-
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
Show ellipsis menu in the List View #35170
Show ellipsis menu in the List View #35170
Conversation
@mtias @jameskoster @kevin940726: What do you think of this as an alternative to / in addition to "List view as an icon button" in #33926? If we're into the basic premise then I can work on polishing it up. It's much easier to do this than to add a dedicated button as it exists already and so we don't have to solve #32294. |
Size Change: +2.83 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
I think this could be fine to try, but the hover outline effect feels a bit awkward now in this split view. Perhaps we should consider removing it. |
This is cool. I took it for a spin. Here's a GIF of the experience in the site editor: I'm seeing a few things there:
Here's the list view in the post editor: Note how I can click to select a block and it scrolls into view. It's possible the difference in behavior is due to the ellipsis being optimized for the navigation context, where selecting the block closes the modal: I know that @gwwar has been looking at some improvements to the nav block list view recently, she might have some insights into the inner workings. Aside from getting the select behavior right, I wonder if we should change what makes the ellipsis appear. It feels a bit noisy, and the fact that the blue focus style doesn't extend the full width tips the balance a bit. Could we make the ellipsis appear only on-select? |
Personally I don't mind the ellipsis appearing on hover, but agree that the outlines are looking a bit odd. I suppose it was built this way since outline.mp4Alternatively, we can argue that if the ellipsis appears on hover then it is essentially doing the same job as the outline – visually highlighting the hovered block: no-outline.mp4If so we can probably remove it altogether as Matias suggested. |
To clarify, I'm fine with the ellipsis appearing only on hover / focus — I don't think the split outline is working well. |
I think it's great to give folks more options to perform actions from list view. I've found myself trying to delete items from the interface. Have we considered a right-click to show options, or has this already been ruled out? These are secondary actions and I personally find the ellipsis to be distracting in this context. Not rendering another column would also leave open the option to move away from using tree-grid. I'd need to double check this, but I think the keyboard shortcut could be the menu key or shift+F10 |
As the project becomes more application like, I do think a right-click menu could eventually make sense, but it seems like something to explore once it becomes more necessary. At the moment there are valuable options in the default right click menu. To reduce the noise, the ellipsis could be visible only when a block has been selected first. Not a strong opinion. |
Deleting and insert before / after would be great to have in the menu. |
I think this is great and a better experience than icon button in #33926. We just need to figure out the design and the accessibility story of it (tab order, outline style, etc). |
It should use a roving tab index, and be navigable using arrow, just testing and it still works, so hopefully just the visual style to resolve.
Good observations. IIRC it was done this way because List View used to be in a popover, and selecting the block would unfocus List View causing the popover to close. So the solution was a 'soft selection', but this might be buggy now. Also no longer needed in the site editor with the persistent list view, so this could be changed. There's also a 'Go to block' option in the ellipsis menu that can be removed if that does change. |
@jameskoster I think a user using keyboard navigation would still need to know which button has focus, the button that selects the block or the button that opens the dropdown. Maybe hover can behave differently though. What do you think? |
Yes, we need the outlines for focus but not hover. |
Thanks for the feedback everyone! I'll work on showing the outline on focus but not hover and removing the "soft selection".
@mtias: These three options are already in the ellipsis menu. Or am I misunderstanding you? 🙂
Yep as @talldan says the accessibility story here is already pretty good as it uses a tree grid with roving tab index. It's not until we want to add a variable number of action buttons to the List View that things get complicated, see #32294. I think this can be "later's problem" for now. |
@noisysocks I mean that it's great to get them finally exposed as part of this work :D |
OK, I've removed the "soft selection" so that selecting an item in the List View always selects the block and made it so that the button outline appears on focus but not hover. I've also enabled these changes in the post editor so that the ellipsis menu can indicate which item is being hovered over as per the second mockup in #35170 (comment). It's also just a useful thing to have. I think we need to do something about the blue background that appears when a block with children is selected, but I'm not sure what. It looks a little awkward that the background doesn't go to the edge of the sidebar: If we do make the background go to the edge, it looks even more awkward because of the button focus outline: Any ideas? Or is it only a problem in my mind? 😀 |
packages/block-editor/src/components/block-settings-menu-controls/index.js
Show resolved
Hide resolved
packages/edit-site/src/components/edit-template-part-menu-button/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/edit-template-part-menu-button/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-settings-menu-controls/index.js
Show resolved
Hide resolved
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.
LGTM 👍
Here's a quick before shot: Here's after:
Let's address this before we merge. The current state is not great. I'll dive into a quick mockup. |
Question: how do I set focus on the block, from the list view? Roving focus and enter selects the block in the list view. But how do I get to the block in the canvas? |
eb8e182
to
510af3e
Compare
Looks great! Thanks so much. I'll get to work on it.
Fixed. Good catch 😀 |
The ellipsis turns blue on hover. It's not particularly noticeable though. |
The behavior now feels pretty solid: But it would be the cherry on top if we could also get the full-width-on-select style going before we land this. |
OK, I think I've got the focus styling working as described in #35170 (comment). Please feel free to push up any tweaks. Kapture.2021-10-08.at.18.30.45.mp4 |
Yes, makes sense to make it visible on select all the time. |
Sorry, I meant the white ellipsis that appears on the selected block. |
Done!
Ah I see! A hover state would be nice. How should it appear, though? We can't turn it blue because then it will not be visible against the blue background. |
I think this is in a pretty good state so going to merge once tests pass. We can keep iterating in follow-up PRs 🙂 |
cc @noisysocks @kevin940726 I think this one broke expand/collapse clicks. Keyboard events look okay Edit: added #35526 CleanShot.2021-10-11.at.11.28.34.mp4CleanShot.2021-10-11.at.11.28.47.mp4 |
Oops! Thanks @gwwar. |
Description
Enables the List View's ellipsis menu (block settings menu) in the Site Editor. This lets you copy, duplicate, remove, etc. blocks from within the List View.
It's a fairly straightforward PR because this functionality was added in #22427 but only enabled in the Navigation screen.
It also means that you can access the isolated template part editor by opening the menu and selecting e.g. Edit header. As such, this PR serves as a "quick win" alternative to "List view as an icon button" in #33926.
How has this been tested?
Screenshots
Kapture.2021-09-28.at.15.53.55.mp4
Checklist:
*.native.js
files for terms that need renaming or removal).