-
Notifications
You must be signed in to change notification settings - Fork 153
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
Utilizes ModalBase to lock scroll on mobile filters #5659
Utilizes ModalBase to lock scroll on mobile filters #5659
Conversation
Just realizing that button for "Filter" will look a bit weird on Desktop if this view ever happens to be triggered there. We really need an un-styled button primitive in palette — like the equivalent of |
Error RangeError
Dangerfile
|
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.
Whoa, ModalBase looks handy 😄
7337b41
to
cec9e1a
Compare
LGTM save for the odd little snapshot failure in CI |
cec9e1a
to
8e7872e
Compare
Unexciting :) most of the nice things in the ModalBase like the focus management would apply more to desktop uses but this at least does a better job of locking the scrolling on mobile.
Also snuck in tapping on filter to scroll to top since that's expected from a UX perspective but wouldn't work due to the modal context.