[6.7] Zoom-out (contentOnly): show more menu in block toolbar #66311
Conversation
ellatrix
added
[Type] Bug
|
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: +59 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
Could we just show duplicate and delete? I find having group, ungroup, and create pattern to be an odd experience, in particular group/ungroup as nothing changes visually: testinggggg.mov |
|
@annezazu I had just deleted them yes. |
|
Hmm noticing an empty menu randomly. I'm not quite sure why it appears but I can replicate it if I play around with this PR enough:
Here's a quick video where this showed up: testing.more.mov |
|
I'd expect "Copy" as well. Perhaps "add before" and "add after", though those could be separate—to engage with the zoom out behavior that occurs when using one of those inserters. This is what I'd consider ideal:
|
I suspect it's from the grouping you did, which is now gone. Does it still appear after that change? |
|
@richtabor Copy was intentionally removed in #61127. Are you sure we should restore it here? |
|
@getdave Does this appear in trunk as well? What content do you have to reproduce it? |
|
Ah, I see it on the demo page actually, sorry. |
|
We ehave no guarantee what kind of selection this applies to so the contents of the settings dropdown varies a lot. |
|
@getdave So the reason you see that button is because #65560 (from @youknowriad) was not backported. |
|
I'm still seeing "create pattern" as an option.
Can't replicate anymore 🥳 |
Looks like we now have that PR backported in #66339. |
getdave
added
the
[Priority] High
|
Testing locally with the backport applied I'm still seeing the rogue menu items when editing These items are adding a lot of additional complexity to Zoom Out mode in 6.7. As such I"m bumping up the priority on this one with a view to finding a fix before RC2 next week. I think we should be aiming for this UI which also means getting rid of the 
|
I've pushed a commit that may solve that (feel free to do what you want with it!), essentially not showing those options for any children of the post content block. I think this makes sense, as the children of Post Content are not ever considered part of the template AFAIK. I'm still seeing 'Create pattern' a lot when testing. |
|
Thanks @talldan. This does indeed solve the Issue of the unwanted "Template" items in the Site Editor. Unfortunately it does not seem to fix the Post Editor (try editing the same 
I'm wonder whether we just skip rendering these items in Zoom Out mode? It's unlikely we'd ever want them. I feel like this is a valid case to tie this logic to. Also, @richtabor how do you feel about getting ride of the |
|
Noted above on my end but I’d like create pattern to go away personally. It’s a confusing UX as is if you click it. |
|
Why didn't we rebase this first? |
|
Given @getdave's commit, I'll revert my one. |
talldan
force-pushed
the
try/show-block-toolbar-more
branch
from
0befd2f
to
6ed5370
Compare
Now done. Rather than revert I dropped the commit I added, as it meant not solving a conflict. Was there some other change in wp/6.7 that would be beneficial? |
I think #66339 |
|
Well, looking at the latest changes that component is now entirely hidden? Is that needed after #66339? |
getdave
force-pushed
the
try/show-block-toolbar-more
branch
from
6ed5370
to
d321d31
Compare
I rebase against latest |
| { ! isZoomOut && ( | ||
| <ContentOnlySettingsMenu /> | ||
| ) } |
There was a problem hiding this comment.
Would it be better to subscribe to state only in the ContentOnlySettingsMenu? That would isolate the subscription and also keep it closer to the affected UI.
The downside would be that any subscriptions in ContentOnlySettingsMenu would be run.
|
Flaky tests detected in 0050f8a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11659714780
|
Yes, let's take it out. |
|
I've added copy back and removed the pattern related group of options. |
|
Should this be targeted at 6.7? Technically, it's a feature, and we're already in the RC phase. Could we punt this to 6.7.1 or 6.8? Sorry if I'm missing something! c.c. @colorful-tones @ndiego |
|
The main piece we need for 6.7 is the ability to delete using the more menu. I say this needs to land for the release. |
There was a problem hiding this comment.
The main piece we need for 6.7 is the ability to delete using the more menu. I say this needs to land for the release.
I agree. It can be tricky this close to the last RC before the final release, but from my perspective this would be good to get in:
- The change is relatively small and doesn't feel too dangerous to land to me
- The ability to delete from the block settings menu is important enough to warrant this change
- Being able to copy, duplicate, and delete feels like a good simple set of controls to me
- It's testing well for me in the site editor, page editor, and post editor in zoom out mode, and I haven't encountered any issues not in zoom out
I only had a small amount of time to test this before wrapping up for the day, so it might be worth getting a second review on this, but it LGTM and I think would be good to get in for 6.7 if we can.
kevin940726
force-pushed
the
try/show-block-toolbar-more
branch
from
44fa058
to
0050f8a
Compare
|
I rebased the branch to see if it passes the CI. |
|
We also need this merged addressed in |
What?
Since zoom-out mode is automatically
contentOnlymode, this condition will cause the "more" menu in the block toolbar to disappear. It's actually shown in trunk and was fixed here:https://github.com/WordPress/gutenberg/pull/65485/files#diff-9ac2a4466e7c0a85aaff0b40c5d7bc64c894132528ca3c879db4bebb438617dcL245-L247
I do wonder if we should try to backport all of #65485 or not. Cc @youknowriad.
Also, since this is
contentOnlymode, should the group/ungroup buttons be removed? I went ahead and removed them, but not sure if they were left in place intentionally.Why?
There's no way to trash patterns in zoom-out mode in 6.7 without the use of the keyboard. Fixes #66287
How?
Remove the requirement to be in the default editing mode.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast