Try improving ListView drag and drop performance by computing blocksData ahead of time
#50210
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
talldan
added
[Type] Performance
6 tasks
|
Size Change: +178 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
andrewserong
reviewed
May 1, 2023
There was a problem hiding this comment.
Thanks for picking this up! I like the idea of computing the data much less frequently. One question this change potentially raises is what to do about blocks rendered as placeholders in a long list view when the drag is initially started and then a user drags up or down the list to where the windowed/placeholder rows are rendered.
Would it be worth outputting the expected data-block, data-expanded, etc in the placeholder row around here: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/list-view/branch.js#L199?
|
I tried instrumenting When you start adding a lot of blocks, there are other performance issues in the editor that cause more issues than drag and drop (Things like scrolling and even toggling the list view sidebar open is noticeably slow). |
|
Flaky tests detected in 7467553. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4848418920
|
|
Thanks for giving this one a look anyway, it was worth exploring 🙂 |
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?
Originally, when ListView drag and drop was introduced, there was a performance optimization in that the expensive
blocksDatawas computed only once when the user started dragging.At some point this has regressed and the data is now computed on every trigger of the dragover event.
This PR ensures
blocksDatais only be computed on drag start.Why?
The block list doesn't change when a user is dragging, so this isn't needed.
This could be revised when collaborative editing is introduced, as then the block list may change while the user is dragging 😬. Hopefully there will be other ways to recompute the data (i.e. whenever the
blockOrderchanges?)How?
Store blocksData in a
refwhen the user starts dragging and clear it when the user stops.Testing Instructions
The easiest way to test is to set a logpoint in the
getBlocksDatafunction. Then start dragging and you should see only one line of console output.