-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Navigation] Fix navigation hide on key press #2607
Conversation
da4d402
to
5dee996
Compare
💦 Potential splash zone of changes introduced to
DetailsAll files potentially affected (total: 1)📄
|
I'm just on a call right now but I'll add a test before the end of the day |
a2ad90a
to
5b51e61
Compare
b8cb9d9
to
97bb2fa
Compare
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.
Nitpick about the casting, otherwise looks great 👍
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.
Nice catch ✅ 🎩
Co-Authored-By: Andrew Musgrave <andrew.musgrave@shopify.com>
c89d7e6
to
dfec046
Compare
Co-Authored-By: Andrew Musgrave <andrew.musgrave@shopify.com>
dfec046
to
e5a0077
Compare
* Fix nav hide bug * Update src/components/Frame/tests/Frame.test.tsx Co-Authored-By: Andrew Musgrave <andrew.musgrave@shopify.com> * Update src/components/Frame/tests/Frame.test.tsx Co-Authored-By: Andrew Musgrave <andrew.musgrave@shopify.com> Co-authored-by: Andrew Musgrave <andrew.musgrave@shopify.com>
WHY are these changes introduced?
There's an issue in production where if any item within the Navigation component has focus and you hit the escape key, the nav will try to hide itself off to the left.
Screencast
There's no github issue for this. I randomly stumbled upon it while working on something else.
WHAT is this pull request doing?
This pr adds a check to make sure the nav is both active and on a small screen before calling dismiss.
🎩
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
Copy-paste this code in
playground/Playground.tsx
:🎩 checklist
README.md
with documentation changes