Skip to content

Try: Add component for handling keyboard events#1944

Merged
aduth merged 3 commits intomasterfrom
add/key-sequence-utility
Jul 19, 2017
Merged

Try: Add component for handling keyboard events#1944
aduth merged 3 commits intomasterfrom
add/key-sequence-utility

Conversation

@aduth
Copy link
Copy Markdown
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 ellatrix and westonruter July 19, 2017 15:17
Copy link
Copy Markdown
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
Copy Markdown
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