-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DataViews: allow register/unregister fields #67175
Conversation
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: +302 B (+0.02%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
@@ -28,29 +24,44 @@ interface Author { | |||
} | |||
|
|||
function usePostFields(): UsePostFieldsReturn { | |||
const postType = 'page'; // TODO: this could be page or post (experimental). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have two post types that work with the same fields: the Pages
page pulls data for the page
post type, and the experimental post screen (enable in "Gutenberg > Experiments > Redesigned post dashboard") does the same with posts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if ultimately these hooks should received kind/name just like the registration function. I think we'll have this use-case pretty quickly when we start working on the tags/categories dataviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, we'd have:
useFields( 'postType', 'pages' )
instead ofusePostFields( 'page' )
useFields( 'taxonomy', 'category' )
instead ofuseTaxonomyFields( 'category' )
Under the hood and conceptually, fields and actions are already connected to core-data. Making the kind
an argument could work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given we would also use this for taxonomies would it make sense to move these actions/selectors to the core
data store instead? Given this is more entity specific.
Or in general do we expect every place that uses DataViews/DataForms within WordPress to also import the @wordpress/editor
package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conversation is relevant: plan is to move them to @wordpress/fields
package when it becomes stable.
unlock( registry.dispatch( editorStore ) ).setIsReady( | ||
'postType', | ||
postType, | ||
'fields' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we avoid this argument and just have a global registration function for entities that register both actions and fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed registerPostTypeActions
to registerPostTypeSchema
and consolidated everything there 3e6782e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good so far
); | ||
|
||
const fields = [ | ||
featuredImageField, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need the post type config to add some fields conditionally? For example featured image is the post type supports it, etc..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! But I don't want to do it in this PR, I'd rather work on small steps that land fast :)
There's a few other things we need to do: for example, somewhere we need to keep a list of post types <=> fields (see related conversation), but also a bit of validation (return early if post type is unknown), etc..
Would that work for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tracked this follow-up in the overview issue #61084
@ntsekouras @youknowriad would you think this PR needs to do anything else before landing? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nik has a good point about the post type supports but we can do that later.
Part of #61084
What?
This PR updates the
usePostFields
hook to take the fields from a central registry. It also implement the low-level APIs to allow register/unregister a field for any post type.This new API is only available in the Gutenberg plugin and it's private for now.
Why?
This follows suit of what we've done for actions. It is part of how we envision making the entities (dataviews) extensible, see #61084
How?
wp.editor.registerEntityField
andwp.editor.unregisterEntityField
. 7443615usePostFields
hook to take fields from the registry. 8649f93isReady
state to acomodateactions
andfields
. df078fdAfter feedback:
registerEntitySchema
and have a singleisReady
for the post type: 3e6782eTesting Instructions
Go to the Pages page and verify that everything works as expected.
Test unregistering a field. After unregistering it, the field won't be available. In the Pages page, open the console and type:
Screen.Recording.2024-11-20.at.18.23.10.mov
Screen.Recording.2024-11-20.at.18.24.40.mov