-
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
Remove parent block selector from Block Settings Menu #50443
Conversation
Size Change: -138 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
Flaky tests detected in b45909e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4919020883
|
My initial impulse is good eye. Yes let's remove it. Then.... I began thinking of various ways a user might want to do something and might be used to do something. So many different paths to take. Some settings are repeated in multiple places. I checked a dev site which has Gutenberg plugin version 15.7.0 active. Clicking the Stack block and the 3 dot drop down menu the first option I see is copy block. I do not see a "Select parent block (stack)"... |
You have to have a parent block, to see the parent block selector. Select a block within the Stack to see the control.
It's more about when and where controls are replicated—and in some cases its fine. In this case however, it shifts all the other controls down. If you're used to seeing "Copy block" as the first item, you won't — sometimes. That sometimes leads to the editor feeling inconsistent. |
I agree quite strongly: there is no need for the duplication here. I'm not sure why it was added in the first place. And because of that question (why was it added in the first place), I'll just widen the ping range a bit to see if anyone has context. CC: @WordPress/gutenberg-core — if no-one chimes in, let's remove it. Less, but better! |
I believe it was introduced as an enhancement for smaller screens when the parent selector in Block Toolbar isn't visible. Context: #23800 (comment) and #40105. |
Oh that's useful context. Don't we still have clickthrough on mobile? That is, you select bottom-up parent first? |
Can we add it just on mobile for now? |
Plus one. That would likely also benefit embedded contexts. |
Different people use different workflows... I realize that this button seems like duplication, but that's not necessarily a bad thing - unless we want to force all users to take the same path. I'm not against removing this button... But it's useful to keep in mind that not everyone works the same way. What seems like the best/only method to some is a nightmare to others. If we remove the button, then we're making the list view an essential part of the editing experience. If that is the case, then it should probably not be hidden/collapsed by default (not everyone knows how to find it). If the list view is exposed by default, new users won't have to keep searching for buttons. It's there, and they can hide it if they don't need it. |
@aristath just making sure I understand both you and the PR correctly. My understanding of the PR is that it removes the "Select parent" menu item in the ellipsis menu, not that it removes the button from the block toolbar. Which button are you referring to? If the one in the toolbar, it should remain. It should also exist in top toolbar mode, btw, and the fact that it doesn't yet is actually an issue to fix. It exists in the mockups: |
My bad! I misunderstood the purpose of the PR and thought it was removing the button from the toolbar, not the ellipsis menu! In that case, yes, I agree 💯 |
Thank you for your effort; admittedly, I didn't know until just now that I could select the parent block within the block toolbar; I had been selecting the parent block with the parent block settings (which is proposed to be removed with this PR) or with the List View. In my experience small UX changes like this, if the users don't perceive that the changes improve the editing experience, confuse editors and foster frustration. (trying to get actionable feedback in an large open source project from users base whose preferences and familiarity with the block editor vary greatly, is probably difficult, but perhaps worthwhile). Since the parent block selector is still available in the block toolbar, I feel more comfortable since no features or functions are being taken away from users. |
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 love how much this PR removes code from this menu :) Tested and the menu itsm isn't there and nothing else seems to break.
@Mamaduka what would we need to do, or should we tag, about making this conditional? |
@richtabor, sorry this got lost somewhere in my backlogs. We'll need to conditionally render the menu items for mobile screens when the parent block selector isn't available in Block Toolbar. |
Is that something easy/likely to get in soon — or should we go for an alternative (moving the control to the last item in the MenuGroup)? |
@Mamaduka just following up again here. Is this something you could help with? Should it happen before this PR? |
@richtabor, sure! But probably after WCUS 👍 |
@jasmussen what's wrong with adding the parent selector on mobile in the block toolbar just like the one on the top toolbar current implementation? |
Nothing? To an extent it was designed to work in both cases. See #52032. |
@Mamaduka whats the blocker here? We could alternatively move the select parent control to the bottom of the first MenuGroup for now, if we can't get movement on this. It's unexpected to shift all the other primary controls down, just because you're in an innerBlock instance. Would that be an appropriate stop-gap while figuring out how/where to move on the mobile front? |
@richtabor, I think we can close this PR now that #50443 is merged. |
What?
Removes the extraneous "Select parent block (blockName)" menu item from the Block Settings Dropdown. The parent selector is already available as part of the block toolbar.
Why?
Cleaning up and simplifying the menu in support of #49271.
How?
Removes the relevant code for the menu item.
Testing Instructions
Screenshots or screencast