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

Writing Flow: Move selection to block's focus handler #5289

Merged
merged 11 commits into from
Mar 7, 2018
10 changes: 6 additions & 4 deletions edit-post/components/visual-editor/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@

// This is a focus style shown for blocks that need an indicator even when in an isEditing state
// like for example an image block that receives arrowkey focus.
.edit-post-visual-editor .editor-block-list__block:not( .is-selected ) .editor-block-list__block-edit {
box-shadow: 0 0 0 0 $white, 0 0 0 0 $dark-gray-900;
transition: .1s box-shadow .05s;
.edit-post-visual-editor .editor-block-list__block:not( .is-selected ) {
.editor-block-list__block-edit {
box-shadow: 0 0 0 0 $white, 0 0 0 0 $dark-gray-900;
transition: .1s box-shadow .05s;
}

&:focus {
&:focus .editor-block-list__block-edit {
box-shadow: 0 0 0 1px $white, 0 0 0 3px $dark-gray-900;
}
}
Expand Down
38 changes: 18 additions & 20 deletions editor/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ export class BlockListBlock extends Component {
// eslint-disable-next-line react/no-find-dom-node
node = findDOMNode( node );

this.wrapperNode = node;

this.props.blockRef( node, this.props.uid );
}

Expand All @@ -214,7 +216,11 @@ export class BlockListBlock extends Component {
focusTabbable() {
const { initialPosition } = this.props;

if ( this.node.contains( document.activeElement ) ) {
// Focus is captured by the wrapper node, so while focus transition
// should only consider tabbables within editable display, since it
// may be the wrapper itself or a side control which triggered the
// focus event, don't unnecessary transition to an inner tabbable.
if ( this.wrapperNode.contains( document.activeElement ) ) {
return;
}

Expand Down Expand Up @@ -370,20 +376,10 @@ export class BlockListBlock extends Component {
* specifically handles the case where block does not set focus on its own
* (via `setFocus`), typically if there is no focusable input in the block.
*
* @param {FocusEvent} event A focus event
*
* @return {void}
*/
onFocus( event ) {
// Firefox-specific: Firefox will redirect focus of an already-focused
// node to its parent, but assign a property before doing so. If that
// property exists, ensure that it is the node, or abort.
const { explicitOriginalTarget } = event.nativeEvent;
if ( explicitOriginalTarget && explicitOriginalTarget !== this.node ) {
return;
}

if ( event.target === this.node && ! this.props.isSelected ) {
onFocus() {
if ( ! this.props.isSelected && ! this.props.isMultiSelected ) {
this.props.onSelect();
}
}
Expand Down Expand Up @@ -422,7 +418,12 @@ export class BlockListBlock extends Component {
} else {
this.props.onSelectionStart( this.props.uid );

if ( ! this.props.isSelected ) {
// Allow user to escape out of a multi-selection to a singular
// selection of a block via click. This is handled here since
// onFocus excludes blocks involved in a multiselection, as
// focus can be incurred by starting a multiselection (focus
// moved to first block's multi-controls).
if ( this.props.isMultiSelected ) {
this.props.onSelect();
}
}
Expand Down Expand Up @@ -588,13 +589,14 @@ export class BlockListBlock extends Component {
className={ wrapperClassName }
data-type={ block.name }
onTouchStart={ this.onTouchStart }
onFocus={ this.onFocus }
onClick={ this.onClick }
tabIndex="0"
childHandledEvents={ [
'onKeyPress',
'onDragStart',
'onMouseDown',
'onKeyDown',
'onFocus',
] }
{ ...wrapperProps }
>
Expand Down Expand Up @@ -627,9 +629,7 @@ export class BlockListBlock extends Component {
onDragStart={ this.preventDrag }
onMouseDown={ this.onPointerDown }
onKeyDown={ this.onKeyDown }
onFocus={ this.onFocus }
className={ BlockListBlock.className }
tabIndex="0"
className="editor-block-list__block-edit"
aria-label={ blockLabel }
data-block={ block.uid }
>
Expand Down Expand Up @@ -767,8 +767,6 @@ const mapDispatchToProps = ( dispatch, ownProps ) => ( {
},
} );

BlockListBlock.className = 'editor-block-list__block-edit';

BlockListBlock.childContextTypes = {
BlockList: noop,
canUserUseUnfilteredHTML: noop,
Expand Down
13 changes: 8 additions & 5 deletions editor/components/block-list/layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { Component } from '@wordpress/element';
import './style.scss';
import BlockListBlock from './block';
import BlockInsertionPoint from './insertion-point';
import IgnoreNestedEvents from './ignore-nested-events';
import BlockSelectionClearer from '../block-selection-clearer';
import DefaultBlockAppender from '../default-block-appender';
import {
Expand Down Expand Up @@ -233,11 +234,13 @@ class BlockListLayout extends Component {
renderBlockMenu={ renderBlockMenu }
/>
) ) }
<DefaultBlockAppender
rootUID={ rootUID }
lastBlockUID={ last( blockUIDs ) }
layout={ defaultLayout }
/>
<IgnoreNestedEvents childHandledEvents={ [ 'onFocus', 'onClick', 'onKeyDown' ] }>
<DefaultBlockAppender
rootUID={ rootUID }
lastBlockUID={ last( blockUIDs ) }
layout={ defaultLayout }
/>
</IgnoreNestedEvents>
</BlockSelectionClearer>
);
}
Expand Down
5 changes: 0 additions & 5 deletions editor/components/block-mover/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { compose } from '@wordpress/element';
import './style.scss';
import { getBlockMoverLabel } from './mover-label';
import { getBlockIndex, getBlock } from '../../store/selectors';
import { selectBlock } from '../../store/actions';

/**
* Module constants
Expand Down Expand Up @@ -94,10 +93,6 @@ export function BlockMover( { onMoveUp, onMoveDown, isFirst, isLast, uids, block
function createOnMove( type, dispatch, ownProps ) {
return () => {
const { uids, rootUID } = ownProps;
if ( uids.length === 1 ) {
dispatch( selectBlock( first( uids ) ) );
}

dispatch( { type, uids, rootUID } );
};
}
Expand Down
2 changes: 1 addition & 1 deletion editor/components/navigable-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class NavigableToolbar extends Component {

// 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 .editor-block-list__block-edit' );
const selectedBlock = document.querySelector( '.editor-block-list__block.is-selected' );
if ( !! selectedBlock ) {
event.stopPropagation();
selectedBlock.focus();
Expand Down
25 changes: 2 additions & 23 deletions editor/components/writing-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class WritingFlow extends Component {
}

getEditables( target ) {
const outer = target.closest( '.editor-block-list__block-edit' );
const outer = target.closest( '.editor-block-list__block' );
if ( ! outer || target === outer ) {
return [ target ];
}
Expand All @@ -74,7 +74,7 @@ class WritingFlow extends Component {
node.nodeName === 'INPUT' ||
node.nodeName === 'TEXTAREA' ||
node.contentEditable === 'true' ||
node.classList.contains( 'editor-block-list__block-edit' )
node.classList.contains( 'editor-block-list__block' )
) );
}

Expand Down Expand Up @@ -128,25 +128,6 @@ class WritingFlow extends Component {
return editables.length > 0 && index === edgeIndex;
}

/**
* Function called to ensure the block parent of the target node is selected.
*
* @param {DOMElement} target
*/
selectParentBlock( target ) {
if ( ! target ) {
return;
}

const parentBlock = target.hasAttribute( 'data-block' ) ? target : target.closest( '[data-block]' );
if (
parentBlock &&
( ! this.props.selectedBlockUID || parentBlock.getAttribute( 'data-block' ) !== this.props.selectedBlockUID )
) {
this.props.onSelectBlock( parentBlock.getAttribute( 'data-block' ) );
}
}

onKeyDown( event ) {
const { selectedBlockUID, selectionStart, hasMultiSelection } = this.props;

Expand Down Expand Up @@ -184,12 +165,10 @@ class WritingFlow extends Component {
} else if ( isVertical && isVerticalEdge( target, isReverse, isShift ) ) {
const closestTabbable = this.getClosestTabbable( target, isReverse );
placeCaretAtVerticalEdge( closestTabbable, isReverse, this.verticalRect );
this.selectParentBlock( closestTabbable );
event.preventDefault();
} else if ( isHorizontal && isHorizontalEdge( target, isReverse, isShift ) ) {
const closestTabbable = this.getClosestTabbable( target, isReverse );
placeCaretAtHorizontalEdge( closestTabbable, isReverse );
this.selectParentBlock( closestTabbable );
event.preventDefault();
}
}
Expand Down