-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Editor: Unify the more menu #60910
Editor: Unify the more menu #60910
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: -861 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
Good PR, seems close. Post editor before and after: The main difference here is the sorting of the tools, primarily. It really is a bit of a kitchen-sink, that section, so they don't have to be resorted, but a better sorting may be:
Site editor before and after: Same sorting item. Fine to omit the fullscreen toggle, it doesn't exist for the site editor. The big difference here is that now you can load sidebar plugins in the site editor, such as Jetpack. And Jetpack doesn't really do anything in the site editor, and it can't be pinned like it can in the post editor. Could we/should we omit this? |
For the moment, the sorting of "Welcome Guide" and "Manage Patterns" can't be modified because they are specific to each editor so they come last. For Jepatck, I kind of disagree, I think it should load in the site editor. If it's useful for the "page editor" in the post editor, it should be useful in the site editor too. |
Also I'm pretty sure the jetpack change didn't not happen in this PR. |
4b7d769
to
5b4ab8f
Compare
How useful is the "manage patterns" link? |
I should've clarified, I'm not necessarily against including it. It just seems like a biggish change, at least conceptually, and perhaps something worth extracting. Or maybe it's fine? The prominence is managed, after all.
It used to be the only way to get to these in the post editor. In the site editor the link is not very useful. Given there's now a dedicated "My patterns" section in the inserter, I wonder if we can move the link to this dropdown? Or just to that section in general? |
Flaky tests detected in 5b4ab8f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8780438026
|
There is something wrong with Screen.Recording.2024-04-22.at.11.28.56.AM.mov |
@ntsekouras yep, small mistake, it should be fixed now |
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.
How useful is the "manage patterns" link?
I guess it's more useful in non block themes.
Also I'm pretty sure the jetpack change didn't not happen in this PR.
Yes, this seems to have happened in some other unification PRs. I agree that they should be shown in both editors, although the inability to pin items is an inconsistency we should address(not here though).
Code wise this looks good to me, thanks!
Oh I didn't know bout this inconsistency, I'll look into it separately. |
It seems a bit random at this point. Why no "Manage templates", "Manage pages", etc? I'd remove it.
Do we still need this? It feels more like a dev tool that might be better suited to plugins (create block theme)? Re sorting, the menu feels very long... it might be good to migrate to the new dropdown menu component in the future and place plugins/tools in flyouts. |
For me create-block-theme should be core kind of (not all of it) but I believe core should be able to export good themes right away. |
Yes I suppose that's the point. It feels like one small part of the theme creation process, so a bit random on its own, and in this location. But if we plan to bring more theme tools and flows to core then totally fine to leave it. |
I agree the position is very random. |
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.
Everything seems to be working fine for me. I think we could live without the "Manage patterns" link, but we don't necessarily have to remove it here.
Yes, I've been thinking to land this as is (without too much impact) and consider each of these particular links on their own. |
On the plugins point, I wonder if it would be good to allow them to define a scope in which they can be accessed. e.g. "Always", "Template editing", "Focused editing", "Post editing", "Zoomed out", etc. I agree the Jetpack tools are not very useful when you're editing a template / pattern. |
@jameskoster That's already possible. I believe some plugins are already doing it because the post editor can already edit all post types. So it's just a matter of checking the active post type |
Related #52632
What?
This unifies the MoreMenu component between the post and site editors. There are still the following differences between the two menus:
In order to keep these small differences in place while sharing the same code, I've used the existing ToolsMoreMenuGroup Slot and introduced a new private and similar ViewMoreMenuGroup. The order of the menu items is slightly different than trunk because the menu items that come through the slot are inserter after the "built-in" menu items.
Testing Instructions
1- Check the MoreMenu of both post and site editors.