From 9767af6b0c3095b2cf8317d5bef03c1002a684aa Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 11:21:13 -0500 Subject: [PATCH 1/8] Block List: Fix select previous on backspace behavior Regression of #5025, where prop was changed from `previousBlock` to `previousBlockUid`, but neglected to update the instance of the prop reference in keydown handler --- editor/components/block-list/block.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/editor/components/block-list/block.js b/editor/components/block-list/block.js index c2628584b10df7..99f2b6bc5cbe0e 100644 --- a/editor/components/block-list/block.js +++ b/editor/components/block-list/block.js @@ -438,13 +438,13 @@ export class BlockListBlock extends Component { case DELETE: // Remove block on backspace. if ( target === this.node ) { - const { uid, onRemove, isLocked, previousBlock, onSelect } = this.props; + const { uid, onRemove, isLocked, previousBlockUid, onSelect } = this.props; event.preventDefault(); if ( ! isLocked ) { onRemove( uid ); - if ( previousBlock ) { - onSelect( previousBlock.uid, -1 ); + if ( previousBlockUid ) { + onSelect( previousBlockUid, -1 ); } } } From 31e70d58a519d9bfb21da5a9d29fb2fb29fa8cc1 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 12:07:38 -0500 Subject: [PATCH 2/8] Utils: Improve spec compliancy of text field --- utils/dom.js | 17 +++++++++------ utils/test/dom.js | 54 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/utils/dom.js b/utils/dom.js index afb6799396be1d..1088d56b413bdf 100644 --- a/utils/dom.js +++ b/utils/dom.js @@ -318,16 +318,21 @@ export function placeCaretAtVerticalEdge( container, isReverse, rect, mayUseScro } /** - * Check whether the given node in an input field. + * Check whether the given element is a text field, where text field is defined + * by the ability to select within the input, or that it is contenteditable. + * + * See: https://html.spec.whatwg.org/#textFieldSelection * * @param {HTMLElement} element The HTML element. * - * @return {boolean} True if the element is an input field, false if not. + * @return {boolean} True if the element is an text field, false if not. */ -export function isInputField( { nodeName, contentEditable } ) { +export function isTextField( element ) { + const { nodeName, selectionStart, contentEditable } = element; + return ( - nodeName === 'INPUT' || - nodeName === 'TEXTAREA' || + ( nodeName === 'INPUT' && selectionStart !== null ) || + ( nodeName === 'TEXTAREA' ) || contentEditable === 'true' ); } @@ -339,7 +344,7 @@ export function isInputField( { nodeName, contentEditable } ) { * @return {boolean} True if there is selection, false if not. */ export function documentHasSelection() { - if ( isInputField( document.activeElement ) ) { + if ( isTextField( document.activeElement ) ) { return true; } diff --git a/utils/test/dom.js b/utils/test/dom.js index b908403ecb4df2..9f3bc16db01def 100644 --- a/utils/test/dom.js +++ b/utils/test/dom.js @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import { isHorizontalEdge, placeCaretAtHorizontalEdge, isInputField } from '../dom'; +import { isHorizontalEdge, placeCaretAtHorizontalEdge, isTextField } from '../dom'; describe( 'DOM', () => { let parent; @@ -92,13 +92,53 @@ describe( 'DOM', () => { } ); } ); - describe( 'isInputfield', () => { - it( 'should return true for an input element', () => { - expect( isInputField( document.createElement( 'input' ) ) ).toBe( true ); + describe( 'isTextField', () => { + /** + * A sampling of input types expected not to be text eligible. + * + * @type {string[]} + */ + const NON_TEXT_INPUT_TYPES = [ + 'button', + 'checkbox', + 'image', + 'hidden', + 'radio', + 'submit', + ]; + + /** + * A sampling of input types expected to be text eligible. + * + * @type {string[]} + */ + const TEXT_INPUT_TYPES = [ + 'text', + 'password', + 'search', + 'url', + ]; + + it( 'should return false for non-text input elements', () => { + NON_TEXT_INPUT_TYPES.forEach( ( type ) => { + const input = document.createElement( 'input' ); + input.type = type; + + expect( isTextField( input ) ).toBe( false ); + } ); + } ); + + it( 'should return true for text input elements', () => { + TEXT_INPUT_TYPES.forEach( ( type ) => { + const input = document.createElement( 'input' ); + input.type = type; + + expect( isTextField( input ) ).toBe( true ); + } ); } ); it( 'should return true for an textarea element', () => { - expect( isInputField( document.createElement( 'textarea' ) ) ).toBe( true ); + expect( isTextField( document.createElement( 'textarea' ) ) ).toBe( true ); } ); it( 'should return true for a contenteditable element', () => { @@ -106,11 +146,11 @@ describe( 'DOM', () => { div.contentEditable = 'true'; - expect( isInputField( div ) ).toBe( true ); + expect( isTextField( div ) ).toBe( true ); } ); it( 'should return true for a normal div element', () => { - expect( isInputField( document.createElement( 'div' ) ) ).toBe( false ); + expect( isTextField( document.createElement( 'div' ) ) ).toBe( false ); } ); } ); } ); From c950541755d68c08ff73c009792caad142ee2464 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 12:17:36 -0500 Subject: [PATCH 3/8] Block: Prefer focus text field on selection, fallback to wrapper --- editor/components/block-list/block.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/editor/components/block-list/block.js b/editor/components/block-list/block.js index 99f2b6bc5cbe0e..0382d098eb9c98 100644 --- a/editor/components/block-list/block.js +++ b/editor/components/block-list/block.js @@ -13,6 +13,7 @@ import { Component, findDOMNode, compose } from '@wordpress/element'; import { keycodes, focus, + isTextField, placeCaretAtHorizontalEdge, placeCaretAtVerticalEdge, } from '@wordpress/utils'; @@ -202,15 +203,15 @@ export class BlockListBlock extends Component { } // Find all tabbables within node. - const tabbables = focus.tabbable.find( this.node ) - .filter( ( node ) => node !== this.node ); + const textInputs = focus.tabbable.find( this.node ).filter( isTextField ); // If reversed (e.g. merge via backspace), use the last in the set of // tabbables. const isReverse = -1 === initialPosition; - const target = ( isReverse ? last : first )( tabbables ); + const target = ( isReverse ? last : first )( textInputs ); if ( ! target ) { + this.wrapperNode.focus(); return; } From a4d52572933bcea1791052267aee1919898a4997 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 13:05:12 -0500 Subject: [PATCH 4/8] Writing Flow: Refactor getClosestTabbable to support non-tabbable target --- editor/components/writing-flow/index.js | 102 +++++++++++++++--------- editor/utils/dom.js | 29 +++++++ utils/focus/tabbable.js | 2 +- 3 files changed, 96 insertions(+), 37 deletions(-) diff --git a/editor/components/writing-flow/index.js b/editor/components/writing-flow/index.js index 2e91cea7b30245..ef258e42f26780 100644 --- a/editor/components/writing-flow/index.js +++ b/editor/components/writing-flow/index.js @@ -2,7 +2,6 @@ * External dependencies */ import { connect } from 'react-redux'; -import 'element-closest'; import { find, reverse, get } from 'lodash'; /** @@ -12,6 +11,7 @@ import { Component } from '@wordpress/element'; import { keycodes, focus, + isTextField, computeCaretRect, isHorizontalEdge, isVerticalEdge, @@ -33,6 +33,10 @@ import { multiSelect, selectBlock, } from '../../store/actions'; +import { + isBlockFocusStop, + isInSameBlock, +} from '../../utils/dom'; /** * Module Constants @@ -57,50 +61,67 @@ class WritingFlow extends Component { this.verticalRect = null; } - getEditables( target ) { - const outer = target.closest( '.editor-block-list__block' ); - if ( ! outer || target === outer ) { - return [ target ]; - } - - const elements = outer.querySelectorAll( '[contenteditable="true"]' ); - return [ ...elements ]; - } - - getVisibleTabbables() { - return focus.tabbable - .find( this.container ) - .filter( ( node ) => ( - node.nodeName === 'INPUT' || - node.nodeName === 'TEXTAREA' || - node.contentEditable === 'true' || - node.classList.contains( 'editor-block-list__block' ) - ) ); - } - + /** + * Returns the optimal tab target from the given focused element in the + * desired direction. A preference is made toward text fields, falling back + * to the block focus stop if no other candidates exist for the block. + * + * @param {Element} target Currently focused text field. + * @param {boolean} isReverse True if considering as the first field. + * + * @return {?Element} Optimal tab target, if one exists. + */ getClosestTabbable( target, isReverse ) { - let focusableNodes = this.getVisibleTabbables(); + // Since the current focus target is not guaranteed to be a text field, + // find all focusables. Tabbability is considered later. + let focusableNodes = focus.focusable.find( this.container ); if ( isReverse ) { focusableNodes = reverse( focusableNodes ); } - focusableNodes = focusableNodes.slice( focusableNodes.indexOf( target ) ); + // Consider as candidates those focusables after the current target. + // It's assumed this can only be reached if the target is focusable + // (on its keydown event), so no need to verify it exists in the set. + focusableNodes = focusableNodes.slice( focusableNodes.indexOf( target ) + 1 ); - return find( focusableNodes, ( node, i, array ) => { - if ( node.contains( target ) ) { + function isTabCandidate( node, i, array ) { + // Not a candidate if the node is not tabbable. + if ( ! focus.tabbable.isTabbableIndex( node ) ) { return false; } - const nextNode = array[ i + 1 ]; + // Prefer text fields, but settle for block focus stop. + if ( ! isTextField( node ) && ! isBlockFocusStop( node ) ) { + return false; + } - // Skip node if it contains a focusable node. - if ( nextNode && node.contains( nextNode ) ) { + // If navigating out of a block (in reverse), don't consider its + // block focus stop. + if ( node.contains( target ) ) { return false; } + // In case of block focus stop, check to see if there's a better + // text field candidate within. + for ( let offset = 1, nextNode; ( nextNode = array[ i + offset ] ); offset++ ) { + // Abort if no longer testing descendents of focus stop. + if ( ! node.contains( nextNode ) ) { + break; + } + + // Apply same tests by recursion. This is important to consider + // nestable blocks where we don't want to settle for the inner + // block focus stop. + if ( isTabCandidate( nextNode, i + offset, array ) ) { + return false; + } + } + return true; - } ); + } + + return find( focusableNodes, isTabCandidate ); } expandSelection( currentStartUid, isReverse ) { @@ -121,11 +142,20 @@ class WritingFlow extends Component { } } - isEditableEdge( moveUp, target ) { - const editables = this.getEditables( target ); - const index = editables.indexOf( target ); - const edgeIndex = moveUp ? 0 : editables.length - 1; - return editables.length > 0 && index === edgeIndex; + /** + * Returns true if the given target field is the last in its block which + * can be considered for tab transition. For example, in a block with two + * text fields, this would return true when reversing from the first of the + * two fields, but false when reversing from the second. + * + * @param {Element} target Currently focused text field. + * @param {boolean} isReverse True if considering as the first field. + * + * @return {boolean} Whether field is at edge for tab transition. + */ + isTabbableEdge( target, isReverse ) { + const closestTabbable = this.getClosestTabbable( target, isReverse ); + return ! isInSameBlock( target, closestTabbable ); } onKeyDown( event ) { @@ -154,7 +184,7 @@ class WritingFlow extends Component { // Shift key is down and existing block multi-selection event.preventDefault(); this.expandSelection( selectionStart, isReverse ); - } else if ( isNav && isShift && this.isEditableEdge( isReverse, target ) && isNavEdge( target, isReverse, true ) ) { + } else if ( isNav && isShift && this.isTabbableEdge( target, isReverse ) && isNavEdge( target, isReverse, true ) ) { // Shift key is down, but no existing block multi-selection event.preventDefault(); this.expandSelection( selectedBlockUID, isReverse ); diff --git a/editor/utils/dom.js b/editor/utils/dom.js index 21208bec1f52e2..5e4c7db2b7d764 100644 --- a/editor/utils/dom.js +++ b/editor/utils/dom.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import 'element-closest'; + /** * Given a block UID, returns the corresponding DOM node for the block, if * exists. As much as possible, this helper should be avoided, and used only @@ -10,3 +15,27 @@ export function getBlockDOMNode( uid ) { return document.querySelector( '[data-block="' + uid + '"]' ); } + +/** + * Returns true if the given HTMLElement is a block focus stop. Blocks without + * their own text fields rely on the focus stop to be keyboard navigable. + * + * @param {HTMLElement} element Element to test. + * + * @return {boolean} Whether element is a block focus stop. + */ +export function isBlockFocusStop( element ) { + return element.classList.contains( 'editor-block-list__block' ); +} + +/** + * Returns true if two elements are contained within the same block. + * + * @param {HTMLElement} a First element. + * @param {HTMLElement} b Second element. + * + * @return {boolean} Whether elements are in the same block. + */ +export function isInSameBlock( a, b ) { + return a.closest( '[data-block]' ) === b.closest( '[data-block]' ); +} diff --git a/utils/focus/tabbable.js b/utils/focus/tabbable.js index 88280228f96a97..e70cba256401d1 100644 --- a/utils/focus/tabbable.js +++ b/utils/focus/tabbable.js @@ -27,7 +27,7 @@ function getTabIndex( element ) { * * @return {boolean} Whether element is tabbable. */ -function isTabbableIndex( element ) { +export function isTabbableIndex( element ) { return getTabIndex( element ) !== -1; } From 11919abef7f1424885a2fb64dbf447246244e46f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 15:53:56 -0500 Subject: [PATCH 5/8] Block List: Extract typing monitor to separate component --- edit-post/components/visual-editor/index.js | 27 ++-- editor/components/block-list/block.js | 107 +------------ editor/components/index.js | 1 + editor/components/observe-typing/README.md | 18 +++ editor/components/observe-typing/index.js | 166 ++++++++++++++++++++ 5 files changed, 206 insertions(+), 113 deletions(-) create mode 100644 editor/components/observe-typing/README.md create mode 100644 editor/components/observe-typing/index.js diff --git a/edit-post/components/visual-editor/index.js b/edit-post/components/visual-editor/index.js index a439953e390826..cdf593a640b015 100644 --- a/edit-post/components/visual-editor/index.js +++ b/edit-post/components/visual-editor/index.js @@ -6,6 +6,7 @@ import { CopyHandler, PostTitle, WritingFlow, + ObserveTyping, EditorGlobalKeyboardShortcuts, BlockSelectionClearer, MultiSelectScrollIntoView, @@ -26,18 +27,20 @@ function VisualEditor( { hasFixedToolbar, isLargeViewport } ) { - - - ( - - - { children } - - ) } - /> - + + + + ( + + + { children } + + ) } + /> + + ); } diff --git a/editor/components/block-list/block.js b/editor/components/block-list/block.js index 0382d098eb9c98..758de4b7b6a36e 100644 --- a/editor/components/block-list/block.js +++ b/editor/components/block-list/block.js @@ -54,8 +54,6 @@ import { removeBlock, replaceBlocks, selectBlock, - startTyping, - stopTyping, updateBlockAttributes, toggleSelection, } from '../../store/actions'; @@ -76,7 +74,7 @@ import { getSelectedBlocksInitialCaretPosition, } from '../../store/selectors'; -const { BACKSPACE, ESCAPE, DELETE, ENTER, UP, RIGHT, DOWN, LEFT } = keycodes; +const { BACKSPACE, ESCAPE, DELETE, ENTER } = keycodes; export class BlockListBlock extends Component { constructor() { @@ -87,8 +85,6 @@ export class BlockListBlock extends Component { this.setAttributes = this.setAttributes.bind( this ); this.maybeHover = this.maybeHover.bind( this ); this.hideHoverEffects = this.hideHoverEffects.bind( this ); - this.maybeStartTyping = this.maybeStartTyping.bind( this ); - this.stopTypingOnMouseMove = this.stopTypingOnMouseMove.bind( this ); this.mergeBlocks = this.mergeBlocks.bind( this ); this.onFocus = this.onFocus.bind( this ); this.preventDrag = this.preventDrag.bind( this ); @@ -99,13 +95,11 @@ export class BlockListBlock extends Component { this.onTouchStart = this.onTouchStart.bind( this ); this.onClick = this.onClick.bind( this ); this.selectOnOpen = this.selectOnOpen.bind( this ); - this.onSelectionChange = this.onSelectionChange.bind( this ); this.hadTouchStart = false; this.state = { error: null, isHovered: false, - isSelectionCollapsed: true, }; } @@ -127,46 +121,23 @@ export class BlockListBlock extends Component { } componentDidMount() { - if ( this.props.isTyping ) { - document.addEventListener( 'mousemove', this.stopTypingOnMouseMove ); - } - document.addEventListener( 'selectionchange', this.onSelectionChange ); - if ( this.props.isSelected ) { this.focusTabbable(); } } componentWillReceiveProps( newProps ) { - if ( newProps.isTyping || newProps.isSelected ) { + if ( newProps.isTypingWithinBlock || newProps.isSelected ) { this.hideHoverEffects(); } } componentDidUpdate( prevProps ) { - // Bind or unbind mousemove from page when user starts or stops typing - if ( this.props.isTyping !== prevProps.isTyping ) { - if ( this.props.isTyping ) { - document.addEventListener( 'mousemove', this.stopTypingOnMouseMove ); - } else { - this.removeStopTypingListener(); - } - } - if ( this.props.isSelected && ! prevProps.isSelected ) { this.focusTabbable(); } } - componentWillUnmount() { - this.removeStopTypingListener(); - document.removeEventListener( 'selectionchange', this.onSelectionChange ); - } - - removeStopTypingListener() { - document.removeEventListener( 'mousemove', this.stopTypingOnMouseMove ); - } - setBlockListRef( node ) { // Disable reason: The root return element uses a component to manage // event nesting, but the parent block list layout needs the raw DOM @@ -298,31 +269,6 @@ export class BlockListBlock extends Component { } } - maybeStartTyping() { - // We do not want to dispatch start typing if state value already reflects - // that we're typing (dispatch noise) - if ( ! this.props.isTyping ) { - this.props.onStartTyping(); - } - } - - stopTypingOnMouseMove( { clientX, clientY } ) { - const { lastClientX, lastClientY } = this; - - // We need to check that the mouse really moved - // Because Safari trigger mousemove event when we press shift, ctrl... - if ( - lastClientX && - lastClientY && - ( lastClientX !== clientX || lastClientY !== clientY ) - ) { - this.props.onStopTyping(); - } - - this.lastClientX = clientX; - this.lastClientY = clientY; - } - mergeBlocks( forward = false ) { const { block, previousBlockUid, nextBlockUid, onMerge } = this.props; @@ -339,10 +285,6 @@ export class BlockListBlock extends Component { } else { onMerge( previousBlockUid, block.uid ); } - - // Manually trigger typing mode, since merging will remove this block and - // cause onKeyDown to not fire - this.maybeStartTyping(); } insertBlocksAfter( blocks ) { @@ -421,18 +363,6 @@ export class BlockListBlock extends Component { createBlock( 'core/paragraph' ), ], this.props.order + 1 ); } - - // Pressing enter should trigger typing mode after the content has split - this.maybeStartTyping(); - break; - - case UP: - case RIGHT: - case DOWN: - case LEFT: - // Arrow keys do not fire keypress event, but should still - // trigger typing mode. - this.maybeStartTyping(); break; case BACKSPACE: @@ -449,9 +379,6 @@ export class BlockListBlock extends Component { } } } - - // Pressing backspace should trigger typing mode - this.maybeStartTyping(); break; case ESCAPE: @@ -471,19 +398,6 @@ export class BlockListBlock extends Component { } } - onSelectionChange() { - if ( ! this.props.isSelected ) { - return; - } - - const selection = window.getSelection(); - const isCollapsed = selection.rangeCount > 0 && selection.getRangeAt( 0 ).collapsed; - // We only keep track of the collapsed selection for selected blocks. - if ( isCollapsed !== this.state.isSelectionCollapsed && this.props.isSelected ) { - this.setState( { isSelectionCollapsed: isCollapsed } ); - } - } - render() { const { block, @@ -500,6 +414,7 @@ export class BlockListBlock extends Component { isMultiSelected, isFirstMultiSelected, isLastInSelection, + isTypingWithinBlock, } = this.props; const isHovered = this.state.isHovered && ! this.props.isMultiSelecting; const { name: blockName, isValid } = block; @@ -509,11 +424,11 @@ export class BlockListBlock extends Component { // The block as rendered in the editor is composed of general block UI // (mover, toolbar, wrapper) and the display of the block content. - // If the block is selected and we're typing the block should not appear as selected unless the selection is not collapsed. + // If the block is selected and we're typing the block should not appear. // Empty paragraph blocks should always show up as unselected. const isEmptyDefaultBlock = isUnmodifiedDefaultBlock( block ); + const isSelectedNotTyping = isSelected && ! isTypingWithinBlock; const showSideInserter = ( isSelected || isHovered ) && isEmptyDefaultBlock; - const isSelectedNotTyping = isSelected && ( ! this.props.isTyping || ! this.state.isSelectionCollapsed ); const shouldAppearSelected = ! showSideInserter && isSelectedNotTyping; const shouldShowMovers = shouldAppearSelected || isHovered || ( isEmptyDefaultBlock && isSelectedNotTyping ); const shouldShowSettingsMenu = shouldShowMovers; @@ -571,7 +486,6 @@ export class BlockListBlock extends Component { onClick={ this.onClick } tabIndex="0" childHandledEvents={ [ - 'onKeyPress', 'onDragStart', 'onMouseDown', 'onKeyDown', @@ -603,7 +517,6 @@ export class BlockListBlock extends Component { { isFirstMultiSelected && } { isLastInSelection: state.blockSelection.end === uid, // We only care about this prop when the block is selected // Thus to avoid unnecessary rerenders we avoid updating the prop if the block is not selected. - isTyping: isSelected && isTyping( state ), + isTypingWithinBlock: isSelected && isTyping( state ), order: getBlockIndex( state, uid, rootUID ), meta: getEditedPostAttribute( state, 'meta' ), mode: getBlockMode( state, uid ), @@ -702,14 +615,6 @@ const mapDispatchToProps = ( dispatch, ownProps ) => ( { dispatch( clearSelectedBlock() ); }, - onStartTyping() { - dispatch( startTyping() ); - }, - - onStopTyping() { - dispatch( stopTyping() ); - }, - onInsertBlocks( blocks, index ) { const { rootUID, layout } = ownProps; diff --git a/editor/components/index.js b/editor/components/index.js index f874a9e7c16be3..e67d431fb1b44e 100644 --- a/editor/components/index.js +++ b/editor/components/index.js @@ -65,6 +65,7 @@ export { default as Inserter } from './inserter'; export { default as MultiBlocksSwitcher } from './block-switcher/multi-blocks-switcher'; export { default as MultiSelectScrollIntoView } from './multi-select-scroll-into-view'; export { default as NavigableToolbar } from './navigable-toolbar'; +export { default as ObserveTyping } from './observe-typing'; export { default as PreserveScrollInReorder } from './preserve-scroll-in-reorder'; export { default as Warning } from './warning'; export { default as WritingFlow } from './writing-flow'; diff --git a/editor/components/observe-typing/README.md b/editor/components/observe-typing/README.md new file mode 100644 index 00000000000000..06f5e85bfdc878 --- /dev/null +++ b/editor/components/observe-typing/README.md @@ -0,0 +1,18 @@ +Observe Typing +============== + +`` is a component used in managing the editor's internal typing flag. When used to wrap content — typically the top-level block list — it observes keyboard and mouse events to set and unset the typing flag. The typing flag is used in considering whether the block border and controls should be visible. While typing, these elements are hidden for a distraction-free experience. + +## Usage + +Wrap the component where blocks are to be rendered with ``: + +```jsx +function VisualEditor() { + return ( + + + + ); +} +``` diff --git a/editor/components/observe-typing/index.js b/editor/components/observe-typing/index.js new file mode 100644 index 00000000000000..af5502d7ef3675 --- /dev/null +++ b/editor/components/observe-typing/index.js @@ -0,0 +1,166 @@ +/** + * External dependencies + */ +import { includes } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component, compose } from '@wordpress/element'; +import { withSelect, withDispatch } from '@wordpress/data'; +import { isTextField, keycodes } from '@wordpress/utils'; + +const { UP, RIGHT, DOWN, LEFT, ENTER, BACKSPACE } = keycodes; + +/** + * Set of key codes upon which typing is to be initiated on a keydown event. + * + * @type {number[]} + */ +const KEY_DOWN_ELIGIBLE_KEY_CODES = [ UP, RIGHT, DOWN, LEFT, ENTER, BACKSPACE ]; + +/** + * Returns true if a given keydown event can be inferred as intent to start + * typing, or false otherwise. A keydown is considered eligible if it is a + * text navigation without shift active. + * + * @param {KeyboardEvent} event Keydown event to test. + * + * @return {boolean} Whether event is eligible to start typing. + */ +function isKeyDownEligibleForStartTyping( event ) { + const { keyCode, shiftKey } = event; + return ! shiftKey && includes( KEY_DOWN_ELIGIBLE_KEY_CODES, keyCode ); +} + +class ObserveTyping extends Component { + constructor() { + super( ...arguments ); + + this.stopTypingOnSelectionUncollapse = this.stopTypingOnSelectionUncollapse.bind( this ); + this.stopTypingOnMouseMove = this.stopTypingOnMouseMove.bind( this ); + this.startTypingInTextField = this.startTypingInTextField.bind( this ); + + this.lastMouseMove = null; + } + + componentDidMount() { + this.toggleEventBindings( this.props.isTyping ); + } + + componentDidUpdate( prevProps ) { + if ( this.props.isTyping !== prevProps.isTyping ) { + this.toggleEventBindings( this.props.isTyping ); + } + } + + componentWillUnmount() { + this.toggleEventBindings( false ); + } + + /** + * Bind or unbind events to the document when typing has started or stopped + * respectively, or when component has become unmounted. + * + * @param {boolean} isBound Whether event bindings should be applied. + */ + toggleEventBindings( isBound ) { + const bindFn = isBound ? 'addEventListener' : 'removeEventListener'; + document[ bindFn ]( 'selectionchange', this.stopTypingOnSelectionUncollapse ); + document[ bindFn ]( 'mousemove', this.stopTypingOnMouseMove ); + } + + /** + * On mouse move, unset typing flag if user has moved cursor. + * + * @param {MouseEvent} event Mousemove event. + */ + stopTypingOnMouseMove( event ) { + const { clientX, clientY } = event; + + // We need to check that the mouse really moved because Safari triggers + // mousemove events when shift or ctrl are pressed. + if ( this.lastMouseMove ) { + const { + clientX: lastClientX, + clientY: lastClientY, + } = this.lastMouseMove; + + if ( lastClientX !== clientX || lastClientY !== clientY ) { + this.props.onStopTyping(); + } + } + + this.lastMouseMove = { clientX, clientY }; + } + + /** + * On selection change, unset typing flag if user has made an uncollapsed + * (shift) selection. + */ + stopTypingOnSelectionUncollapse() { + const selection = window.getSelection(); + const isCollapsed = selection.rangeCount > 0 && selection.getRangeAt( 0 ).collapsed; + + if ( ! isCollapsed ) { + this.props.onStopTyping(); + } + } + + /** + * Handles a keypress or keydown event to infer intention to start typing. + * + * @param {KeyboardEvent} event Keypress or keydown event to interpret. + */ + startTypingInTextField( event ) { + const { isTyping, onStartTyping } = this.props; + const { type, target } = event; + + // Abort early if already typing, or key press is incurred outside a + // text field (e.g. arrow-ing through toolbar buttons). + if ( isTyping || ! isTextField( target ) ) { + return; + } + + // Special-case keydown because certain keys do not emit a keypress + // event. Conversely avoid keydown as the canonical event since there + // are many keydown which are explicitly not targeted for typing. + if ( type === 'keydown' && ! isKeyDownEligibleForStartTyping( event ) ) { + return; + } + + onStartTyping(); + } + + render() { + const { children } = this.props; + + // Disable reason: This component is responsible for capturing bubbled + // keyboard events which are interpreted as typing intent. + + /* eslint-disable jsx-a11y/no-static-element-interactions */ + return ( +
+ { children } +
+ ); + /* eslint-enable jsx-a11y/no-static-element-interactions */ + } +} + +export default compose( [ + withSelect( ( select ) => { + return { + isTyping: select( 'core/editor' ).isTyping(), + }; + } ), + withDispatch( ( dispatch ) => { + return { + onStartTyping: dispatch( 'core/editor' ).startTyping, + onStopTyping: dispatch( 'core/editor' ).stopTyping, + }; + } ), +] )( ObserveTyping ); From f48ba1b35cac914358c5f3c5496342b42b2ae483 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 15:54:56 -0500 Subject: [PATCH 6/8] Block List: Don't deselect block on escape press Escape doesn't clear focus, so causes problems that block is not selected but retains focus (since isSelected state is synced with focus) --- editor/components/block-list/block.js | 7 +------ test/e2e/integration/002-adding-blocks.js | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/editor/components/block-list/block.js b/editor/components/block-list/block.js index 758de4b7b6a36e..75da71b50cbd12 100644 --- a/editor/components/block-list/block.js +++ b/editor/components/block-list/block.js @@ -74,7 +74,7 @@ import { getSelectedBlocksInitialCaretPosition, } from '../../store/selectors'; -const { BACKSPACE, ESCAPE, DELETE, ENTER } = keycodes; +const { BACKSPACE, DELETE, ENTER } = keycodes; export class BlockListBlock extends Component { constructor() { @@ -380,11 +380,6 @@ export class BlockListBlock extends Component { } } break; - - case ESCAPE: - // Deselect on escape. - this.props.onDeselect(); - break; } } diff --git a/test/e2e/integration/002-adding-blocks.js b/test/e2e/integration/002-adding-blocks.js index 7ee63e76bf929b..f69c667e4027d2 100644 --- a/test/e2e/integration/002-adding-blocks.js +++ b/test/e2e/integration/002-adding-blocks.js @@ -13,7 +13,7 @@ describe( 'Adding blocks', () => { // Default block appender is provisional cy.get( lastBlockSelector ).then( ( firstBlock ) => { cy.get( '.editor-default-block-appender' ).click(); - cy.focused().type( '{esc}' ); + cy.get( '.editor-post-title__input' ).click(); cy.get( lastBlockSelector ).should( 'have.text', firstBlock.text() ); } ); From bf49ca97cbec69ca8e3eefa1d64b34bfa7cb7523 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 16:04:47 -0500 Subject: [PATCH 7/8] Block List: Fix delete or insert after focused block wrapper node --- editor/components/block-list/block.js | 46 +++++++++++++++------------ 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/editor/components/block-list/block.js b/editor/components/block-list/block.js index 75da71b50cbd12..7b95a45ca6fec8 100644 --- a/editor/components/block-list/block.js +++ b/editor/components/block-list/block.js @@ -89,7 +89,7 @@ export class BlockListBlock extends Component { this.onFocus = this.onFocus.bind( this ); this.preventDrag = this.preventDrag.bind( this ); this.onPointerDown = this.onPointerDown.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); + this.deleteOrInsertAfterWrapper = this.deleteOrInsertAfterWrapper.bind( this ); this.onBlockError = this.onBlockError.bind( this ); this.insertBlocksAfter = this.insertBlocksAfter.bind( this ); this.onTouchStart = this.onTouchStart.bind( this ); @@ -349,36 +349,41 @@ export class BlockListBlock extends Component { } } - onKeyDown( event ) { + /** + * Interprets keydown event intent to remove or insert after block if key + * event occurs on wrapper node. This can occur when the block has no text + * fields of its own, particularly after initial insertion, to allow for + * easy deletion and continuous writing flow to add additional content. + * + * @param {KeyboardEvent} event Keydown event. + */ + deleteOrInsertAfterWrapper( event ) { const { keyCode, target } = event; + if ( target !== this.wrapperNode || this.props.isLocked ) { + return; + } + switch ( keyCode ) { case ENTER: // Insert default block after current block if enter and event // not already handled by descendant. - if ( target === this.node && ! this.props.isLocked ) { - event.preventDefault(); - - this.props.onInsertBlocks( [ - createBlock( 'core/paragraph' ), - ], this.props.order + 1 ); - } + this.props.onInsertBlocks( [ + createBlock( 'core/paragraph' ), + ], this.props.order + 1 ); + event.preventDefault(); break; case BACKSPACE: case DELETE: // Remove block on backspace. - if ( target === this.node ) { - const { uid, onRemove, isLocked, previousBlockUid, onSelect } = this.props; - event.preventDefault(); - if ( ! isLocked ) { - onRemove( uid ); - - if ( previousBlockUid ) { - onSelect( previousBlockUid, -1 ); - } - } + const { uid, onRemove, previousBlockUid, onSelect } = this.props; + onRemove( uid ); + + if ( previousBlockUid ) { + onSelect( previousBlockUid, -1 ); } + event.preventDefault(); break; } } @@ -479,11 +484,11 @@ export class BlockListBlock extends Component { onTouchStart={ this.onTouchStart } onFocus={ this.onFocus } onClick={ this.onClick } + onKeyDown={ this.deleteOrInsertAfterWrapper } tabIndex="0" childHandledEvents={ [ 'onDragStart', 'onMouseDown', - 'onKeyDown', ] } { ...wrapperProps } > @@ -514,7 +519,6 @@ export class BlockListBlock extends Component { ref={ this.bindBlockNode } onDragStart={ this.preventDrag } onMouseDown={ this.onPointerDown } - onKeyDown={ this.onKeyDown } className="editor-block-list__block-edit" aria-label={ blockLabel } data-block={ block.uid } From 5886309783cc837b11431ec9dc270784e3259262 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Mar 2018 16:22:50 -0500 Subject: [PATCH 8/8] Rich Text: Ensure format toolbar manages its own dismissal Previously only closed on esc when editing link, not adding new link TODO: Consolidate editing state --- blocks/rich-text/format-toolbar/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blocks/rich-text/format-toolbar/index.js b/blocks/rich-text/format-toolbar/index.js index edc1c54a61b749..39b42751fdc211 100644 --- a/blocks/rich-text/format-toolbar/index.js +++ b/blocks/rich-text/format-toolbar/index.js @@ -64,7 +64,7 @@ class FormatToolbar extends Component { onKeyDown( event ) { if ( event.keyCode === ESCAPE ) { - if ( this.state.isEditingLink ) { + if ( this.state.isEditingLink || this.state.isAddingLink ) { event.stopPropagation(); this.dropLink(); }