Skip to content

Commit

Permalink
Fix buggy behaviour on fast enter to split blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
ellatrix committed Oct 4, 2018
1 parent 2fe8cc0 commit 0162cba
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 82 deletions.
94 changes: 14 additions & 80 deletions packages/editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
find,
identity,
isNil,
noop,
isEqual,
} from 'lodash';
import memize from 'memize';
Expand All @@ -22,7 +21,7 @@ import {
getScrollContainer,
} from '@wordpress/dom';
import { createBlobURL } from '@wordpress/blob';
import { BACKSPACE, DELETE, ENTER, LEFT, RIGHT, rawShortcut } from '@wordpress/keycodes';
import { BACKSPACE, DELETE, ENTER, rawShortcut } from '@wordpress/keycodes';
import { Slot } from '@wordpress/components';
import { withDispatch, withSelect } from '@wordpress/data';
import { rawHandler, children, getBlockTransforms, findTransform } from '@wordpress/blocks';
Expand Down Expand Up @@ -59,7 +58,7 @@ import TokenUI from './tokens/ui';
* Browser dependencies
*/

const { Node, getSelection } = window;
const { getSelection } = window;

/**
* Zero-width space character used by TinyMCE as a caret landing point for
Expand All @@ -82,7 +81,6 @@ export class RichText extends Component {
this.onChange = this.onChange.bind( this );
this.onNodeChange = this.onNodeChange.bind( this );
this.onDeleteKeyDown = this.onDeleteKeyDown.bind( this );
this.onHorizontalNavigationKeyDown = this.onHorizontalNavigationKeyDown.bind( this );
this.onKeyDown = this.onKeyDown.bind( this );
this.onKeyUp = this.onKeyUp.bind( this );
this.onPropagateUndo = this.onPropagateUndo.bind( this );
Expand Down Expand Up @@ -167,10 +165,7 @@ export class RichText extends Component {

editor.on( 'init', this.onInit );
editor.on( 'nodechange', this.onNodeChange );
editor.on( 'keydown', this.onKeyDown );
editor.on( 'keyup', this.onKeyUp );
editor.on( 'BeforeExecCommand', this.onPropagateUndo );
editor.on( 'focus', this.onFocus );
// The change event in TinyMCE fires every time an undo level is added.
editor.on( 'change', this.onCreateUndoLevel );

Expand Down Expand Up @@ -527,50 +522,6 @@ export class RichText extends Component {
event.stopImmediatePropagation();
}

/**
* Handles a horizontal navigation key down event to handle the case where
* TinyMCE attempts to preventDefault when on the outside edge of an inline
* boundary when arrowing _away_ from the boundary, not within it. Replaces
* the TinyMCE event `preventDefault` behavior with a noop, such that those
* relying on `defaultPrevented` are not misinformed about the arrow event.
*
* If TinyMCE#4476 is resolved, this handling may be removed.
*
* @see https://github.com/tinymce/tinymce/issues/4476
*
* @param {tinymce.EditorEvent<KeyboardEvent>} event Keydown event.
*/
onHorizontalNavigationKeyDown( event ) {
const { focusNode } = getSelection();
const { nodeType, nodeValue } = focusNode;

if ( nodeType !== Node.TEXT_NODE ) {
return;
}

if ( nodeValue.length !== 1 || nodeValue[ 0 ] !== TINYMCE_ZWSP ) {
return;
}

const { keyCode } = event;

// Consider to be moving away from inline boundary based on:
//
// 1. Within a text fragment consisting only of ZWSP.
// 2. If in reverse, there is no previous sibling. If forward, there is
// no next sibling (i.e. end of node).
const isReverse = keyCode === LEFT;
const edgeSibling = isReverse ? 'previousSibling' : 'nextSibling';
if ( ! focusNode[ edgeSibling ] ) {
// Note: This is not reassigning on the native event, rather the
// "fixed" TinyMCE copy, which proxies its preventDefault to the
// native event. By reassigning here, we're effectively preventing
// the proxied call on the native event, but not otherwise mutating
// the original event object.
event.preventDefault = noop;
}
}

/**
* Handles a keydown event from TinyMCE.
*
Expand All @@ -584,26 +535,16 @@ export class RichText extends Component {
this.onDeleteKeyDown( event );
}

const isHorizontalNavigation = keyCode === LEFT || keyCode === RIGHT;
if ( isHorizontalNavigation ) {
this.onHorizontalNavigationKeyDown( event );
}

// If we click shift+Enter on inline RichTexts, we avoid creating two contenteditables
// We also split the content and call the onSplit prop if provided.
if ( keyCode === ENTER ) {
event.preventDefault();

if ( this.props.onReplace ) {
const text = getTextContent( this.getRecord() );
const transformation = findTransform( this.enterPatterns, ( item ) => {
return item.regExp.test( text );
} );

if ( transformation ) {
// Calling onReplace() will destroy the editor, so it's
// important that we stop other handlers (e.g. ones
// registered by TinyMCE) from also handling this event.
event.stopImmediatePropagation();
event.preventDefault();
this.props.onReplace( [
transformation.transform( { content: text } ),
] );
Expand All @@ -612,27 +553,17 @@ export class RichText extends Component {
}

if ( this.props.multiline ) {
if ( ! this.props.onSplit ) {
return;
}

const record = this.getRecord();

if ( ! isEmptyLine( record ) ) {
return;
}

event.preventDefault();

this.props.onSplit( ...split( record ).map( this.valueToFormat ) );
} else {
event.preventDefault();

if ( event.shiftKey || ! this.props.onSplit ) {
this.editor.execCommand( 'InsertLineBreak', false, event );
if ( ! this.props.onSplit || ! isEmptyLine( record ) ) {
this.onChange( insert( record, '\u2028' ) );
} else {
this.splitContent();
this.props.onSplit( ...split( record ).map( this.valueToFormat ) );
}
} else if ( event.shiftKey || ! this.props.onSplit ) {
this.editor.execCommand( 'InsertLineBreak', false, event );
} else {
this.splitContent();
}
}
}
Expand Down Expand Up @@ -910,6 +841,9 @@ export class RichText extends Component {
key={ key }
onPaste={ this.onPaste }
onInput={ this.onInput }
onKeyDown={ this.onKeyDown }
onKeyUp={ this.onKeyUp }
onFocus={ this.onFocus }
multilineTag={ MultilineTag }
setRef={ this.setRef }
/>
Expand Down
80 changes: 78 additions & 2 deletions packages/editor/src/components/rich-text/tinymce.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
* External dependencies
*/
import tinymce from 'tinymce';
import { isEqual } from 'lodash';
import { isEqual, noop } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { Component, createElement } from '@wordpress/element';
import { BACKSPACE, DELETE } from '@wordpress/keycodes';
import { BACKSPACE, DELETE, ENTER, LEFT, RIGHT } from '@wordpress/keycodes';
import { toHTMLString } from '@wordpress/rich-text';
import { children } from '@wordpress/blocks';

