-
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
Add useScaleCanvas: Web Animations API for zoom in/out animation #66917
Conversation
Size Change: +117 B (+0.01%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
035b913
to
f0f90e8
Compare
Flaky tests detected in fb16c4f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12018296199
|
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. |
5e23644
to
a74987a
Compare
#66618, which is applying the correct zoom in/out on |
201b18a
to
857ca64
Compare
@cbravobernal - This will be the hopeful final state of the scaling animation. It has a large improvement over the one that got merged into 6.7 (but also has a lot more code). |
It isn't necessary to set the previous values at the beginning of the useEffect, because that is handled by the starting frame of the web animations api.
…ansitionTo.scrollTop
b06c369
to
b41d282
Compare
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 like the rebase worked well! The typing added seems useful.
|
||
@mixin editor-canvas-resize-animation($additional-transition-rules...) { | ||
transition: all 400ms cubic-bezier(0.46, 0.03, 0.52, 0.96), $additional-transition-rules; | ||
@include reduce-motion("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.
Removed the mixin, as it's only used once.
packages/block-editor/src/components/iframe/use-scale-canvas.js
Outdated
Show resolved
Hide resolved
// so we need to adjust the height of the content to match the scale by using negative margins. | ||
$extra-content-height: calc(#{$content-height} * (1 - #{$scale})); | ||
$total-frame-height: calc(2 * #{$frame-size} / #{$scale}); | ||
$total-height: calc(#{$extra-content-height} + #{$total-frame-height} + 2px); |
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.
what is 2px?
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.
It's the same 2px from before; the line just got moved.
I'm not entirely sure what it does, but I assume it's a 1px border somewhere that isn't accounted for in the height.
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 gave the code a read and the PR some testing. I didn't find anything to block this. I like the code normalisation where we have a new hook to deal with the scaling in/out UI updates.
What?
Use web animation api to handle the zoom animation in a more performant way. Also allows for reversing the animation during the zoom animation if you click the zoom button twice quickly.
Fixes zooming out scaling from wrong point when sidebars have been opened after zoom out was initiated.
Why?
Improve implementation for maintainability, performance, and fix edge cases.
How?
When changing scale (zoom in/out):
useScaleCanvas
hook to manage zoom level and animationsis-zoomed-out
class to the iframe document htmlstartAnimationRef
to trueuseEffect
runs that:transitionFrom
andtransitionTo
refs with the properties of{ scaleValue, frameSize, clientHeight, scrollTop }
. This gives the animation everything it needs to know to transition to/from the values.transitionFrom.scrollTop
so that the canvas visually stays in the same positiontransitionTo.scrollTop
value so everything looks seamless.transitionFrom
andtransitionTo
refs are swapped, and the animation is reversed.Known bugs
Testing Instructions
Zooming from Top
Screen.Recording.2024-11-06.at.2.38.52.PM.mov
Zooming from below Top
Screen.Recording.2024-11-06.at.2.39.17.PM.mov
Zooming out after scale container has changed sizes
On
trunk
, this snaps to a much smaller size.Before
Screen.Recording.2024-11-14.at.11.43.56.AM.mov
After
Screen.Recording.2024-11-14.at.11.32.29.AM.mov
Open/close sidebar scaling animations
Try to break it :)
Testing Instructions for Keyboard
Screenshots or screencast
Before
Screen.Recording.2024-11-06.at.3.29.26.PM.mov
After
Screen.Recording.2024-11-06.at.12.27.31.PM.mov