From 0162cba26942b6953b4d19e96b182d0e91c60a9b Mon Sep 17 00:00:00 2001 From: iseulde Date: Thu, 4 Oct 2018 00:44:29 -0400 Subject: [PATCH] Fix buggy behaviour on fast enter to split blocks --- .../editor/src/components/rich-text/index.js | 94 +++---------------- .../src/components/rich-text/tinymce.js | 80 +++++++++++++++- packages/rich-text/src/to-tree.js | 12 +++ 3 files changed, 104 insertions(+), 82 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 6eb76ddafb319..6f158162cfde1 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -7,7 +7,6 @@ import { find, identity, isNil, - noop, isEqual, } from 'lodash'; import memize from 'memize'; @@ -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'; @@ -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 @@ -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 ); @@ -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 ); @@ -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} 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. * @@ -584,14 +535,9 @@ 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 ) => { @@ -599,11 +545,6 @@ export class RichText extends Component { } ); 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 } ), ] ); @@ -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(); } } } @@ -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 } /> diff --git a/packages/editor/src/components/rich-text/tinymce.js b/packages/editor/src/components/rich-text/tinymce.js index c8180622c91e8..5d3cb79517dec 100644 --- a/packages/editor/src/components/rich-text/tinymce.js +++ b/packages/editor/src/components/rich-text/tinymce.js @@ -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'; @@ -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. * @@ -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() { @@ -168,6 +185,8 @@ export default class TinyMCE extends Component { setup: ( editor ) => { this.editor = editor; this.props.onSetup( editor ); + + editor.on( 'keydown', this.onKeyDown ); }, } ); } @@ -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} 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 { @@ -203,6 +273,9 @@ export default class TinyMCE extends Component { isPlaceholderVisible, onPaste, onInput, + onKeyDown, + onKeyUp, + onFocus, multilineTag, } = this.props; @@ -239,6 +312,9 @@ export default class TinyMCE extends Component { dangerouslySetInnerHTML: { __html: initialHTML }, onPaste, onInput, + onKeyDown, + onKeyUp, + onFocus, } ); } } diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index 8f228654c74bb..8b95e783f29bc 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -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 {