Extensibility: Make Block Bindings work with editor.BlockEdit hook
#67370
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: -120 B (-0.01%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
gziolo
force-pushed
the
fix/hooks-order-block-bindings
branch
from
b24d527 to
985eb9b
Compare
gziolo
added
[Feature] Block bindings
[Feature] Extensibility
gziolo
changed the title
BlockEdit so it works with existing hooks
gziolo
changed the title
BlockEdit so it works with existing hookseditor.BlockEdit hook
gziolo
requested review from
ellatrix,
ntwb,
nerrad,
ajitbohra and
tellthemachines
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
I will investigate further the failing e2e test for the Button block and Post Meta source. We need to ensure that |
I hope that 7c4d983 resolves the issue. |
|
Flaky tests detected in 5c4ff54. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12090287025
|
packages/block-editor/src/components/block-edit/with-block-bindings-support.js
Show resolved
Hide resolved
SantosGuillamot
approved these changes
Dec 2, 2024
packages/block-editor/src/components/block-edit/with-block-bindings-support.js
Show resolved
Hide resolved
packages/block-editor/src/components/block-edit/with-block-bindings-support.js
Show resolved
Hide resolved
Comment on lines
+58
to
+60
| if ( ! props.attributes?.metadata?.bindings?.content ) { | ||
| return el( BlockEdit, props ); | ||
| } |
There was a problem hiding this comment.
To avoid affecting other tests where content is connected, maybe it is safer to provide a unique name for this specific test? For example, checking props.attributes?.metadata?.bindings === 'bindings-inspector-controls'.
There was a problem hiding this comment.
Can you explain how this generic UI control could affect other e2e tests and what the idea is to prevent it? bindings is an object so I'm not sure how to compare that with a string.
There was a problem hiding this comment.
Sorry, I meant metadata?.name, not metadata?.bindings.
The idea behind it was that right now it is adding the extra control for all the tests with content bound. I guess it shouldn't affect, but I thought it could be safer to provide a unique property (for example, a name to the block) from tests that need the extra control and check against that instead.
There was a problem hiding this comment.
What are other conditions that would get closer to real-life usage? I don't think extenders would ever target their integrations based on the name provided by the user through UI for the block name.
There was a problem hiding this comment.
That'd be specific to the test. But it's okay, we can keep the control even for those tests where it is not needed.
|
👋🏻 Looks like this had a pretty big negative impact on the |
|
Yes, I will try to fix it tomorrow or revert depending how on how it goes. |
|
I opened a revert with #67516 to check whether it resolves the performance regression. In the meantime, I will continue investigating locally. |
im3dabasia
pushed a commit
to im3dabasia/gutenberg
that referenced
this pull request
Dec 4, 2024
…ordPress#67370) * Block Bindings: Move the place when the attributes get overriden * Fix failing unit tests * Wrap Edit with bindings logic only when the block supports it * Extend the test plugin with `editor.BlockEdit` filter * Add a new test covering the extensibility inside inspector controls * Fix the issue with missing context established by sources Co-authored-by: gziolo <[email protected]> Co-authored-by: SantosGuillamot <[email protected]>
michalczaplinski
pushed a commit
that referenced
this pull request
Dec 5, 2024
…67370) * Block Bindings: Move the place when the attributes get overriden * Fix failing unit tests * Wrap Edit with bindings logic only when the block supports it * Extend the test plugin with `editor.BlockEdit` filter * Add a new test covering the extensibility inside inspector controls * Fix the issue with missing context established by sources Co-authored-by: gziolo <[email protected]> Co-authored-by: SantosGuillamot <[email protected]>
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?
It wasn't possible to use
editor.BlockEditfilter to extend the block with additional controls and successfully usesetAttributesorattributesfor connected attribute using Block Bindings.Why?
The way all the logic for enhancing
attributesandsetAttributeswas connected too deep in the React components chain. It was below the point whereeditor.BlockEditallows overriding the Edit component. In effect the defaultattributesandsetAttributeswere passed to the component as if it wasn't connected with Block Bindings mechanism.How?
Refactored the code to move the logic higher in the hierarchy so all required child blocks receive access to enhanced
attributesandsetAttributesprops.Testing Instructions
I included a new e2e test that covers the extensibility point for
editor.BlockEdithooks that uses the enhanced props correctly by consuming them through input field injected into inspector controls:The simplest way to test it is using the Gutenberg Test Block Bindings plugin included with e2e tests:
text_field_value.Content.