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

[a11y]: Dropdown MultiSelect button should have role="combobox" instead of button #12596

Closed
2 tasks done
Natuu opened this issue Nov 10, 2022 · 11 comments · Fixed by #13173
Closed
2 tasks done

[a11y]: Dropdown MultiSelect button should have role="combobox" instead of button #12596

Natuu opened this issue Nov 10, 2022 · 11 comments · Fixed by #13173

Comments

@Natuu
Copy link

Natuu commented Nov 10, 2022

Package

carbon-components-react

Browser

Chrome

Operating System

Windows

Package version

7.59.3 (from @console/pal)

React version

17.0.2

Automated testing tool and ruleset

IBM Equal Access Accessibility Checker - latest version

Assistive technology

No response

Description

The buttons inside the MultiSelect causes issue because parent is a button instead of a role="combobox"

WCAG 2.1 Violation

IBM 4.1.2 Name, Role, Value

Reproduction/example

https://v10.carbondesignsystem.com/components/dropdown/code

Steps to reproduce

  1. Choose MultiSelect
  2. Select an item
  3. Close the input
  4. Run accessibility checker

Code of Conduct

@tay1orjones
Copy link
Member

I don't think we'd be able to change the role to combobox without significantly altering the behavior and semantics of the MultiSelect. This is why we have Combobox as a separate component.

That said, I can confirm the issue in the v11 storybook.
image

@mbgower I'm not sure how we might approach this one. Do you have any thoughts? At first glance I'm wondering if this might be a false positive.

@mbgower
Copy link

mbgower commented Feb 3, 2023

I'm pulling @tombrunet in on this. Too technical for me. My hunch is that you need to make the tag a peer of the listbox. If it is at the same level, it shouldn't spit out this error. But that's ill-informed conjecture :)
They've been adding a lot of these aria to html rules, and I suspect this is one of the new ones kicking in.

@tw15egan
Copy link
Collaborator

@mbgower I've put up a PR (#13173) that makes the tag a peer of the list box and resolves the issue. That is how we were already structuring FilterableMultiSelect, which was not throwing this violation, so it makes sense to mimic that structure.

@mbgower
Copy link

mbgower commented Feb 16, 2023

@tw15egan I think there are some problems with that approach, on investigation. I just met with Tom and went through these different components, and we have suggestions across the board on the whole 'dropdown' family. Tom is going to summarize some of those here, but I'm also sending you a small video we just made.

@tombrunet
Copy link
Contributor

The high level issue is that most widgets aren't intended to be containers (other than maybe grid). If a widget has content (like button), that content is typically just a shorthand for the aria-label. So the question is, how do you handle these widgets that have multiple mouse actions associated with them?

The net is, the multiple mouse actions are a mouse accommodation for the fact that the mouse is limited on what it can do on the widget - focus or activate mostly. A keyboard doesn't have that limitation, so it doesn't necessarily need all of those other visual buttons to perform actions. For a combobox / dropdown, typically, the escape key is used to clear while on the combobox (no need to tab off to perform the action).

A good reference for this is the APG guide for combobox:
https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-autocomplete-both/

