-
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
Reuse and unify post and page actions, accross the different use cases. #60486
Reuse and unify post and page actions, accross the different use cases. #60486
Conversation
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
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: +950 B (0%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
62ab396
to
81d86ad
Compare
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 great @jorgefilipecosta, nice work! 👍
It tests well for me. I only spotted a couple of very minor styling issues and tweak for an inline comment.
When a page is in the trash and the page actions button in the site editor's sidebar nav screen is disabled, the hover styles for the button icon seem off.
In the sidebar nav screen still but for a page that isn't in the trash, the icon also appears to disappear be the button is clicked. Is that by design?
Screen.Recording.2024-04-08.at.6.37.09.PM.mp4
These tweaks could be done in a follow-up, if we want them, so as to not block this PR.
Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Hi @aaronrobertshaw, the active style has been resolved, and the PR is available at #60592. Regarding the hover issue, I have observed that we can no longer open a trashed page on the editor, so it seems to not be a problem anymore. I have added a disabled attribute to the edit button, for testing and unfortunately it has the same issue, so it seems like a general style problem with buttons on dark background. |
…s. (WordPress#60486) Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Good point. I forgot that improvement was in the works. I can confirm also it is not an issue anymore. |
const POST_ACTIONS_WHILE_EDITING = [ | ||
'view-post', | ||
'view-post-revisions', | ||
'rename-post', |
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.
The rename-post
action is probably redundant for standard post types. The title is available in Canvas, and if the post type doesn't support the title
field, then renaming should be disabled in the editor.
Motivation ========== Actions and commands -------------------- In the context of Data Views, there has been a lot of recent work towards providing a set of actions operating on posts, templates, patterns (e.g. rename post, edit post, duplicate template), and ultimately other entities. These actions, however, aren't unique to Data Views, and indeed exist in several different contexts (e.g. Site Editor inner sidebar, new Admin "shell" sidebar, Pages index view, Post Editor), so the next step was to unify actions across packages (e.g. #60486, #60754). The first unification effort led to an abstraction around a hook, `usePostActions`, but the consensus now is to remove it and expose the actions directly (#61040). Meanwhile, it has been noted that there is a strong parallel between these _actions_ and the Command Palette's _commands_, which has its own API already. This isn't a 1:1 mapping, but we should determine what the overlap is. Actions and side effects ------------------------ There is a limit to how much we can unify, because the context in which actions are triggered will determine what secondary effects are desired. For example, trashing a post inside the post editor should result in the client navigating elsewhere (e.g. edit.php), but there should be no such effect when trashing from a Data View index. The current solution for this is to let consumers of the `PostActions` component pass a callback as `onActionPerformed`. It works but there's a risk that it's too flexible, so I kept wondering about what kind of generalisations we could make here before we opened this up as an API. Extensibility ------------- As tracked in #61084, our system -- what ever it turns to be -- needs to become extensible soon. Somewhere in our GitHub conversations there was a suggestion to set up an imperative API like `registerAction` that third parties could leverage. I think that's fair, though we'll need to determine what kind of registry we want (scope and plurality). An imperative API that can be called in an initialisation step rather than as a call inside the render tree (e.g. `<Provider value=...>` or `useRegisterAction(...)`) is more convenient for developers, but introduces indirection. In this scenario, how do we implement those aforementioned _contextual side effects_ (e.g. navigate to page)? The experiment ============== It was in this context that I had the terrible thought of leveraging wp.hooks to provide a private API (to dogfood in Gutenberg core packages). But, of course, hooks are keyed by strings, and so they are necessarily public -- i.e., a third party can call `applyFilters('privateFilter'`, even if `privateFilter` is not meant to be used outside of core. This branch changes that assumption: hook names *must* be strings, *except* if they match a small set of hard-coded symbols. These symbols are only accessible via the lock/unlock API powered by the `private-apis` package. Thus, core packages can communicate amongst each other via hooks that no third party can use. For example: - An action triggers `doAction` with a symbol corresponding to its name (e.g. `postActions.renamePost`). - A consumer of actions, like the Page index view (PagePages), triggers a more contextual action (e.g. `pagePages.renamePost`). - A different component hooks to one of these actions, according to the intended specificity, to trigger a side effect like navigation. See for yourself: upon `pagePages.editPost`, the necessary navigation to said post is triggered by a subscriber of that action. Assessment ========== Having tried it, I think this is a poor idea. "Private hooks" as a concept is a cool way to see how far `private-apis` can take us, but they seem like the wrong tool for the current problem. Still, I wanted to share the work, hence this verbose commit. I think our next steps should be: - Finish the actions refactor (#61040) - Impose constraints on ourselves to try to achieve our feature goals with less powerful constructs than `onActionPerformed`. I'm still convinced we haven't done enough work to generalise side effects. Consider it along with the commands API. - Try a more classic registry-based approach for actions (`registerAction`)
Supersedes: #59849
This PR introduces a post actions component in the editor package that implements the post and page actions UI in a single place (editor packages):
And reuses the actions in the following scenarios:
This PR makes the PageActions component unnecessary and removes it.
With the merge of #60395 we will also remove TemplateActions.
Screenshots
Testing Instructions for Keyboard
Verified in all the cases where the page actions are used that they work as expected.