Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix buggy behaviour on fast enter to split blocks #10328

Closed
wants to merge 1 commit into from
Closed

Fix buggy behaviour on fast enter to split blocks #10328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 14 additions & 80 deletions packages/editor/src/components/rich-text/index.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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