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

[Discover] chore: Improve scrolling experience #9298

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Maosaic
Copy link
Contributor

@Maosaic Maosaic commented Jan 29, 2025

Description

Improve scrolling experience on Discover page:

  1. Make vertical scrolling happen inside table rather instead of the entire right panel.(This change also bring horizontal scrolling to the bottom of the viewport instead of staying at the end of the page)
  2. Make table header sticky.

Issues Resolved

  1. If user scroll down the table on Discover the entire view scrolls down. The user is not able to see their columns that they have selected nor can they even see the query that they ran.
  2. There is no indication of overflow horizontally wise the user will not know that they can scroll and depending on their setup that might not even be able to scroll horizontally

Screenshot

Screen.Recording.2025-01-27.at.11.25.51.AM.mp4

Testing the changes

Changelog

  • feat: Improve scrolling experience on Discover page.

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Joey Liu <jiyili@amazon.com>
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.70%. Comparing base (d227bc5) to head (7f8d801).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9298      +/-   ##
==========================================
- Coverage   61.71%   61.70%   -0.02%     
==========================================
  Files        3816     3816              
  Lines       91822    91829       +7     
  Branches    14542    14543       +1     
==========================================
- Hits        56672    56666       -6     
- Misses      31495    31507      +12     
- Partials     3655     3656       +1     
Flag Coverage Δ
Linux_1 28.99% <ø> (-0.01%) ⬇️
Linux_2 56.46% <ø> (ø)
Linux_3 39.18% <ø> (+<0.01%) ⬆️
Linux_4 28.90% <ø> (-0.01%) ⬇️
Windows_1 29.00% <ø> (-0.02%) ⬇️
Windows_2 56.41% <ø> (ø)
Windows_3 39.18% <ø> (+<0.01%) ⬆️
Windows_4 28.90% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@angle943
Copy link
Contributor

Hey the funcitonality looks good! I checked it out to test if another annoyance i encountered during my testing was fixed (sometimes the table would jump around a bit), but I was not able to replicate it right now (both in your PR but also in main).

One slight UX issue though, do we want the bottom bar for the header to be stickied too? Its a little jarring seeing it like this:

Screenshot 2025-01-30 at 9 51 20 AM

@Maosaic
Copy link
Contributor Author

Maosaic commented Jan 30, 2025

No sure if there is a perfect solution for the bottom border, since adding it will also "visually breaking" row height when scrolling.

@angle943
Copy link
Contributor

angle943 commented Jan 30, 2025

@Maosaic here's a couple examples i found in some of the widely used tables:

  1. MUI - https://mui.com/material-ui/react-table/#sticky-header
Screenshot 2025-01-30 at 10 25 26 AM
  1. Cloudscape (although this example is for stickied columns)
Screenshot 2025-01-30 at 10 27 08 AM
  1. Bootstrap (https://examples.bootstrap-table.com/)

  2. ANT design (https://ant.design/docs/blog/virtual-table)

I can find a couple more examples but i think the standard is either to have the border or a shadow

EDIT: I found one that is like yours though:

Screenshot 2025-01-30 at 10 34 38 AM

@kavilla kavilla added backport 2.x discover for discover reinvent labels Jan 30, 2025
Signed-off-by: Joey Liu <jiyili@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants