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.

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

Writing Flow: Consider horizontal handled by stopPropagation in RichText #6712

Merged
merged 6 commits into from
Jun 29, 2018
Merged
69 changes: 63 additions & 6 deletions editor/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,15 @@ import {
getScrollContainer,
} from '@wordpress/dom';
import { createBlobURL } from '@wordpress/blob';
import { BACKSPACE, DELETE, ENTER, rawShortcut } from '@wordpress/keycodes';
import {
BACKSPACE,
DELETE,
ENTER,
LEFT,
RIGHT,
TINYMCE_ZERO_WIDTH_SPACE,
rawShortcut,
} from '@wordpress/keycodes';
import { withInstanceId, withSafeTimeout, Slot } from '@wordpress/components';
import { withSelect } from '@wordpress/data';
import { rawHandler } from '@wordpress/blocks';
Expand All @@ -44,6 +52,12 @@ import { withBlockEditContext } from '../block-edit/context';
import { domToFormat, valueToString } from './format';
import TokenUI from './tokens/ui';

/**
* Browser dependencies
*/

const { Node } = window;

/**
* Returns true if the node is the inline node boundary. This is used in node
* filtering prevent the inline boundary from being included in the split which
Expand All @@ -58,7 +72,7 @@ import TokenUI from './tokens/ui';
*/
export function isEmptyInlineBoundary( node ) {
const text = node.nodeName === 'A' ? node.innerText : node.textContent;
return text === '\uFEFF';
return text === TINYMCE_ZERO_WIDTH_SPACE;
}

/**
Expand Down Expand Up @@ -112,6 +126,7 @@ export class RichText extends Component {
this.onChange = this.onChange.bind( this );
this.onNewBlock = this.onNewBlock.bind( this );
this.onNodeChange = this.onNodeChange.bind( this );
this.onHorizontalNavigationKeyDown = this.onHorizontalNavigationKeyDown.bind( this );
this.onKeyDown = this.onKeyDown.bind( this );
this.onKeyUp = this.onKeyUp.bind( this );
this.changeFormats = this.changeFormats.bind( this );
Expand Down Expand Up @@ -433,6 +448,42 @@ export class RichText extends Component {
left: position.left - containerPosition.left + ( position.width / 2 ),
};
}
/**
* Handles a horizontal navigation key down event to stop propagation if it
* can be inferred that it will be handled by TinyMCE (notably transitions
* out of an inline boundary node).
*
* @param {KeyboardEvent} event Keydown event.
*/
onHorizontalNavigationKeyDown( event ) {
const { focusNode, focusOffset } = window.getSelection();
const { nodeType, nodeValue } = focusNode;

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

const isReverse = event.keyCode === LEFT;

let offset = focusOffset;
if ( isReverse ) {
offset--;
}

// [WORKAROUND]: When in a new paragraph in a new inline boundary node,
// while typing the zero-width space occurs as the first child instead
// of at the end of the inline boundary where the caret is. This should
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite follow the sentence "This should only be exempt when focusNode is not only the ZWSP, which occurs when caret is placed on the right outside edge of inline boundary."

Could you rephrase it to explain what this is saying? I don't quite follow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I was reading this again earlier and was having a hard time following my own explanation, so yes, I definitely agree it could do for some rephrasing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside: Created issue for this in TinyMCE at tinymce/tinymce#4472

// only be exempt when focusNode is not _only_ the ZWSP, which occurs
// when caret is placed on the right outside edge of inline boundary.
if ( ! isReverse && focusOffset === nodeValue.length &&
nodeValue.length > 1 && nodeValue[ 0 ] === TINYMCE_ZERO_WIDTH_SPACE ) {
offset = 0;
}

if ( nodeValue[ offset ] === TINYMCE_ZERO_WIDTH_SPACE ) {
event.stopPropagation();
}
}

/**
* Handles a keydown event from TinyMCE.
Expand All @@ -442,18 +493,19 @@ export class RichText extends Component {
onKeyDown( event ) {
const dom = this.editor.dom;
const rootNode = this.editor.getBody();
const { keyCode } = event;

if (
( event.keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) ||
( event.keyCode === DELETE && isHorizontalEdge( rootNode, false ) )
( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) ||
( keyCode === DELETE && isHorizontalEdge( rootNode, false ) )
) {
if ( ! this.props.onMerge && ! this.props.onRemove ) {
return;
}

this.onCreateUndoLevel();

const forward = event.keyCode === DELETE;
const forward = keyCode === DELETE;

if ( this.props.onMerge ) {
this.props.onMerge( forward );
Expand All @@ -471,9 +523,14 @@ export class RichText extends Component {
event.stopImmediatePropagation();
}

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 ( event.keyCode === ENTER ) {
if ( keyCode === ENTER ) {
if ( this.props.multiline ) {
if ( ! this.props.onSplit ) {
return;
Expand Down
10 changes: 10 additions & 0 deletions packages/keycodes/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ export const CTRL = 'ctrl';
export const COMMAND = 'meta';
export const SHIFT = 'shift';

/**
* 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}
*/
export const TINYMCE_ZERO_WIDTH_SPACE = '\uFEFF';

/**
* Return true if platform is MacOS.
*
Expand Down