Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Table Text Got Truncated in Small Screen Issue #9906

Closed
wants to merge 2 commits into from

Conversation

Goodyear2017
Copy link
Contributor

@Goodyear2017 Goodyear2017 commented Apr 4, 2024

@Goodyear2017 Goodyear2017 requested a review from a team as a code owner April 4, 2024 20:37
@erdembayar
Copy link
Contributor

Please include screenshots for UI change. Could you please include before and after changes?

@Goodyear2017
Copy link
Contributor Author

Goodyear2017 commented Apr 11, 2024

@erdembayar Please see the screenshot, user can scroll left and right to see the full text after css change.
image
image

@agr
Copy link
Contributor

agr commented Apr 12, 2024

@ryuyu, haven't we been flagged for horizontal scrollbars in the past?

@ryuyu
Copy link
Contributor

ryuyu commented Apr 12, 2024

I believe the previous flags were for horizontal scroll bars for entire page.
Scroll bars that were contexted to table contents I BELIEVE were ok, so this looks like it is probably ok to me.

ryuyu
ryuyu previously approved these changes Apr 12, 2024
@@ -1726,3 +1726,15 @@ img.contributors-contributor-avatar {
font-size: 1.1em;
padding: 5px 15px 5px 15px;
}
.container.page-statistics-overview td {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have both a less and css file. Should we update the .less file instead of the css file in this case?
On other day I saw @martinrrm is making changes in less file generating the css changes from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erdembayar Just made the change, move the css from site.css to less file. Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need to generate .css change from .less file and check-in both into our code repository.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinrrm could you please confirm if above is correct process?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goodyear2017 For changes in styles, we change the .less file and then update the css with grunt. We have some docs on how to update those files here https://github.com/NuGet/NuGetGallery/blob/main/docs/Frontend.md, let me know if you have questions. Thanks

@martinrrm
Copy link
Contributor

@agr @ryuyu @erdembayar Do we want to add horizontal scrolls to each row of the table? I'm not sure if that is the best approach, we could add one horizontal scroll to the table and make the table columns the size of the text, this could make some columns not visible unless scrolling to the side.

@Goodyear2017
Copy link
Contributor Author

@martinrrm We have a table on our site scroll the whole table in small screen, the accessibility team approved it.

@Goodyear2017
Copy link
Contributor Author

I will close this PR and create another one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants