-
Notifications
You must be signed in to change notification settings - Fork 4.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
Site Editor: Improve the frame animation #60363
Conversation
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. |
@@ -27,6 +27,7 @@ | |||
"react-native": "src/index", | |||
"dependencies": { | |||
"@babel/runtime": "^7.16.0", | |||
"@react-spring/web": "^9.4.5", |
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 adds a new dependency to the site editor but I think it's worth it.
* @param {Object} $1 Options | ||
* @param {*} $1.triggerAnimationOnChange Variable used to trigger the animation if it changes. | ||
*/ | ||
function useMovingAnimation( { triggerAnimationOnChange } ) { |
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.
Potentially this can be a generic hook in the compose package but I think the recent perf optimization that we've made to the block animation made it less likely to be shared. Maybe it's fine for it to be duplicated. (cc @ellatrix )
The other difference with the block animation is that this one animates the "width" and "height" as well as the position.
Size Change: +16.6 kB (+1%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
How does the current animation work? |
I'm trying to test, but I'm not sure what differences I should be looking for. |
The current animation uses framer motion layout animation but only animates position so it results in a weird animation in both root of the site editor and list views. |
Cool to see an iteration on this. I reckon we should sync up the animation effects, currently the top bar seems to move much quicker than the frame, which feels a bit pedestrian by comparison. Generally I think this is a move in the right direction, but if other panels are open (List view, Inspector/Styles) when you open/close the editor it feels a bit clunky: clunky.mp4I don't know if we can do it here, but long term it would probably be good if those panels (and the breadcrumb) behaved as discrete elements, like the top toolbar. If that's feasible I'd be happy to mockup a prototype in Figma. Edit: Alternatively, the panel treatment could also be applied to the top toolbar (and tidied up in terms of animation). It feels strange that they behave differently. We should probably mock this up in Figma regardless so we have consensus about what to aim for. |
I don't think it will happen here, I wanted to focus on the animation of the frame alone to keep the PR small. That said, I'm curious to see mockups here, we should improve these interactions as much as we can. For the header moving to the frame, I'm planning to explore that in a follow-up. |
I updated this to use the same animation duration for the header and the frame (300ms) |
I must be blind but I'm not seeing the difference 😅 Also, do you think we'll want to move away from Motion to React Spring if this is more flexible? |
The width of the frame in trunk doesn't really animate when leaving the edit mode (it moves from 100% to the final frame size without animating, you just see the frame moving afterwards).
I don't really know, but for this particular animation, it's just impossible to do with our previous method properly. |
Aaah, I see it now. |
@@ -158,7 +158,6 @@ jobs: | |||
- name: Docker debug information | |||
run: | | |||
docker -v | |||
docker-compose -v |
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.
Is this temporary to check what's causing the failures?
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.
I think this is what was causing the failures actually. I guess these days it's docker compose
and not docker-compose
and Github probably updated docker or something so the PHP tests started failing, I expect that they'll fail elsewhere as well.
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.
Shall we merge this so we fix the tests in other PRs as well?
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.
The only thing that's a bit strange to me is that all the text reflows, but I guess there's not really any way to avoid that without zooming.
The alternative way is the default behavior of the "layout" animation of framer motion but it's worse because it distorts the text. |
This definitely feels better than the abrupt width change. |
Alternative to #58924
What?
The site editor frame animation when entering and existing the edit mode is a bit clunky on trunk (especially when exiting the frame). This PR solves that by relying on a technique we've used before in the block animations in the canvas.
Basically, we compute the destination position, restore the previous position and animate the "ref" of the frame to reach the final destination.
Why?
I think this improves the experience a lot. The site editor feels smoother.
Testing Instructions
1- Try entering and exiting the editor either from the list view or from regular views.