-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Align hook: improve BlockEdit filter #48422
Conversation
Size Change: +282 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
); | ||
}, | ||
[ clientId, isSelected, name ] | ||
); | ||
const context = { |
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.
Is this context a public API, if it is, I think we should "lock" the new key somehow to make it private.
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.
How do you "lock" a key?
I'm thinking we should maybe create a separate boolean context for this that is private to blockEditor which can be used in the Fills and pass it down as a prop to BlockEdit for the HoCs so they don't need to call a hook.
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 either a separate context or store a locked function that returns the boolean in this context.
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 probably prefer the former to avoid the proliferation of contexts
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.
Why should we lock it though? Don't we want plugin authors to use these in their filters?
}, | ||
[ clientId, isSelected, name ] | ||
); | ||
return useBlockEditContext().shouldDisplayControls; |
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.
useBlockControlsFill
calls useDisplayBlockControls
but also adds some additional checks that are now part of useDisplayBlockControls
so there's probably some code to remove from useBlockControlsFill
Flaky tests detected in 4ffc496. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4264760720
|
Closing in favour of #48809. |
What?
Split out from #48420.
Only add the controls (and run related checks) which the block is selected and supports alignment.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast