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

Enable keyboard navigation #31

Merged
merged 3 commits into from
Jan 29, 2020
Merged

Enable keyboard navigation #31

merged 3 commits into from
Jan 29, 2020

Conversation

alex-ju
Copy link
Contributor

@alex-ju alex-ju commented Jan 27, 2020

This PR:

  • adds previous and next pointers to the Topic class
  • creates a focusTopic helper function to help move the focus to a specific item
  • binds keyboard events on arrow keys to use the Topic pointers (previous, next, parent, childList) to navigate topics

Trello card

Preview

mc-navigation

Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job on the tests 👏 the only thing I'm not sure about is the additional prev/next parameters, where we don't actually pass next at any point. Not sure how much better my suggested approach is, but I do think it would be less confusing.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
Create previous/next pointers for the topics list and use them to
navigate the taxonomy tree
@alex-ju alex-ju force-pushed the enable-keyboard-navigation branch from 62ba8ef to 7ce4045 Compare January 28, 2020 17:17
Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@alex-ju alex-ju merged commit 73bf3c7 into master Jan 29, 2020
@alex-ju alex-ju deleted the enable-keyboard-navigation branch January 29, 2020 10:39
@alex-ju alex-ju mentioned this pull request Feb 6, 2020
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.

2 participants