Block Bindings: Use post meta label from register_meta in block bindings workflows
#65099
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
[Type] Enhancement
SantosGuillamot
changed the title
register_meta in block bindings workflows
|
Size Change: +346 B (+0.02%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
cbravobernal
force-pushed
the
experiment/add-meta-post-type-api
branch
2 times, most recently
from
964b861
to
af79c2b
Compare
SantosGuillamot
force-pushed
the
experiment/add-label-to-register-meta-for-bindings
branch
from
b5dc992
to
3464cdc
Compare
gziolo
reviewed
Sep 17, 2024
SantosGuillamot
changed the title
register_meta in block bindings workflowsregister_meta in block bindings workflows
51 tasks
|
I've been taking a look at the failing e2e tests and I believe it is a matter of rebasing to |
This reverts commit a43e391.
SantosGuillamot
force-pushed
the
experiment/add-label-to-register-meta-for-bindings
branch
from
9b657e5
to
efa3b5d
Compare
|
Now that the base PR has been merged, and e2e are passing after rebasing, I believe this pull request is ready for review and potentially merge. The main changes affect the |
SantosGuillamot
requested review from
spacedmonkey,
ellatrix 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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @dannyreaktiv. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
gziolo
reviewed
Sep 18, 2024
| @@ -61,7 +61,7 @@ function BlockBindingsPanelDropdown( { fieldsList, attribute, binding } ) { | |||
| { registeredSources[ name ].label } | |||
| </DropdownMenuV2.GroupLabel> | |||
| ) } | |||
| { Object.entries( fields ).map( ( [ key, value ] ) => ( | |||
| { Object.entries( fields ).map( ( [ key, args ] ) => ( | |||
| <DropdownMenuV2.RadioItem | |||
| key={ key } | |||
| onChange={ () => | |||
There was a problem hiding this comment.
This part needs further enhancements to become more flexible. In particular:
[ attribute ]: {
source: name,
args: { key },
}In my opinion, every source should construct args argument, which for Post Meta happens to be { key }. For Pattern Overrides that would be {} or undefined, and every other source could decide what that it.
In effect, I believe the object item from the fields should have the following shape:
{
label: string,
value: string,
args: any,
}This way it's up to the implementor to provide all that's necessary to make it work when defining getFieldsList fir the source.
There was a problem hiding this comment.
I plan to create an issue to discuss the whole getFieldsList API to ensure it feels good before making it public.
I believe I shouldn't have used the args variable name here because it doesn't refer to the source args, but the object returned by getFieldsList. There are two different things:
- The binding object: It specifies which attribute is connected to which source, which can already use any
argsthey want. For example, in post meta we usekeyto specify which custom field the attribute is connected to:
"content": {
source: "core/post-meta",
args: { key: "my_custom_field" },
}
- The
getFieldsListAPI: This is used just to get a list of ALL the bindable fields in order to show them in the UI and let the user select. In this case, I believe we only need thelabeland thevalue. As it is a callback, each source can decide what to show as the label and the value.
I hope we can clarify it by improving the getFieldsList API (probably needs a new name) and that part of the code, because I agree it feels confusing right now.
|
It tests great. Let's get this one in on time for the upcoming Gutenberg plugin RC1 so it's included in WordPress 6.7. We can work on the code quality changes separately. Without the label: 
With the label, different states in the editor: 
|
gziolo
approved these changes
Sep 18, 2024
SantosGuillamot
deleted the
experiment/add-label-to-register-meta-for-bindings
branch
fabiankaegy
added
the
Needs Dev Note
This was referenced Sep 26, 2024
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?
Fixes #65066
This pull request is built on top of #64072 because it reuses the
getRegisteredPostMetaresolver.In this PR, I'm experimenting with using the label exposed there to consume it in the places where we are using block bindings. These are the use cases covered so far:
In the attributes panel
You can appreciate the difference both in the attributes section and in the dropdown.
In rich text placeholders for empty attributes
When the user can edit
When the user can not edit
When no label is provided, it still works as before
Why?
Right now, we are relying on the key, which is a bit technical. A recurrent feedback received is that it'd be nice to have a human-readable label.
How?
The main change done here is changing slightly how the
getFieldsListAPI works (still private). Until now, it was returning an object of key: value. Now, I explore returning key: args. And including in those args both the label and the value. That way, it provides flexibility for each source. In the case of post meta, we get the value usinggetRegisteredPostMetaresolver.Return from
getFieldsListAPI:Testing Instructions