-
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
Block switcher: don't get all parsed patterns when unopened #57020
Conversation
Size Change: +107 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
Flaky tests detected in c32fb23. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7195796165
|
@@ -137,7 +228,7 @@ export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => { | |||
!! possibleBlockTransformations.length && canRemove && ! isTemplate; | |||
const hasPossibleBlockVariationTransformations = | |||
!! blockVariationTransformations?.length; | |||
const hasPatternTransformation = !! patterns?.length && canRemove; | |||
|
|||
if ( | |||
! hasBlockStyles && | |||
! hasPossibleBlockTransformations && |
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.
Unfortunately we can't do this refactoring because we could end up with en empty Dropdown. This highlights an existing bug though because we should add the && ! hasPatternTransformation
check here. If we have no styles or block transformations, but we have patterns, now we do not render the Dropdown.
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.
But it exists in trunk :D
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, I guess we need lighter has*
selectors?
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 probably some has
selectors. I actually introduced a private one yesterday - not for this case. We would need something that filters first by blockType
and then parses, but what concerns me a bit is that with lazy loading the patterns parsing will have to be refactored anyway. So maybe wait a bit for these? 🤔
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.
But it exists in trunk :D
I opened a PR for the bug in trunk
What?
Currently we seem to be parsing a lot of patterns when simply showing the block switcher (when selecting a block), at least on the first call of
__experimentalGetPatternTransformItems
. We should only call the__experimentalGetPatternTransformItems
selector when the dropdown is rendered.Maybe this also helps stabilise this metric because the first select will be heavy.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast