Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow keyboard accessible sorting #12

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jbinto
Copy link

@jbinto jbinto commented Dec 6, 2017

We couldn't figure out a way to use the existing sortabular API to trigger sorts both by mouse click and keyboard.

This PR changes the sort function to have an onKeyDown handler by default, so you can trigger sorts using the ENTER key on the keyboard. (You can opt out of this behavior by passing disableKeyboard: true to sort.)

Let me know if you'd prefer a different approach, or if there's a better way to do this using the existing API.

@codecov-io
Copy link

codecov-io commented Dec 6, 2017

Codecov Report

Merging #12 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #12      +/-   ##
==========================================
+ Coverage   97.94%   97.98%   +0.04%     
==========================================
  Files          10       10              
  Lines         146      149       +3     
  Branches       52       55       +3     
==========================================
+ Hits          143      146       +3     
  Misses          2        2              
  Partials        1        1
Impacted Files Coverage Δ
src/sort.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fb4f51...182179e. Read the comment docs.

@bebraw
Copy link
Member

bebraw commented Dec 10, 2017

I don't like the flag. It would be better to inject the functionality through props. You could refactor the code as an utility and write a small example/test that does this. I agree with the idea (keyboard sorting). If you can change the design like that, I don't mind merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants