-
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: Bootstrap Actions Extensibility API #62052
Conversation
14db3d4
to
73fd8f1
Compare
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: +307 B (+0.02%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
@youknowriad is there any way to be aware of the context where the post action is rendered/added? I wonder whether it makes sense to be able to control whether an action should show up in the data view vs in the post actions of a single post? 🤔 |
There are some indirect ways, we may want to introduce something like "scope" config later but for now, I believe we should encourage folks to write agnostic actions. Could be one follow-up to consider. |
Thanks for getting this started. Seems like a reasonable approach; let's see where it leads us. |
I love the idea, and this style of API should be more future-proof than using a filter or a SlotFill. |
@@ -402,4 +403,5 @@ export default combineReducers( { | |||
listViewPanel, | |||
listViewToggleRef, | |||
publishSidebarActive, | |||
dataviews: dataviewsReducer, |
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 named like that because the plan is to have a separate package eventually? Because the actions are used beyond the Data Views, so I don't really get why name it like that.
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, I can rename, what would you suggest? I guess you can think of it as the package name.
Thanks Riad! Have you considered registering an action by providing the What is the expectation and gains of having the |
This is a good alternative suggestion.
I aligned the API with the entities abstraction in core-data, so you can have actions and fields for things like site, taxonomies,... that are not post types. Based on this, I feel like the entities abstraction mapping make more sense because I see WP providing these dataviews with the same filters as well. WDYT? |
For start I think the structure we'll use in store, doesn't matter much right now and could be updated anytime. For the consumers facing part though ( Maybe we have to answer how related is the core-data abstraction with the actions API. In actions the callbacks are specified inside the action's props and they could do anything (ex view revisions). I think the core-data abstraction is used to help with the entity handling mostly. Also, can we have actions that apply for more than one entity? For example could the rename action be almost identical (at least in UI) for post types and taxonomies. Probably it could, but maybe it should be kept separate. How could this affect the API? My first thoughts would be that we could keep an internal single registration API if we want, but expose specific ones like registerPostTypeAction, registerTaxonomyAction, etc.., with respective fields ( postTypes, terms, etc..) I'm rambling a bit, but just thinking out loud to get more thoughts on this.
Can you expand a bit on this? |
name: string, | ||
config: Action< Item > | ||
) { | ||
return { |
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 know this is the bootstrap PR, but we should add validation, even if it's in a follow up.
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.
That's one of the reasons I added the types, for us at least, everything is typed, so there's less need for validation but indeed, validation for third-parties would be great. I wonder if there's a runtime lib that can use typescript types to generate validation automatically.
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 added a todo list for this to the issue.
I think the main action is:
Personally, it seems clear to me that we shouldn't try to reuse actions between taxonomies and post types for instance, in that sense a global action with "taxonomies" or "post types" configs, doesn't really make sense to me. This is the main reason that I think the proposed API is a bit better. Now there's a case that we have where we want the same action for multiple post types. So I was thinking about
to address this. |
I still think the proposed API is a bit more verbose but we can always add some wrapper functions(syntactic sugar) like registerPostTypeAction, etc.. Not that important for now. We can start with this, but I think these should be addressed here:
I'll say again that the wildcard API is something I don't like and I think we'll end up needing extra props eventually to satisfy cases like: include all, except one.. I guess that's fine though, or in such cases the With actions being extensible, we would also need to add an |
We have always refrained from having order in our APIs to avoid plugins competing for positions. So I think for this one, we should be consistent and continue until that decision is reversed. |
Yeah, I know it's a tough one, but usually we would overcome this by having a slot and render the third party components there. We'll see in the future, but I guess for example that the |
I went through @youknowriad's and @ntsekouras's discussion and have some loose thoughts:
Thinking about post types in the traditional PHP sense: CPTs don't manually re-register/re-implement every bit of functionality that they want from the base Finally, I don't hold this opinion strongly, but |
I think inherently the actions (and fields) are closer to entities than "editor". Some entities can be edited in forms outside the block editor for instance, and still have actions and fields. So for me, the usage of "editor" is just a temporary hack. I think this API makes more sense in "core-data" or its own dedicated package.
I agree but I think we shouldn't try to create a new concept now. How would you tie registering "site actions" into this API without inventing a new concept (or picking entities) |
We'd either assume the compromise and expose the entities model to users, or mediate via entity-specific functions. Don't see a better way. |
f1a45b3
to
39d7603
Compare
Flaky tests detected in 39d7603. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9461933800
|
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 don't want to block this with the illusion of indecision. Mapping the API to the entities domain seems fine, and we can always make adjustments.
@annezazu @fabiankaegy Going to merge this. It would be cool to try to get some outreach around this at some point. I wonder whether we should first make the core actions removable. |
Working on the outreach piece. Make Core post in the work and will ensure this is shared for this week's dev chat meeting. Awesome job getting this done! |
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: mcsf <mcsf@git.wordpress.org> Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
What?
This PR bootstraps an API to allow third-party developers to register and unregister post type actions.
Right now post type actions appear in two places:
Actions allow users to interact with the selected, active or bulk selected entity/post/record. The API will be initially only available in the Gutenberg plugin for some time (until we get testing, feedback, iterations...)
As mentioned on the issue, initially I'm adding this API to the editor package but I think a dedicated package is probably going to be needed quickly.
you can test the API by opening the editor (or dataviews) and calling the function like so:
The action will appear in the actions dropdown.
Note