Block bindings: Bring bindings UI in Site Editor #64072
Merged
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
|
This is a similar use case to what @Mamaduka contributed recently for gutenberg/packages/core-data/src/selectors.ts Line 1149 in 9186964 gutenberg/packages/core-data/src/resolvers.js Line 382 in 9186964 |
|
This is probably the right PR - #59243. @cbravobernal, it would be helpful if you could expand on why this is needed. |
cbravobernal
force-pushed
the
experiment/add-meta-post-type-api
branch
from
379ef7c to
ff0cd99
Compare
The idea is to be able to add post meta data bindings to any kind of template. Like this: Screen.Recording.2024-07-31.at.12.00.44.mov |
|
Size Change: +3.26 kB (+0.18%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
Comment on lines
90
to
91
| // TODO: Last item 'post' should not be hardcoded. | ||
| .getEntityRecord( 'root', 'postType', 'post' ); |
There was a problem hiding this comment.
This one is tricky. We might need to infer from the template slug the entity it refers to, replicating the template hierarchy. I remember doing something similar in the past and coming up with a relationship table between slugs and entities. I'll explore deeper this point.
There was a problem hiding this comment.
It also largely depends on the context, when in Query Loop that explicitly picks a post type different that post, there might be different meta fields defined. In particular, in templates, that might be even more tricky as you often have general templates like single, which applies to all possible post types, but you can also go very specific when providing the template for the products archive page, which would offer more inferred specificity. Maybe for a start, the list of allowed post meta should be grouped by post type and that would be a user decision to pick the one they feel fits best. Alternatively, it could be only meta registered for all post types (when the dev passes and empty string):
|
As mentioned here, I believe one of the main challenges of this pull request is to identify in which template the user is. Depending on the template, the custom fields to show in the UI will be different. Some examples:
There are a bunch of different use cases. I believe we could create a relationship between the template and the entities it applies to using the template slug. Something similar to the classic themes Template Hierarchy. This could be beneficial not only for this use case but for other ones. Another example could be to know what should be the default of the query loop. I made an experimental commit (not finished) to showcase what I mean and covering the different template slugs that could exist: link. Maybe this information is available in another place, but I wasn't able to find it. Another possibility could be adding a new field to the Any thoughts? I'm happy to keep investigating any approach. Apart from that, we need to explore how it will work with other external sources that might face a similar problem as well. |
64 tasks
gziolo
reviewed
Aug 2, 2024
This was referenced Aug 21, 2024
cbravobernal
force-pushed
the
experiment/add-meta-post-type-api
branch
2 times, most recently
from
d2ccb1e to
71bb464
Compare
cbravobernal
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. |
cbravobernal
commented
Aug 29, 2024
|
Why is this not using the schema? This was asked here. But I don't see where it was ever addressed? |
|
Code has been updated with this change #64072 (comment) |
|
I'll try to clarify the Let's imagine I have a website with posts, pages, and movies (Custom Post Type). And I register some custom fields:
This is the code I'm usingfunction testing_fields_and_sources() {
// Register movie custom post type.
register_post_type(
'movie',
array(
'label' => 'Movie',
'public' => true,
'supports' => array( 'title', 'editor', 'comments', 'revisions', 'trackbacks', 'author', 'excerpt', 'page-attributes', 'thumbnail', 'custom-fields', 'post-formats' ),
'has_archive' => true,
'show_in_rest' => true,
),
);
// Register fields
register_meta(
'post',
'any_post_type',
array(
'show_in_rest' => true,
'type' => 'string',
'default' => 'Any Post Type',
)
);
register_meta(
'post',
'only_for_movies',
array(
'show_in_rest' => true,
'type' => 'string',
'default' => 'Movie Specific',
'object_subtype' => 'movie',
)
);
register_meta(
'post',
'only_for_posts',
array(
'show_in_rest' => true,
'type' => 'string',
'default' => 'Post Specific',
'object_subtype' => 'post',
)
);
}
add_action( 'init', 'testing_fields_and_sources' );So far so good. Let's review how this translates while creating connections in templates. Depending on the template I am, i expect to see different fields.
This last case doesn't work because we are adding a fallback to Basically, we wanted a way to get the fields registered WITHOUT Having said that, if it isn't clear how to do so, I believe we could work on that part in a follow-up pull request. I don't think registering a meta field with Apart from that, I made this commit which potentially fixes the |
SantosGuillamot
approved these changes
Sep 17, 2024
There was a problem hiding this comment.
The pull request looks good to me aside from the problem mentioned here where fields registered only for "posts" are shown in templates that could apply to any post type. But I believe we can explore that in a follow-up PR.
I'd appreciate a second check because I have been deeply involved in the code.
Some aspects I checked:
- I can connect custom fields in CPT templates like "movies" or "videogames".
- Only the relevant fields are shown in those templates.
- It shows the default values and it fallback to the meta key when they don't exist.
- Other contexts like post editing seem to work as before.
|
In the latest commit, I removed support for templates that can apply to any post type. I think it is safer to go this way and explore how to handle it in a follow-up PR. |
|
Merging this so it lands in next Gutenberg release. |
gziolo
reviewed
Sep 18, 2024
gziolo
reviewed
Sep 18, 2024
gziolo
reviewed
Sep 18, 2024
gziolo
reviewed
Sep 18, 2024
gziolo
reviewed
Sep 18, 2024
|
I left a couple of notes to better understand the implementation. Great work figuring out how to expose the list of meta fields. When paired with the new label property on meta fields, this is going to work nicely in WordPress 6.7. |
gziolo
reviewed
Sep 18, 2024
| // `useSelect` is used purposely here to ensure `getFieldsList` | ||
| // is updated whenever there are updates in block context. | ||
| // `source.getFieldsList` may also call a selector via `registry.select`. | ||
| const _fieldsList = {}; |
There was a problem hiding this comment.
What is the reason to keep that const outside of useSelect and modify it with the selector?
There was a problem hiding this comment.
If I move the const inside the useSelect, it interprets it is a new object each time it goes through it, resulting in this warning:
The 'useSelect' hook returns different values when called with the same state and parameters. This can lead to unnecessary rerenders.
I must say I am not fully familiar with how this should work, but it is the only way I found to avoid that message.
There was a problem hiding this comment.
Ideally you should memoize the fieldsList value but here it's almost impossible because the data come from so many different sources. Any getFieldsList callback can select from any store it wants.
The easiest solution is to:
- return the
fieldsListas the top-level object in theuseSelectresult.const fieldsList = useSelect( ... ) - move the
canUpdateBlockBindingsselect to its ownuseSelectcall. The two selects don't have anything in common anyway, the select from completely different stores.
That should remove the warning.
There was a problem hiding this comment.
I've started exploring this here. As commented there, it seems just making these changes is not enough. We can keep the discussion in the other PR.
gziolo
reviewed
Sep 18, 2024
This was referenced Sep 18, 2024
2 tasks
97 tasks
This comment was marked as off-topic.
This comment was marked as off-topic.
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?
Include registered
metadata to the core-data schema, so the user can list the registered meta values on the block bindings UI when editing a template.This allows creating layouts by calling custom meta data and custom binding sources, using the UI.
Testing Instructions
1.) Register your meta in a PHP snippet
all_templates_keycan be selected.videogame_coverandall_templates_keycan be selected.