-
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
Improve zoom transition #64179
Improve zoom transition #64179
Conversation
Size Change: +79 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
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. |
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.
Looks so much smoother!
...props.style, | ||
height: props.style?.height, | ||
transition: 'all .3s', | ||
transition: 'all 0.5s cubic-bezier(0.65, 0, 0.45, 1)', // Maps to the .block-editor-iframe__html transition. |
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.
Why don't we move this to the iframe/content.scss
file to make it easeir? I don't think we need this defined in the JS anyways.
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.
.edit-site-visual-editor__editor-canvas
already is used in scss elsewhere. 🤷♂️
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.
Yeah, I think we should move it there. I don't see why it should be defined in the JS over CSS.
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 agree with defining it in CSS instead of JS and not in Iframe
’s styles because as long as it’s innate to the component it exacerbates #50449 by slowing down how quickly the pattern preview reaches its final height. It’s one thing I've been meaning to make a PR for. It can be done separately of course. packages/block-editor/src/components/block-canvas/style.scss
seems a decent place to put it.
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.
Didn't know this was needed until testing it out 😄 Way smoother
Transition seems vastly better. Does it also account for some of the jitters that happen if you have sidebars (inserter or inspector) open? There's currently some jumpiness there in places. Otherwise, seems like this mostly needs a code review. |
There are still some refinements we can do to the sidebar transitions. #64110 helped a good bit, when not in zoom out; but the timings of those transition states can be finessed. |
4fec73e
to
c66a8d9
Compare
Flaky tests detected in c66a8d9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10220040753
|
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.
Much smoother transition than on trunk, and also moves the animations to a shared location in the CSS instead of spread about in JS and CSS.
What?
Makes zooming out much smoother. I'd also like to apply the same transitions to when you switch to an alternate device preview (Tablet and Mobile). Currently those transitions are formed elsewhere.
Why?
The current transition is choppy, transitioning some attributes—but not all.
Testing Instructions
Visuals
Trunk
Note that this UI is from #63870, but the transitions are from trunk. The background color is applied without transition, as well as the initial placement to give zoom out spacing from the editor editor.
CleanShot.2024-08-01.at.14.33.39.mp4
Proposal
All elements transition together and slowed down just a little—overall, less jarring.
CleanShot.2024-08-01.at.14.30.14.mp4