Skip to content

Commit

Permalink
Writing Flow: Move selection to block's focus handler (#5289)
Browse files Browse the repository at this point in the history
* Writing Flow: Select block by own focus handler

* Block List: Mark default appender events as block-handled

* Writing Flow: ...

* Block: Move tabIndex to wrapper component

See: #2934
See: https://codepen.io/aduth/pen/MQxRME

Something needs to capture the focus event, which is skipped by button press on the block's toolbar, but bubbles up wrongly to the parent's edit wrapper (which has tabIndex) and causes parent block to select itself.

* Writing Flow: Update block focusable target

* Block: Remove unused block classname constant

Originally used for down-to-new-paragraph in #3973, removed as of #5271

* Block List: Capture focus event on block wrapper

Corresponding to move of tabIndex from inner edit element to wrapper, we also need to capture focus from wrapper. This way, when focus is applied via WritingFlow, the block appropriately becomes selected. This may also make "onSelect" of movers unnecessary.

* Block List: Remove pointer handling of onSelect

Can now rely on focus behavior captured from wrapper node to reflect selection onto blocks, even those without their own focusable elements

* Block List: Avoid select on focus if in multi-selection

Multi-selection causes focus event to be triggered on the block, but at that point `isSelected` is false, so it will trigger `onSelect` and a subsequent `focusTabbables`, thus breaking the intended multi-selection flow.

* Block Mover: Remove explicit select on block move

Focus will bubble to block wrapper (or in case of Firefox/Safari, where button click itself does not trigger focus, focus on the wrapper itself) to incur self-select.

* Block List: Singular select block on clicking multi-selected

This reverts commit 90a5dab.
  • Loading branch information
aduth authored Mar 7, 2018
1 parent cf4cc73 commit 136f2ba
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 58 deletions.
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 @@ -174,6 +174,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 @@ -191,7 +193,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 @@ -347,20 +353,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 @@ -399,7 +395,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 @@ -565,13 +566,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 @@ -604,9 +606,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 @@ -744,8 +744,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

0 comments on commit 136f2ba

Please sign in to comment.