Block editor: rewrite moving animation for better load performance #57133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -980 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 4bb81c4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7233772414
|
youknowriad
reviewed
Dec 16, 2023
packages/block-editor/src/components/use-moving-animation/index.js
Outdated
Show resolved
Hide resolved
youknowriad
reviewed
Dec 16, 2023
ellatrix
commented
Dec 16, 2023
| ref.current.style.transform = finishedMoving | ||
| ? null // Set to `null` to explicitly remove the transform. | ||
| : `translate3d(${ x }px,${ y }px,0)`; | ||
| ref.current.style.zIndex = zIndex; |
There was a problem hiding this comment.
Maybe we can also set this at before animation start, then restore at the end or on cleanup.
There was a problem hiding this comment.
(Same for transformOrigin)
There was a problem hiding this comment.
I left the original code here. Didn't want to alter things too much and I was hoping to see a before start and stop API.
youknowriad
reviewed
Dec 16, 2023
packages/block-editor/src/components/use-moving-animation/index.js
Outdated
Show resolved
Hide resolved
youknowriad
reviewed
Dec 16, 2023
youknowriad
approved these changes
Dec 16, 2023
andrewserong
reviewed
Jan 8, 2024
| * If the block count exceeds the threshold, we disable the reordering animation | ||
| * to avoid laginess. | ||
| */ | ||
| const BLOCK_ANIMATION_THRESHOLD = 200; |
There was a problem hiding this comment.
By moving this into useMovingAnimation, the moving animation is now prevented in the list view when the global block count is greater than 200. Was that intentional?
I think for the list view case, where list items are displayed conditionally based on windowing logic, we might not need to guard it behind this global block count threshold. The reason I'm thinking about this is because over in #56625 I'm exploring displacing list view items when dragging within the list view, and useMovingAnimation provides a handy way of smoothly animating a dropped list view item into its final position.
I noticed while editing the blog home template in TT4 that the getGlobalBlockCount() already starts out pretty high (165 in my test site) so it's quite easy to get to over 200 and then lose the animation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
In this PR, we use the lower level, imperative API from react-spring. This gives us better performance for load since we only create an animation controller at the time that it's needed.
I believe this also makes the code easier to understand. There's just a single layout effect instead of all the complex state, multiple effects, and an obscure declarative API. Sometimes imperative rules!
Why?
I made some additional optimisations: checking reduce motion through match media is expensive. Let's run a few times again:
First run: -6%
Second run: -8.5%
First run: load -4%
Second: -2.2%
Third: -3.2%
Fourth: 3.6%
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast