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
Block editor: avoid list re-rendering on select #57188
Conversation
Size Change: +21 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
if ( hasSelectedBlocks && hasVisibleBlocks ) { | ||
break; | ||
} | ||
} |
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.
Could this be a private selectors, something like getSelectedBlocksAtLevel( rootClientId )
and getVisibleBlocksAtLevel( rootClientID )
or something like that?
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 was thinking about that, but it didn't feel worth it to create another selector for this, especially since it's such a weird optimisation. Also we already have access to the block order here. What do you think?
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.
Ideally we also prevent re-rendering the current list with the selected block, but I don't know of a way to do that without introducing another block editor store subscription per block.
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.
To be honest, with private APIs, I think the purpose becomes clearer with a selector that without.
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.
Ok, will add them as selectors then :)
03b739d
to
a7f0f3c
Compare
What?
This PR avoids the re-rendering of
Items
if the selection changes, but the list doesn't include the selected (or visible) block at all.Note: while we could create another
useSelect
call for each block, check if it's selected, and then turn on the AsyncProvider, there's a balance to be struck between optimising type and block select. I think typing is more important that block select, so let's avoid introducing anotheruseSelect
per block.But we can avoid re-renders for other block lists. For example, in the large posts, there are many list and quote blocks.
Why?
First run: -5% (select/focus)
Second run: -4.6%
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast