zillow / react-slider

Accessible, CSS agnostic, slider component for React.
https://zillow.github.io/react-slider
MIT License
891 stars 232 forks source link

Add keyboard accessibility enhancement #113

Closed B-Reif closed 7 years ago

B-Reif commented 7 years ago

This PR adds keyboard accessibility to react-slider (and closes #41).

refactors

additions

misc

mpowaga commented 7 years ago

Thanks for working on this @B-Reif! Is this supposed to work with example in index.html or it needs to be updated? Because I can't change the slider value with arrow keys using the example. Tested on OS X with Safari and Chrome.

B-Reif commented 7 years ago

I just tested it with Chrome without any issue. Are you able to tab-focus the slider handles, and then the keybinds aren't working? Or can you not even tab to them?

mpowaga commented 7 years ago

Just checked again and it worked this time after setting a focus with tab.

It looks like Y-axis is reverted, i.e. up arrow moves the slider down and down arrow up. Is this intentional? I'd expect that it worked the other way around.

B-Reif commented 7 years ago

I was going by examples from http://oaa-accessibility.org/example/32/ where Up/Left always decrements and Down/Right always increments.

The vertical slider example has invert: true so the arrow key functionality seems backward; it works as expected if it's not inverted.

We can flip the arrow key functions if invert: true. This might cause some confusion for blind users though. Let me know which way you'd like to go.

mpowaga commented 7 years ago

We definitely should stick to accessibility standards. Looks good to me. Can you just add missing semicolons? Then I'll merge. Thanks!

B-Reif commented 7 years ago

Ok, should be good to go.