-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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: Remove horizontal scroll navigation from SQL Lab #17356
fix: Remove horizontal scroll navigation from SQL Lab #17356
Conversation
Marking as ready for review to get the docker build, will tag people when it's actually ready |
/testenv up |
@etr2460 Ephemeral environment spinning up at http://52.35.144.201:8080. Credentials are |
Codecov Report
@@ Coverage Diff @@
## master #17356 +/- ##
=======================================
Coverage 77.06% 77.06%
=======================================
Files 1036 1036
Lines 55757 55757
Branches 7628 7628
=======================================
Hits 42969 42969
Misses 12532 12532
Partials 256 256
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
30cc709
to
cc98071
Compare
/testenv down |
/testenv up |
@etr2460 Ephemeral environment spinning up at http://18.236.74.45:8080. Credentials are |
Can we turn it off for the whole app? I hate that pattern! 😆 |
We can consider it, would appreciate a UX designer/PM's take on that first |
I'd need to do some research on the accessibility issues around turning it off for the entire app, but big thumbs up to doing it here! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<3
Ephemeral environment shutdown and build artifacts deleted. |
@etr2460 @betodealmeida Table chart would need horizontal scrolling at least. Some other tables may also need it. |
Oh, if I understood correctly we're not getting rid of horizontal scrolling (that's also needed in SQL Lab), but navigating between pages by using horizontal scrolling. |
SUMMARY
People commonly complain about accidentally navigating forwards or backwards in chrome when horizontally scrolling in SQL Lab. Most of our app doesn't have such a horizontal scrolling paradigm, but SQL Lab is the exception. So let's remove that horizontal scrolling navigation behavior in SQL Lab
TESTING INSTRUCTIONS
Spin up a testenv and test. make sure you can't horizontal scroll to go forward and backwards in sql lab. But when you go back to a dashboard/explore see that you can
Before:
before.with.scroll.mov
After:
after.no.scroll.mov
ADDITIONAL INFORMATION
to: @graceguo-supercat @ktmud @rusackas @michael-s-molina