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: Collapse range in horizontal edge check by selection direction #6467

Closed
wants to merge 25 commits into from
Closed
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
654eb22
Writing Flow: Collapse range in horizontal edge check by direction
aduth Apr 27, 2018
f7dcc8a
Try alternative approach
ellatrix Apr 30, 2018
bef44c3
Writing Flow: Move collapsed selection condition to WritingFlow
aduth May 2, 2018
a8728bf
Writing Flow: Vertical navigate from uncollapsed selections
aduth May 2, 2018
79fc388
Writing Flow: Use Selection#isCollapsed in place of hasCollapsedSelec…
aduth May 2, 2018
83d3ff7
Rich Text: Verify collapsed selection in delete behavior
aduth May 2, 2018
33184f9
Utils: Improve range rect detection for br-separated content
aduth May 2, 2018
f2ae9ef
Testing: Add Writing Flow E2E tests
aduth May 2, 2018
d81ac98
Try getRectangleFromRange fix without manipulating live DOM
ellatrix May 2, 2018
284900e
Utils: Update caret to selection in edge tests comments
aduth May 3, 2018
bc252af
Writing Flow: Invert position when isReverse not aligned to backward
aduth May 3, 2018
a961282
Try direct selection
ellatrix May 3, 2018
6431770
Only look at focusNode
ellatrix May 3, 2018
c0a0fc3
Utils: Document horizontal edge focusOffset
aduth May 3, 2018
b5a54df
Utils: Use temporary text node for rect fallback
aduth May 3, 2018
e087ddd
Writing Flow: Test horizontal navigation as handled
aduth May 3, 2018
7b174ad
Element: Fix Fragment render error on empty children
aduth May 10, 2018
a528868
Testing: Add global guard against ZWSP in E2E content retrieval
aduth May 10, 2018
d6f1ab9
RichText: Remove ZWSP on focusout
aduth May 10, 2018
4523390
RichText: Consider emptiness by Children count utility
aduth May 10, 2018
31596df
Writing Flow: Consider horizontal handled by stopPropagation in RichText
aduth May 10, 2018
fa4ad6d
RichText: Get content by converting tree node to element
aduth May 10, 2018
2afb649
Writing Flow: Consider as edge by effective caret position
aduth May 10, 2018
dab93de
Rich Text: Return element fragment as array
aduth May 10, 2018
9dde841
Testing: Attempt to resolve race conditions with TinyMCE initialization
aduth May 10, 2018
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
36 changes: 35 additions & 1 deletion editor/components/rich-text/format.js
Original file line number Diff line number Diff line change
@@ -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.
*
179 changes: 146 additions & 33 deletions editor/components/rich-text/index.js
Original file line number Diff line number Diff line change
@@ -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 ) {
12 changes: 9 additions & 3 deletions editor/components/writing-flow/index.js
Original file line number Diff line number Diff line change
@@ -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();
6 changes: 4 additions & 2 deletions element/serialize.js
Original file line number Diff line number Diff line change
@@ -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 ];

6 changes: 6 additions & 0 deletions element/test/serialize.js
Original file line number Diff line number Diff line change
@@ -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> );

Loading