Site Editor: Improve the header animation #60408
Conversation
youknowriad
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +1.67 kB (0%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
|
Looks good, and I know this is something that's been advocated for. But it's also something we tried in the past, and then specifically moved it out of the frame, in order to better orchestrate animation elsewhere. Are those motivations no longer valid? |
Can you clarify this? I think at some point we just wanted the animation to come from the top but I don't feel like that's necessary or that it brings any value. For me it just adds complexity. |
|
Mainly about keeping the frame to be content only. I don't have a strong opinion, and happy to move forward, animation works. |
|
I think it's hard to answer that question without a comprehensive prototype that considers all the variables. That said, this feels like an improvement on trunk to me. Mostly because there's only a single element animating as a whole, rather than two separate elements which feels a bit disjointed, particularly in the side-by-side data view layout. I still think it would be a good idea to prototype the ideal, as the animation details can also be fine-tuned, but that doesn't need to hold up this PR. |
|
Just to visualize, the inspector causes a jump both in trunk and in this branch. This branch:
Trunk:
That's not necessarily a blocker, but it does speak to the larger orchestration challenge. For example, will the inspector need to live outside of this frame still, as far as the unification work with data views goes? If yes, then this PR might not be the right approach after all. |
|
I think this PR is important in more than just the animation, let me explain.
It is clear that the current header is not something "global" to all the pages of the site editor, it's clear that it's specific to "editor" pages. Meaning that I believe the second approach (this PR) is more suited for our use-cases and is de facto simpler: It doesn't require formalizing a new "header" area as it seems that the only pages that need it today are the ones that are "editors" and they can just inject it in the "frame" area. |
I don't really think the inspector or other "editor specific" panels should leave outside of the frame. Anything that moves outside of the frame forces us create a new "routable area" in the site editor. An area that pages can decide which UI to inject in It's easy to see that this doesn't scale very well, especially since most of these are editor specific UI. |
|
@youknowriad that makes sense to me, though it's worth connecting the dot about potentially invoking the inspector from data views for quick/bulk edits. Does that affect anything in the logic outlined above? 
|
youknowriad
force-pushed
the
update/site-editor-header
branch
from
11011a7 to
52fadc5
Compare
|
So what should we do here? Can I get an approval maybe? |
|
This aligns with the prototype we've been exploring so I think it's worth trying. I updated the animation profile to match the prototype. It would be nice to apply this to the frame as well, but I couldn't work out how to do that. |
|
+1 on this being an improvement and simplification in terms of moving parts and technically too. I see the preview frame as site in view that can transition to edit mode, so header inside makes sense to me. |
|
@jameskoster The frame animation uses React sprint, it is possible to use "easing" for React sprint animations but I don't understand how to translate the config you provided for framer motion to the config required for "easings" in React Spring https://react-spring.dev/docs/advanced/config#easings For now, I just adapted the duration of the animation. |
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org>
|
I ❤️ this change. Yet a
One thing I tried to fix it was moving I hope this is helpful and I'm very interested to see what a good solution looks like. Oh, in case anyone would like to verify here’s the commit from this PR 3ece636 and the parent commit (where issues mentioned aren’t present) a7a62f7. |
|
The distraction free animation is already tracked and I've been working on a potential fix here #61579 (which I believe is the right fix, but I wonder if it's the right time for it). I'm not sure whether it impacts the other one too but it might be good to track it as well. |
|
I've added an issue for the Document Bar animation ☝️. I tested #61579 for it and saw no impact. I didn’t rebase the branch but doubt that would have made a difference. I also went ahead and made #61874 to test the fix I found. No tests failed but that doesn’t mean I trust it. Riad, if you can can have look maybe you'd have insight of whether or not it’s a bad idea. |
Follow-up to #60363
Also based on #58924
What?
This PR moves the editor header into the frame (which is more logical IMO) and improve the animation.
Result:
Screen.Recording.2024-04-03.at.9.56.53.AM.mov