Skip to content

Commit

Permalink
Toolbar: Return focus to selection when escape pressed (#5551)
Browse files Browse the repository at this point in the history
* Keyboard Shortcuts: Remove global escape clear selected block

This doesn't apply in most text fields, and where it does apply is often unexpected (e.g. contextual toolbar)

* Toolbar: Return focus to selection when escape pressed

* Keyboard Shortcuts: Clear multi-selection by escape
  • Loading branch information
aduth authored Mar 14, 2018
1 parent 52f72b4 commit 4bac4fe
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 38 deletions.
69 changes: 42 additions & 27 deletions editor/components/editor-global-keyboard-shortcuts/index.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,24 @@
/**
* External dependencies
*/
import { connect } from 'react-redux';
import { first, last } from 'lodash';

/**
* WordPress dependencies
*/
import { Component, Fragment, compose } from '@wordpress/element';
import { KeyboardShortcuts, withContext } from '@wordpress/components';

/**
* Internal dependencies
*/
import { getBlockOrder, getMultiSelectedBlockUids } from '../../store/selectors';
import {
clearSelectedBlock,
multiSelect,
redo,
undo,
autosave,
removeBlocks,
} from '../../store/actions';
import { withSelect, withDispatch } from '@wordpress/data';

class EditorGlobalKeyboardShortcuts extends Component {
constructor() {
super( ...arguments );

this.selectAll = this.selectAll.bind( this );
this.undoOrRedo = this.undoOrRedo.bind( this );
this.save = this.save.bind( this );
this.deleteSelectedBlocks = this.deleteSelectedBlocks.bind( this );
this.clearMultiSelection = this.clearMultiSelection.bind( this );
}

selectAll( event ) {
Expand Down Expand Up @@ -64,6 +53,16 @@ class EditorGlobalKeyboardShortcuts extends Component {
}
}

/**
* Clears current multi-selection, if one exists.
*/
clearMultiSelection() {
const { hasMultiSelection, clearSelectedBlock } = this.props;
if ( hasMultiSelection ) {
clearSelectedBlock();
}
}

render() {
return (
<Fragment>
Expand All @@ -74,7 +73,7 @@ class EditorGlobalKeyboardShortcuts extends Component {
'mod+shift+z': this.undoOrRedo,
backspace: this.deleteSelectedBlocks,
del: this.deleteSelectedBlocks,
escape: this.props.clearSelectedBlock,
escape: this.clearMultiSelection,
} }
/>
<KeyboardShortcuts
Expand All @@ -88,28 +87,44 @@ class EditorGlobalKeyboardShortcuts extends Component {
}
}

export default compose(
connect(
( state ) => {
return {
uids: getBlockOrder( state ),
multiSelectedBlockUids: getMultiSelectedBlockUids( state ),
};
},
{
export default compose( [
withSelect( ( select ) => {
const {
getBlockOrder,
getMultiSelectedBlockUids,
hasMultiSelection,
} = select( 'core/editor' );

return {
uids: getBlockOrder(),
multiSelectedBlockUids: getMultiSelectedBlockUids(),
hasMultiSelection: hasMultiSelection(),
};
} ),
withDispatch( ( dispatch ) => {
const {
clearSelectedBlock,
multiSelect,
redo,
undo,
removeBlocks,
autosave,
} = dispatch( 'core/editor' );

return {
clearSelectedBlock,
onMultiSelect: multiSelect,
onRedo: redo,
onUndo: undo,
onRemove: removeBlocks,
onSave: autosave,
}
),
};
} ),
withContext( 'editor' )( ( settings ) => {
const { templateLock } = settings;

return {
isLocked: !! templateLock,
};
} ),
)( EditorGlobalKeyboardShortcuts );
] )( EditorGlobalKeyboardShortcuts );
46 changes: 36 additions & 10 deletions editor/components/navigable-toolbar/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
/**
* External dependencies
*/
import { cond, matchesProperty } from 'lodash';

/**
* WordPress dependencies
*/
import { NavigableMenu, KeyboardShortcuts } from '@wordpress/components';
import { Component, findDOMNode } from '@wordpress/element';
import { focus, keycodes } from '@wordpress/utils';

/**
* Browser dependencies
*/

const { Node, getSelection } = window;

/**
* Module Constants
*/
Expand All @@ -13,9 +24,14 @@ const { ESCAPE } = keycodes;
class NavigableToolbar extends Component {
constructor() {
super( ...arguments );

this.bindNode = this.bindNode.bind( this );
this.focusToolbar = this.focusToolbar.bind( this );
this.onToolbarKeyDown = this.onToolbarKeyDown.bind( this );
this.focusSelection = this.focusSelection.bind( this );

this.switchOnKeyDown = cond( [
[ matchesProperty( [ 'keyCode' ], ESCAPE ), this.focusSelection ],
] );
}

bindNode( ref ) {
Expand All @@ -32,17 +48,27 @@ class NavigableToolbar extends Component {
}
}

onToolbarKeyDown( event ) {
if ( event.keyCode !== ESCAPE ) {
/**
* Programmatically shifts focus to the element where the current selection
* exists, if there is a selection.
*/
focusSelection() {
// Ensure that a selection exists.
const selection = getSelection();
if ( ! selection ) {
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-block-list__block.is-selected' );
if ( !! selectedBlock ) {
event.stopPropagation();
selectedBlock.focus();
// Focus node may be a text node, which cannot be focused directly.
// Find its parent element instead.
const { focusNode } = selection;
let focusElement = focusNode;
if ( focusElement.nodeType !== Node.ELEMENT_NODE ) {
focusElement = focusElement.parentElement;
}

if ( focusElement ) {
focusElement.focus();
}
}

Expand All @@ -54,7 +80,7 @@ class NavigableToolbar extends Component {
role="toolbar"
deep
ref={ this.bindNode }
onKeyDown={ this.onToolbarKeyDown }
onKeyDown={ this.switchOnKeyDown }
{ ...props }
>
<KeyboardShortcuts
Expand Down
18 changes: 17 additions & 1 deletion editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,23 @@ export function isBlockWithinSelection( state, uid ) {
}

/**
* Whether in the process of multi-selecting or not.
* Returns true if a multi-selection has been made, or false otherwise.
*
* @param {Object} state Editor state.
*
* @return {boolean} Whether multi-selection has been made.
*/
export function hasMultiSelection( state ) {
const { start, end } = state.blockSelection;
return start !== end;
}

/**
* Whether in the process of multi-selecting or not. This flag is only true
* while the multi-selection is being selected (by mouse move), and is false
* once the multi-selection has been settled.
*
* @see hasMultiSelection
*
* @param {Object} state Global application state.
*
Expand Down
36 changes: 36 additions & 0 deletions editor/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const {
getNextBlockUid,
isBlockSelected,
isBlockWithinSelection,
hasMultiSelection,
isBlockMultiSelected,
isFirstMultiSelectedBlock,
getBlockMode,
Expand Down Expand Up @@ -1804,6 +1805,41 @@ describe( 'selectors', () => {
} );
} );

describe( 'hasMultiSelection', () => {
it( 'should return false if no selection', () => {
const state = {
blockSelection: {
start: null,
end: null,
},
};

expect( hasMultiSelection( state ) ).toBe( false );
} );

it( 'should return false if singular selection', () => {
const state = {
blockSelection: {
start: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1',
end: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1',
},
};

expect( hasMultiSelection( state ) ).toBe( false );
} );

it( 'should return true if multi-selection', () => {
const state = {
blockSelection: {
start: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1',
end: '9db792c6-a25a-495d-adbd-97d56a4c4189',
},
};

expect( hasMultiSelection( state ) ).toBe( true );
} );
} );

describe( 'isBlockMultiSelected', () => {
const state = {
editor: {
Expand Down

0 comments on commit 4bac4fe

Please sign in to comment.