From 5fc8f97cc1607a22a903dcd7be4463f843b8b2bf Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 10 Oct 2017 16:26:50 +0100 Subject: [PATCH 1/7] Accessibility: Keyboard shortcuts to navigate to/from/in the block's toolbar --- editor/block-toolbar/index.js | 62 ++++++++++++++++++++++++++++++++++- utils/keycodes.js | 1 + 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index 2639aed32a2086..7e57b5ff2ddfb4 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -11,6 +11,7 @@ import classnames from 'classnames'; import { IconButton, Toolbar } from '@wordpress/components'; import { Component, Children } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; +import { focus, keycodes } from '@wordpress/utils'; /** * Internal Dependencies @@ -20,6 +21,11 @@ import BlockSwitcher from '../block-switcher'; import BlockMover from '../block-mover'; import BlockRightMenu from '../block-settings-menu'; +/** + * Module Constants + */ +const { LEFT, RIGHT, ESCAPE, ALT } = keycodes; + function FirstChild( { children } ) { const childrenArray = Children.toArray( children ); return childrenArray[ 0 ] || null; @@ -29,17 +35,71 @@ 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.state = { showMobileControls: false, }; } + componentDidMount() { + document.addEventListener( 'keyup', this.onKeyUp ); + } + + componentWillUnmout() { + document.removeEventListener( 'keyup', this.onKeyUp ); + } + + bindNode( ref ) { + this.toolbar = ref; + } + toggleMobileControls() { this.setState( ( state ) => ( { showMobileControls: ! state.showMobileControls, } ) ); } + onKeyDown( event ) { + // This is required to avoid messing up with the WritingFlow navigation + event.stopPropagation(); + } + + onKeyUp( event ) { + // Is there a better way to focus the selected block + const selectedBlock = document.querySelector( '.editor-visual-editor__block.is-selected' ); + const tabbables = focus.tabbable.find( this.toolbar ); + const indexOfTabbable = tabbables.indexOf( document.activeElement ); + + if ( event.keyCode === ALT ) { + if ( tabbables.length ) { + tabbables[ 0 ].focus(); + } + return; + } + + switch ( event.keyCode ) { + case ESCAPE: + if ( indexOfTabbable !== -1 && selectedBlock ) { + selectedBlock.focus(); + } + break; + case LEFT: + if ( indexOfTabbable > 0 ) { + tabbables[ indexOfTabbable - 1 ].focus(); + } + event.stopPropagation(); + break; + case RIGHT: + if ( indexOfTabbable !== -1 && indexOfTabbable !== tabbables.length - 1 ) { + tabbables[ indexOfTabbable + 1 ].focus(); + } + event.stopPropagation(); + break; + } + } + render() { const { showMobileControls } = this.state; const { uid } = this.props; @@ -57,7 +117,7 @@ class BlockToolbar extends Component { transitionLeave={ false } component={ FirstChild } > -
+
{ ! showMobileControls && [ , diff --git a/utils/keycodes.js b/utils/keycodes.js index fdafef005fd969..c3d63bb678c8c2 100644 --- a/utils/keycodes.js +++ b/utils/keycodes.js @@ -1,6 +1,7 @@ export const BACKSPACE = 8; export const TAB = 9; export const ENTER = 13; +export const ALT = 18; export const ESCAPE = 27; export const SPACE = 32; export const LEFT = 37; From 8c64118a406f2ba07ce16a3f00da4c39601bdac3 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 11 Oct 2017 10:58:56 +0100 Subject: [PATCH 2/7] Block Toolbar: Replace alt with command and support alt + f10 --- editor/block-toolbar/index.js | 30 +++++++++++++++++++++++++++--- utils/keycodes.js | 3 +++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index 7e57b5ff2ddfb4..b77ef18c4691a1 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -24,13 +24,21 @@ import BlockRightMenu from '../block-settings-menu'; /** * Module Constants */ -const { LEFT, RIGHT, ESCAPE, ALT } = keycodes; +const { LEFT, RIGHT, ESCAPE, F10 } = keycodes; function FirstChild( { children } ) { const childrenArray = Children.toArray( children ); return childrenArray[ 0 ] || null; } +function isMac() { + return window.navigator.platform.toLowerCase().indexOf( 'mac' ) !== -1; +} + +function metaKeyPressed( event ) { + return isMac() ? event.metaKey : ( event.ctrlKey && ! event.altKey ); +} + class BlockToolbar extends Component { constructor() { super( ...arguments ); @@ -38,17 +46,24 @@ class BlockToolbar extends Component { this.bindNode = this.bindNode.bind( this ); this.onKeyUp = this.onKeyUp.bind( this ); this.onKeyDown = this.onKeyDown.bind( this ); + this.stopPropagation = this.stopPropagation.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 ); } componentWillUnmout() { document.removeEventListener( 'keyup', this.onKeyUp ); + document.removeEventListener( 'keydown', this.onKeyDown ); } bindNode( ref ) { @@ -62,17 +77,26 @@ class BlockToolbar extends Component { } onKeyDown( event ) { + if ( metaKeyPressed( event ) ) { + this.metaCount++; + } + } + + stopPropagation( event ) { // This is required to avoid messing up with the WritingFlow navigation event.stopPropagation(); } onKeyUp( event ) { + const isMeta = this.metaCount === 1; + this.metaCount = 0; + // Is there a better way to focus the selected block const selectedBlock = document.querySelector( '.editor-visual-editor__block.is-selected' ); const tabbables = focus.tabbable.find( this.toolbar ); const indexOfTabbable = tabbables.indexOf( document.activeElement ); - if ( event.keyCode === ALT ) { + if ( isMeta || ( event.keyCode === F10 && event.altKey ) ) { if ( tabbables.length ) { tabbables[ 0 ].focus(); } @@ -117,7 +141,7 @@ class BlockToolbar extends Component { transitionLeave={ false } component={ FirstChild } > -
+
{ ! showMobileControls && [ , diff --git a/utils/keycodes.js b/utils/keycodes.js index c3d63bb678c8c2..a503b315d2b447 100644 --- a/utils/keycodes.js +++ b/utils/keycodes.js @@ -1,6 +1,7 @@ export const BACKSPACE = 8; export const TAB = 9; export const ENTER = 13; +export const CTRL = 17; export const ALT = 18; export const ESCAPE = 27; export const SPACE = 32; @@ -9,3 +10,5 @@ export const UP = 38; export const RIGHT = 39; export const DOWN = 40; export const DELETE = 46; + +export const F10 = 121; From ac7936d32cbed752e241264a83295b3e0a637288 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 11 Oct 2017 12:54:10 +0100 Subject: [PATCH 3/7] Block Toolbar: Avoid running selectors on eack key --- editor/block-toolbar/index.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index b77ef18c4691a1..80c277937ef30a 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -88,15 +88,17 @@ class BlockToolbar extends Component { } onKeyUp( event ) { - const isMeta = this.metaCount === 1; + const shouldFocusToolbar = this.metaCount === 1 || ( event.keyCode === F10 && event.altKey ); this.metaCount = 0; - // Is there a better way to focus the selected block - const selectedBlock = document.querySelector( '.editor-visual-editor__block.is-selected' ); + if ( ! shouldFocusToolbar && [ ESCAPE, LEFT, RIGHT ].indexOf( event.keyCode ) === -1 ) { + return; + } + const tabbables = focus.tabbable.find( this.toolbar ); const indexOfTabbable = tabbables.indexOf( document.activeElement ); - if ( isMeta || ( event.keyCode === F10 && event.altKey ) ) { + if ( shouldFocusToolbar ) { if ( tabbables.length ) { tabbables[ 0 ].focus(); } @@ -104,11 +106,14 @@ class BlockToolbar extends Component { } switch ( event.keyCode ) { - case ESCAPE: + case ESCAPE: { + // Is there a better way to focus the selected block + const selectedBlock = document.querySelector( '.editor-visual-editor__block.is-selected' ); if ( indexOfTabbable !== -1 && selectedBlock ) { selectedBlock.focus(); } break; + } case LEFT: if ( indexOfTabbable > 0 ) { tabbables[ indexOfTabbable - 1 ].focus(); From 2a407a03d1c0384efc43cd42987664b0efc4f600 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 11 Oct 2017 13:06:54 +0100 Subject: [PATCH 4/7] Block Toolbar: Fix typo componentWillUnmount --- editor/block-toolbar/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index 80c277937ef30a..eec6bc0d9900db 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -61,7 +61,7 @@ class BlockToolbar extends Component { document.addEventListener( 'keydown', this.onKeyDown ); } - componentWillUnmout() { + componentWillUnmount() { document.removeEventListener( 'keyup', this.onKeyUp ); document.removeEventListener( 'keydown', this.onKeyDown ); } From 6f127507d5181d0dd304780216557b7189093c1d Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 13 Oct 2017 09:05:06 +0100 Subject: [PATCH 5/7] Block Toolbar: Reuse Navigable Menu instead of custom keyUp handlers --- components/navigable-menu/README.md | 8 ++++ components/navigable-menu/index.js | 6 +-- editor/block-toolbar/index.js | 67 +++++++++++------------------ 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/components/navigable-menu/README.md b/components/navigable-menu/README.md index 202180a556fa3d..2750f261748c7c 100644 --- a/components/navigable-menu/README.md +++ b/components/navigable-menu/README.md @@ -38,3 +38,11 @@ A callback invoked when the menu navigates to one of its children passing the in - Type: `Function` - Required: No + +## deep + +A boolean to look for navigable children in the direct children or any descendant. + +- Type: `Boolean` +- Required: No +- default: false diff --git a/components/navigable-menu/index.js b/components/navigable-menu/index.js index 11ea3cbe1ebb18..d4ce50ee065e52 100644 --- a/components/navigable-menu/index.js +++ b/components/navigable-menu/index.js @@ -26,18 +26,18 @@ class NavigableMenu extends Component { } onKeyDown( event ) { - const { orientation = 'vertical', onNavigate = noop } = this.props; + const { orientation = 'vertical', onNavigate = noop, deep = false } = this.props; if ( ( orientation === 'vertical' && [ UP, DOWN, TAB ].indexOf( event.keyCode ) === -1 ) || ( orientation === 'horizontal' && [ RIGHT, LEFT, TAB ].indexOf( event.keyCode ) === -1 ) ) { return; } - const tabbables = focus.tabbable .find( this.container ) - .filter( ( node ) => node.parentElement === this.container ); + .filter( ( node ) => deep || node.parentElement === this.container ); const indexOfTabbable = tabbables.indexOf( document.activeElement ); + if ( indexOfTabbable === -1 ) { return; } diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index eec6bc0d9900db..7607d6c66c0b48 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -8,8 +8,8 @@ import classnames from 'classnames'; /** * WordPress Dependencies */ -import { IconButton, Toolbar } from '@wordpress/components'; -import { Component, Children } from '@wordpress/element'; +import { IconButton, Toolbar, NavigableMenu } from '@wordpress/components'; +import { Component, Children, findDOMNode } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { focus, keycodes } from '@wordpress/utils'; @@ -24,7 +24,7 @@ import BlockRightMenu from '../block-settings-menu'; /** * Module Constants */ -const { LEFT, RIGHT, ESCAPE, F10 } = keycodes; +const { ESCAPE, F10 } = keycodes; function FirstChild( { children } ) { const childrenArray = Children.toArray( children ); @@ -46,7 +46,7 @@ class BlockToolbar extends Component { this.bindNode = this.bindNode.bind( this ); this.onKeyUp = this.onKeyUp.bind( this ); this.onKeyDown = this.onKeyDown.bind( this ); - this.stopPropagation = this.stopPropagation.bind( this ); + this.onToolbarKeyDown = this.onToolbarKeyDown.bind( this ); this.state = { showMobileControls: false, }; @@ -67,7 +67,7 @@ class BlockToolbar extends Component { } bindNode( ref ) { - this.toolbar = ref; + this.toolbar = findDOMNode( ref ); } toggleMobileControls() { @@ -82,50 +82,29 @@ class BlockToolbar extends Component { } } - stopPropagation( event ) { - // This is required to avoid messing up with the WritingFlow navigation - event.stopPropagation(); - } - onKeyUp( event ) { const shouldFocusToolbar = this.metaCount === 1 || ( event.keyCode === F10 && event.altKey ); this.metaCount = 0; - if ( ! shouldFocusToolbar && [ ESCAPE, LEFT, RIGHT ].indexOf( event.keyCode ) === -1 ) { - return; - } - - const tabbables = focus.tabbable.find( this.toolbar ); - const indexOfTabbable = tabbables.indexOf( document.activeElement ); - if ( shouldFocusToolbar ) { + const tabbables = focus.tabbable.find( this.toolbar ); if ( tabbables.length ) { tabbables[ 0 ].focus(); } + } + } + + onToolbarKeyDown( event ) { + if ( event.keyCode !== ESCAPE ) { return; } - switch ( event.keyCode ) { - case ESCAPE: { - // Is there a better way to focus the selected block - const selectedBlock = document.querySelector( '.editor-visual-editor__block.is-selected' ); - if ( indexOfTabbable !== -1 && selectedBlock ) { - selectedBlock.focus(); - } - break; - } - case LEFT: - if ( indexOfTabbable > 0 ) { - tabbables[ indexOfTabbable - 1 ].focus(); - } - event.stopPropagation(); - break; - case RIGHT: - if ( indexOfTabbable !== -1 && indexOfTabbable !== tabbables.length - 1 ) { - tabbables[ indexOfTabbable + 1 ].focus(); - } - event.stopPropagation(); - break; + // Is there a better way to focus the selected block + // TODO: separate focused/selected block state and use Redux actions instead + const selectedBlock = document.querySelector( '.editor-visual-editor__block.is-selected .editor-visual-editor__block-edit' ); + if ( !! selectedBlock ) { + event.stopPropagation(); + selectedBlock.focus(); } } @@ -146,8 +125,14 @@ class BlockToolbar extends Component { transitionLeave={ false } component={ FirstChild } > -
-
+ +
{ ! showMobileControls && [ , , @@ -169,7 +154,7 @@ class BlockToolbar extends Component { }
-
+ ); } From 59259b7ec4d85da3e6cf3cafe8f0b0a995142e76 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 13 Oct 2017 09:06:49 +0100 Subject: [PATCH 6/7] Drop unused keycodes --- utils/keycodes.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/utils/keycodes.js b/utils/keycodes.js index a503b315d2b447..6dfa5c7d61120d 100644 --- a/utils/keycodes.js +++ b/utils/keycodes.js @@ -1,8 +1,6 @@ export const BACKSPACE = 8; export const TAB = 9; export const ENTER = 13; -export const CTRL = 17; -export const ALT = 18; export const ESCAPE = 27; export const SPACE = 32; export const LEFT = 37; From 3522271194d3399b3cb44a2689a1f7aab401af2f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 13 Oct 2017 11:43:26 +0100 Subject: [PATCH 7/7] Utils: Move the isMac util to the DOM utils --- editor/block-toolbar/index.js | 5 +---- editor/utils/dom.js | 9 +++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/editor/block-toolbar/index.js b/editor/block-toolbar/index.js index 7607d6c66c0b48..6fd3582b07c25b 100644 --- a/editor/block-toolbar/index.js +++ b/editor/block-toolbar/index.js @@ -20,6 +20,7 @@ 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 @@ -31,10 +32,6 @@ function FirstChild( { children } ) { return childrenArray[ 0 ] || null; } -function isMac() { - return window.navigator.platform.toLowerCase().indexOf( 'mac' ) !== -1; -} - function metaKeyPressed( event ) { return isMac() ? event.metaKey : ( event.ctrlKey && ! event.altKey ); } diff --git a/editor/utils/dom.js b/editor/utils/dom.js index f6396752e573de..4b213a815394f2 100644 --- a/editor/utils/dom.js +++ b/editor/utils/dom.js @@ -86,3 +86,12 @@ 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; +}