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

Try: Add component for handling keyboard events #1944

Merged
merged 3 commits into from
Jul 19, 2017
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Jul 19, 2017

Fixes #1850
Supersedes #1908
Related: #1943

This pull request explores adding a new KeyboardEvents component to assist in handling keyboard events.

<KeyboardShorcuts shortcuts={ {
	'mod+a': this.setAllSelected,
} } />

See full README.md documentation

Implementation notes:

Currently, it is implemented to capture keyboard events globally, but I expect this could be further enhanced in the future to add scoping to a specific element context by optionally supporting children of KeyboardShortcuts, where a combination of ref, findDOMNode, and renderedNode.contains( event.target ) confirms that the keyboard event occurred within the rendered children.

Testing instructions:

Verify that there is no regression in "Select All" keyboard behavior (#1211).

@aduth aduth added the General Interface Parts of the UI which don't fall neatly under other labels. label Jul 19, 2017
@aduth aduth requested review from westonruter and ellatrix July 19, 2017 15:17
Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

This looks great to me. 👍

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

It's beautiful. 😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants