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: horizontal resource table scroll #11313

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

JammingBen
Copy link
Collaborator

@JammingBen JammingBen commented Aug 1, 2024

Description

Prevents resource tables from being horizontally scrollable for a11y reasons. Instead, information that is available elsewhere will get hidden on smaller displays.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@JammingBen JammingBen self-assigned this Aug 1, 2024
@JammingBen JammingBen force-pushed the fix/horizontal-scroll-on-small-screens branch 2 times, most recently from bacb1b2 to 4337cbb Compare August 1, 2024 11:39
Prevents resource tables from being horizontally scrollable for a11y reasons. Instead, information that is available elsewhere will get hidden on smaller displays.
@JammingBen JammingBen force-pushed the fix/horizontal-scroll-on-small-screens branch from 4337cbb to 9f7a2c4 Compare August 1, 2024 12:29
@JammingBen JammingBen marked this pull request as ready for review August 1, 2024 12:53
@kulmann
Copy link
Member

kulmann commented Aug 5, 2024

ftr I changed the PR description from fixes to works towards because I found another issue. We need to check first if that needs to be fixed as well...

@kulmann
Copy link
Member

kulmann commented Aug 5, 2024

Can we make sure that all action buttons above the file list are one row and break into the next line from the beginning? Also, line break only kind of works if I shrink the browser width even further, see screenshot (where you can actually see both of my issues):

Screenshot 2024-08-05 at 05 49 20

@kulmann
Copy link
Member

kulmann commented Aug 5, 2024

Can we have some margin-bottom on the filters?

Screenshot 2024-08-05 at 05 52 07

@kulmann
Copy link
Member

kulmann commented Aug 5, 2024

Spaces overview still scrolls horizontally:

Screenshot 2024-08-05 at 05 53 37

@JammingBen
Copy link
Collaborator Author

JammingBen commented Aug 5, 2024

Can we make sure that all action buttons above the file list are one row and break into the next line from the beginning?

Yes I can do that, but selection will be jumpy then because we add new vertical space that wasn't there before.

Also, line break only kind of works if I shrink the browser width even further, see screenshot (where you can actually see both of my issues):

It's fine with the smallest pre-setting of the browser, no? We should have a limit IMO, nobody uses a screen <320px. If we really want to support that, I feel like this is a separate story. Edit: Ah, but there is a difference between dev mode and production build - again 😬 With production build, it still shows allows horizontal scroll.

Can we have some margin-bottom on the filters?

I discussed that with @tbsbdr : it's not that easy because it's just a flex-wrap. So it would need a hand-crafted solution.

In general I thought the goal of this issue is to make it a11y-conform. Making things actually look nice on small screens is certainly possible, but we would need to allocate a lot more time for that.

Spaces overview still scrolls horizontally:

I can't reproduce that 🤔 What's your vertical screen size?

image

This is bad because it causes the screen to allow horizontal scroll on small screens.
Copy link

sonarcloud bot commented Aug 5, 2024

@JammingBen
Copy link
Collaborator Author

@kulmann I fixed the horizontal scroll for production build, can you please check if the spaces page is still scrollable?

For the breadcrumbs, as discussed, let's not solve this via a new line because it's too jumpy. This could be solved via a context menu, but not in the scope of this.

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Works good now 👍 When I navigate to the spaces overview while having 100% and then zooming in to 400% breaks the spaces overview, but it works fine on page reload then.

@JammingBen JammingBen merged commit 32d81a8 into master Aug 5, 2024
3 checks passed
@kulmann kulmann deleted the fix/horizontal-scroll-on-small-screens branch September 5, 2024 19:48
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.

2 participants