Skip to content
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

Site Editor Animation: We have a mismatch of animation when toggling the site editor sidebar #48630

Conversation

tomdevisser
Copy link
Member

@tomdevisser tomdevisser commented Mar 1, 2023

What?

We currently have a bit of a mismatch in terms of animation when you move between browsing / editing

Why?

The Site Icon and Save button remain in place, but the menu items (very quickly) slide away to the left. Ideally everything in the dark sidebar should remain stationary, solidifying the idea that the frame sits "on top" of everything else.

How?

Removed the animation on the x axis.

Testing Instructions

Go to the site editor and toggle it

fixed-design.mov

Props jameskoster
Fixes #48595

The Site Icon and Save button remain in place, but the menu items (very quickly) slide away to the left. Ideally everything in the dark sidebar should remain stationary, solidifying the idea that the frame sits "on top" of everything else.

Props jameskoster
Fixes WordPress#48595
@tomdevisser tomdevisser requested a review from ajitbohra as a code owner March 1, 2023 11:37
@tomdevisser tomdevisser changed the title We have a mismatch of animation when toggling the site editor sidebar Site Editor Animation: We have a mismatch of animation when toggling the site editor sidebar Mar 1, 2023
@tomdevisser tomdevisser self-assigned this Mar 1, 2023
@tomdevisser tomdevisser added the [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") label Mar 1, 2023
@jameskoster
Copy link
Contributor

Thanks for working on this so quickly :)

The video looks good to me. In isolation I don't mind the opacity animation, but I don't think it really speaks to the idea of the frame being a separate object from the dark material beneath. In that respect it feels a bit misplaced. Perhaps it's worth trying without for a comparison?

@jasmussen
Copy link
Contributor

Fast work, thank you! I'd agree with Jay: conceptually you can think of the dark material as the desk, and the site on the right being a paper on that desk. The desk doesn't need to fade out as the paper scales up. The metaphor doesn't work 100%, but some of the thoughts on the faux physics involved can help us figure out some consistency across.

@tomdevisser
Copy link
Member Author

@jameskoster @jasmussen All right, this one was a bit trickier to find as there was an opacity change on both the sidebar content wrapper and the little menu wrapper as well, happening at the same time. Good thing we found this, and when I notice how much simpler the component becomes by just omitting the opacity change I think your decision was the right one.

@tomdevisser
Copy link
Member Author

no-opacity.mov

@tomdevisser
Copy link
Member Author

I do notice the menu item disappearing when closing the sidebar. Will try to fix that as well.

@jameskoster
Copy link
Contributor

I do notice the menu item disappearing when closing the sidebar. Will try to fix that as well.

Ah yes, that does become more apparent. Thanks again for the energy on this :)

@tomdevisser
Copy link
Member Author

@jameskoster @jasmussen Changed it, but this means the Sidebar has to stay in markup when in edit mode. This doesn't seem to be a problem when testing things, no z-index clashes or anything, and I only changed it when not on mobile. But it would be nice to get a second opinion from a dev for this just to be sure it's okay.

cc @Mamaduka @felixarntz @aaronrobertshaw is one of you available to answer this?

@tomdevisser
Copy link
Member Author

fix-visual-bug.mov

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ping, @tomdevisser.

Unfortunately, I'm not very good with animations 😅

I left a few minor comments regarding the code but will leave the final decision to others.

Comment on lines +95 to +96
! isMobileViewport ||
! isEditorPage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to test how this affects performance. Rendering a component when we don't use it doesn't seem like the right way to do it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the reason I pinged you, wondered about this. But the only other way we can achieve this look is by taking this approach, or delaying the animation which will definitely make the UI feel slow.

I would like to test the performance for you, but have no idea how I would go about that. Could you point me in the right direction there? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to rendering an unused component, having it in the DOM could have some a11y impacts could it not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasmussen @jameskoster This in combination with the y-axis enter and exit animation having to return makes me think this'll just have to stay the way it is.

@jameskoster
Copy link
Contributor

@tomdevisser I noticed we lost the animation as you move between panels :D

animation.mp4

@tomdevisser
Copy link
Member Author

@tomdevisser I noticed we lost the animation as you move between panels :D

True, that's the same animation! It's either they have an in and out animation or they don't as far as I can tell.

@jameskoster
Copy link
Contributor

Oh, I think we need to keep that in-panel animation. That's most likely a blocker to removing the animation between edit/browse.

@tomdevisser
Copy link
Member Author

Oh, I think we need to keep that in-panel animation. That's most likely a blocker to removing the animation between edit/browse.

Ah okay, but that was the whole point of the issue right? Only removing the opacity is gonna look weird with the movement left/right. Does that mean we're gonna close the issue?

@jameskoster
Copy link
Contributor

The issue was to remove the sidebar animation when you move between browse/edit views. Apologies, perhaps that should have been clearer.

@tomdevisser
Copy link
Member Author

The issue was to remove the sidebar animation when you move between browse/edit views. Apologies, perhaps that should have been clearer.

No worries, I understand the issue, but the element has an animation on entry and exit. Wether that entry happens on toggling edit/browse view or while browsing doesn't matter to the animation component, it's the same animation. So it's either there or it's not as far as I can tell.

@jameskoster
Copy link
Contributor

Hmm perhaps @youknowriad has an idea?

We'd like to remove the slide-out animation of the sidebar when you move between edit/browse views, but preserve it when you drill down/up.

@youknowriad
Copy link
Contributor

Yeah, it's not really clear to me how you can make Framer motion animations work in one way but not the other. You can try to tweak the "duration time" of the interaction depending on the flag (canvas mode) but I'm not sure how that would work.

@ciampo
Copy link
Contributor

ciampo commented Mar 14, 2023

I don't think that we should remove the animation entirely from Navigator.

From what I see, could this be solved by allowing Navigator to disable the initial animation?

@ciampo
Copy link
Contributor

ciampo commented Mar 14, 2023

Proposed a new approach in #49062

@stokesman
Copy link
Contributor

I stumbled across this one and it looks to be well outdated. Pardon me if I wield a heavy hand in closing it.

@stokesman stokesman closed this Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing")
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site Editor dark sidebar contents should remain stationary when entering edit view
8 participants