search

PolymerElements / iron-menu-behavior

Accessible menu behavior
18 stars 28 forks source link

Fixing #57 Moving focus to next item (in paper-menu) does not skip disabled items. #58

Closed giltza closed 8 years ago

giltza commented 8 years ago

Fixing #57 We are looking at the 'disabled' html dom attribute rather then the iron-control-state 'disabled' property for more generic support.

tjsavage commented 8 years ago

One thing we should check is a11y - it might actually be the right behavior to tab through disabled state items, so that the screen reader can read the item. @bicknellr and @notwaldorf what do you think?

giltza commented 8 years ago

@tjsavage , @notwaldorf , @bicknellr Let me know what to do. I personally think that traversing through disabled items has no purpose and the standard is to skip them (Check out menus in your OS). But I have not problem adding a property configuration that will be default to the current state.

bicknellr commented 8 years ago

I think it's pretty far-fetched to say that people would be using an attribute called disabled on a item of a selectable set for something other than making that item unselectable, but it's possible.. Maybe it could be used to disable something internal to that item? @notwaldorf, care to perform the ritual weighing of the change?

notwaldorf commented 8 years ago

Pretty much every native element that I can think of is skipped when disabled: http://jsbin.com/leyoja/edit?html,output, so I'm fairly sure that the platform intention is to skip disabled inputs.

So then I nerded out and dug out the spec and it says:

An [...] element that is disabled must prevent any click events that are queued on the user interaction task source from being dispatched on the element.

which also suggests that disabled items should not be interacted with (and tabbing sounds like an interaction, I think)

bicknellr commented 8 years ago

@giltza, sounds like this is ok without a flag. There's a debugger in the tests and I think it would be nice to have a test that checks that these won't loop forever / otherwise break if all the items are disabled. (They look fine at first glance though.) Otherwise, LGTM.

Also, @notwaldorf: :cake: is as close as I could get; no :brownie:s were available.

giltza commented 8 years ago

@tjsavage , @notwaldorf , @bicknellr a. Removed debugger; (sorry about that) b. Made sure that in case of all disabled item menus, no item gets focused on menu focus. c. Added additional tests for empty menus and all disabled item menus.

bicknellr commented 8 years ago

Great, thanks!