-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add up and down arrow support - #30 #41
Add up and down arrow support - #30 #41
Conversation
User is now able to navigate up and down through ToDoItems with the up and down arrow keys and it will stop at the top and bottom of the list and do nothing.
Do you want me to fix these on my forked repo branch and open a new pull request? Or are you just going to make those few quick let->const changes? I'm happy to do what I need to do, but as I said in the issue thread - I'm a bit new to all this so I need some instructions on how you'd like me to proceed so that this can get approved and merged! @bhujoshi |
autoFocus | ||
onKeyDown={(e) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do it with react state instead of dom manupulation.
You should declear a new state for focused item and use that to focus the item and manupulate that state on pressing up and down arrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry - got a little busy with a few other things, but I'll be working on updating that to use React state instead of dom manipulation early this week. Thanks for the feedback.
@kamalkishor1991 - I made the updates, I think you should be able to see them now: Can you take a look and merge it in or provide additional feedback please? |
User is now able to navigate up and down through ToDoItems with the up and down arrow keys