-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Accessibility: Keyboard shortcuts to navigate to/from/in the block's toolbar #2960
Changes from all commits
5fc8f97
8c64118
ac7936d
2a407a0
6f12750
59259b7
3522271
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,9 +8,10 @@ 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'; | ||
|
||
/** | ||
* Internal Dependencies | ||
|
@@ -19,19 +20,51 @@ 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; | ||
|
||
function FirstChild( { children } ) { | ||
const childrenArray = Children.toArray( children ); | ||
return childrenArray[ 0 ] || null; | ||
} | ||
|
||
function metaKeyPressed( event ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it have been possible to use the https://github.com/WordPress/gutenberg/tree/master/components/keyboard-shortcuts There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried but it failed to trigger on "command" key only, not sure why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Might have been running into caveat of Mousetrap not firing callbacks while input is focused. See #3031 for workaround. |
||
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.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 ) { | ||
this.toolbar = findDOMNode( ref ); | ||
} | ||
|
||
toggleMobileControls() { | ||
|
@@ -40,6 +73,38 @@ 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(); | ||
} | ||
} | ||
} | ||
|
||
onToolbarKeyDown( event ) { | ||
if ( event.keyCode !== ESCAPE ) { | ||
return; | ||
} | ||
|
||
// 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(); | ||
} | ||
} | ||
|
||
render() { | ||
const { showMobileControls } = this.state; | ||
const { uid } = this.props; | ||
|
@@ -57,8 +122,14 @@ class BlockToolbar extends Component { | |
transitionLeave={ false } | ||
component={ FirstChild } | ||
> | ||
<div className={ toolbarClassname }> | ||
<div className="editor-block-toolbar__group"> | ||
<NavigableMenu | ||
className={ toolbarClassname } | ||
ref={ this.bindNode } | ||
orientation="horizontal" | ||
role="toolbar" | ||
deep | ||
> | ||
<div className="editor-block-toolbar__group" onKeyDown={ this.onToolbarKeyDown }> | ||
{ ! showMobileControls && [ | ||
<BlockSwitcher key="switcher" uid={ uid } />, | ||
<Slot key="slot" name="Formatting.Toolbar" />, | ||
|
@@ -80,7 +151,7 @@ class BlockToolbar extends Component { | |
} | ||
</Toolbar> | ||
</div> | ||
</div> | ||
</NavigableMenu> | ||
</CSSTransitionGroup> | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,3 +86,12 @@ export function placeCaretAtEdge( container, start = false ) { | |
sel.addRange( range ); | ||
container.focus(); | ||
} | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks 🙇 |
||
* 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; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,5 @@ export const UP = 38; | |
export const RIGHT = 39; | ||
export const DOWN = 40; | ||
export const DELETE = 46; | ||
|
||
export const F10 = 121; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, we should have all keycodes here to make code easier to read 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it uses DOM properties to determine,
isMac
doesn't seem like a DOM-specific utility function.