From 13c2bdf50074205a895f17e61eb4266f1afe5c49 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 19 Jul 2017 11:07:07 -0400 Subject: [PATCH 1/3] Try: Add component for handling keyboard events --- components/index.js | 1 + components/keyboard-shortcuts/README.md | 52 +++++++++++++++++++++++++ components/keyboard-shortcuts/index.js | 32 +++++++++++++++ editor/modes/visual-editor/index.js | 23 +++++------ package.json | 1 + utils/keycodes.js | 1 - 6 files changed, 98 insertions(+), 12 deletions(-) create mode 100644 components/keyboard-shortcuts/README.md create mode 100644 components/keyboard-shortcuts/index.js diff --git a/components/index.js b/components/index.js index 2f2ba501c66cd5..e29243f1bff079 100644 --- a/components/index.js +++ b/components/index.js @@ -7,6 +7,7 @@ export { default as ExternalLink } from './external-link'; export { default as FormToggle } from './form-toggle'; export { default as FormTokenField } from './form-token-field'; export { default as IconButton } from './icon-button'; +export { default as KeyboardShortcuts } from './keyboard-shortcuts'; export { default as Notice } from './notice'; export { default as NoticeList } from './notice/list'; export { default as Panel } from './panel'; diff --git a/components/keyboard-shortcuts/README.md b/components/keyboard-shortcuts/README.md new file mode 100644 index 00000000000000..43a2b5a0b0e71c --- /dev/null +++ b/components/keyboard-shortcuts/README.md @@ -0,0 +1,52 @@ +Keyboard Shortcuts +================== + +`` is a component which renders no children of its own, but instead handles keyboard sequences during the lifetime of the rendering element. + +It uses the [Mousetrap](https://craig.is/killing/mice) library to implement keyboard sequence bindings. + +## Example + +Render `` with a `shortcuts` prop object: + +```jsx +class SelectAllDetection extends Component { + constructor() { + super( ...arguments ); + + this.setAllSelected = this.setAllSelected.bind( this ); + + this.state = { isAllSelected: false }; + } + + setAllSelected() { + this.setState( { isAllSelected: true } ); + } + + render() { + return ( +
+ + Combination pressed? { isAllSelected ? 'Yes' : 'No' } +
+ ); + } +} +``` + +## Props + +The component accepts the following props: + +### shortcuts + +An object of shortcut bindings, where each key is a keyboard combination, the value of which is the callback to be invoked when the key combination is pressed. + +- Type: `Object` +- Required: No + +## References + +- [Mousetrap documentation](https://craig.is/killing/mice) diff --git a/components/keyboard-shortcuts/index.js b/components/keyboard-shortcuts/index.js new file mode 100644 index 00000000000000..ac75430e0fe909 --- /dev/null +++ b/components/keyboard-shortcuts/index.js @@ -0,0 +1,32 @@ +/** + * External dependencies + */ +import Mousetrap from 'mousetrap'; +import { forEach } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component } from 'element'; + +class KeyboardShortcuts extends Component { + componentWillMount() { + this.toggleBindings( true ); + } + + componentWillUnmount() { + this.toggleBindings( false ); + } + + toggleBindings( isActive ) { + forEach( this.props.shortcuts, ( callback, key ) => { + Mousetrap[ isActive ? 'bind' : 'unbind' ]( key, callback ); + } ); + } + + render() { + return null; + } +} + +export default KeyboardShortcuts; diff --git a/editor/modes/visual-editor/index.js b/editor/modes/visual-editor/index.js index a5c951056796ad..67d258d0901027 100644 --- a/editor/modes/visual-editor/index.js +++ b/editor/modes/visual-editor/index.js @@ -9,7 +9,7 @@ import { first, last } from 'lodash'; */ import { __ } from 'i18n'; import { Component, findDOMNode } from 'element'; -import { CHAR_A } from 'utils/keycodes'; +import { KeyboardShortcuts } from 'components'; /** * Internal dependencies @@ -27,7 +27,7 @@ class VisualEditor extends Component { this.bindContainer = this.bindContainer.bind( this ); this.bindBlocksContainer = this.bindBlocksContainer.bind( this ); this.onClick = this.onClick.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); + this.selectAll = this.selectAll.bind( this ); } componentDidMount() { @@ -52,16 +52,14 @@ class VisualEditor extends Component { } } - onKeyDown( event ) { - const { uids } = this.props; - if ( - ! isEditableElement( document.activeElement ) && - ( event.ctrlKey || event.metaKey ) && - event.keyCode === CHAR_A - ) { - event.preventDefault(); - this.props.multiSelect( first( uids ), last( uids ) ); + selectAll( event ) { + if ( isEditableElement( document.activeElement ) ) { + return; } + + const { uids } = this.props; + event.preventDefault(); + this.props.multiSelect( first( uids ), last( uids ) ); } render() { @@ -77,6 +75,9 @@ class VisualEditor extends Component { onKeyDown={ this.onKeyDown } ref={ this.bindContainer } > + diff --git a/package.json b/package.json index 50db4f10cd51b1..4f2bc0e2e86e0e 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "lodash": "^4.17.4", "moment": "^2.18.1", "moment-timezone": "^0.5.13", + "mousetrap": "^1.6.1", "prop-types": "^15.5.10", "react": "^15.5.4", "react-autosize-textarea": "^0.4.2", diff --git a/utils/keycodes.js b/utils/keycodes.js index 3363e4f1ade462..f7f3b6cfe2db86 100644 --- a/utils/keycodes.js +++ b/utils/keycodes.js @@ -7,4 +7,3 @@ export const UP = 38; export const RIGHT = 39; export const DOWN = 40; export const DELETE = 46; -export const CHAR_A = 'A'.charCodeAt( 0 ); From f9e2156a9ef71d1626f19d90337d781d0135d6d3 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 19 Jul 2017 11:12:32 -0400 Subject: [PATCH 2/3] Warn about consistent callback reference for shortcuts --- components/keyboard-shortcuts/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/keyboard-shortcuts/README.md b/components/keyboard-shortcuts/README.md index 43a2b5a0b0e71c..ac2749094c9853 100644 --- a/components/keyboard-shortcuts/README.md +++ b/components/keyboard-shortcuts/README.md @@ -36,6 +36,8 @@ class SelectAllDetection extends Component { } ``` +__Note:__ The value of each shortcut should be a consistent function reference, not an anonymous function. Otherwise, the callback will not be correctly unbound when the component unmounts. + ## Props The component accepts the following props: From 20d535d3f079b5211ea4db0d2059fa07c37ef677 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 19 Jul 2017 11:14:04 -0400 Subject: [PATCH 3/3] Remove editable detection from keyboard handling Already default behavior for Mousetrap https://craig.is/killing/mice#api.bind.text-fields --- components/keyboard-shortcuts/README.md | 2 ++ editor/modes/visual-editor/index.js | 5 ----- editor/utils/dom.js | 13 ----------- editor/utils/test/dom.js | 30 ------------------------- 4 files changed, 2 insertions(+), 48 deletions(-) delete mode 100644 editor/utils/dom.js delete mode 100644 editor/utils/test/dom.js diff --git a/components/keyboard-shortcuts/README.md b/components/keyboard-shortcuts/README.md index ac2749094c9853..14defab6142d8e 100644 --- a/components/keyboard-shortcuts/README.md +++ b/components/keyboard-shortcuts/README.md @@ -38,6 +38,8 @@ class SelectAllDetection extends Component { __Note:__ The value of each shortcut should be a consistent function reference, not an anonymous function. Otherwise, the callback will not be correctly unbound when the component unmounts. +__Note:__ The callback will not be invoked if the key combination occurs in an editable field. + ## Props The component accepts the following props: diff --git a/editor/modes/visual-editor/index.js b/editor/modes/visual-editor/index.js index 67d258d0901027..8feb5ddf30d6b0 100644 --- a/editor/modes/visual-editor/index.js +++ b/editor/modes/visual-editor/index.js @@ -19,7 +19,6 @@ import VisualEditorBlockList from './block-list'; import PostTitle from '../../post-title'; import { getBlockUids } from '../../selectors'; import { clearSelectedBlock, multiSelect } from '../../actions'; -import { isEditableElement } from '../../utils/dom'; class VisualEditor extends Component { constructor() { @@ -53,10 +52,6 @@ class VisualEditor extends Component { } selectAll( event ) { - if ( isEditableElement( document.activeElement ) ) { - return; - } - const { uids } = this.props; event.preventDefault(); this.props.multiSelect( first( uids ), last( uids ) ); diff --git a/editor/utils/dom.js b/editor/utils/dom.js deleted file mode 100644 index ec3feb0f17ba71..00000000000000 --- a/editor/utils/dom.js +++ /dev/null @@ -1,13 +0,0 @@ - -/** - * Utility function to check whether the domElement provided is editable or not - * An editable element means we can type in it to edit its content - * This includes inputs and contenteditables - * - * @param {DomElement} domElement DOM Element - * @return {Boolean} Whether the DOM Element is editable or not - */ -export function isEditableElement( domElement ) { - return [ 'textarea', 'input', 'select' ].indexOf( domElement.tagName.toLowerCase() ) !== -1 - || !! domElement.isContentEditable; -} diff --git a/editor/utils/test/dom.js b/editor/utils/test/dom.js deleted file mode 100644 index 37381e3d6d084d..00000000000000 --- a/editor/utils/test/dom.js +++ /dev/null @@ -1,30 +0,0 @@ -/** - * Internal dependencies - */ -import { isEditableElement } from '../dom'; - -describe( 'isEditableElement', () => { - it( 'should return false for non editable nodes', () => { - const div = document.createElement( 'div' ); - - expect( isEditableElement( div ) ).toBe( false ); - } ); - - it( 'should return true for inputs', () => { - const input = document.createElement( 'input' ); - - expect( isEditableElement( input ) ).toBe( true ); - } ); - - it( 'should return true for textareas', () => { - const textarea = document.createElement( 'textarea' ); - - expect( isEditableElement( textarea ) ).toBe( true ); - } ); - - it( 'should return true for selects', () => { - const select = document.createElement( 'select' ); - - expect( isEditableElement( select ) ).toBe( true ); - } ); -} );