Block Bindings: Create utils to update or remove bindings #64102
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
SantosGuillamot
added
[Feature] Synced Patterns
SantosGuillamot
requested review from
talldan,
gziolo,
artemiomorales,
kevin940726 and
cbravobernal
SantosGuillamot
requested review from
tellthemachines and
ellatrix
as code owners
SantosGuillamot
changed the title
|
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. |
|
Size Change: +36 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
61 tasks
artemiomorales
approved these changes
Aug 16, 2024
| @@ -286,7 +231,7 @@ export const BlockBindingsPanel = ( { name, metadata } ) => { | |||
| } ); | |||
| // Only add source if the list is not empty. | |||
| if ( sourceList ) { | |||
| fieldsList[ label ] = { ...sourceList }; | |||
| fieldsList[ sourceName ] = { ...sourceList }; | |||
There was a problem hiding this comment.
Why were we using the label here before? It seems like that was brittle. It appears that this change is unrelated to the adding of the utils in this PR, and is just an additional code cleanup, and it looks good. I'm just curious.
There was a problem hiding this comment.
It seems that we were just using the values of the registeredSources object instead of their keys.
I'm not sure if if sources label were always the same as the registeredSources key, but this approach seems safer, as the label should allow whitespaces or uppercases.
There was a problem hiding this comment.
Why were we using the label here before?
I believe the name wasn't exposed before. But this seemed safer to me as well.
gziolo
reviewed
Aug 23, 2024
Comment on lines
+86
to
+88
| const { metadata } = getBlockAttributes( clientId ); | ||
| const newMetadata = { ...metadata }; | ||
| delete newMetadata.bindings; |
There was a problem hiding this comment.
That could be a single line:
const { metadata: { bindings, ...metadata } } = getBlockAttributes( clientId );
gziolo
reviewed
Aug 23, 2024
| export function useBlockBindingsUtils() { | ||
| const { clientId } = useBlockEditContext(); | ||
| const { updateBlockAttributes } = useDispatch( blockEditorStore ); | ||
| const { getBlockAttributes } = useSelect( blockEditorStore ); |
There was a problem hiding this comment.
It would be more optimal to have:
const { getBlockAttributes } = useRegistry().select( blockEditorStore );See more in this guide.
gziolo
reviewed
Aug 23, 2024
| bindings: newBindings, | ||
| }; | ||
|
|
||
| if ( Object.keys( newMetadata.bindings ).length === 0 ) { |
There was a problem hiding this comment.
Here and in other places it would increase the readability if the helper function was present:
function isObjectEmpty( object ) {
return ! object || Object.keys( object ).length === 0;
}
// later
if ( isObjectEmpty( newMetadata.bindings ) ) {This could also be further refactored so it doesn't require the step to remove the undefined key:
const newMetadata = {
...metadata,
...( ! isObjectEmpty( newBindings ) && {
bindings: newBindings,
} ),
};
|
Nice refactoring. It looks like two methods allow to accomplish all necessary operations: const { updateBlockBindings, removeAllBlockBindings ) = useBlockBindingsUtils();
// Add or update a binding.
updateBlockBindings( { attributeName: { source: 'my/source' } );
// Remove a binding - that could also be included in the inline docs.
updateBlockBindings( { attributeName: undefined );
// Remove all block bindings for a block.
removeAllBlockBindings();When opening this API to the public, it might also be worth considering allowing passing a different // Remove all block bindings for another block.
const { removeAllBlockBindings ) = useBlockBindingsUtils( anotherClientId );
removeAllBlockBindings(); |
bph
pushed a commit
to bph/gutenberg
that referenced
this pull request
Aug 31, 2024
…64102) * Add `useBlockBindingsUtils` * Use utils in custom fields UI * Use utils in pattern overrides * Properly populate custom fields * Add comments * Update wrong comment Co-authored-by: SantosGuillamot <[email protected]> Co-authored-by: artemiomorales <[email protected]> Co-authored-by: cbravobernal <[email protected]>
bph
added
the
Needs Dev Note
|
@SantosGuillamot @gziolo @cbravobernal
Curious, was there some boilerplate code y'all used to test this? I wanted to test it, but did not see anything in the notes of this ticket. 🕵️ 🤔 |
|
When this was first added, it was still a private API used by You can find an example of how it could be used in the readme. However, trying to create a quick code snippet, I've realized that we didn't address passing a |
|
I've started this pull request to explore that possibility: #65818 It's hard to test manually because hooks need to run inside components or hooks, but I add some unit tests that should serve as reference and to give more confidence on the APIs. |
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 requests explores the idea of exposing two new block bindings utils to easily add/remove/update bindings in the editor:
updateBlockAttributes. You pass an object with all the bindings to update, and to remove one users can passundefined.Why?
The same code was used for the custom fields UI and pattern overrides. Additionally, it could be useful for external developers in case they want to create their own UIs.
How?
Creating a
useBlockBindingsUtilshook that return the needed function. It is inspired by other hooks like useGradient.Testing Instructions
It doesn't add new functionalities. Automated tests should pass.