Writing Flow: Collapse range in horizontal edge check by selection direction

editor/components/rich-text/format.js

@@ -1,14 +1,20 @@
 /**
  * External dependencies
  */
-import { omitBy } from 'lodash';
+import { omitBy, get } from 'lodash';
 import { nodeListToReact } from 'dom-react';
 
 /**
  * WordPress dependencies
  */
 import { createElement, renderToString } from '@wordpress/element';
 
+/**
+ * Browser dependencies
+ */
+
+const { Node } = window;
+
 /**
  * Transforms a WP Element to its corresponding HTML string.
  *
@@ -62,6 +68,34 @@ export function createTinyMCEElement( type, props, ...children ) {
 	);
 }
 
+/**
+ * Given a TinyMCE Node instance, returns an equivalent WordPress element.
+ *
+ * @param {tinyMCE.html.Node} node TinyMCE node
+ *
+ * @return {WPElement} WordPress element
+ */
+export function tinyMCENodeToElement( node ) {
+	if ( node.type === Node.TEXT_NODE ) {
+		return node.value;
+	}
+
+	const children = [];
+
+	let child = node.firstChild;
+	while ( child ) {
+		children.push( tinyMCENodeToElement( child ) );
+		child = child.next;
+	}
+
+	if ( node.type === Node.DOCUMENT_FRAGMENT_NODE ) {
+		return children;
+	}
+
+	const attributes = get( node.attributes, [ 'map' ], {} );
+	return createElement( node.name, attributes, ...children );
+}
+
 /**
  * Transforms an array of DOM Elements to their corresponding WP element.
  *

editor/components/rich-text/index.js

@@ -18,7 +18,7 @@ import 'element-closest';
 /**
  * WordPress dependencies
  */
-import { Component, Fragment, compose, RawHTML, createRef } from '@wordpress/element';
+import { Component, Fragment, compose, RawHTML, Children, createRef } from '@wordpress/element';
 import {
 	keycodes,
 	createBlobURL,
@@ -43,9 +43,33 @@ import { pickAriaProps } from './aria';
 import patterns from './patterns';
 import { EVENTS } from './constants';
 import { withBlockEditContext } from '../block-edit/context';
-import { domToFormat, valueToString } from './format';
+import {
+	domToFormat,
+	valueToString,
+	tinyMCENodeToElement,
+} from './format';
+
+/**
+ * Browser dependencies
+ */
+
+const { getSelection, Node } = window;
+
+/**
+ * Module constants
+ */
+
+const { LEFT, RIGHT, BACKSPACE, DELETE, ENTER, rawShortcut } = keycodes;
 
-const { BACKSPACE, DELETE, ENTER, rawShortcut } = keycodes;
+/**
+ * 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';
 
 /**
  * Returns true if the node is the inline node boundary. This is used in node
@@ -61,7 +85,7 @@ const { BACKSPACE, DELETE, ENTER, rawShortcut } = keycodes;
  */
 export function isEmptyInlineBoundary( node ) {
 	const text = node.nodeName === 'A' ? node.innerText : node.textContent;
-	return text === '\uFEFF';
+	return text === TINYMCE_ZWSP;
 }
 
 /**
@@ -122,6 +146,7 @@ export class RichText extends Component {
 		this.onPaste = this.onPaste.bind( this );
 		this.onCreateUndoLevel = this.onCreateUndoLevel.bind( this );
 		this.setFocusedElement = this.setFocusedElement.bind( this );
+		this.removeZwsp = this.removeZwsp.bind( this );
 
 		this.state = {
 			formats: {},
@@ -186,6 +211,7 @@ export class RichText extends Component {
 		editor.on( 'PastePreProcess', this.onPastePreProcess, true /* Add before core handlers */ );
 		editor.on( 'paste', this.onPaste, true /* Add before core handlers */ );
 		editor.on( 'input', this.onChange );
+		editor.on( 'focusout', this.removeZwsp );
 		// The change event in TinyMCE fires every time an undo level is added.
 		editor.on( 'change', this.onCreateUndoLevel );
 
@@ -202,6 +228,27 @@ export class RichText extends Component {
 		}
 	}
 
+	/**
+	 * Cleans up after TinyMCE when leaving the field, removing lingering zero-
+	 * width space characters. Without removal, future horizontal navigation
+	 * into the field would land on the zero-width space, where it's preferred
+	 * to consistently land within an inline boundary where the zero-width
+	 * space had existed to delineate.
+	 */
+	removeZwsp() {
+		const rootNode = this.editor.getBody();
+
+		const stack = [ ...rootNode.childNodes ];
+		while ( stack.length ) {
+			const node = stack.pop();
+			if ( node.nodeType === Node.TEXT_NODE && node.nodeValue === TINYMCE_ZWSP ) {
+				node.parentNode.removeChild( node );
+			}
+
+			stack.push( ...node.childNodes );
+		}
+	}
+
 	/**
 	 * Allows prop event handlers to handle an event.
 	 *
@@ -440,40 +487,101 @@ export class RichText extends Component {
 	}
 
 	/**
-	 * Handles a keydown event from tinyMCE.
+	 * Handles a Backspace or Delete keydown event to delegate merge or remove
+	 * if key event occurs while at the extent edge of the field. Prevents
+	 * default browser behavior if delegated to prop callback handler.
 	 *
-	 * @param {KeydownEvent} event The keydow event as triggered by tinyMCE.
+	 * @param {KeyboardEvent} event Keydown event.
 	 */
-	onKeyDown( event ) {
-		const dom = this.editor.dom;
+	onDeleteKeyDown( event ) {
+		const { onMerge, onRemove } = this.props;
+		if ( ! onMerge && ! onRemove ) {
+			return;
+		}
+
+		if ( ! getSelection().isCollapsed ) {
+			return;
+		}
+
+		const isForward = ( event.keyCode === DELETE );
 		const rootNode = this.editor.getBody();
+		if ( ! isHorizontalEdge( rootNode, ! isForward ) ) {
+			return;
+		}
 
-		if (
-			( event.keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) ||
-			( event.keyCode === DELETE && isHorizontalEdge( rootNode, false ) )
-		) {
-			if ( ! this.props.onMerge && ! this.props.onRemove ) {
-				return;
-			}
+		this.onCreateUndoLevel();
 
-			this.onCreateUndoLevel();
+		if ( onMerge ) {
+			onMerge( isForward );
+		}
 
-			const forward = event.keyCode === DELETE;
+		if ( onRemove && this.isEmpty() ) {
+			onRemove( isForward );
+		}
 
-			if ( this.props.onMerge ) {
-				this.props.onMerge( forward );
-			}
+		event.preventDefault();
 
-			if ( this.props.onRemove && this.isEmpty() ) {
-				this.props.onRemove( forward );
-			}
+		// Calling onMerge() or onRemove() will destroy the editor, so it's
+		// important that other handlers (e.g. ones registered by TinyMCE) do
+		// not also attempt to handle this event.
+		event.stopImmediatePropagation();
+	}
 
-			event.preventDefault();
+	/**
+	 * 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;
 
-			// Calling onMerge() or onRemove() 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();
+		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
+		// 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_ZWSP ) {
+			offset = 0;
+		}
+
+		if ( nodeValue[ offset ] === TINYMCE_ZWSP ) {
+			event.stopPropagation();
+		}
+	}
+
+	/**
+	 * Handles a keydown event from tinyMCE.
+	 *
+	 * @param {KeyboardEvent} event Keydown event.
+	 */
+	onKeyDown( event ) {
+		const { keyCode } = event;
+		const dom = this.editor.dom;
+		const rootNode = this.editor.getBody();
+
+		const isDeleteKey = keyCode === BACKSPACE || keyCode === DELETE;
+		if ( isDeleteKey ) {
+			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
@@ -716,9 +824,7 @@ export class RichText extends Component {
 			case 'string':
 				return this.editor.getContent();
 			default:
-				return this.editor.dom.isEmpty( this.editor.getBody() ) ?
-					[] :
-					domToFormat( this.editor.getBody().childNodes || [], 'element', this.editor );
+				return tinyMCENodeToElement( this.editor.getContent( { format: 'tree' } ) );
 		}
 	}
 
@@ -755,8 +861,15 @@ export class RichText extends Component {
 	 * @return {boolean} Whether field is empty.
 	 */
 	isEmpty() {
-		const { value } = this.props;
-		return ! value || ! value.length;
+		const { value, format } = this.props;
+		if ( ! value ) {
+			return true;
+		}
+
+		return (
+			format === 'string' ||
+			! Children.count( value )
+		);
 	}
 
 	isFormatActive( format ) {

editor/components/writing-flow/index.js

@@ -28,6 +28,12 @@ import {
 	isInSameBlock,
 } from '../../utils/dom';
 
+/**
+ * Browser constants
+ */
+
+const { getSelection } = window;
+
 /**
  * Module Constants
  */
@@ -206,7 +212,7 @@ class WritingFlow extends Component {
 
 		if ( isShift && ( hasMultiSelection || (
 			this.isTabbableEdge( target, isReverse ) &&
-			isNavEdge( target, isReverse, true )
+			isNavEdge( target, isReverse )
 		) ) ) {
 			// Shift key is down, and there is multi selection or we're at the end of the current block.
 			this.expandSelection( isReverse );
@@ -215,14 +221,14 @@ class WritingFlow extends Component {
 			// Moving from block multi-selection to single block selection
 			this.moveSelection( isReverse );
 			event.preventDefault();
-		} else if ( isVertical && isVerticalEdge( target, isReverse, isShift ) ) {
+		} else if ( isVertical && isVerticalEdge( target, isReverse ) ) {
 			const closestTabbable = this.getClosestTabbable( target, isReverse );
 
 			if ( closestTabbable ) {
 				placeCaretAtVerticalEdge( closestTabbable, isReverse, this.verticalRect );
 				event.preventDefault();
 			}
-		} else if ( isHorizontal && isHorizontalEdge( target, isReverse, isShift ) ) {
+		} else if ( isHorizontal && getSelection().isCollapsed && isHorizontalEdge( target, isReverse ) ) {
 			const closestTabbable = this.getClosestTabbable( target, isReverse );
 			placeCaretAtHorizontalEdge( closestTabbable, isReverse );
 			event.preventDefault();

element/serialize.js

@@ -406,14 +406,14 @@ export function renderNativeComponent( type, props, context = {} ) {
 		// Textarea children can be assigned as value prop. If it is, render in
 		// place of children. Ensure to omit so it is not assigned as attribute
 		// as well.
-		content = renderChildren( [ props.value ], context );
+		content = renderChildren( props.value, context );
 		props = omit( props, 'value' );
 	} else if ( props.dangerouslySetInnerHTML &&
 			typeof props.dangerouslySetInnerHTML.__html === 'string' ) {
 		// Dangerous content is left unescaped.
 		content = props.dangerouslySetInnerHTML.__html;
 	} else if ( typeof props.children !== 'undefined' ) {
-		content = renderChildren( castArray( props.children ), context );
+		content = renderChildren( props.children, context );
 	}
 
 	if ( ! type ) {
@@ -465,6 +465,8 @@ export function renderComponent( Component, props, context = {} ) {
 function renderChildren( children, context = {} ) {
 	let result = '';
 
+	children = castArray( children );
+
 	for ( let i = 0; i < children.length; i++ ) {
 		const child = children[ i ];
 

element/test/serialize.js

@@ -199,6 +199,12 @@ describe( 'renderElement()', () => {
 		expect( result ).toBe( 'Hello' );
 	} );
 
+	it( 'renders Fragment with undefined children', () => {
+		const result = renderElement( <Fragment /> );
+
+		expect( result ).toBe( '' );
+	} );
+
 	it( 'renders RawHTML as its unescaped children', () => {
 		const result = renderElement( <RawHTML>{ '<img/>' }</RawHTML> );