Expand All @@ -18,6 +18,22 @@ import { children } from '@wordpress/blocks';
*/
import { diffAriaProps, pickAriaProps } from './aria';

/**
* Browser dependencies
*/

const { Node, getSelection } = window;

/**
* Zero-width space character used by TinyMCE as a caret landing point for
* inline boundary nodes.
*
* @see tinymce/src/core/main/ts/text/Zwsp.ts
*
* @type {string}
*/
const TINYMCE_ZWSP = '\uFEFF';

/**
* Determines whether we need a fix to provide `input` events for contenteditable.
*
Expand Down Expand Up @@ -101,6 +117,7 @@ export default class TinyMCE extends Component {
constructor() {
super();
this.bindEditorNode = this.bindEditorNode.bind( this );
this.onKeyDown = this.onKeyDown.bind( this );
}

componentDidMount() {
Expand Down Expand Up @@ -168,6 +185,8 @@ export default class TinyMCE extends Component {
setup: ( editor ) => {
this.editor = editor;
this.props.onSetup( editor );

editor.on( 'keydown', this.onKeyDown );
},
} );
}
Expand All @@ -193,6 +212,57 @@ export default class TinyMCE extends Component {
}
}

/**
* Handles a horizontal navigation key down event to handle the case where
* TinyMCE attempts to preventDefault when on the outside edge of an inline
* boundary when arrowing _away_ from the boundary, not within it. Replaces
* the TinyMCE event `preventDefault` behavior with a noop, such that those
* relying on `defaultPrevented` are not misinformed about the arrow event.
*
* If TinyMCE#4476 is resolved, this handling may be removed.
*
* @see https://github.com/tinymce/tinymce/issues/4476
*
* @param {tinymce.EditorEvent<KeyboardEvent>} event Keydown event.
*/
onKeyDown( event ) {
// Disable TinyMCE enter behaviour.
if ( event.keyCode === ENTER ) {
event.preventDefault();
}

if ( event.keyCode !== LEFT && event.keyCode !== RIGHT ) {
return;
}

const { focusNode } = getSelection();
const { nodeType, nodeValue } = focusNode;

if ( nodeType !== Node.TEXT_NODE ) {
return;
}

if ( nodeValue.length !== 1 || nodeValue[ 0 ] !== TINYMCE_ZWSP ) {
return;
}

// Consider to be moving away from inline boundary based on:
//
// 1. Within a text fragment consisting only of ZWSP.
// 2. If in reverse, there is no previous sibling. If forward, there is
// no next sibling (i.e. end of node).
const isReverse = event.keyCode === LEFT;
const edgeSibling = isReverse ? 'previousSibling' : 'nextSibling';
if ( ! focusNode[ edgeSibling ] ) {
// Note: This is not reassigning on the native event, rather the
// "fixed" TinyMCE copy, which proxies its preventDefault to the
// native event. By reassigning here, we're effectively preventing
// the proxied call on the native event, but not otherwise mutating
// the original event object.
event.preventDefault = noop;
}
}

render() {
const ariaProps = pickAriaProps( this.props );
const {
Expand All @@ -203,6 +273,9 @@ export default class TinyMCE extends Component {
isPlaceholderVisible,
onPaste,
onInput,
onKeyDown,
onKeyUp,
onFocus,
multilineTag,
} = this.props;

Expand Down Expand Up @@ -239,6 +312,9 @@ export default class TinyMCE extends Component {
dangerouslySetInnerHTML: { __html: initialHTML },
onPaste,
onInput,
onKeyDown,
onKeyUp,
onFocus,
} );
}
}
12 changes: 12 additions & 0 deletions packages/rich-text/src/to-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,21 @@ export function toTree( value, multilineTag, settings ) {
} );
}

// If there is selection at 0, handle it before characters are inserted.

if ( onStartIndex && start === 0 && i === 0 ) {
onStartIndex( tree, pointer, multilineIndex );
}

if ( onEndIndex && end === 0 && i === 0 ) {
onEndIndex( tree, pointer, multilineIndex );
}

if ( character !== '\ufffc' ) {
if ( character === '\n' ) {
pointer = append( getParent( pointer ), { type: 'br', object: true } );
// Ensure pointer is text node.
pointer = append( getParent( pointer ), '' );
} else if ( ! isText( pointer ) ) {
pointer = append( getParent( pointer ), character );
} else {
Expand Down

0 comments on commit 0162cba

Please sign in to comment.