-
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
Move the edit button in the site editor sidebar to a contextual widget #46700
Conversation
4bbfc27
to
1bd75b2
Compare
1bd75b2
to
eb1a2a0
Compare
Size Change: -92 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Thanks for this. I think there's a lot of sense in isolating the site information in a "hub" like that, letting the waterfall navigation slide below it. It is also a way, potentially, to allow for more context to show next to the Edit button in the future. Can the edit button be 32px tall? |
Also the W logo / site icon for balance? The W logo is currently 30x30, inside a 36px button, so will need some tweaking. Edit: I noticed the sidebar seems to have shrunk a bit (width), and the top toolbar animation has disappeared. Edit 2: One other small thing: When moving from browse to edit, the nav panel slides out to the left. Instead of sliding back in when moving from edit to browse it appears instantly. I don't have a strong preference re sliding vs fading, but they should probably be consistent? |
Unfortunately, this animation is controlled by a lower-level reusable component "Navigator", a fix might not be easy here. Maybe I can try to disable the sliding and only leave fading. |
How is this looking? |
I think it's looking good! At the risk of repeating Jay on animation, the inserter button appearing "under" the hub animation feels awkward. Could we speed up the hub animation so it finishes before the white toolbar drops down? Or can we animate width of the white bar (might be awkward). Conceptually, it makes sense that the hub is an element that floats above the bottom dark gray canvas, and above the white toolbar and inspector elements in elevation. That's already the case in this animation. But I think either speeding up the hub-collapsing animation might thread the needle. Alternately, maybe the contents of the top toolbar (all the buttons) could fade in as the toolbar drops down. That might also give the appearance that they aren't interactive yet. |
The hub animates faster now. |
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.
Some end-to-end test failures may be relevant.
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.
This feels like a solid upgrade over trunk, especially if paired with #46736, so greenlighting it as such:
I still think we need to make the nav sidebar 360px wide, and I wish there was a way we could gate the browse view mostly as an experiment, but since that's not really feasible. at least his increases contextuality and usability a fair bit.
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.
Worked well on my tests 👍 And the code looks good.
Related #36667
What?
This PR redesigns the sidebar of the browse mode to match the video shared here https://user-images.githubusercontent.com/846565/208665227-82537dfb-2893-4fb6-8f84-823f1e70cc07.mp4
Why?
We have noticed that the "edit" button is a bit hidden and lacks context/connection to the frame. This PR adds a "widget" on top of the sidebar to add that missing context and move the edit button there for more clarity.
How?
Note
Screen.Recording.2022-12-21.at.11.23.56.AM.mov