-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Alternative approach to adding a close icon for navigation sidebar #31031
Conversation
Noting that we could also pursue @jameskoster's approach
In this approach, the |
Size Change: +152 B (0%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
Thanks so much for exploring these. Out of the two, I have a preference for the existing toggle, even if we add an extra close button affordance to it. Mostly because that lets focus on the W menu "stay in place" rather than shift around when opening closing. If we do go with this alternate approach, we need to consider sites that have a logo defined: |
You're welcome 👍
I understand this to mean you'd like to retain the toggle interaction in and around the site icon and avoid having a 2nd I see the logic because that would allow the interaction to remain "under" the cursor whether toggling open/closed. In that case we're heading towards simply replacing the site icon with an Shall I do one final exploration of that route? |
I'm closing this one based on |
Description
Alternative approach to #30964 in order to address #29529.
This approach uses a single
NavigationToggle
component. It has two states:The site icon (the
W
logo) is extracted to its ownNavigationIcon
component and now has no interactivity. It's purely a visual element. You now have to click theX
icon to close the panel but that's far more intuitive and is an established pattern in the editor (eg: theList View
sidebar has this).This approach is better because
X
icon as the way to toggle the panel.See that PR for all details.
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).