Skip to content
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

ToolsPanel: It's difficult to close the ToolsPanel menu #41546

Open
apeatling opened this issue Jun 3, 2022 · 47 comments
Open

ToolsPanel: It's difficult to close the ToolsPanel menu #41546

apeatling opened this issue Jun 3, 2022 · 47 comments
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Bug An existing feature does not function as intended

Comments

@apeatling
Copy link
Contributor

apeatling commented Jun 3, 2022

Prior: #41376

Description

With the changes in #40716 the ToolsPanel menu now stays open when selecting a specific option. A byproduct of this is that the menu now has no clear way to be closed as previously it would close upon selection.

By clicking outside of the menu you may inadvertently select something else on the canvas, causing the sidebar state to be lost.

2022-06-03 10 17 33

@apeatling apeatling added [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Bug An existing feature does not function as intended labels Jun 3, 2022
@apeatling apeatling moved this to ⏱ Not Started in Design Tools Project Jun 3, 2022
@apeatling
Copy link
Contributor Author

apeatling commented Jun 3, 2022

When I use this, I naturally expect to be able to click anywhere in the browser window to close the menu. @bdurette made the same observation in the previous thread.

This seems like a no brainer to me as a first iteration. Although we don't have any other examples of this in Gutenberg outside of modals -- I would bet most users would also naturally expect to be able to do this. Trying to do this is the reason this issue has come up.

Going further we could also explore a dedicated close button within the menu in addition to being able to click away. Figma is an example of this:

Screen Shot 2022-06-03 at 9 57 16 AM

@apeatling apeatling added the Needs Design Feedback Needs general design feedback. label Jun 3, 2022
@fotisps
Copy link

fotisps commented Jun 5, 2022

Close icon with close text would be a good solution in order to close each popup that could accidentally lose the state by misclicking. It also makes it clear to the user how the primary closing works rather than giving the false impression that closing only comes from clicking outside.

@jameskoster
Copy link
Contributor

I naturally expect to be able to click anywhere in the browser window to close the menu. [...] This seems like a no brainer to me as a first iteration.

I agree, this feels like a good first step.

A dedicated close button already exists – re-clicking the ellipsis will close the menu. Perhaps there's something we can do to make that more obvious?

@mtias
Copy link
Member

mtias commented Jun 25, 2022

I'm also not sure I agree with the menu not self-closing when you select an item. Seems to be optimizing for the less common cases. Can we look at reverting that part?

@apeatling
Copy link
Contributor Author

@talldan I think you changed the menu from self closing for accessibility reasons. Am I remember this correctly? Are there any other options here that would keep the menu self closing when selecting?

@talldan
Copy link
Contributor

talldan commented Jul 8, 2022

@talldan I think you changed the menu from self closing for accessibility reasons. Am I remember this correctly?

That's correct. The reason being that when dropdown menus close like that it results in a loss of focus.

There was also feedback on the PR that keeping the menu open is better. Previously, if a user wanted to select multiple items the only way was to keep reopening the menu.

Are there any other options here that would keep the menu self closing when selecting?

The accessibility feedback has always been that menus should stay open like this unless the act of clicking on an item deliberately transfers focus (e.g. to a newly inserted block). Perhaps focus could transfer to the newly added tool? I don't know for sure if that would be acceptable, it'd be worth getting further accessibility feedback.

A dedicated close button already exists – re-clicking the ellipsis will close the menu. Perhaps there's something we can do to make that more obvious?

The opener icon could change to an 'X'?

@jameskoster
Copy link
Contributor

I think auto-closing is worth trying, assuming that shifting focus to the added tool is acceptable from an a11y perspective.

@talldan talldan added the Needs Accessibility Feedback Need input from accessibility label Jul 11, 2022
@priethor priethor added this to Polish Apr 28, 2023
@richtabor
Copy link
Member

There was also feedback on the PR that keeping the menu open is better. Previously, if a user wanted to select multiple items the only way was to keep reopening the menu.

Agreed. I don't think we should auto-close. It's not uncommon to enable multiple tools at a time, re the typography controls.

@richtabor
Copy link
Member

Is this still relevant, or can we close?

@jameskoster
Copy link
Contributor

Looking at it with fresh eyes, it seems the problem isn't that the toolspanel menu is difficult to close, it's that clicking on the canvas to close it will likely select a different block resulting in a context shift in the Inspector.

What if: when a popover menu is open, clicking outside only has one behavior (close the popover) regardless of what you click on? This seems to be a fairly standard practise, as seen right here on github:

(Note how clicking on the Comment button doesn't actually click it, while the popover is open).

@priethor priethor moved this to Needs design in Polish Apr 30, 2023
@simonwammerfors
Copy link

simonwammerfors commented May 1, 2023

I find this part of the editor UI to be quite cumbersome. Even though I'm an experienced user of the editor by now, I often accidentally select a different block trying to close those menus in the sidebar. And then I have to navigate back to where I was. Limiting the behaviour of a click outside the menu (like @jameskoster suggests above) would help a lot.

I also often experience that the selections I make in those menus don't persist if I then accidentally navigate away from the selected block. If I select a paragraph block, navigate to the styles tab and then turn on the line-height panel, but accidentally selects another block (or just de-selects the current block) trying to close the menu, then I have to repeat the process when I select the paragraph block again. A guess that's a different issue though.

@richtabor
Copy link
Member

I also often experience that the selections I make in those menus don't persist if I then accidentally navigate away from the selected block.

Agreed. I’d expect them to persist.

@priethor priethor removed this from Polish May 2, 2023
@hanneslsm
Copy link

hanneslsm commented Jul 5, 2023

How can we move this forward? Looks like we all agree the issue exists, so here are the options:

  1. Adding a close icon
  2. Clicking anywhere should close the menu, instead of clicking the underlying element.
  3. I'd like to add that changing the position would also help, as in Figma. See screenshot above: ToolsPanel: It's difficult to close the ToolsPanel menu #41546 (comment)

How about doing all three?

@jameskoster
Copy link
Contributor

What if: when a popover menu is open, clicking outside only has one behavior (close the popover) regardless of what you click on?

This seems worth a try imo. What do you think @apeatling ?

@hanneslsm
Copy link

hanneslsm commented Jul 6, 2023

What if: when a popover menu is open, clicking outside only has one behavior (close the popover) regardless of what you click on?

This seems worth a try imo. What do you think @apeatling ?

In this case I'd like to add that it might be worth exploring to make the shadow a little bit bigger. This adds to the mental model we're acting on a different "layer" and the behaviour is slightly different than for the other options, like those:
image

This is the shadow Github uses.
image

@jameskoster
Copy link
Contributor

I tend to agree that more 'obvious' shadows would do a better job of communicating the different 'layers' in the UI.

That said it needs to be considered holistically accounting for elements like modals, snackbars, the 'frame' in the site editor, command palette, and so on. This is most likely a separate endeavor.

@joedolson
Copy link
Contributor

I have consistently found it frustrating that the ToolPanel opens directly on top of the tools it's activating. What's most natural for me is to open the tool, turn something on, and then immediately interact with the new tool. E.g., enable Line Height, then set the Line Height. Moving your focus into the Line Height tool would automatically close the ToolPanel. However, I can't do that, because the panel covers the settings.

If the ToolPanel auto closed on selection, having it cover the related tools isn't a problem; but now that it stays open, that creates a more difficult flow.

I think that adjusting the position of the tool panel to not cover the relevant tools would make this flow more natural.

It would probably also be beneficial to change the ellipse into a close icon to make it more clear that you can use it to close.

All of the problems with clicking somewhere and losing access to the tools (e.g., clicking on the canvas, focusing a block, etc.) stem from the fact that there's no way to visually distinguish between different regions of the page.

@joedolson joedolson removed the Needs Accessibility Feedback Need input from accessibility label Aug 11, 2023
@talldan
Copy link
Contributor

talldan commented Oct 21, 2023

I like it too 👍

It'd be good to make sure it works well on mobile. The duotone/color panels are ok on mobile, but could probably do with a little bit of polish.

@richtabor
Copy link
Member

It'd be good to make sure it works well on mobile. The duotone/color panels are ok on mobile, but could probably do with a little bit of polish.

@talldan I have a draft PR #55785 which moves the panel over. Ideas on how I'd position it for mobile?

@afercia
Copy link
Contributor

afercia commented Nov 6, 2023

I like both of those changes; removing that arbitrary separation would definitely help in cognitive load, and not covering the information you're supposed to be working with would be very helpful.

I'd agree both changes greatly improve the general usability and accessibility of these panels.

@hanneslsm
Copy link

@richtabor With your PR #55785 this issue can be closed, right?
Also, with #64108 the discussed elevation can be tackled in future.

@ciampo
Copy link
Contributor

ciampo commented Dec 16, 2024

Hey folks, in the context of potentially stabilizing the ToolsPanel component, could y'all help understand whether this issue is still relevant?

Do we still need to improve the way the menu is closed? If so, a few options come to mind:

  • Make the dropdown menu modal (should easy enough to do if we migrate to the private Menu component), although that has other implications (see @wordpress/components: modality of popover-based components #63674)
  • Add an explicit "Close" menu item (although I'm not sure if this sits well with the design and content of other menus in Gutenberg)

Thank you!

@afercia
Copy link
Contributor

afercia commented Dec 16, 2024

Add an explicit "Close" menu item (

The ToolsPanel is an ARIA menu. It can only contain menu items (and related variants), groups and separators. I'm not sure a Close button can be used, unless it's a menuitem but that wouldn't fit the design I guess.

More in general, I have concerns about the usage of ToolsPanel but that's out of the scope of this issue.

@jameskoster
Copy link
Contributor

It's a bit easier to dismiss the menu now that it appears to the left of the Inspector. I don't know if that's adequate to close the issue though.

It remains problematic that clicking the canvas to close the menu typically results in the selection of a different block. Based on current behavior, migrating to Menu would address that, but perhaps that behavior will change in the future depending on #63674?

@joedolson
Copy link
Contributor

I'm not sure I follow your argument, @afercia; I don't see a good reason that a Close button can't be a menu item within that menu. It seems sound to me that one of the item in a menu could be to close that menu.

@richtabor
Copy link
Member

It remains problematic that clicking the canvas to close the menu typically results in the selection of a different block.

I question how prevalent this is in practice. My theory is that you engage with the controls applied via the menu next; which is why the popover is positioned to not cover the controls.

@afercia
Copy link
Contributor

afercia commented Dec 17, 2024

I'm not sure I follow your argument, @afercia; I don't see a good reason that a Close button can't be a menu item within that menu. It seems sound to me that one of the item in a menu could be to close that menu.

@joedolson maybs I wasn't clear. That's what I meant: a Close button should be a menuitem inside the ARIA menu. It can't be a normal button outside of it.

@jameskoster
Copy link
Contributor

I question how prevalent this is in practice.

I agree it's much less of an issue now that the menu appears outside the Inspector panel.

@richtabor
Copy link
Member

Why not follow the precedent set by Radix popover for the close element?

We should do our best to not reinvent.

@jameskoster
Copy link
Contributor

jameskoster commented Dec 19, 2024

Someone else can confirm, but I believe the difference is that this UI is a menu, not a popover (Modal dialog). There's a lot of detailed discussion on this in #63674, and I think (😅) there's some consensus that menus do not require a dismiss button.

Radix' Dropdown Menu is probably a more accurate comparison.

Edit: Based on the outcome of 63674 I suppose it could eventually become possible to create a dialog menu, whereby the requirements change, and the dismiss button comes back into the equation :)

@afercia
Copy link
Contributor

afercia commented Dec 19, 2024

As mentioned earlier, The ToolsPanel is an ARIA menu. It cannot contain extraneous content.
A stray button to close it would invalidate the ARIA pattern. Anything inside an ARIA menu must be either a menu item, a group or a separator. No other content is allowed.
I would appreciate to not have to repeat the same feedback over and over again, thanks.

@richtabor
Copy link
Member

Radix' Dropdown Menu is probably a more accurate comparison.

The Radix Dropdown Menu does not have a close button.

@richtabor
Copy link
Member

There also should not be a "Close" menu item among the same list as all the other tools. It performs an entirely different action from all the other menu items in that list. Not a standard.

@jameskoster
Copy link
Contributor

My point was that if we wanted to add a dismiss button like the popover example, we'd need to re-engineer this UI to use a different component. I'm not convinced it's worth the effort. The menu is much easier to close now than it was, and feedback in #63674 suggests that esc is an adequately accessible way for keyboard users to close menus.

@afercia
Copy link
Contributor

afercia commented Dec 20, 2024

If we want the ToolsPanel to be a menu (ARIA menu) with menu/menuitems semantics and arrow keys navigation as expected in an ARIA menu, it can't contain a Close button, period.

If we want a Close button, then ToolsPanel should be refactored to a Popover and should not contain an ARIA menu. In this case, the UI inside the Popover would be just a set of buttons.

As an aside, I spent a couple minutes inspecting the mentioned Radix popover example and I have to say a few minutes were enough to realize that's not a good example to point to. While Radix claims to be a 'library optimized for fast development, easy maintenance, and accessibility' and I do see some good intent there to provide some accessibility level, that Radix popover example as it is today is far from being accessible and isn't a good example to follow. I would recommend much more caution before pointing developers to examples or external libraries that are far from implementing the best practices we aim to see in this project.

@ciampo
Copy link
Contributor

ciampo commented Dec 20, 2024

If we want the ToolsPanel to be a menu (ARIA menu) with menu/menuitems semantics and arrow keys navigation as expected in an ARIA menu, it can't contain a Close button, period.

@afercia I'm curious as to why we could not have a dialog containing various UI elements, including a menu (correctly implemented, with only menu items as children) rendered inline. Could you expand on this? Thank you!

@afercia
Copy link
Contributor

afercia commented Dec 20, 2024

@ciampo an ARIA menu is supposed to provide arrows navigation directly after activating the trigger button that opens it. No more no less than that. The advantage of arrows navigation is evident in the ARIA Authoring Practices Guide (APG) here for example, where menus are within a toolbar where there's only one tab stop and the rest of navigation works with arrow keys. It's a pattern to reduce tab stops.

A dialog with a collapsed menu that opens when clicking its trigger button would be a little weird. A menu open by default wouldn't be a real menu in the first place. At that point I'd rather consider a dialog that contains a list of buttons, with only tab key navigation.

@ciampo
Copy link
Contributor

ciampo commented Dec 20, 2024

I agree that a collapsed dropdown menu inside a dialog would be weird in this scenario, and I also agree that a menu should behave like a composite widget, allowing the end user to treat the whole menu as a single tab stop and to navigate its menu items via arrow keys.

I re-read the APG and the MDN docs and I can't find an indication that "a menu open by default wouldn't be a real menu". In fact, I read that a menu is "a list of actions or functions that require complex functionality, such as composite widget focus management and first-character navigation. A menu can be a permanently visible list of controls or a widget that can be made to open and close." (source)

Anyway, I don't want to go off-topic — I don't think that using a menu or a list of buttons would be a big deal on my side, especially if it allows us to explore a new UX that works best for everyone.

@afercia
Copy link
Contributor

afercia commented Dec 20, 2024

I re-read the APG and the MDN docs and I can't find an indication that "a menu open by default wouldn't be a real menu". I

@ciampo in the same MDN source you mentioned, see:

If the menu is visually persistent, consider the menubar role instead.

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/menubar_role

A menubar is a presentation of menu that usually remains visible and is usually presented horizontally.

MDN is a very authoritative source but sometimes it uses conversational, so to say, terminology. In case of doubts please always refer to the specifications. To me, it's clear that the menu role is meant to be used for 'dropdowns'. A menubar role for a presentation of menu that usually remains visible and is usually presented horizontally.

@richtabor
Copy link
Member

If we don't need or want a close button, can we close this issue?

@richtabor
Copy link
Member

As an aside, I spent a couple minutes inspecting the mentioned Radix popover example and I have to say a few minutes were enough to realize that's not a good example to point to. While Radix claims to be a 'library optimized for fast development, easy maintenance, and accessibility' and I do see some good intent there to provide some accessibility level, that Radix popover example as it is today is far from being accessible and isn't a good example to follow. I would recommend much more caution before pointing developers to examples or external libraries that are far from implementing the best practices we aim to see in this project.

I'd appreciate if you wrote a blog post or something sharing your feedback. I'm sure the Radix folks would be interested in reading your thoughts, if it were to help them improve what is widely regarded as an industry standard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Bug An existing feature does not function as intended
Projects
Status: Not Started
Development

No branches or pull requests