Some basic rules of thumb:

  • Escape key: If a dropdown box is open, simply close the dropdown. Else: if there is content in the input, clear the input. Else: If something is selected, clear the selection (unless it's a select where you always keep a value once selected).
  • Clear buttons are adjacent to the combobox but not tabbable (tabindex=-1). These buttons are there for mouse users, not keyboard users (since they can use the escape key)

@tw15egan
Copy link
Collaborator

tw15egan commented Feb 16, 2023

Okay, that makes sense. I've adjusted the PR for now to include the following:

  • Tag is no longer in the tab order but can be clicked for mouse users.
  • Once an item is selected, esc once closes the menu, and once more clears the selection.
  • Same for filterable multiselect, but it clears the input / closes menu on first esc key press, and clears the selection on second esc press.
  • Combobox now retains the value when esc is pressed, and the menu is open. Pressing esc again clears the input.

Just pushed up some changes to the same PR, deploy preview should be updated in a few minutes

https://deploy-preview-13173--carbon-components-react.netlify.app/

@mbgower
Copy link

mbgower commented Feb 16, 2023

All nice changes!
The other one I'm seeing, which I texted you about separately, unfortunately, is that the string that gets inserted in the mult-select filter should go away when the user leaves the input

In a combobox, in some implementations, if I type in “o” and then tab away from the input, the “o” persists. To me, that is a little weird but in a true combobox that is an allowed action, since you don’t just choose existing options. You can add new strings that become options.
However, in a multiselect (whether filterable or not) that is not a possible thing. You cannot add new values. So IMO when focus leaves the input in filterable multi-select, any filter string typed in should disappear into the ether.
That means the Esc key consistently can be used to clear the tag in a multiselect when a user reaches an input with a 2x (or whatever number) tag

https://deploy-preview-13173--carbon-components-react.netlify.app/?path=/story/components-multiselect--filterable

I don't think we want something like this to persist when the focus has moved on...
image

Also, pressing "Esc" is not clearing it either once it's already in the input. (And, it also lacks the affordance to tab to the x, which I agree we don't want/need, but when Esc does NOT work, we lose the regular way to invoke the 'delete/clear' function). Note that the user CAN clear the string except by just typing something else (since it's highlight by default on refocus) or backspace. They can also press Delete if the letters are highlighted

@tw15egan
Copy link
Collaborator

@mbgower just pushed some updates that should address these issues 👍🏻

@mbgower
Copy link

mbgower commented Feb 17, 2023

You work fast, my friend :)
I think there's_ something not quite working properly there.
I can get the list to reappear even when there's... it's retaining the listbox location even after clearing values (when they appear again). Let me just poke at it a bit...

@alisonjoseph alisonjoseph moved this from ⏱ Backlog to 🚦 In Review in Design System Feb 17, 2023
@mbgower
Copy link

mbgower commented Feb 17, 2023

Say, while doing so, and comparing interaction across all these things I noticed that pressing Enter was NOT opening the filtering multiselect listbox when it had focus. I thought it should, so went and looked at the others just to make sure everything was consistent. While doing so, noticed a couple of other things (sorry!):

  1. On your dropdown, Enter works fine to toggle the list open (and put focus on the item selected) and closed. I noticed that Down Arrow on a closed dropdown is opening but ALSO advancing past the currently selected item. I don't think that should happen. We definitely want the down arrow to open the listbox, but my expectation is that the action is identical to the Enter key: opens listbox and puts focus on the currently selected item. I think the APG select-only comboxbox reflects this pattern pretty accurately. BTW, I think I had an existing ticket on this one.
  2. Your default multiselect is opening on Enter, but not opening on Down Arrow. (the complete opposite of the filerable variant.) Any reason for that? My expectation would be that the Enter would reveal the listbox, as would the Down Arrow. Unfortunately, there's really nothing directly parallel to this in the APG.
  3. Still talking about the multiselect, the behaviour on open has some considerations. Normally I'd expect the list to open with the focus going to the first item (And BTW, the behaviour with multiselect is that where something is already selected it becomes the first item). However, because Enter is also used to select, this gets a little tricky. If the focus goes to something in the list, a user trying to toggle the list closed by pressing Enter could inadvertently either select the first unselected item or unselect the first selected item. So, my inclination is to say that opening a multiselect with Enter should open the list, but not put focus inside the list. The user would have to arrow down to select items. I'd invite discussion on this. IF the multiselect is opened with the Down Arrow, I think I'd expect regular behaviour: open and move focus to first item.
  4. And just to return to the top of this comment, I confirm my expectation that the filtering multiselect should open on Enter

@mbgower
Copy link

mbgower commented Feb 17, 2023

PS, in the time it took me to start that, have a couple of meetings, and update it and post the comment, I see you've partially addressed my prior comment on interaction curiosities.

Now, I'd like Tom to weigh in on the behaviour I'm now seeing on the filterable mutliselect, which is that if I've entered in a filter string and am arrowing through the filtered items that match, when I press Esc, the list collapses AND the string is cleared (leaving me just with the 2x or whatever label showing how many are selected).

I think that works, and I also think it makes an interesting comparison with the action we got you to do on the combo box (pressing Esc once only collapse, not clears string). @tombrunet would you please play with it and give your two bits?

Obviously for consistency, it would make more sense to just collapse the list, like the combo box. I'm just having troubles imagining why we need that intermediary step in the filterable multiselect. Unlike the combo box, there is no case for the string to stick around as a potential new value. I can add to or alter the string without the need to collapse the list. So to me, if someone collapses this variant, ditching the string almost seems like an intentional feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
6 participants