Block Bindings: enhance block attribute binding to external sources #58895
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
retrofox
requested review from
Mamaduka,
gziolo,
ellatrix,
michalczaplinski and
SantosGuillamot
|
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. |
retrofox
force-pushed
the
update/add-block-binding-support
branch
from
7f7d9ec
to
d24e430
Compare
retrofox
added
the
[Feature] Custom Fields
retrofox
force-pushed
the
update/add-block-binding-support
branch
from
b7a0188
to
80a49fc
Compare
|
Size Change: +152 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 52c04f3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8010367582
|
retrofox
force-pushed
the
update/add-block-binding-support
branch
from
454c3ce
to
688c4db
Compare
retrofox
commented
Feb 12, 2024
packages/block-editor/src/components/block-binding-support/index.js
Outdated
Show resolved
Hide resolved
retrofox
force-pushed
the
update/add-block-binding-support
branch
from
a786604
to
eb036e5
Compare
|
There is no such concept as entities on the server. How do you envision |
I haven't thought too much about that scenario :-(, for different reasons. The main one is that it isn't a requirement in our development. Handling the data propagation in the editor's context would, in principle, be sufficient for our purposes. In any case, and now thinking like part of core, I guess the ability to link entities with the attributes of any block would be a more than exciting feature. In that case, rendering the HTML on the backend for the blocks bound to the entities seems to be a good challenge. Within the function responsible for processing and generating the markup, we should collect the data that is ultimately provided by the endpoint linked to the entity. Finaly, once the data has been collected, regenerate the markup but with the entities, ignoring the static data provided by the block attributes. Pretty simple to say :-) |
|
It should be handled by the gutenberg_process_block_bindings function? |
packages/block-editor/src/components/block-binding-support/with-block-binding-support.js
Outdated
Show resolved
Hide resolved
|
Nice work @retrofox ! Aside from the question of whether should / want to create a separate I'd like that we make sure to address any discrepancy between the current implementation and this new one: e.g. the placeholders I mention above. |
retrofox
commented
Feb 12, 2024
packages/block-editor/src/components/block-binding-support/with-block-binding-support.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-binding-support/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-binding-support/index.js
Outdated
Show resolved
Hide resolved
|
I took the PR for a spin yesterday. I just had to adapt the post meta source to the new format and I also added another |
|
Overall, this tests well for me now! 🙂 The next step for me would be to address the remaining feedback and make sure it works with the placeholders. I am not yes 100% sure that we haven't missed some functionality while refactoring from the previous iteration so would have to test it some more! |
I'd like know a little bit better about this one, since I think having only one effect could avoid race conditions, infinite loops, etc |
retrofox
commented
Feb 14, 2024
packages/block-editor/src/components/block-binding-support/with-block-binding-support.js
Outdated
Show resolved
Hide resolved
Hmm, I think it should be possible to move everything to one useEffect and then have another effect just for setting the initial value. Let me try that!
Damián beat me to it. 🙂 |
retrofox
force-pushed
the
update/add-block-binding-support
branch
from
5d5bbdb
to
9c577be
Compare
Co-authored-by: Michal <[email protected]>
Co-authored-by: Michal <[email protected]>
michalczaplinski
force-pushed
the
update/add-block-binding-support
branch
from
abac79e
to
db32d18
Compare
|
Closing (at least for now) in favor of #59403 |
For anyone following along, I’d like to clarify next steps. We intend to land the majority of changes included in this branch later this week with new PR to fix the known issues present in the WordPress 6.5 release. That's why the part where the reduced in the block editor store updates whenever the connected custom data source changes is going to be further explored seperately. |
gziolo
removed
the
Backport to WP 6.6 Beta/RC
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?
This pull request enhances the mechanism by which block attributes are bound to external source properties. The primary aim is to prevent incorrect hook calls that adversely affect performance.
On this PR:
Component Introduction: Two new components,
<BlockBindingBridge />and<BlockBindingConnector />, have been introduced. These components isolate and encapsulate connecting block attributes with external source properties.Hook Invocation Improvement: To circumvent the issue of incorrect hook invocation within a loop, the source object is now passed down to the
<BlockBindingConnector />. This ensures that the hook is called at the top level of the component, aligning with React's best practices.API Modification: The
useSource()API has changed. Previously, a useValue method was provided, but it has been removed in favor of directly exposing the source property's value and its handler through value and updateValue props, respectively. The updated API structure is as follows:Why?
Because calling the hook wrongly is not recommended. It could bring performance issues.
How?
This PR addresses the issue by introducing and components to streamline the connection between block attributes and external sources, thereby preventing incorrect hook calls. It refines the useSource() API to directly expose the source property's value.
Testing Instructions
Pretty similar to #58085
Test paragraph
text_custom_fieldvalue is defined ((001) Meta valuein my testing block), then it should be shown in the UI: