Skip to content
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

Toolbar: Return focus to selection when escape pressed #5551

Merged
merged 3 commits into from
Mar 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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