From 7d036d0c39c4060d34e57f2839e9793f826c03be Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 2 Oct 2017 20:06:13 -0400 Subject: [PATCH 1/4] Components: Enhance KeyboardShortcuts to optionally bind globally To capture events within inputs --- components/keyboard-shortcuts/README.md | 10 +++- components/keyboard-shortcuts/index.js | 15 ++++- components/keyboard-shortcuts/test/index.js | 61 +++++++++++++++++++++ 3 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 components/keyboard-shortcuts/test/index.js diff --git a/components/keyboard-shortcuts/README.md b/components/keyboard-shortcuts/README.md index 14defab6142d8e..351b047f04978a 100644 --- a/components/keyboard-shortcuts/README.md +++ b/components/keyboard-shortcuts/README.md @@ -15,6 +15,7 @@ class SelectAllDetection extends Component { super( ...arguments ); this.setAllSelected = this.setAllSelected.bind( this ); + this.save = this.save.bind( this ); this.state = { isAllSelected: false }; } @@ -23,11 +24,16 @@ class SelectAllDetection extends Component { this.setState( { isAllSelected: true } ); } + save() { + this.props.onSave(); + } + render() { return (
Combination pressed? { isAllSelected ? 'Yes' : 'No' }
@@ -38,7 +44,7 @@ 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. +__Note:__ The callback will not be invoked if the key combination occurs in an editable field, unless you pass the callback as an array with the callback and `true` as entries of the array respectively. Refer to the example above, and see the `shortcuts` prop documentation for more information. ## Props @@ -46,7 +52,7 @@ 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. +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. To capture a key event globally (including within input fields), assign as the value an array with the function callback as the first argument and `true` as the second argument. - Type: `Object` - Required: No diff --git a/components/keyboard-shortcuts/index.js b/components/keyboard-shortcuts/index.js index f7fed78b6e9ef5..13b46d10c0d235 100644 --- a/components/keyboard-shortcuts/index.js +++ b/components/keyboard-shortcuts/index.js @@ -2,6 +2,7 @@ * External dependencies */ import Mousetrap from 'mousetrap'; +import 'mousetrap/plugins/global-bind/mousetrap-global-bind'; import { forEach } from 'lodash'; /** @@ -13,7 +14,19 @@ class KeyboardShortcuts extends Component { componentWillMount() { this.mousetrap = new Mousetrap; forEach( this.props.shortcuts, ( callback, key ) => { - this.mousetrap.bind( key, callback ); + // Normalize callback, which can be passed as either a function or + // an array of [ callback, isGlobal ] + let isGlobal = false; + if ( Array.isArray( callback ) ) { + isGlobal = callback[ 1 ]; + callback = callback[ 0 ]; + } + + if ( isGlobal ) { + this.mousetrap.bindGlobal( key, callback ); + } else { + this.mousetrap.bind( key, callback ); + } } ); } diff --git a/components/keyboard-shortcuts/test/index.js b/components/keyboard-shortcuts/test/index.js new file mode 100644 index 00000000000000..196ccd80ae6918 --- /dev/null +++ b/components/keyboard-shortcuts/test/index.js @@ -0,0 +1,61 @@ +/** + * External dependencies + */ +import { mount } from 'enzyme'; + +/** + * Internal dependencies + */ +import KeyboardShortcuts from '../'; + +describe( 'KeyboardShortcuts', () => { + afterEach( () => { + while ( document.body.firstChild ) { + document.body.removeChild( document.body.firstChild ); + } + } ); + + function keyPress( which, target ) { + [ 'keydown', 'keypress', 'keyup' ].forEach( ( eventName ) => { + const event = new window.Event( eventName, { bubbles: true } ); + event.keyCode = which; + event.which = which; + target.dispatchEvent( event ); + } ); + } + + it( 'should capture key events', () => { + const spy = jest.fn(); + mount( + + ); + + keyPress( 68, document ); + + expect( spy ).toHaveBeenCalled(); + } ); + + it( 'should capture key events globally', () => { + const spy = jest.fn(); + const attachNode = document.createElement( 'div' ); + document.body.appendChild( attachNode ); + + const wrapper = mount( +
+ + +
, + { attachTo: attachNode } + ); + + keyPress( 68, wrapper.find( 'textarea' ).getDOMNode() ); + + expect( spy ).toHaveBeenCalled(); + } ); +} ); From a6340ac68e667e2f35a90ab2ba28ea37462e295e Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 16 Oct 2017 15:45:31 -0400 Subject: [PATCH 2/4] Block Toolbar: Refactor using KeyboardShorcuts component --- editor/block-toolbar/index.js | 51 +++++++++-------------------------- editor/utils/dom.js | 9 ------- 2 files changed, 13 insertions(+), 47 deletions(-) diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index baeadf5d01d315..2003859d75dea2 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -7,7 +7,7 @@ import classnames from 'classnames'; /** * WordPress Dependencies */ -import { IconButton, Toolbar, NavigableMenu, Slot } from '@wordpress/components'; +import { IconButton, Toolbar, NavigableMenu, Slot, KeyboardShortcuts } from '@wordpress/components'; import { Component, Children, findDOMNode } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { focus, keycodes } from '@wordpress/utils'; @@ -19,47 +19,29 @@ import './style.scss'; import BlockSwitcher from '../block-switcher'; import BlockMover from '../block-mover'; import BlockRightMenu from '../block-settings-menu'; -import { isMac } from '../utils/dom'; /** * Module Constants */ -const { ESCAPE, F10 } = keycodes; +const { ESCAPE } = keycodes; function FirstChild( { children } ) { const childrenArray = Children.toArray( children ); return childrenArray[ 0 ] || null; } -function metaKeyPressed( event ) { - return isMac() ? event.metaKey : ( event.ctrlKey && ! event.altKey ); -} - class BlockToolbar extends Component { constructor() { super( ...arguments ); + this.toggleMobileControls = this.toggleMobileControls.bind( this ); this.bindNode = this.bindNode.bind( this ); - this.onKeyUp = this.onKeyUp.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); + this.focusToolbar = this.focusToolbar.bind( this ); this.onToolbarKeyDown = this.onToolbarKeyDown.bind( this ); + this.state = { showMobileControls: false, }; - - // it's not easy to know if the user only clicked on a "meta" key without simultaneously clicking on another key - // We keep track of the key counts to ensure it's reliable - this.metaCount = 0; - } - - componentDidMount() { - document.addEventListener( 'keyup', this.onKeyUp ); - document.addEventListener( 'keydown', this.onKeyDown ); - } - - componentWillUnmount() { - document.removeEventListener( 'keyup', this.onKeyUp ); - document.removeEventListener( 'keydown', this.onKeyDown ); } bindNode( ref ) { @@ -72,21 +54,10 @@ class BlockToolbar extends Component { } ) ); } - onKeyDown( event ) { - if ( metaKeyPressed( event ) ) { - this.metaCount++; - } - } - - onKeyUp( event ) { - const shouldFocusToolbar = this.metaCount === 1 || ( event.keyCode === F10 && event.altKey ); - this.metaCount = 0; - - if ( shouldFocusToolbar ) { - const tabbables = focus.tabbable.find( this.toolbar ); - if ( tabbables.length ) { - tabbables[ 0 ].focus(); - } + focusToolbar() { + const tabbables = focus.tabbable.find( this.toolbar ); + if ( tabbables.length ) { + tabbables[ 0 ].focus(); } } @@ -128,6 +99,10 @@ class BlockToolbar extends Component { role="toolbar" deep > +
{ ! showMobileControls && [ , diff --git a/editor/utils/dom.js b/editor/utils/dom.js index 4b213a815394f2..f6396752e573de 100644 --- a/editor/utils/dom.js +++ b/editor/utils/dom.js @@ -86,12 +86,3 @@ export function placeCaretAtEdge( container, start = false ) { sel.addRange( range ); container.focus(); } - -/** - * Checks whether the user is on MacOS or not - * - * @return {Boolean} Is Mac or Not - */ -export function isMac() { - return window.navigator.platform.toLowerCase().indexOf( 'mac' ) !== -1; -} From 931414621cb5061dcf9dbb99aace167f09b163fe Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 17 Oct 2017 10:34:36 -0400 Subject: [PATCH 3/4] Components: Handle global and key event as props --- components/keyboard-shortcuts/README.md | 26 ++++++++++++++------- components/keyboard-shortcuts/index.js | 16 +++---------- components/keyboard-shortcuts/test/index.js | 26 ++++++++++++++++++++- editor/block-toolbar/index.js | 12 ++++++---- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/components/keyboard-shortcuts/README.md b/components/keyboard-shortcuts/README.md index 351b047f04978a..b7f75b47f2780f 100644 --- a/components/keyboard-shortcuts/README.md +++ b/components/keyboard-shortcuts/README.md @@ -15,7 +15,6 @@ class SelectAllDetection extends Component { super( ...arguments ); this.setAllSelected = this.setAllSelected.bind( this ); - this.save = this.save.bind( this ); this.state = { isAllSelected: false }; } @@ -24,16 +23,11 @@ class SelectAllDetection extends Component { this.setState( { isAllSelected: true } ); } - save() { - this.props.onSave(); - } - render() { return (
Combination pressed? { isAllSelected ? 'Yes' : 'No' }
@@ -44,19 +38,33 @@ 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, unless you pass the callback as an array with the callback and `true` as entries of the array respectively. Refer to the example above, and see the `shortcuts` prop documentation for more information. - ## 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. To capture a key event globally (including within input fields), assign as the value an array with the function callback as the first argument and `true` as the second argument. +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 +## bindGlobal + +By default, a callback will not be invoked if the key combination occurs in an editable field. Pass `bindGlobal` as `true` if the key events should be observed globally, including within editable fields. + +- Type: `Boolean` +- Required: No + +_Tip:_ If you need some but not all keyboard events to be observed globally, simply render two distinct `KeyboardShortcuts` elements, one with and one without the `bindGlobal` prop. + +## event + +By default, a callback is invoked in response to the `keydown` event. To override this, pass `event` with the name of a specific keyboard event. + +- Type: `String` +- Required: No + ## References - [Mousetrap documentation](https://craig.is/killing/mice) diff --git a/components/keyboard-shortcuts/index.js b/components/keyboard-shortcuts/index.js index 13b46d10c0d235..24b1b48dbe3018 100644 --- a/components/keyboard-shortcuts/index.js +++ b/components/keyboard-shortcuts/index.js @@ -14,19 +14,9 @@ class KeyboardShortcuts extends Component { componentWillMount() { this.mousetrap = new Mousetrap; forEach( this.props.shortcuts, ( callback, key ) => { - // Normalize callback, which can be passed as either a function or - // an array of [ callback, isGlobal ] - let isGlobal = false; - if ( Array.isArray( callback ) ) { - isGlobal = callback[ 1 ]; - callback = callback[ 0 ]; - } - - if ( isGlobal ) { - this.mousetrap.bindGlobal( key, callback ); - } else { - this.mousetrap.bind( key, callback ); - } + const { bindGlobal, eventName } = this.props; + const bindFn = bindGlobal ? 'bindGlobal' : 'bind'; + this.mousetrap[ bindFn ]( key, callback, eventName ); } ); } diff --git a/components/keyboard-shortcuts/test/index.js b/components/keyboard-shortcuts/test/index.js index 196ccd80ae6918..f27cccfff33c4c 100644 --- a/components/keyboard-shortcuts/test/index.js +++ b/components/keyboard-shortcuts/test/index.js @@ -46,8 +46,9 @@ describe( 'KeyboardShortcuts', () => { const wrapper = mount(
, @@ -58,4 +59,27 @@ describe( 'KeyboardShortcuts', () => { expect( spy ).toHaveBeenCalled(); } ); + + it( 'should capture key events on specific event', () => { + const spy = jest.fn(); + const attachNode = document.createElement( 'div' ); + document.body.appendChild( attachNode ); + + const wrapper = mount( +
+ + +
, + { attachTo: attachNode } + ); + + keyPress( 68, wrapper.find( 'textarea' ).getDOMNode() ); + + expect( spy ).toHaveBeenCalled(); + expect( spy.mock.calls[ 0 ][ 0 ].type ).toBe( 'keyup' ); + } ); } ); diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index 2003859d75dea2..ca5feb7fb4637d 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -99,10 +99,14 @@ class BlockToolbar extends Component { role="toolbar" deep > - +
{ ! showMobileControls && [ , From 26016802582015e096a0d88a9a959c6ee8fec552 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 17 Oct 2017 10:49:57 -0400 Subject: [PATCH 4/4] Components: Warn about KeyboardShortcuts prop updates --- components/keyboard-shortcuts/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/keyboard-shortcuts/README.md b/components/keyboard-shortcuts/README.md index b7f75b47f2780f..f7a023c3a0340f 100644 --- a/components/keyboard-shortcuts/README.md +++ b/components/keyboard-shortcuts/README.md @@ -36,8 +36,6 @@ 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: @@ -49,6 +47,10 @@ An object of shortcut bindings, where each key is a keyboard combination, the va - Type: `Object` - Required: No +__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 `KeyboardShortcuts` component will not update to reflect a changed `shortcuts` prop. If you need to change shortcuts, mount a separate `KeyboardShortcuts` element, which can be achieved by assigning a unique `key` prop. + ## bindGlobal By default, a callback will not be invoked if the key combination occurs in an editable field. Pass `bindGlobal` as `true` if the key events should be observed globally, including within editable fields.