Make elements keyboard accessible #83
Conversation
|
LGTM. but should the tabIndex be different？ |
benjycui
changed the title
|
A tabindex of 0 makes the element focusable using the keyboard. It's best to use '0' unless you want to explicitly change the natural html order. Ok, we've made the final changes. As for tests, it looks like they were failing with version 6.x of rc-tools, but will pass with 5.x. We did not revert the version in this commit, but that might be necessary for the tests to pass. If you're interested we can update the tests to use the enzyme library. Let us know. |
|
rc-tools 6.x have moved all test command to a new package called rc-test，you can just use rc-test to replace the relative test command，then ci should be no problem any more. But if you are willing to refactor test with jest and enzyme，that would be great too. You can see https://github.com/react-component/slider test code as a example. |
|
@paranoidjk it appears that CI was broken before this PR. We checked that we did not create new errors by running the tests before and after this feature update. Would this be ok to merge at this point? |
src/Pagination.jsx
Outdated
| @@ -190,6 +190,12 @@ class Pagination extends React.Component { | |||
| return this.state.current < this._calcPage(); | |||
| } | |||
|
|
|||
| _runIfEnter(event, callback) { | |||
| if (event.keyCode === 'Enter') { | |||
There was a problem hiding this comment.
You should use event .key === 'Enter' or event.charCode === 13, see https://facebook.github.io/react/docs/events.html#keyboard-events
There was a problem hiding this comment.
Ok, the update should be ready.
|
@geramirez As i mentioned earlier, just follow this react-component/checkbox@cd7c340 to fix the ci. |
Signed-off-by: Gabriel Ramirez <gabriel.e.ramirez@uscis.dhs.gov> Signed-off-by: Patrick Oyarzun <patrick.t.oyarzun@uscis.dhs.gov> Signed-off-by: Gabriel Ramirez <gabriel.e.ramirez@uscis.dhs.gov> Signed-off-by: Patrick Oyarzun <patrick.t.oyarzun@uscis.dhs.gov>
|
Ok, @paranoidjk the build is fixed. |
paranoidjk
changed the title
This is a work in progress, I just wanted to get some feedback if the changes are OK. Then I will add tests and clean up the code.