spectrum
spectrum copied to clipboard
Add keyboard navigation to the color palette, for accessibility.
Allow tabbing into the color picker, and focus to be set on a square in the color picker. Allow using arrow keys to move around the color palette and set focus. Allow Enter key to select the focused color. This is done with a new keydown handler.
This is needed in order to comply with accessibility requirements. Note that the color gradient is not yet accessible, just the palette.
https://github.com/bgrins/spectrum/issues/295
Thanks for this. I've pulled this down and played with it, it's a very good start. As this kind of navigation gets tricky, I've noticed a couple of issues:
- when I click on a swatch, it'd be nice if arrow navigation started working for consistency
- when I press enter to select a swatch, it'd be nice if arrow navigation continued to work
- when trying to go up/down between nodes within a single row (but that have wrapped visually like in the top colorpicker on http://bgrins.github.io/spectrum/) it doesn't move as I'd expect. This is probably really hard to get right, since it's not obvious which node is visually below the other - they are all siblings. The only way I can think of to address this would be to use elementFromPoint to try and find an element if possible, and if not fall back to the current algorithm. Let me try and put something together with that and respond later.
Here are a couple of other bugs that are maybe not as important in a first version:
- when I press escape, I'd like the colorpicker to close (this is an existing general issue in https://github.com/bgrins/spectrum/issues/281, but it'd be great to get it fixed)
- when pressing 'left' on the first swatch in a row or 'right' on the last, it'd be nice if it continued wrapping into the previous/next row.
OK, I've updated the https://github.com/bgrins/spectrum/compare/ajssd-master branch to handle the left/right and up/down behavior I was discussing. Let me know what you think so far.
I'd still like if we could get navigation to work after pressing Enter, and also if it was enabled after clicking on a swatch. I think we also need to make escape close the picker so people have a way to get out of the picker if they want to use the keyboard to scroll the page, for instance.
We also need some basic test coverage for this in https://github.com/bgrins/spectrum/blob/master/test/tests.js.
I think we also need to make escape close the picker so people have a way to get out of the picker if they want to use the keyboard to scroll the page, for instance.
Just landed ESC closing the picker on master for #281
I added two more minor accessibility-related changes, both aria-labels on user-navigable elements (the initial/current color swatches, and the input field). An aria-label is read aloud by screen readers but is otherwise not visible; it is required for accessibility when there isn't an explicit label.
I made one additional change -- instead of putting the tabindex on the focused swatch (sp-thumb-el) now the tabindex is on on the palette itself. This way, we don't have to add and remove tab stops as the user tabs around. Also, if a color is chosen from the rgb picker on the right, and that color is not in the palette, previously there was no way to regain focus in the palette. Now it is possible. Please consider accepting this pull request in order to make Spectrum keyboard-navigable and compliant with Accessibility requirements.
This still needs tests, you can add a new test function at the bottom of https://github.com/bgrins/spectrum/blob/master/test/tests.js and run them locally either by loading the test/index.html page in a browser or by running grunt test
after setting it up with: https://github.com/bgrins/spectrum#building-spectrum-locally.
jQuery allows you to create new events to trigger custom key presses like this: http://api.jquery.com/category/events/event-object/
OK I've gone through and made a few notes. I will still need to apply this locally and do a test, but I think getting basic accessibility in place (once there is some test coverage) will be a great start and we can handle enhancements in follow up work. Thanks for following up with this!
What's missing in this PR to hit master?
I could help
Nothing that I know of ... we have been pulling from this branch since Sep 1, without issue. I would recommend Brian fold this into master and release 1.7.1.
Brian, would it be possible to merge these changes into the forthcoming 1.8.0 release? My team is interested in leveraging Spectrum in our product, however we will most likely be unable to take the dependency until accessibility support has been added. Thanks!
I'm sorry for dropping this. I've asked @miketoth if he could review the patch and advise if we can land it straight away or if it needs changes.
Thanks bgrins! I should be able to review this tomorrow
Hey guys. I'm fairly new to github branching stuff. Is this accessibility stuff checked into the main branch? E.g. if I get the main branch, it will have accessibility already. If it isn't checked into the main branch, this branch has no conflicts with the main. So I could also use this child branch? Thanks
Downloaded the branch. Would like better keyboard functionality, unless I've missed something. Going to branch this branch.
Hi guys, I appreciate your effort for doing these accessibility updates. I'm interested to use this updated version. Can it be already merged with master?
The thing is I'm managing my project with Composer by using asset-packagist and there is no option to use these specific branches as you can see https://asset-packagist.org/package/npm-asset/spectrum-colorpicker
Thanks for understanding!
@dekisha if they do merge it. I hope they merge the changes on my fork. I did some more accessibility changes. Check my merge request.