Skip to content

Commit 07f6ec6

Browse files
authored
Writing Flow: Fix all the broken flows (#5513)
* 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 * Utils: Improve spec compliancy of text field * Block: Prefer focus text field on selection, fallback to wrapper * Writing Flow: Refactor getClosestTabbable to support non-tabbable target * Block List: Extract typing monitor to separate component * 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) * Block List: Fix delete or insert after focused block wrapper node * 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
1 parent 0ec3408 commit 07f6ec6

File tree

12 files changed

+391
-194
lines changed

12 files changed

+391
-194
lines changed

blocks/rich-text/format-toolbar/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class FormatToolbar extends Component {
6464

6565
onKeyDown( event ) {
6666
if ( event.keyCode === ESCAPE ) {
67-
if ( this.state.isEditingLink ) {
67+
if ( this.state.isEditingLink || this.state.isAddingLink ) {
6868
event.stopPropagation();
6969
this.dropLink();
7070
}

edit-post/components/visual-editor/index.js

+15-12
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
CopyHandler,
77
PostTitle,
88
WritingFlow,
9+
ObserveTyping,
910
EditorGlobalKeyboardShortcuts,
1011
BlockSelectionClearer,
1112
MultiSelectScrollIntoView,
@@ -26,18 +27,20 @@ function VisualEditor( { hasFixedToolbar, isLargeViewport } ) {
2627
<EditorGlobalKeyboardShortcuts />
2728
<CopyHandler />
2829
<MultiSelectScrollIntoView />
29-
<WritingFlow>
30-
<PostTitle />
31-
<BlockList
32-
showContextualToolbar={ ! isLargeViewport || ! hasFixedToolbar }
33-
renderBlockMenu={ ( { children, onClose } ) => (
34-
<Fragment>
35-
<BlockInspectorButton onClick={ onClose } />
36-
{ children }
37-
</Fragment>
38-
) }
39-
/>
40-
</WritingFlow>
30+
<ObserveTyping>
31+
<WritingFlow>
32+
<PostTitle />
33+
<BlockList
34+
showContextualToolbar={ ! isLargeViewport || ! hasFixedToolbar }
35+
renderBlockMenu={ ( { children, onClose } ) => (
36+
<Fragment>
37+
<BlockInspectorButton onClick={ onClose } />
38+
{ children }
39+
</Fragment>
40+
) }
41+
/>
42+
</WritingFlow>
43+
</ObserveTyping>
4144
</BlockSelectionClearer>
4245
);
4346
}

editor/components/block-list/block.js

+35-130
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { Component, findDOMNode, compose } from '@wordpress/element';
1313
import {
1414
keycodes,
1515
focus,
16+
isTextField,
1617
placeCaretAtHorizontalEdge,
1718
placeCaretAtVerticalEdge,
1819
} from '@wordpress/utils';
@@ -53,8 +54,6 @@ import {
5354
removeBlock,
5455
replaceBlocks,
5556
selectBlock,
56-
startTyping,
57-
stopTyping,
5857
updateBlockAttributes,
5958
toggleSelection,
6059
} from '../../store/actions';
@@ -75,7 +74,7 @@ import {
7574
getSelectedBlocksInitialCaretPosition,
7675
} from '../../store/selectors';
7776

78-
const { BACKSPACE, ESCAPE, DELETE, ENTER, UP, RIGHT, DOWN, LEFT } = keycodes;
77+
const { BACKSPACE, DELETE, ENTER } = keycodes;
7978

8079
export class BlockListBlock extends Component {
8180
constructor() {
@@ -86,25 +85,21 @@ export class BlockListBlock extends Component {
8685
this.setAttributes = this.setAttributes.bind( this );
8786
this.maybeHover = this.maybeHover.bind( this );
8887
this.hideHoverEffects = this.hideHoverEffects.bind( this );
89-
this.maybeStartTyping = this.maybeStartTyping.bind( this );
90-
this.stopTypingOnMouseMove = this.stopTypingOnMouseMove.bind( this );
9188
this.mergeBlocks = this.mergeBlocks.bind( this );
9289
this.onFocus = this.onFocus.bind( this );
9390
this.preventDrag = this.preventDrag.bind( this );
9491
this.onPointerDown = this.onPointerDown.bind( this );
95-
this.onKeyDown = this.onKeyDown.bind( this );
92+
this.deleteOrInsertAfterWrapper = this.deleteOrInsertAfterWrapper.bind( this );
9693
this.onBlockError = this.onBlockError.bind( this );
9794
this.insertBlocksAfter = this.insertBlocksAfter.bind( this );
9895
this.onTouchStart = this.onTouchStart.bind( this );
9996
this.onClick = this.onClick.bind( this );
10097
this.selectOnOpen = this.selectOnOpen.bind( this );
101-
this.onSelectionChange = this.onSelectionChange.bind( this );
10298
this.hadTouchStart = false;
10399

104100
this.state = {
105101
error: null,
106102
isHovered: false,
107-
isSelectionCollapsed: true,
108103
};
109104
}
110105

@@ -126,46 +121,23 @@ export class BlockListBlock extends Component {
126121
}
127122

128123
componentDidMount() {
129-
if ( this.props.isTyping ) {
130-
document.addEventListener( 'mousemove', this.stopTypingOnMouseMove );
131-
}
132-
document.addEventListener( 'selectionchange', this.onSelectionChange );
133-
134124
if ( this.props.isSelected ) {
135125
this.focusTabbable();
136126
}
137127
}
138128

139129
componentWillReceiveProps( newProps ) {
140-
if ( newProps.isTyping || newProps.isSelected ) {
130+
if ( newProps.isTypingWithinBlock || newProps.isSelected ) {
141131
this.hideHoverEffects();
142132
}
143133
}
144134

145135
componentDidUpdate( prevProps ) {
146-
// Bind or unbind mousemove from page when user starts or stops typing
147-
if ( this.props.isTyping !== prevProps.isTyping ) {
148-
if ( this.props.isTyping ) {
149-
document.addEventListener( 'mousemove', this.stopTypingOnMouseMove );
150-
} else {
151-
this.removeStopTypingListener();
152-
}
153-
}
154-
155136
if ( this.props.isSelected && ! prevProps.isSelected ) {
156137
this.focusTabbable();
157138
}
158139
}
159140

160-
componentWillUnmount() {
161-
this.removeStopTypingListener();
162-
document.removeEventListener( 'selectionchange', this.onSelectionChange );
163-
}
164-
165-
removeStopTypingListener() {
166-
document.removeEventListener( 'mousemove', this.stopTypingOnMouseMove );
167-
}
168-
169141
setBlockListRef( node ) {
170142
// Disable reason: The root return element uses a component to manage
171143
// event nesting, but the parent block list layout needs the raw DOM
@@ -202,15 +174,15 @@ export class BlockListBlock extends Component {
202174
}
203175

204176
// Find all tabbables within node.
205-
const tabbables = focus.tabbable.find( this.node )
206-
.filter( ( node ) => node !== this.node );
177+
const textInputs = focus.tabbable.find( this.node ).filter( isTextField );
207178

208179
// If reversed (e.g. merge via backspace), use the last in the set of
209180
// tabbables.
210181
const isReverse = -1 === initialPosition;
211-
const target = ( isReverse ? last : first )( tabbables );
182+
const target = ( isReverse ? last : first )( textInputs );
212183

213184
if ( ! target ) {
185+
this.wrapperNode.focus();
214186
return;
215187
}
216188

@@ -297,31 +269,6 @@ export class BlockListBlock extends Component {
297269
}
298270
}
299271

300-
maybeStartTyping() {
301-
// We do not want to dispatch start typing if state value already reflects
302-
// that we're typing (dispatch noise)
303-
if ( ! this.props.isTyping ) {
304-
this.props.onStartTyping();
305-
}
306-
}
307-
308-
stopTypingOnMouseMove( { clientX, clientY } ) {
309-
const { lastClientX, lastClientY } = this;
310-
311-
// We need to check that the mouse really moved
312-
// Because Safari trigger mousemove event when we press shift, ctrl...
313-
if (
314-
lastClientX &&
315-
lastClientY &&
316-
( lastClientX !== clientX || lastClientY !== clientY )
317-
) {
318-
this.props.onStopTyping();
319-
}
320-
321-
this.lastClientX = clientX;
322-
this.lastClientY = clientY;
323-
}
324-
325272
mergeBlocks( forward = false ) {
326273
const { block, previousBlockUid, nextBlockUid, onMerge } = this.props;
327274

@@ -338,10 +285,6 @@ export class BlockListBlock extends Component {
338285
} else {
339286
onMerge( previousBlockUid, block.uid );
340287
}
341-
342-
// Manually trigger typing mode, since merging will remove this block and
343-
// cause onKeyDown to not fire
344-
this.maybeStartTyping();
345288
}
346289

347290
insertBlocksAfter( blocks ) {
@@ -406,56 +349,41 @@ export class BlockListBlock extends Component {
406349
}
407350
}
408351

409-
onKeyDown( event ) {
352+
/**
353+
* Interprets keydown event intent to remove or insert after block if key
354+
* event occurs on wrapper node. This can occur when the block has no text
355+
* fields of its own, particularly after initial insertion, to allow for
356+
* easy deletion and continuous writing flow to add additional content.
357+
*
358+
* @param {KeyboardEvent} event Keydown event.
359+
*/
360+
deleteOrInsertAfterWrapper( event ) {
410361
const { keyCode, target } = event;
411362

363+
if ( target !== this.wrapperNode || this.props.isLocked ) {
364+
return;
365+
}
366+
412367
switch ( keyCode ) {
413368
case ENTER:
414369
// Insert default block after current block if enter and event
415370
// not already handled by descendant.
416-
if ( target === this.node && ! this.props.isLocked ) {
417-
event.preventDefault();
418-
419-
this.props.onInsertBlocks( [
420-
createBlock( 'core/paragraph' ),
421-
], this.props.order + 1 );
422-
}
423-
424-
// Pressing enter should trigger typing mode after the content has split
425-
this.maybeStartTyping();
426-
break;
427-
428-
case UP:
429-
case RIGHT:
430-
case DOWN:
431-
case LEFT:
432-
// Arrow keys do not fire keypress event, but should still
433-
// trigger typing mode.
434-
this.maybeStartTyping();
371+
this.props.onInsertBlocks( [
372+
createBlock( 'core/paragraph' ),
373+
], this.props.order + 1 );
374+
event.preventDefault();
435375
break;
436376

437377
case BACKSPACE:
438378
case DELETE:
439379
// Remove block on backspace.
440-
if ( target === this.node ) {
441-
const { uid, onRemove, isLocked, previousBlock, onSelect } = this.props;
442-
event.preventDefault();
443-
if ( ! isLocked ) {
444-
onRemove( uid );
445-
446-
if ( previousBlock ) {
447-
onSelect( previousBlock.uid, -1 );
448-
}
449-
}
450-
}
451-
452-
// Pressing backspace should trigger typing mode
453-
this.maybeStartTyping();
454-
break;
380+
const { uid, onRemove, previousBlockUid, onSelect } = this.props;
381+
onRemove( uid );
455382

456-
case ESCAPE:
457-
// Deselect on escape.
458-
this.props.onDeselect();
383+
if ( previousBlockUid ) {
384+
onSelect( previousBlockUid, -1 );
385+
}
386+
event.preventDefault();
459387
break;
460388
}
461389
}
@@ -470,19 +398,6 @@ export class BlockListBlock extends Component {
470398
}
471399
}
472400

473-
onSelectionChange() {
474-
if ( ! this.props.isSelected ) {
475-
return;
476-
}
477-
478-
const selection = window.getSelection();
479-
const isCollapsed = selection.rangeCount > 0 && selection.getRangeAt( 0 ).collapsed;
480-
// We only keep track of the collapsed selection for selected blocks.
481-
if ( isCollapsed !== this.state.isSelectionCollapsed && this.props.isSelected ) {
482-
this.setState( { isSelectionCollapsed: isCollapsed } );
483-
}
484-
}
485-
486401
render() {
487402
const {
488403
block,
@@ -499,6 +414,7 @@ export class BlockListBlock extends Component {
499414
isMultiSelected,
500415
isFirstMultiSelected,
501416
isLastInSelection,
417+
isTypingWithinBlock,
502418
} = this.props;
503419
const isHovered = this.state.isHovered && ! this.props.isMultiSelecting;
504420
const { name: blockName, isValid } = block;
@@ -508,11 +424,11 @@ export class BlockListBlock extends Component {
508424
// The block as rendered in the editor is composed of general block UI
509425
// (mover, toolbar, wrapper) and the display of the block content.
510426

511-
// If the block is selected and we're typing the block should not appear as selected unless the selection is not collapsed.
427+
// If the block is selected and we're typing the block should not appear.
512428
// Empty paragraph blocks should always show up as unselected.
513429
const isEmptyDefaultBlock = isUnmodifiedDefaultBlock( block );
430+
const isSelectedNotTyping = isSelected && ! isTypingWithinBlock;
514431
const showSideInserter = ( isSelected || isHovered ) && isEmptyDefaultBlock;
515-
const isSelectedNotTyping = isSelected && ( ! this.props.isTyping || ! this.state.isSelectionCollapsed );
516432
const shouldAppearSelected = ! showSideInserter && isSelectedNotTyping;
517433
const shouldShowMovers = shouldAppearSelected || isHovered || ( isEmptyDefaultBlock && isSelectedNotTyping );
518434
const shouldShowSettingsMenu = shouldShowMovers;
@@ -568,12 +484,11 @@ export class BlockListBlock extends Component {
568484
onTouchStart={ this.onTouchStart }
569485
onFocus={ this.onFocus }
570486
onClick={ this.onClick }
487+
onKeyDown={ this.deleteOrInsertAfterWrapper }
571488
tabIndex="0"
572489
childHandledEvents={ [
573-
'onKeyPress',
574490
'onDragStart',
575491
'onMouseDown',
576-
'onKeyDown',
577492
] }
578493
{ ...wrapperProps }
579494
>
@@ -602,10 +517,8 @@ export class BlockListBlock extends Component {
602517
{ isFirstMultiSelected && <BlockMultiControls rootUID={ rootUID } /> }
603518
<IgnoreNestedEvents
604519
ref={ this.bindBlockNode }
605-
onKeyPress={ this.maybeStartTyping }
606520
onDragStart={ this.preventDrag }
607521
onMouseDown={ this.onPointerDown }
608-
onKeyDown={ this.onKeyDown }
609522
className="editor-block-list__block-edit"
610523
aria-label={ blockLabel }
611524
data-block={ block.uid }
@@ -677,7 +590,7 @@ const mapStateToProps = ( state, { uid, rootUID } ) => {
677590
isLastInSelection: state.blockSelection.end === uid,
678591
// We only care about this prop when the block is selected
679592
// Thus to avoid unnecessary rerenders we avoid updating the prop if the block is not selected.
680-
isTyping: isSelected && isTyping( state ),
593+
isTypingWithinBlock: isSelected && isTyping( state ),
681594
order: getBlockIndex( state, uid, rootUID ),
682595
meta: getEditedPostAttribute( state, 'meta' ),
683596
mode: getBlockMode( state, uid ),
@@ -701,14 +614,6 @@ const mapDispatchToProps = ( dispatch, ownProps ) => ( {
701614
dispatch( clearSelectedBlock() );
702615
},
703616

704-
onStartTyping() {
705-
dispatch( startTyping() );
706-
},
707-
708-
onStopTyping() {
709-
dispatch( stopTyping() );
710-
},
711-
712617
onInsertBlocks( blocks, index ) {
713618
const { rootUID, layout } = ownProps;
714619

editor/components/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export { default as Inserter } from './inserter';
6565
export { default as MultiBlocksSwitcher } from './block-switcher/multi-blocks-switcher';
6666
export { default as MultiSelectScrollIntoView } from './multi-select-scroll-into-view';
6767
export { default as NavigableToolbar } from './navigable-toolbar';
68+
export { default as ObserveTyping } from './observe-typing';
6869
export { default as PreserveScrollInReorder } from './preserve-scroll-in-reorder';
6970
export { default as Warning } from './warning';
7071
export { default as WritingFlow } from './writing-flow';

0 commit comments

Comments
 (0)