PolymerElements / paper-dropdown-menu

A Material Design browser select element
https://www.webcomponents.org/element/PolymerElements/paper-dropdown-menu
61 stars 107 forks source link

Fix accessibility descriptions. (...) #317

Closed bicknellr closed 4 years ago

bicknellr commented 4 years ago

paper-dropdown-menu-light is not a combobox: a combobox contains a user-editable text area for arbitrary inputs with a popup for suggestions (usually a 'listbox') and this element does not allow arbitrary user input. paper-dropdown-menu-light is a 'collapsible dropdown listbox'. See this link for an example of this behavior: https://www.w3.org/TR/wai-aria-practices-1.1/examples/listbox/listbox-collapsible.html.

Given these issues with the current state of the element, I've made these changes:

Also, even though this element and its demos imply that it acts as a listbox-popup-opening button, the current element only sets aria-haspopup to true rather than explicitly "listbox". So, I've decided not to change it in case someone using this element is relying on the current generic value of true and showing something other than a "listbox".

e111077 commented 4 years ago

related PR: #71 related Issue: #30

I would say it SEEEMS obviated by host attribute role button. We may need to update listbox.

bicknellr commented 4 years ago

WDYT about the latest commit having changed the focusable thing from being the paper-dropdown-menu itself to an element in its shadow root? I was kinda worried that this will break users that expect to set role, tabindex, and other aria- attributes directly on it but I think setting delegatesFocus on the shadow root would fix the role and tabindex parts. I still need to look more into this.

(edit: btw there's some notes in the last commit f913cec if you're looking for more context.)

e111077 commented 4 years ago

This is actually what I was doing for mwc-select. It seems to work just fine with a screen reader and tabbing. I've been following this advice though using aria-haspopup=listbox instead and setting role listbox on the list

https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html

e111077 commented 4 years ago

THOUGH, we programmatically change tabindex as focus changes in the listbox. Idk if we handle that here

bicknellr commented 4 years ago

Yeah, that's exactly the example I was trying to match, glad to hear mwc-select is doing this. For paper-dropdown-menu I think aria-haspopup="true" / "menu" should work: https://www.w3.org/TR/wai-aria-1.1/#aria-haspopup ? Also, AFAICT, paper-listbox containing paper-items (like the demo) handles setting role="option" as well as toggling aria-selected and tabindex as needed, so it seems out of the scope of paper-dropdown-menu itself. One other thing is that this needs PolymerElements/paper-input#701 to pass the right role and aria-haspopup values to the input in the paper-menu.

bicknellr commented 4 years ago

Though, there's also https://www.w3.org/TR/wai-aria-practices-1.1/examples/menu-button/menu-button-actions.html , which seems to match with the name 'paper-dropdown-menu' more, but I guess the examples are actually pointing to it being the other type (collapsible dropdown listbox). So, maybe I should change it to aria-haspopup="listbox", if that's what it's really meant to be in spite of the name.

e111077 commented 4 years ago

So the only thing on=listbox is that I had to fight the lit analyzer linter which says it should be =true. I went with listbox though because it came from w3c

bicknellr commented 4 years ago

Yeah, I'm going to change the aria-haspopup value.