From 7f1d3fcafe5b0ed51433ef289055c322dc88053e Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 23 Jan 2019 10:29:53 +0100 Subject: [PATCH 01/19] Avoid flickering by setting boundary class when creating the DOM tree --- packages/rich-text/src/to-tree.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index be23219851a96..a44cafe1e64bc 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -3,6 +3,7 @@ */ import { getFormatType } from './get-format-type'; +import { getActiveFormat } from './get-active-format'; import { LINE_SEPARATOR, OBJECT_REPLACEMENT_CHARACTER, @@ -19,7 +20,7 @@ function fromFormat( { type, attributes, unregisteredAttributes, object } ) { const elementAttributes = { ...unregisteredAttributes }; for ( const name in attributes ) { - const key = formatType.attributes[ name ]; + const key = formatType.attributes ? formatType.attributes[ name ] : false; if ( key ) { elementAttributes[ key ] = attributes[ name ]; @@ -146,14 +147,25 @@ export function toTree( { return; } + const { type, attributes = {}, object } = format; + const activeFormat = getActiveFormat( value, type ); + + if ( format === activeFormat ) { + attributes[ 'data-mce-selected' ] = 'inline-boundary'; + } + const parent = getParent( pointer ); - const newNode = append( parent, fromFormat( format ) ); + const newNode = append( parent, fromFormat( { + type, + attributes, + object, + } ) ); if ( isText( pointer ) && getText( pointer ).length === 0 ) { remove( pointer ); } - pointer = append( format.object ? parent : newNode, '' ); + pointer = append( object ? parent : newNode, '' ); } ); } From fa8722e79615a6d5200390f434e90d0e85b457bc Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 23 Jan 2019 10:46:17 +0100 Subject: [PATCH 02/19] Remove TinyMCE boundaries --- packages/editor/src/components/rich-text/index.js | 4 +++- packages/editor/src/components/rich-text/tinymce.js | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 21ae5a91b5cac..428fecc4b6490 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -377,7 +377,8 @@ export class RichText extends Component { * Handles the `selectionchange` event: sync the selection to local state. */ onSelectionChange() { - const { start, end, formats } = this.createRecord(); + const value = this.createRecord(); + const { start, end, formats } = value; if ( start !== this.state.start || end !== this.state.end ) { const isCaretWithinFormattedText = this.props.isCaretWithinFormattedText; @@ -388,6 +389,7 @@ export class RichText extends Component { } this.setState( { start, end } ); + this.applyRecord( value ); } } diff --git a/packages/editor/src/components/rich-text/tinymce.js b/packages/editor/src/components/rich-text/tinymce.js index 529fa15c1a54c..297a6d496b728 100644 --- a/packages/editor/src/components/rich-text/tinymce.js +++ b/packages/editor/src/components/rich-text/tinymce.js @@ -191,7 +191,6 @@ export default class TinyMCE extends Component { // already with dangerouslySetInnerHTML, we don't need this to be // verified. verify_html: false, - inline_boundaries_selector: 'a[href],code,b,i,strong,em,del,ins,sup,sub', plugins: [], forced_root_block: multilineTag || false, // Allow TinyMCE to keep one undo level for comparing changes. From e269593e5cb71db99e210fd684c8b3d03d26f556 Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 30 Jan 2019 11:11:33 +0100 Subject: [PATCH 03/19] Remove TinyMCE so it doesn't interfere with our boundaries --- .../src/components/rich-text/editable.js | 196 +++++++++ .../editor/src/components/rich-text/index.js | 17 +- .../src/components/rich-text/style.scss | 6 +- .../src/components/rich-text/tinymce.js | 380 ------------------ 4 files changed, 207 insertions(+), 392 deletions(-) create mode 100644 packages/editor/src/components/rich-text/editable.js delete mode 100644 packages/editor/src/components/rich-text/tinymce.js diff --git a/packages/editor/src/components/rich-text/editable.js b/packages/editor/src/components/rich-text/editable.js new file mode 100644 index 0000000000000..883e2b1ec70bd --- /dev/null +++ b/packages/editor/src/components/rich-text/editable.js @@ -0,0 +1,196 @@ +/** + * External dependencies + */ +import { isEqual } from 'lodash'; +import classnames from 'classnames'; + +/** + * WordPress dependencies + */ +import { Component, createElement } from '@wordpress/element'; +import { BACKSPACE, DELETE } from '@wordpress/keycodes'; + +/** + * Internal dependencies + */ +import { diffAriaProps, pickAriaProps } from './aria'; + +/** + * Browser dependencies + */ + +const { userAgent } = window.navigator; + +/** + * Applies a fix that provides `input` events for contenteditable in Internet Explorer. + * + * @param {Element} editorNode The root editor node. + * + * @return {Function} A function to remove the fix (for cleanup). + */ +function applyInternetExplorerInputFix( editorNode ) { + /** + * Dispatches `input` events in response to `textinput` events. + * + * IE provides a `textinput` event that is similar to an `input` event, + * and we use it to manually dispatch an `input` event. + * `textinput` is dispatched for text entry but for not deletions. + * + * @param {Event} textInputEvent An Internet Explorer `textinput` event. + */ + function mapTextInputEvent( textInputEvent ) { + textInputEvent.stopImmediatePropagation(); + + const inputEvent = document.createEvent( 'Event' ); + inputEvent.initEvent( 'input', true, false ); + inputEvent.data = textInputEvent.data; + textInputEvent.target.dispatchEvent( inputEvent ); + } + + /** + * Dispatches `input` events in response to Delete and Backspace keyup. + * + * It would be better dispatch an `input` event after each deleting + * `keydown` because the DOM is updated after each, but it is challenging + * to determine the right time to dispatch `input` since propagation of + * `keydown` can be stopped at any point. + * + * It's easier to listen for `keyup` in the capture phase and dispatch + * `input` before `keyup` propagates further. It's not perfect, but should + * be good enough. + * + * @param {KeyboardEvent} keyUp + * @param {Node} keyUp.target The event target. + * @param {number} keyUp.keyCode The key code. + */ + function mapDeletionKeyUpEvents( { target, keyCode } ) { + const isDeletion = BACKSPACE === keyCode || DELETE === keyCode; + + if ( isDeletion && editorNode.contains( target ) ) { + const inputEvent = document.createEvent( 'Event' ); + inputEvent.initEvent( 'input', true, false ); + inputEvent.data = null; + target.dispatchEvent( inputEvent ); + } + } + + editorNode.addEventListener( 'textinput', mapTextInputEvent ); + document.addEventListener( 'keyup', mapDeletionKeyUpEvents, true ); + return function removeInternetExplorerInputFix() { + editorNode.removeEventListener( 'textinput', mapTextInputEvent ); + document.removeEventListener( 'keyup', mapDeletionKeyUpEvents, true ); + }; +} + +const IS_PLACEHOLDER_VISIBLE_ATTR_NAME = 'data-is-placeholder-visible'; +const CLASS_NAME = 'editor-rich-text__editable'; + +/** + * Whether or not the user agent is Internet Explorer. + * + * @type {boolean} + */ +const IS_IE = userAgent.indexOf( 'Trident' ) >= 0; + +export default class Editable extends Component { + constructor() { + super(); + this.bindEditorNode = this.bindEditorNode.bind( this ); + this.onFocus = this.onFocus.bind( this ); + } + + onFocus() { + if ( this.props.onFocus ) { + this.props.onFocus(); + } + } + + // We must prevent rerenders because the browser will modify the DOM. React + // will rerender the DOM fine, but we're losing selection and it would be + // more expensive to do so as it would just set the inner HTML through + // `dangerouslySetInnerHTML`. Instead RichText does it's own diffing and + // selection setting. + // + // Because we never update the component, we have to look through props and + // update the attributes on the wrapper nodes here. `componentDidUpdate` + // will never be called. + shouldComponentUpdate( nextProps ) { + this.configureIsPlaceholderVisible( nextProps.isPlaceholderVisible ); + + if ( ! isEqual( this.props.style, nextProps.style ) ) { + this.editorNode.setAttribute( 'style', '' ); + Object.assign( this.editorNode.style, nextProps.style ); + } + + if ( ! isEqual( this.props.className, nextProps.className ) ) { + this.editorNode.className = classnames( nextProps.className, CLASS_NAME ); + } + + const { removedKeys, updatedKeys } = diffAriaProps( this.props, nextProps ); + removedKeys.forEach( ( key ) => + this.editorNode.removeAttribute( key ) ); + updatedKeys.forEach( ( key ) => + this.editorNode.setAttribute( key, nextProps[ key ] ) ); + + return false; + } + + configureIsPlaceholderVisible( isPlaceholderVisible ) { + const isPlaceholderVisibleString = String( !! isPlaceholderVisible ); + if ( this.editorNode.getAttribute( IS_PLACEHOLDER_VISIBLE_ATTR_NAME ) !== isPlaceholderVisibleString ) { + this.editorNode.setAttribute( IS_PLACEHOLDER_VISIBLE_ATTR_NAME, isPlaceholderVisibleString ); + } + } + + bindEditorNode( editorNode ) { + this.editorNode = editorNode; + + if ( this.props.setRef ) { + this.props.setRef( editorNode ); + } + + if ( IS_IE ) { + if ( editorNode ) { + // Mounting: + this.removeInternetExplorerInputFix = applyInternetExplorerInputFix( editorNode ); + } else { + // Unmounting: + this.removeInternetExplorerInputFix(); + } + } + } + + render() { + const ariaProps = pickAriaProps( this.props ); + const { + tagName = 'div', + style, + record, + valueToEditableHTML, + className, + isPlaceholderVisible, + onPaste, + onInput, + onKeyDown, + onCompositionEnd, + onBlur, + } = this.props; + + return createElement( tagName, { + ...ariaProps, + className: classnames( className, CLASS_NAME ), + contentEditable: true, + [ IS_PLACEHOLDER_VISIBLE_ATTR_NAME ]: isPlaceholderVisible, + ref: this.bindEditorNode, + style, + suppressContentEditableWarning: true, + dangerouslySetInnerHTML: { __html: valueToEditableHTML( record ) }, + onPaste, + onInput, + onFocus: this.onFocus, + onBlur, + onKeyDown, + onCompositionEnd, + } ); + } +} diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 428fecc4b6490..395fe7fcb79d4 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -55,7 +55,7 @@ import Autocomplete from '../autocomplete'; import BlockFormatControls from '../block-format-controls'; import FormatEdit from './format-edit'; import FormatToolbar from './format-toolbar'; -import TinyMCE, { TINYMCE_ZWSP } from './tinymce'; +import Editable from './editable'; import { pickAriaProps } from './aria'; import { getPatterns } from './patterns'; import { withBlockEditContext } from '../block-edit/context'; @@ -165,7 +165,7 @@ export class RichText extends Component { removeNode: ( node ) => node.getAttribute( 'data-mce-bogus' ) === 'all', unwrapNode: ( node ) => !! node.getAttribute( 'data-mce-bogus' ), removeAttribute: ( attribute ) => attribute.indexOf( 'data-mce-' ) === 0, - filterString: ( string ) => string.replace( TINYMCE_ZWSP, '' ), + filterString: ( string ) => string.replace( '\uFEFF', '' ), prepareEditableTree: this.props.prepareEditableTree, } ); } @@ -814,13 +814,12 @@ export class RichText extends Component { onTagNameChange, } = this.props; + // Generating a key that includes `tagName` ensures that if the tag + // changes, we replace the relevant element. This is needed because we + // prevent Editable component updates. + const key = Tagname; const MultilineTag = this.multilineTag; const ariaProps = pickAriaProps( this.props ); - - // Generating a key that includes `tagName` ensures that if the tag - // changes, we unmount and destroy the previous TinyMCE element, then - // mount and initialize a new child element in its place. - const key = [ 'editor', Tagname ].join(); const isPlaceholderVisible = placeholder && ( ! isSelected || keepPlaceholderOnFocus ) && this.isEmpty(); const classes = classnames( wrapperClassName, 'editor-rich-text' ); const record = this.getRecord(); @@ -855,7 +854,7 @@ export class RichText extends Component { > { ( { listBoxId, activeId } ) => ( - { isPlaceholderVisible && { MultilineTag ? { placeholder } : placeholder } diff --git a/packages/editor/src/components/rich-text/style.scss b/packages/editor/src/components/rich-text/style.scss index 0907de79535d1..41721d0ff11a7 100644 --- a/packages/editor/src/components/rich-text/style.scss +++ b/packages/editor/src/components/rich-text/style.scss @@ -4,7 +4,7 @@ position: relative; } -.editor-rich-text__tinymce { +.editor-rich-text__editable { margin: 0; position: relative; line-height: $editor-line-height; @@ -113,7 +113,7 @@ } // Placeholder text. - & + .editor-rich-text__tinymce { + & + .editor-rich-text__editable { pointer-events: none; // Use opacity to work in various editor styles. @@ -126,7 +126,7 @@ // Captions may have lighter (gray) text, or be shown on a range of different background luminosites. // To ensure legibility, we increase the default placeholder opacity to ensure contrast. - &[data-is-placeholder-visible="true"] + figcaption.editor-rich-text__tinymce { + &[data-is-placeholder-visible="true"] + figcaption.editor-rich-text__editable { opacity: 0.8; } } diff --git a/packages/editor/src/components/rich-text/tinymce.js b/packages/editor/src/components/rich-text/tinymce.js deleted file mode 100644 index 297a6d496b728..0000000000000 --- a/packages/editor/src/components/rich-text/tinymce.js +++ /dev/null @@ -1,380 +0,0 @@ -/** - * External dependencies - */ -import tinymce from 'tinymce'; -import { isEqual, noop } from 'lodash'; -import classnames from 'classnames'; - -/** - * WordPress dependencies - */ -import { Component, createElement } from '@wordpress/element'; -import { BACKSPACE, DELETE, ENTER, LEFT, RIGHT } from '@wordpress/keycodes'; -import { isEntirelySelected } from '@wordpress/dom'; - -/** - * Internal dependencies - */ -import { diffAriaProps, pickAriaProps } from './aria'; - -/** - * Browser dependencies - */ - -const { getSelection } = window; -const { TEXT_NODE } = window.Node; -const { userAgent } = window.navigator; - -/** - * 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_ZWSP = '\uFEFF'; - -/** - * Applies a fix that provides `input` events for contenteditable in Internet Explorer. - * - * @param {Element} editorNode The root editor node. - * - * @return {Function} A function to remove the fix (for cleanup). - */ -function applyInternetExplorerInputFix( editorNode ) { - /** - * Dispatches `input` events in response to `textinput` events. - * - * IE provides a `textinput` event that is similar to an `input` event, - * and we use it to manually dispatch an `input` event. - * `textinput` is dispatched for text entry but for not deletions. - * - * @param {Event} textInputEvent An Internet Explorer `textinput` event. - */ - function mapTextInputEvent( textInputEvent ) { - textInputEvent.stopImmediatePropagation(); - - const inputEvent = document.createEvent( 'Event' ); - inputEvent.initEvent( 'input', true, false ); - inputEvent.data = textInputEvent.data; - textInputEvent.target.dispatchEvent( inputEvent ); - } - - /** - * Dispatches `input` events in response to Delete and Backspace keyup. - * - * It would be better dispatch an `input` event after each deleting - * `keydown` because the DOM is updated after each, but it is challenging - * to determine the right time to dispatch `input` since propagation of - * `keydown` can be stopped at any point. - * - * It's easier to listen for `keyup` in the capture phase and dispatch - * `input` before `keyup` propagates further. It's not perfect, but should - * be good enough. - * - * @param {KeyboardEvent} keyUp - * @param {Node} keyUp.target The event target. - * @param {number} keyUp.keyCode The key code. - */ - function mapDeletionKeyUpEvents( { target, keyCode } ) { - const isDeletion = BACKSPACE === keyCode || DELETE === keyCode; - - if ( isDeletion && editorNode.contains( target ) ) { - const inputEvent = document.createEvent( 'Event' ); - inputEvent.initEvent( 'input', true, false ); - inputEvent.data = null; - target.dispatchEvent( inputEvent ); - } - } - - editorNode.addEventListener( 'textinput', mapTextInputEvent ); - document.addEventListener( 'keyup', mapDeletionKeyUpEvents, true ); - return function removeInternetExplorerInputFix() { - editorNode.removeEventListener( 'textinput', mapTextInputEvent ); - document.removeEventListener( 'keyup', mapDeletionKeyUpEvents, true ); - }; -} - -const IS_PLACEHOLDER_VISIBLE_ATTR_NAME = 'data-is-placeholder-visible'; - -/** - * Whether or not the user agent is Internet Explorer. - * - * @type {boolean} - */ -const IS_IE = userAgent.indexOf( 'Trident' ) >= 0; - -export default class TinyMCE extends Component { - constructor() { - super(); - this.bindEditorNode = this.bindEditorNode.bind( this ); - this.onFocus = this.onFocus.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); - this.initialize = this.initialize.bind( this ); - } - - onFocus() { - if ( this.props.onFocus ) { - this.props.onFocus(); - } - - this.initialize(); - } - - // We must prevent rerenders because RichText, the browser, and TinyMCE will - // modify the DOM. React will rerender the DOM fine, but we're losing - // selection and it would be more expensive to do so as it would just set - // the inner HTML through `dangerouslySetInnerHTML`. Instead RichText does - // it's own diffing and selection setting. - // - // Because we never update the component, we have to look through props and - // update the attributes on the wrapper nodes here. `componentDidUpdate` - // will never be called. - shouldComponentUpdate( nextProps ) { - this.configureIsPlaceholderVisible( nextProps.isPlaceholderVisible ); - - if ( ! isEqual( this.props.style, nextProps.style ) ) { - this.editorNode.setAttribute( 'style', '' ); - Object.assign( this.editorNode.style, nextProps.style ); - } - - if ( ! isEqual( this.props.className, nextProps.className ) ) { - this.editorNode.className = classnames( nextProps.className, 'editor-rich-text__tinymce' ); - } - - const { removedKeys, updatedKeys } = diffAriaProps( this.props, nextProps ); - removedKeys.forEach( ( key ) => - this.editorNode.removeAttribute( key ) ); - updatedKeys.forEach( ( key ) => - this.editorNode.setAttribute( key, nextProps[ key ] ) ); - - return false; - } - - componentWillUnmount() { - if ( ! this.editor ) { - return; - } - - this.editor.destroy(); - delete this.editor; - } - - configureIsPlaceholderVisible( isPlaceholderVisible ) { - const isPlaceholderVisibleString = String( !! isPlaceholderVisible ); - if ( this.editorNode.getAttribute( IS_PLACEHOLDER_VISIBLE_ATTR_NAME ) !== isPlaceholderVisibleString ) { - this.editorNode.setAttribute( IS_PLACEHOLDER_VISIBLE_ATTR_NAME, isPlaceholderVisibleString ); - } - } - - /** - * Initializes TinyMCE. Can only be called once per instance. - */ - initialize() { - if ( this.initialize.called ) { - return; - } - - this.initialize.called = true; - - const { multilineTag } = this.props; - const settings = { - theme: false, - inline: true, - toolbar: false, - browser_spellcheck: true, - entity_encoding: 'raw', - convert_urls: false, - // Disables TinyMCE's parsing to verify HTML. It makes - // initialisation a bit faster. Since we're setting raw HTML - // already with dangerouslySetInnerHTML, we don't need this to be - // verified. - verify_html: false, - plugins: [], - forced_root_block: multilineTag || false, - // Allow TinyMCE to keep one undo level for comparing changes. - // Prevent it otherwise from accumulating any history. - custom_undo_redo_levels: 1, - lists_indent_on_tab: false, - }; - - tinymce.init( { - ...settings, - target: this.editorNode, - setup: ( editor ) => { - this.editor = editor; - - // TinyMCE resets the element content on initialization, even - // when it's already identical to what exists currently. This - // behavior clobbers a selection which exists at the time of - // initialization, thus breaking writing flow navigation. The - // hack here neutralizes setHTML during initialization. - let setHTML; - - editor.on( 'preinit', () => { - setHTML = editor.dom.setHTML; - editor.dom.setHTML = () => {}; - } ); - - editor.on( 'init', () => { - // History is handled internally by RichText. - // - // See: https://github.com/tinymce/tinymce/blob/master/src/core/main/ts/api/UndoManager.ts - [ 'z', 'y' ].forEach( ( character ) => { - editor.shortcuts.remove( `meta+${ character }` ); - } ); - editor.shortcuts.remove( 'meta+shift+z' ); - - // Reset TinyMCE's default formatting shortcuts, since - // RichText supports only registered formats. - // - // See: https://github.com/tinymce/tinymce/blob/master/src/core/main/ts/keyboard/FormatShortcuts.ts - [ 'b', 'i', 'u' ].forEach( ( character ) => { - editor.shortcuts.remove( `meta+${ character }` ); - } ); - [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ].forEach( ( number ) => { - editor.shortcuts.remove( `access+${ number }` ); - } ); - - // Restore the original `setHTML` once initialized. - editor.dom.setHTML = setHTML; - - // In IE11, focus is lost to parent after initialising - // TinyMCE, so we have to set it back. - if ( - IS_IE && - document.activeElement !== this.editorNode && - document.activeElement.contains( this.editorNode ) - ) { - this.editorNode.focus(); - } - } ); - - editor.on( 'keydown', this.onKeyDown, true ); - }, - } ); - } - - bindEditorNode( editorNode ) { - this.editorNode = editorNode; - - if ( this.props.setRef ) { - this.props.setRef( editorNode ); - } - - if ( IS_IE ) { - if ( editorNode ) { - // Mounting: - this.removeInternetExplorerInputFix = applyInternetExplorerInputFix( editorNode ); - } else { - // Unmounting: - this.removeInternetExplorerInputFix(); - } - } - } - - onKeyDown( event ) { - const { keyCode } = event; - const isDelete = keyCode === DELETE || keyCode === BACKSPACE; - - // Disables TinyMCE behaviour. - if ( - keyCode === ENTER || - ( isDelete && isEntirelySelected( this.editorNode ) ) - ) { - event.preventDefault(); - // For some reason this is needed to also prevent the insertion of - // line breaks. - return false; - } - - // 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 - if ( keyCode !== LEFT && keyCode !== RIGHT ) { - return; - } - - const { focusNode } = getSelection(); - const { nodeType, nodeValue } = focusNode; - - if ( nodeType !== 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 { - tagName = 'div', - style, - record, - valueToEditableHTML, - className, - isPlaceholderVisible, - onPaste, - onInput, - onKeyDown, - onCompositionEnd, - onBlur, - } = this.props; - - /* - * The role=textbox and aria-multiline=true must always be used together - * as TinyMCE always behaves like a sort of textarea where text wraps in - * multiple lines. Only the table block editable element is excluded. - */ - if ( tagName !== 'table' ) { - ariaProps.role = 'textbox'; - ariaProps[ 'aria-multiline' ] = true; - } - - // If a default value is provided, render it into the DOM even before - // TinyMCE finishes initializing. This avoids a short delay by allowing - // us to show and focus the content before it's truly ready to edit. - return createElement( tagName, { - ...ariaProps, - className: classnames( className, 'editor-rich-text__tinymce' ), - contentEditable: true, - [ IS_PLACEHOLDER_VISIBLE_ATTR_NAME ]: isPlaceholderVisible, - ref: this.bindEditorNode, - style, - suppressContentEditableWarning: true, - dangerouslySetInnerHTML: { __html: valueToEditableHTML( record ) }, - onPaste, - onInput, - onFocus: this.onFocus, - onBlur, - onKeyDown, - onCompositionEnd, - } ); - } -} From 8435016dc74e3c4af5c91bb05a9f828a9c2305f5 Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 30 Jan 2019 11:55:05 +0100 Subject: [PATCH 04/19] Fix failing unit tests --- .../button/test/__snapshots__/index.js.snap | 4 +- .../heading/test/__snapshots__/index.js.snap | 4 +- .../src/list/test/__snapshots__/index.js.snap | 4 +- .../test/__snapshots__/index.js.snap | 4 +- .../test/__snapshots__/index.js.snap | 4 +- .../test/__snapshots__/index.js.snap | 4 +- .../quote/test/__snapshots__/index.js.snap | 4 +- .../test/__snapshots__/index.js.snap | 8 ++-- .../verse/test/__snapshots__/index.js.snap | 4 +- .../src/components/rich-text/editable.js | 3 ++ .../src/test/__snapshots__/to-dom.js.snap | 37 ++++++++++++++----- packages/rich-text/src/to-tree.js | 12 ++++-- 12 files changed, 59 insertions(+), 33 deletions(-) diff --git a/packages/block-library/src/button/test/__snapshots__/index.js.snap b/packages/block-library/src/button/test/__snapshots__/index.js.snap index e153279891096..ab72e31893bcd 100644 --- a/packages/block-library/src/button/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/button/test/__snapshots__/index.js.snap @@ -18,7 +18,7 @@ exports[`core/button block edit matches snapshot 1`] = ` aria-autocomplete="list" aria-label="Add text…" aria-multiline="true" - class="wp-block-button__link editor-rich-text__tinymce" + class="wp-block-button__link editor-rich-text__editable" contenteditable="true" data-is-placeholder-visible="true" role="textbox" @@ -28,7 +28,7 @@ exports[`core/button block edit matches snapshot 1`] = ` /> diff --git a/packages/block-library/src/heading/test/__snapshots__/index.js.snap b/packages/block-library/src/heading/test/__snapshots__/index.js.snap index 0131b6104ae6a..72e43c5bbb4eb 100644 --- a/packages/block-library/src/heading/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/heading/test/__snapshots__/index.js.snap @@ -13,7 +13,7 @@ exports[`core/heading block edit matches snapshot 1`] = ` aria-autocomplete="list" aria-label="Write heading…" aria-multiline="true" - class="editor-rich-text__tinymce" + class="editor-rich-text__editable" contenteditable="true" data-is-placeholder-visible="true" role="textbox" @@ -23,7 +23,7 @@ exports[`core/heading block edit matches snapshot 1`] = ` />

Write heading…

diff --git a/packages/block-library/src/list/test/__snapshots__/index.js.snap b/packages/block-library/src/list/test/__snapshots__/index.js.snap index d2da3e3413525..93005f79e5566 100644 --- a/packages/block-library/src/list/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/list/test/__snapshots__/index.js.snap @@ -13,7 +13,7 @@ exports[`core/list block edit matches snapshot 1`] = ` aria-autocomplete="list" aria-label="Write list…" aria-multiline="true" - class="editor-rich-text__tinymce" + class="editor-rich-text__editable" contenteditable="true" data-is-placeholder-visible="true" role="textbox" @@ -25,7 +25,7 @@ exports[`core/list block edit matches snapshot 1`] = ` diff --git a/packages/block-library/src/paragraph/test/__snapshots__/index.js.snap b/packages/block-library/src/paragraph/test/__snapshots__/index.js.snap index dfbc48ed59016..52a4f07a4b653 100644 --- a/packages/block-library/src/paragraph/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/paragraph/test/__snapshots__/index.js.snap @@ -21,7 +21,7 @@ exports[`core/paragraph block edit matches snapshot 1`] = ` role="textbox" >


               


diff --git a/packages/block-library/src/quote/test/__snapshots__/index.js.snap b/packages/block-library/src/quote/test/__snapshots__/index.js.snap index e5f0927f03908..886ef7e4560a8 100644 --- a/packages/block-library/src/quote/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/quote/test/__snapshots__/index.js.snap @@ -23,7 +23,7 @@ exports[`core/quote block edit matches snapshot 1`] = ` >


diff --git a/packages/block-library/src/text-columns/test/__snapshots__/index.js.snap b/packages/block-library/src/text-columns/test/__snapshots__/index.js.snap index 5cb16fe1c58e4..4f89f08374630 100644 --- a/packages/block-library/src/text-columns/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/text-columns/test/__snapshots__/index.js.snap @@ -25,7 +25,7 @@ exports[`core/text-columns block edit matches snapshot 1`] = ` role="textbox" >



 node.getAttribute( 'data-mce-bogus' ) === 'all',
-			unwrapNode: ( node ) => !! node.getAttribute( 'data-mce-bogus' ),
-			removeAttribute: ( attribute ) => attribute.indexOf( 'data-mce-' ) === 0,
 			prepareEditableTree: this.props.prepareEditableTree,
 		} );
 	}
@@ -175,11 +172,6 @@ export class RichText extends Component {
 			current: this.editableRef,
 			multilineTag: this.multilineTag,
 			multilineWrapperTags: this.multilineWrapperTags,
-			createLinePadding( doc ) {
-				const element = doc.createElement( 'br' );
-				element.setAttribute( 'data-mce-bogus', '1' );
-				return element;
-			},
 			prepareEditableTree: this.props.prepareEditableTree,
 		} );
 	}
@@ -740,11 +732,6 @@ export class RichText extends Component {
 			value,
 			multilineTag: this.multilineTag,
 			multilineWrapperTags: this.multilineWrapperTags,
-			createLinePadding( doc ) {
-				const element = doc.createElement( 'br' );
-				element.setAttribute( 'data-mce-bogus', '1' );
-				return element;
-			},
 			prepareEditableTree: this.props.prepareEditableTree,
 		} ).body.innerHTML;
 	}
diff --git a/packages/rich-text/src/create.js b/packages/rich-text/src/create.js
index 4dcd1f4e5a6c5..6a74012979e78 100644
--- a/packages/rich-text/src/create.js
+++ b/packages/rich-text/src/create.js
@@ -99,12 +99,6 @@ function toFormat( { type, attributes } ) {
  *                                            multiline.
  * @param {?Array}    $1.multilineWrapperTags Tags where lines can be found if
  *                                            nesting is possible.
- * @param {?Function} $1.removeNode           Function to declare whether the
- *                                            given node should be removed.
- * @param {?Function} $1.unwrapNode           Function to declare whether the
- *                                            given node should be unwrapped.
- * @param {?Function} $1.removeAttribute      Wether to remove an attribute
- *                                            based on the name.
  *
  * @return {Object} A rich text value.
  */
@@ -115,9 +109,6 @@ export function create( {
 	range,
 	multilineTag,
 	multilineWrapperTags,
-	removeNode,
-	unwrapNode,
-	removeAttribute,
 } = {} ) {
 	if ( typeof text === 'string' && text.length > 0 ) {
 		return {
@@ -138,9 +129,6 @@ export function create( {
 		return createFromElement( {
 			element,
 			range,
-			removeNode,
-			unwrapNode,
-			removeAttribute,
 		} );
 	}
 
@@ -149,9 +137,6 @@ export function create( {
 		range,
 		multilineTag,
 		multilineWrapperTags,
-		removeNode,
-		unwrapNode,
-		removeAttribute,
 	} );
 }
 
@@ -267,12 +252,6 @@ function filterString( string ) {
  *                                            multiline.
  * @param {?Array}    $1.multilineWrapperTags Tags where lines can be found if
  *                                            nesting is possible.
- * @param {?Function} $1.removeNode           Function to declare whether the
- *                                            given node should be removed.
- * @param {?Function} $1.unwrapNode           Function to declare whether the
- *                                            given node should be unwrapped.
- * @param {?Function} $1.removeAttribute      Wether to remove an attribute
- *                                            based on the name.
  *
  * @return {Object} A rich text value.
  */
@@ -282,9 +261,6 @@ function createFromElement( {
 	multilineTag,
 	multilineWrapperTags,
 	currentWrapperTags = [],
-	removeNode,
-	unwrapNode,
-	removeAttribute,
 } ) {
 	const accumulator = createEmptyValue();
 
@@ -319,10 +295,7 @@ function createFromElement( {
 			continue;
 		}
 
-		if (
-			( removeNode && removeNode( node ) ) ||
-			( unwrapNode && unwrapNode( node ) && ! node.hasChildNodes() )
-		) {
+		if ( node.getAttribute( 'data-rich-text-padding' ) ) {
 			accumulateSelection( accumulator, node, range, createEmptyValue() );
 			continue;
 		}
@@ -336,37 +309,30 @@ function createFromElement( {
 
 		const lastFormats = accumulator.formats[ accumulator.formats.length - 1 ];
 		const lastFormat = lastFormats && lastFormats[ lastFormats.length - 1 ];
-		let format;
-		let value;
+		const newFormat = toFormat( {
+			type,
+			attributes: getAttributes( { element: node } ),
+		} );
 
-		if ( ! unwrapNode || ! unwrapNode( node ) ) {
-			const newFormat = toFormat( {
-				type,
-				attributes: getAttributes( {
-					element: node,
-					removeAttribute,
-				} ),
-			} );
+		let format;
 
-			if ( newFormat ) {
-				// Reuse the last format if it's equal.
-				if ( isFormatEqual( newFormat, lastFormat ) ) {
-					format = lastFormat;
-				} else {
-					format = newFormat;
-				}
+		if ( newFormat ) {
+			// Reuse the last format if it's equal.
+			if ( isFormatEqual( newFormat, lastFormat ) ) {
+				format = lastFormat;
+			} else {
+				format = newFormat;
 			}
 		}
 
+		let value;
+
 		if ( multilineWrapperTags && multilineWrapperTags.indexOf( type ) !== -1 ) {
 			value = createFromMultilineElement( {
 				element: node,
 				range,
 				multilineTag,
 				multilineWrapperTags,
-				removeNode,
-				unwrapNode,
-				removeAttribute,
 				currentWrapperTags: [ ...currentWrapperTags, format ],
 			} );
 			format = undefined;
@@ -376,9 +342,6 @@ function createFromElement( {
 				range,
 				multilineTag,
 				multilineWrapperTags,
-				removeNode,
-				unwrapNode,
-				removeAttribute,
 			} );
 		}
 
@@ -446,12 +409,6 @@ function createFromElement( {
  *                                            multiline.
  * @param {?Array}    $1.multilineWrapperTags Tags where lines can be found if
  *                                            nesting is possible.
- * @param {?Function} $1.removeNode           Function to declare whether the
- *                                            given node should be removed.
- * @param {?Function} $1.unwrapNode           Function to declare whether the
- *                                            given node should be unwrapped.
- * @param {?Function} $1.removeAttribute      Wether to remove an attribute
- *                                            based on the name.
  * @param {boolean}   $1.currentWrapperTags   Whether to prepend a line
  *                                            separator.
  *
@@ -462,9 +419,6 @@ function createFromMultilineElement( {
 	range,
 	multilineTag,
 	multilineWrapperTags,
-	removeNode,
-	unwrapNode,
-	removeAttribute,
 	currentWrapperTags = [],
 } ) {
 	const accumulator = createEmptyValue();
@@ -489,9 +443,6 @@ function createFromMultilineElement( {
 			multilineTag,
 			multilineWrapperTags,
 			currentWrapperTags,
-			removeNode,
-			unwrapNode,
-			removeAttribute,
 		} );
 
 		// If a line consists of one single line break (invisible), consider the
@@ -532,16 +483,11 @@ function createFromMultilineElement( {
  *
  * @param {Object}    $1                 Named argements.
  * @param {Element}   $1.element         Element to get attributes from.
- * @param {?Function} $1.removeAttribute Wether to remove an attribute based on
- *                                       the name.
  *
  * @return {?Object} Attribute object or `undefined` if the element has no
  *                   attributes.
  */
-function getAttributes( {
-	element,
-	removeAttribute,
-} ) {
+function getAttributes( { element } ) {
 	if ( ! element.hasAttributes() ) {
 		return;
 	}
@@ -553,7 +499,7 @@ function getAttributes( {
 	for ( let i = 0; i < length; i++ ) {
 		const { name, value } = element.attributes[ i ];
 
-		if ( removeAttribute && removeAttribute( name ) ) {
+		if ( name === 'data-rich-text-format-boundary' ) {
 			continue;
 		}
 
diff --git a/packages/rich-text/src/test/__snapshots__/to-dom.js.snap b/packages/rich-text/src/test/__snapshots__/to-dom.js.snap
index 5c531882df419..8a20d0865a0dd 100644
--- a/packages/rich-text/src/test/__snapshots__/to-dom.js.snap
+++ b/packages/rich-text/src/test/__snapshots__/to-dom.js.snap
@@ -3,7 +3,7 @@
 exports[`recordToDom should create a value with formatting 1`] = `
 
   
     test
   
@@ -14,7 +14,7 @@ exports[`recordToDom should create a value with formatting 1`] = `
 exports[`recordToDom should create a value with formatting for split tags 1`] = `
 
   
     test
   
@@ -25,7 +25,7 @@ exports[`recordToDom should create a value with formatting for split tags 1`] =
 exports[`recordToDom should create a value with formatting with attributes 1`] = `
 
   
     test
@@ -85,7 +85,7 @@ exports[`recordToDom should create a value with nested formatting 1`] = `
 
   
     
       test
     
@@ -103,21 +103,25 @@ exports[`recordToDom should create a value without formatting 1`] = `
 exports[`recordToDom should create an empty value 1`] = `
 
   
-  
+
`; exports[`recordToDom should create an empty value from empty tags 1`] = ` -
+
`; -exports[`recordToDom should filter format attributes with settings 1`] = ` +exports[`recordToDom should filter format boundary attributes 1`] = ` test @@ -125,22 +129,22 @@ exports[`recordToDom should filter format attributes with settings 1`] = ` `; -exports[`recordToDom should filter text at end with settings 1`] = ` +exports[`recordToDom should filter zero width space 1`] = ` - test + +
`; -exports[`recordToDom should filter text in format with settings 1`] = ` +exports[`recordToDom should filter zero width space at end 1`] = ` - - test - - + test `; -exports[`recordToDom should filter text outside format with settings 1`] = ` +exports[`recordToDom should filter zero width space in format 1`] = ` test @@ -149,10 +153,12 @@ exports[`recordToDom should filter text outside format with settings 1`] = ` `; -exports[`recordToDom should filter text with settings 1`] = ` +exports[`recordToDom should filter zero width space outside format 1`] = ` + + test + -
`; @@ -167,7 +173,7 @@ exports[`recordToDom should handle br 1`] = ` exports[`recordToDom should handle br with formatting 1`] = `
@@ -199,7 +205,9 @@ exports[`recordToDom should handle empty list value 1`] = `
  • -
    +
  • `; @@ -208,7 +216,9 @@ exports[`recordToDom should handle empty multiline value 1`] = `

    -
    +

    `; @@ -217,15 +227,21 @@ exports[`recordToDom should handle middle empty list value 1`] = `
  • -
    +
  • -
    +
  • -
    +
  • `; @@ -283,7 +299,9 @@ exports[`recordToDom should handle multiline value with empty 1`] = `

    -
    +

    `; @@ -291,11 +309,15 @@ exports[`recordToDom should handle multiline value with empty 1`] = ` exports[`recordToDom should handle nested empty list value 1`] = `
  • -
    +
    • -
      +
  • @@ -337,7 +359,7 @@ exports[`recordToDom should preserve emoji 1`] = ` exports[`recordToDom should preserve emoji in formatting 1`] = ` 🍒 @@ -354,20 +376,9 @@ exports[`recordToDom should preserve non breaking space 1`] = ` exports[`recordToDom should remove br with settings 1`] = ` -
    - -`; - -exports[`recordToDom should remove with children with settings 1`] = ` - - two - -`; - -exports[`recordToDom should remove with settings 1`] = ` - - -
    +
    `; @@ -376,13 +387,3 @@ exports[`recordToDom should replace characters to format HTML with space 1`] = ` `; - -exports[`recordToDom should unwrap with settings 1`] = ` - - te - - st - - - -`; diff --git a/packages/rich-text/src/test/create.js b/packages/rich-text/src/test/create.js index e08f6b82be6b4..5128c55273d78 100644 --- a/packages/rich-text/src/test/create.js +++ b/packages/rich-text/src/test/create.js @@ -29,7 +29,6 @@ describe( 'create', () => { description, multilineTag, multilineWrapperTags, - settings, html, createRange, record, @@ -46,7 +45,6 @@ describe( 'create', () => { range, multilineTag, multilineWrapperTags, - ...settings, } ); const formatsLength = getSparseArrayLength( record.formats ); const createdFormatsLength = getSparseArrayLength( createdRecord.formats ); diff --git a/packages/rich-text/src/test/helpers/index.js b/packages/rich-text/src/test/helpers/index.js index 9673619d57be2..fcf3bd913ed33 100644 --- a/packages/rich-text/src/test/helpers/index.js +++ b/packages/rich-text/src/test/helpers/index.js @@ -527,27 +527,6 @@ export const spec = [ text: 'one', }, }, - { - description: 'should remove with settings', - settings: { - unwrapNode: ( node ) => !! node.getAttribute( 'data-mce-bogus' ), - }, - html: '', - createRange: ( element ) => ( { - startOffset: 0, - startContainer: element, - endOffset: 1, - endContainer: element, - } ), - startPath: [ 0, 0 ], - endPath: [ 0, 0 ], - record: { - start: 0, - end: 0, - formats: [], - text: '', - }, - }, { description: 'should ignore formats at line separator', multilineTag: 'p', @@ -560,10 +539,7 @@ export const spec = [ }, { description: 'should remove br with settings', - settings: { - unwrapNode: ( node ) => !! node.getAttribute( 'data-mce-bogus' ), - }, - html: '
    ', + html: '
    ', createRange: ( element ) => ( { startOffset: 0, startContainer: element, @@ -580,53 +556,8 @@ export const spec = [ }, }, { - description: 'should unwrap with settings', - settings: { - unwrapNode: ( node ) => !! node.getAttribute( 'data-mce-bogus' ), - }, - html: 'test', - createRange: ( element ) => ( { - startOffset: 0, - startContainer: element, - endOffset: 1, - endContainer: element, - } ), - startPath: [ 0, 0 ], - endPath: [ 1, 0, 2 ], - record: { - start: 0, - end: 4, - formats: [ , , [ em ], [ em ] ], - text: 'test', - }, - }, - { - description: 'should remove with children with settings', - settings: { - removeNode: ( node ) => node.getAttribute( 'data-mce-bogus' ) === 'all', - }, - html: 'onetwo', - createRange: ( element ) => ( { - startOffset: 0, - startContainer: element.lastChild, - endOffset: 1, - endContainer: element.lastChild, - } ), - startPath: [ 0, 0 ], - endPath: [ 0, 1 ], - record: { - start: 0, - end: 1, - formats: [ , , , ], - text: 'two', - }, - }, - { - description: 'should filter format attributes with settings', - settings: { - removeAttribute: ( attribute ) => attribute.indexOf( 'data-mce-' ) === 0, - }, - html: 'test', + description: 'should filter format boundary attributes', + html: 'test', createRange: ( element ) => ( { startOffset: 0, startContainer: element, @@ -643,10 +574,7 @@ export const spec = [ }, }, { - description: 'should filter text with settings', - settings: { - filterString: ( string ) => string.replace( '\uFEFF', '' ), - }, + description: 'should filter zero width space', html: '', createRange: ( element ) => ( { startOffset: 0, @@ -664,10 +592,7 @@ export const spec = [ }, }, { - description: 'should filter text at end with settings', - settings: { - filterString: ( string ) => string.replace( '\uFEFF', '' ), - }, + description: 'should filter zero width space at end', html: 'test', createRange: ( element ) => ( { startOffset: 4, @@ -685,10 +610,7 @@ export const spec = [ }, }, { - description: 'should filter text in format with settings', - settings: { - filterString: ( string ) => string.replace( '\uFEFF', '' ), - }, + description: 'should filter zero width space in format', html: 'test', createRange: ( element ) => ( { startOffset: 5, @@ -706,10 +628,7 @@ export const spec = [ }, }, { - description: 'should filter text outside format with settings', - settings: { - filterString: ( string ) => string.replace( '\uFEFF', '' ), - }, + description: 'should filter zero width space outside format', html: 'test', createRange: ( element ) => ( { startOffset: 1, diff --git a/packages/rich-text/src/test/to-dom.js b/packages/rich-text/src/test/to-dom.js index 16fed3f66d0a2..1df1e8e25b229 100644 --- a/packages/rich-text/src/test/to-dom.js +++ b/packages/rich-text/src/test/to-dom.js @@ -34,7 +34,6 @@ describe( 'recordToDom', () => { value: record, multilineTag, multilineWrapperTags, - createLinePadding: ( doc ) => doc.createElement( 'br' ), } ); expect( body ).toMatchSnapshot(); expect( selection ).toEqual( { startPath, endPath } ); diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js index c811bbc618059..071f38aae2a19 100644 --- a/packages/rich-text/src/to-dom.js +++ b/packages/rich-text/src/to-dom.js @@ -113,7 +113,13 @@ function remove( node ) { return node.parentNode.removeChild( node ); } -function padEmptyLines( { element, createLinePadding, multilineWrapperTags } ) { +function createLinePadding( doc ) { + const element = doc.createElement( 'br' ); + element.setAttribute( 'data-rich-text-padding', 'true' ); + return element; +} + +function padEmptyLines( { element, multilineWrapperTags } ) { const length = element.childNodes.length; const doc = element.ownerDocument; @@ -135,7 +141,7 @@ function padEmptyLines( { element, createLinePadding, multilineWrapperTags } ) { element.insertBefore( createLinePadding( doc ), child ); } - padEmptyLines( { element: child, createLinePadding, multilineWrapperTags } ); + padEmptyLines( { element: child, multilineWrapperTags } ); } } } @@ -150,7 +156,6 @@ export function toDom( { value, multilineTag, multilineWrapperTags, - createLinePadding, prepareEditableTree, } ) { let startPath = []; @@ -180,9 +185,7 @@ export function toDom( { isEditableTree: true, } ); - if ( createLinePadding ) { - padEmptyLines( { element: tree, createLinePadding, multilineWrapperTags } ); - } + padEmptyLines( { element: tree, multilineWrapperTags } ); return { body: tree, @@ -205,7 +208,6 @@ export function apply( { current, multilineTag, multilineWrapperTags, - createLinePadding, prepareEditableTree, } ) { // Construct a new element tree in memory. @@ -213,7 +215,6 @@ export function apply( { value, multilineTag, multilineWrapperTags, - createLinePadding, prepareEditableTree, } ); diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index 5928d1b35c5a3..36b24dd5b49da 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -164,7 +164,7 @@ export function toTree( { const { type, attributes = {}, unregisteredAttributes, object } = format; if ( isEditableTree && ! object && format === deepestActiveFormat ) { - attributes[ 'data-mce-selected' ] = 'inline-boundary'; + attributes[ 'data-rich-text-format-boundary' ] = 'true'; } const parent = getParent( pointer ); From ccf61e770596a5df848c4db01d8bc6f76dcaa433 Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 30 Jan 2019 14:51:14 +0100 Subject: [PATCH 08/19] Update styling --- .../src/components/rich-text/style.scss | 63 ++++++------------- 1 file changed, 19 insertions(+), 44 deletions(-) diff --git a/packages/editor/src/components/rich-text/style.scss b/packages/editor/src/components/rich-text/style.scss index 41721d0ff11a7..b19bfebdb1fe9 100644 --- a/packages/editor/src/components/rich-text/style.scss +++ b/packages/editor/src/components/rich-text/style.scss @@ -48,54 +48,29 @@ } } - // Style TinyMCE inline boundaries on select inline text elements. - &:focus { - a, - b, - i, - strong, - em, - del, - ins, - sup, - sub { - &[data-mce-selected] { - padding: 0 2px; - margin: 0 -2px; - border-radius: 2px; - box-shadow: 0 0 0 1px $light-gray-400; - background: $light-gray-400; - - // Enforce a dark text color so active inline boundaries - // are always readable. - // See https://github.com/WordPress/gutenberg/issues/9508 - color: $dark-gray-900; - } - } - - // Link inline boundaries get special colors. - a[data-mce-selected] { - box-shadow: 0 0 0 1px $blue-medium-100; - background: $blue-medium-100; - color: $blue-medium-900; - } + *[data-rich-text-format-boundary] { + border-radius: 2px; + box-shadow: 0 0 0 1px $light-gray-400; + background: $light-gray-400; - // inline boundaries need special treatment because their - // un-selected style is already padded. - code[data-mce-selected] { - background: $light-gray-400; - box-shadow: 0 0 0 1px $light-gray-400; - } + // Enforce a dark text color so active inline boundaries + // are always readable. + // See https://github.com/WordPress/gutenberg/issues/9508 + color: $dark-gray-900; } - img { - &[data-mce-selected] { - outline: none; - } + // Link inline boundaries get special colors. + a[data-rich-text-format-boundary] { + box-shadow: 0 0 0 1px $blue-medium-100; + background: $blue-medium-100; + color: $blue-medium-900; + } - &::selection { - background: none !important; - } + // inline boundaries need special treatment because their + // un-selected style is already padded. + code[data-rich-text-format-boundary] { + background: $light-gray-400; + box-shadow: 0 0 0 1px $light-gray-400; } &[data-is-placeholder-visible="true"] { From 4365fe831d67bcdfdcf1adbf35b60a7b791dd067 Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 30 Jan 2019 14:56:28 +0100 Subject: [PATCH 09/19] valueToFormat shouldn't create editable tree --- packages/editor/src/components/rich-text/index.js | 1 + packages/rich-text/src/to-dom.js | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 543dc0f564ab6..6076a93a0aa28 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -771,6 +771,7 @@ export class RichText extends Component { value, multilineTag: this.multilineTag, multilineWrapperTags: this.multilineWrapperTags, + isEditableTree: false, } ).body.childNodes ); } diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js index 071f38aae2a19..f9625a30b3ac9 100644 --- a/packages/rich-text/src/to-dom.js +++ b/packages/rich-text/src/to-dom.js @@ -157,6 +157,7 @@ export function toDom( { multilineTag, multilineWrapperTags, prepareEditableTree, + isEditableTree = true, } ) { let startPath = []; let endPath = []; @@ -182,10 +183,12 @@ export function toDom( { onEndIndex( body, pointer ) { endPath = createPathToNode( pointer, body, [ pointer.nodeValue.length ] ); }, - isEditableTree: true, + isEditableTree, } ); - padEmptyLines( { element: tree, multilineWrapperTags } ); + if ( isEditableTree ) { + padEmptyLines( { element: tree, multilineWrapperTags } ); + } return { body: tree, From 2e789e79685dacbc2c2c404e0ee05aa8569d8c7f Mon Sep 17 00:00:00 2001 From: iseulde Date: Thu, 31 Jan 2019 16:21:57 +0100 Subject: [PATCH 10/19] Add zero with spaces around formatting --- .../src/test/__snapshots__/to-dom.js.snap | 572 ++++++++++++++++-- packages/rich-text/src/test/helpers/index.js | 70 --- packages/rich-text/src/test/to-dom.js | 4 +- packages/rich-text/src/to-tree.js | 40 +- 4 files changed, 576 insertions(+), 110 deletions(-) diff --git a/packages/rich-text/src/test/__snapshots__/to-dom.js.snap b/packages/rich-text/src/test/__snapshots__/to-dom.js.snap index 8a20d0865a0dd..ca3f711de9154 100644 --- a/packages/rich-text/src/test/__snapshots__/to-dom.js.snap +++ b/packages/rich-text/src/test/__snapshots__/to-dom.js.snap @@ -2,38 +2,81 @@ exports[`recordToDom should create a value with formatting 1`] = ` - - test + + test +  `; +exports[`recordToDom should create a value with formatting 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 5, + ], + "startPath": Array [ + 0, + 0, + 1, + ], +} +`; + exports[`recordToDom should create a value with formatting for split tags 1`] = ` - - test + + test +  `; +exports[`recordToDom should create a value with formatting for split tags 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 3, + ], + "startPath": Array [ + 0, + 0, + 1, + ], +} +`; + exports[`recordToDom should create a value with formatting with attributes 1`] = `
    - test + test +  `; +exports[`recordToDom should create a value with formatting with attributes 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 5, + ], + "startPath": Array [ + 0, + 0, + 1, + ], +} +`; + exports[`recordToDom should create a value with image object 1`] = ` `; +exports[`recordToDom should create a value with image object 2`] = ` +Object { + "endPath": Array [ + 1, + 0, + ], + "startPath": Array [ + 1, + 0, + ], +} +`; + exports[`recordToDom should create a value with image object and formatting 1`] = ` +  +  `; +exports[`recordToDom should create a value with image object and formatting 2`] = ` +Object { + "endPath": Array [ + 0, + 2, + 0, + ], + "startPath": Array [ + 0, + 2, + 0, + ], +} +`; + exports[`recordToDom should create a value with image object and text after 1`] = ` +  te +  st `; +exports[`recordToDom should create a value with image object and text after 2`] = ` +Object { + "endPath": Array [ + 2, + 2, + ], + "startPath": Array [ + 0, + 2, + 0, + ], +} +`; + exports[`recordToDom should create a value with image object and text before 1`] = ` te - st + st +  `; +exports[`recordToDom should create a value with image object and text before 2`] = ` +Object { + "endPath": Array [ + 1, + 2, + 0, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should create a value with nested formatting 1`] = ` - - test +  + + test +  +  `; +exports[`recordToDom should create a value with nested formatting 2`] = ` +Object { + "endPath": Array [ + 0, + 1, + 0, + 5, + ], + "startPath": Array [ + 0, + 1, + 0, + 1, + ], +} +`; + exports[`recordToDom should create a value without formatting 1`] = ` test `; +exports[`recordToDom should create a value without formatting 2`] = ` +Object { + "endPath": Array [ + 0, + 4, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should create an empty value 1`] = ` @@ -109,6 +244,19 @@ exports[`recordToDom should create an empty value 1`] = ` `; +exports[`recordToDom should create an empty value 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should create an empty value from empty tags 1`] = ` @@ -118,17 +266,44 @@ exports[`recordToDom should create an empty value from empty tags 1`] = ` `; +exports[`recordToDom should create an empty value from empty tags 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should filter format boundary attributes 1`] = ` - - test + + test +  `; +exports[`recordToDom should filter format boundary attributes 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 5, + ], + "startPath": Array [ + 0, + 0, + 1, + ], +} +`; + exports[`recordToDom should filter zero width space 1`] = ` @@ -138,30 +313,92 @@ exports[`recordToDom should filter zero width space 1`] = ` `; +exports[`recordToDom should filter zero width space 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should filter zero width space at end 1`] = ` test `; +exports[`recordToDom should filter zero width space at end 2`] = ` +Object { + "endPath": Array [ + 0, + 4, + ], + "startPath": Array [ + 0, + 4, + ], +} +`; + exports[`recordToDom should filter zero width space in format 1`] = ` - - test + + test +  `; +exports[`recordToDom should filter zero width space in format 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 5, + ], + "startPath": Array [ + 0, + 0, + 5, + ], +} +`; + exports[`recordToDom should filter zero width space outside format 1`] = ` - - test + + test +  `; +exports[`recordToDom should filter zero width space outside format 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 5, + ], + "startPath": Array [ + 0, + 0, + 5, + ], +} +`; + exports[`recordToDom should handle br 1`] = ` @@ -170,19 +407,46 @@ exports[`recordToDom should handle br 1`] = ` `; +exports[`recordToDom should handle br 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should handle br with formatting 1`] = ` - - + + 
    +  `; +exports[`recordToDom should handle br with formatting 2`] = ` +Object { + "endPath": Array [ + 0, + 2, + 0, + ], + "startPath": Array [ + 0, + 0, + 1, + ], +} +`; + exports[`recordToDom should handle br with text 1`] = ` te @@ -191,6 +455,19 @@ exports[`recordToDom should handle br with text 1`] = ` `; +exports[`recordToDom should handle br with text 2`] = ` +Object { + "endPath": Array [ + 2, + 0, + ], + "startPath": Array [ + 0, + 2, + ], +} +`; + exports[`recordToDom should handle double br 1`] = ` a @@ -201,6 +478,19 @@ exports[`recordToDom should handle double br 1`] = ` `; +exports[`recordToDom should handle double br 2`] = ` +Object { + "endPath": Array [ + 4, + 0, + ], + "startPath": Array [ + 2, + 0, + ], +} +`; + exports[`recordToDom should handle empty list value 1`] = `
  • @@ -212,6 +502,21 @@ exports[`recordToDom should handle empty list value 1`] = ` `; +exports[`recordToDom should handle empty list value 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + 0, + ], +} +`; + exports[`recordToDom should handle empty multiline value 1`] = `

    @@ -223,6 +528,21 @@ exports[`recordToDom should handle empty multiline value 1`] = ` `; +exports[`recordToDom should handle empty multiline value 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + 0, + ], +} +`; + exports[`recordToDom should handle middle empty list value 1`] = `

  • @@ -246,6 +566,21 @@ exports[`recordToDom should handle middle empty list value 1`] = ` `; +exports[`recordToDom should handle middle empty list value 2`] = ` +Object { + "endPath": Array [ + 1, + 0, + 0, + ], + "startPath": Array [ + 1, + 0, + 0, + ], +} +`; + exports[`recordToDom should handle multiline list value 1`] = `
  • @@ -273,6 +608,25 @@ exports[`recordToDom should handle multiline list value 1`] = ` `; +exports[`recordToDom should handle multiline list value 2`] = ` +Object { + "endPath": Array [ + 0, + 1, + 1, + 1, + 0, + 0, + 1, + ], + "startPath": Array [ + 0, + 0, + 0, + ], +} +`; + exports[`recordToDom should handle multiline value 1`] = `

    @@ -284,6 +638,21 @@ exports[`recordToDom should handle multiline value 1`] = ` `; +exports[`recordToDom should handle multiline value 2`] = ` +Object { + "endPath": Array [ + 1, + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + 1, + ], +} +`; + exports[`recordToDom should handle multiline value with element selection 1`] = `

  • @@ -292,6 +661,21 @@ exports[`recordToDom should handle multiline value with element selection 1`] = `; +exports[`recordToDom should handle multiline value with element selection 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 3, + ], + "startPath": Array [ + 0, + 0, + 3, + ], +} +`; + exports[`recordToDom should handle multiline value with empty 1`] = `

    @@ -306,6 +690,21 @@ exports[`recordToDom should handle multiline value with empty 1`] = ` `; +exports[`recordToDom should handle multiline value with empty 2`] = ` +Object { + "endPath": Array [ + 1, + 0, + 0, + ], + "startPath": Array [ + 1, + 0, + 0, + ], +} +`; + exports[`recordToDom should handle nested empty list value 1`] = `

  • @@ -324,6 +723,25 @@ exports[`recordToDom should handle nested empty list value 1`] = ` `; +exports[`recordToDom should handle nested empty list value 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 0, + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + 0, + 0, + 0, + ], +} +`; + exports[`recordToDom should handle selection before br 1`] = ` a @@ -334,45 +752,105 @@ exports[`recordToDom should handle selection before br 1`] = ` `; +exports[`recordToDom should handle selection before br 2`] = ` +Object { + "endPath": Array [ + 2, + 0, + ], + "startPath": Array [ + 2, + 0, + ], +} +`; + exports[`recordToDom should ignore formats at line separator 1`] = `

    - one + one

    - two + two - + 

    `; +exports[`recordToDom should ignore formats at line separator 2`] = ` +Object { + "endPath": Array [], + "startPath": Array [], +} +`; + exports[`recordToDom should preserve emoji 1`] = ` 🍒 `; +exports[`recordToDom should preserve emoji 2`] = ` +Object { + "endPath": Array [ + 0, + 2, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should preserve emoji in formatting 1`] = ` - - 🍒 + + 🍒 +  `; +exports[`recordToDom should preserve emoji in formatting 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + 3, + ], + "startPath": Array [ + 0, + 0, + 1, + ], +} +`; + exports[`recordToDom should preserve non breaking space 1`] = ` test  test `; +exports[`recordToDom should preserve non breaking space 2`] = ` +Object { + "endPath": Array [ + 0, + 5, + ], + "startPath": Array [ + 0, + 5, + ], +} +`; + exports[`recordToDom should remove br with settings 1`] = ` @@ -382,8 +860,34 @@ exports[`recordToDom should remove br with settings 1`] = ` `; +exports[`recordToDom should remove br with settings 2`] = ` +Object { + "endPath": Array [ + 0, + 0, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; + exports[`recordToDom should replace characters to format HTML with space 1`] = ` `; + +exports[`recordToDom should replace characters to format HTML with space 2`] = ` +Object { + "endPath": Array [ + 0, + 1, + ], + "startPath": Array [ + 0, + 0, + ], +} +`; diff --git a/packages/rich-text/src/test/helpers/index.js b/packages/rich-text/src/test/helpers/index.js index fcf3bd913ed33..5b7b702dbb230 100644 --- a/packages/rich-text/src/test/helpers/index.js +++ b/packages/rich-text/src/test/helpers/index.js @@ -19,8 +19,6 @@ export const spec = [ endOffset: 0, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 0 ], record: { start: 0, end: 0, @@ -37,8 +35,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 1 ], record: { start: 0, end: 1, @@ -55,8 +51,6 @@ export const spec = [ endOffset: 5, endContainer: element.firstChild, } ), - startPath: [ 0, 5 ], - endPath: [ 0, 5 ], record: { start: 5, end: 5, @@ -73,8 +67,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 0 ], record: { start: 0, end: 0, @@ -91,8 +83,6 @@ export const spec = [ endOffset: 4, endContainer: element.firstChild, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 4 ], record: { start: 0, end: 4, @@ -109,8 +99,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 2 ], record: { start: 0, end: 2, @@ -127,8 +115,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 0, 2 ], record: { start: 0, end: 2, @@ -145,8 +131,6 @@ export const spec = [ endOffset: 1, endContainer: element.firstChild, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 0, 4 ], record: { start: 0, end: 4, @@ -163,8 +147,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0, 0, 0 ], - endPath: [ 0, 0, 0, 4 ], record: { start: 0, end: 4, @@ -181,8 +163,6 @@ export const spec = [ endOffset: 1, endContainer: element.querySelector( 'em' ), } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 0, 2 ], record: { start: 0, end: 2, @@ -199,8 +179,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 0, 4 ], record: { start: 0, end: 4, @@ -217,8 +195,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 1, 0 ], - endPath: [ 1, 0 ], record: { start: 0, end: 0, @@ -235,8 +211,6 @@ export const spec = [ endOffset: 1, endContainer: element.querySelector( 'img' ), } ), - startPath: [ 0, 1, 0 ], - endPath: [ 0, 1, 0 ], record: { start: 0, end: 1, @@ -253,8 +227,6 @@ export const spec = [ endOffset: 2, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 1, 2, 0 ], record: { start: 0, end: 5, @@ -271,8 +243,6 @@ export const spec = [ endOffset: 2, endContainer: element, } ), - startPath: [ 0, 1, 0 ], - endPath: [ 1, 2 ], record: { start: 0, end: 5, @@ -289,8 +259,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 0 ], record: { start: 0, end: 0, @@ -307,8 +275,6 @@ export const spec = [ endOffset: 2, endContainer: element, } ), - startPath: [ 0, 2 ], - endPath: [ 2, 0 ], record: { start: 2, end: 3, @@ -325,8 +291,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 2, 0 ], record: { start: 0, end: 1, @@ -343,8 +307,6 @@ export const spec = [ endOffset: 3, endContainer: element, } ), - startPath: [ 2, 0 ], - endPath: [ 4, 0 ], record: { formats: [ , , , , ], text: 'a\n\nb', @@ -361,8 +323,6 @@ export const spec = [ endOffset: 2, endContainer: element, } ), - startPath: [ 2, 0 ], - endPath: [ 2, 0 ], record: { formats: [ , , , , ], text: 'a\n\nb', @@ -380,8 +340,6 @@ export const spec = [ endOffset: 0, endContainer: element.firstChild, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 0, 0 ], record: { start: 0, end: 0, @@ -399,8 +357,6 @@ export const spec = [ endOffset: 0, endContainer: element.lastChild, } ), - startPath: [ 0, 0, 1 ], - endPath: [ 1, 0, 0 ], record: { start: 1, end: 4, @@ -419,8 +375,6 @@ export const spec = [ endOffset: 1, endContainer: element.querySelector( 'ol > li' ).firstChild, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 1, 1, 1, 0, 0, 1 ], record: { start: 0, end: 9, @@ -439,8 +393,6 @@ export const spec = [ endOffset: 0, endContainer: element.firstChild, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 0, 0 ], record: { start: 0, end: 0, @@ -459,8 +411,6 @@ export const spec = [ endOffset: 0, endContainer: element.querySelector( 'ul > li' ), } ), - startPath: [ 0, 0, 0, 0, 0 ], - endPath: [ 0, 0, 0, 0, 0 ], record: { start: 1, end: 1, @@ -479,8 +429,6 @@ export const spec = [ endOffset: 0, endContainer: element.firstChild.nextSibling, } ), - startPath: [ 1, 0, 0 ], - endPath: [ 1, 0, 0 ], record: { start: 1, end: 1, @@ -498,8 +446,6 @@ export const spec = [ endOffset: 0, endContainer: element.lastChild, } ), - startPath: [ 1, 0, 0 ], - endPath: [ 1, 0, 0 ], record: { start: 4, end: 4, @@ -518,8 +464,6 @@ export const spec = [ endOffset: 1, endContainer: element.firstChild, } ), - startPath: [ 0, 0, 3 ], - endPath: [ 0, 0, 3 ], record: { start: 3, end: 3, @@ -530,8 +474,6 @@ export const spec = [ { description: 'should ignore formats at line separator', multilineTag: 'p', - startPath: [], - endPath: [], record: { formats: [ [ em ], [ em ], [ em ], [ em ], [ em ], [ em ], [ em ] ], text: 'one\u2028two', @@ -546,8 +488,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 0 ], record: { start: 0, end: 0, @@ -564,8 +504,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0, 0 ], - endPath: [ 0, 0, 4 ], record: { start: 0, end: 4, @@ -582,8 +520,6 @@ export const spec = [ endOffset: 1, endContainer: element, } ), - startPath: [ 0, 0 ], - endPath: [ 0, 0 ], record: { start: 0, end: 0, @@ -600,8 +536,6 @@ export const spec = [ endOffset: 4, endContainer: element.firstChild, } ), - startPath: [ 0, 4 ], - endPath: [ 0, 4 ], record: { start: 4, end: 4, @@ -618,8 +552,6 @@ export const spec = [ endOffset: 5, endContainer: element.querySelector( 'em' ).firstChild, } ), - startPath: [ 0, 0, 4 ], - endPath: [ 0, 0, 4 ], record: { start: 4, end: 4, @@ -636,8 +568,6 @@ export const spec = [ endOffset: 1, endContainer: element.lastChild, } ), - startPath: [ 0, 0, 4 ], - endPath: [ 0, 0, 4 ], record: { start: 4, end: 4, diff --git a/packages/rich-text/src/test/to-dom.js b/packages/rich-text/src/test/to-dom.js index 1df1e8e25b229..0084e780c91be 100644 --- a/packages/rich-text/src/test/to-dom.js +++ b/packages/rich-text/src/test/to-dom.js @@ -26,8 +26,6 @@ describe( 'recordToDom', () => { multilineTag, multilineWrapperTags, record, - startPath, - endPath, } ) => { it( description, () => { const { body, selection } = toDom( { @@ -36,7 +34,7 @@ describe( 'recordToDom', () => { multilineWrapperTags, } ); expect( body ).toMatchSnapshot(); - expect( selection ).toEqual( { startPath, endPath } ); + expect( selection ).toMatchSnapshot(); } ); } ); } ); diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index 36b24dd5b49da..cc8fb0755e4c0 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -48,7 +48,7 @@ function getDeepestActiveFormat( { formats, start } ) { return; } - const formatsAtStart = formats[ start ]; + const formatsAtStart = formats[ start - 1 ]; if ( ! formatsAtStart ) { return; @@ -146,6 +146,25 @@ export function toTree( { } } + if ( isEditableTree && lastCharacterFormats ) { + let boundaryPointer = pointer; + + lastCharacterFormats.forEach( ( format, formatIndex ) => { + boundaryPointer = getLastChild( boundaryPointer ); + + if ( + characterFormats && + format === characterFormats[ formatIndex ] + ) { + return; + } + + if ( ! format.object && character !== LINE_SEPARATOR ) { + append( getParent( getParent( boundaryPointer ) ), ZERO_WIDTH_NO_BREAK_SPACE ); + } + } ); + } + if ( characterFormats ) { characterFormats.forEach( ( format, formatIndex ) => { if ( @@ -163,7 +182,12 @@ export function toTree( { const { type, attributes = {}, unregisteredAttributes, object } = format; - if ( isEditableTree && ! object && format === deepestActiveFormat ) { + if ( + isEditableTree && + ! object && + character !== LINE_SEPARATOR && + format === deepestActiveFormat + ) { attributes[ 'data-rich-text-format-boundary' ] = 'true'; } @@ -179,7 +203,17 @@ export function toTree( { remove( pointer ); } - pointer = append( object ? parent : newNode, '' ); + if ( isEditableTree ) { + if ( object ) { + pointer = append( parent, '' ); + } else if ( character === LINE_SEPARATOR ) { + pointer = append( newNode, '' ); + } else { + pointer = append( newNode, ZERO_WIDTH_NO_BREAK_SPACE ); + } + } else { + pointer = append( object ? parent : newNode, '' ); + } } ); } From 3d0e224ecb53ab73f866360417a7dc1ab30f5817 Mon Sep 17 00:00:00 2001 From: iseulde Date: Thu, 31 Jan 2019 16:40:13 +0100 Subject: [PATCH 11/19] Remove old boundary compatibility code --- packages/rich-text/src/to-dom.js | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js index f9625a30b3ac9..a6b00e421a1ff 100644 --- a/packages/rich-text/src/to-dom.js +++ b/packages/rich-text/src/to-dom.js @@ -9,7 +9,7 @@ import { createElement } from './create-element'; * Browser dependencies */ -const { TEXT_NODE, ELEMENT_NODE } = window.Node; +const { TEXT_NODE } = window.Node; /** * Creates a path as an array of indices from the given root node to the given @@ -273,34 +273,11 @@ function isRangeEqual( a, b ) { export function applySelection( selection, current ) { const { node: startContainer, offset: startOffset } = getNodeByPath( current, selection.startPath ); const { node: endContainer, offset: endOffset } = getNodeByPath( current, selection.endPath ); - const windowSelection = window.getSelection(); const range = current.ownerDocument.createRange(); - const collapsed = startContainer === endContainer && startOffset === endOffset; - - if ( - collapsed && - startOffset === 0 && - startContainer.previousSibling && - startContainer.previousSibling.nodeType === ELEMENT_NODE && - startContainer.previousSibling.nodeName !== 'BR' - ) { - startContainer.insertData( 0, '\uFEFF' ); - range.setStart( startContainer, 1 ); - range.setEnd( endContainer, 1 ); - } else if ( - collapsed && - startOffset === 0 && - startContainer === TEXT_NODE && - startContainer.nodeValue.length === 0 - ) { - startContainer.insertData( 0, '\uFEFF' ); - range.setStart( startContainer, 1 ); - range.setEnd( endContainer, 1 ); - } else { - range.setStart( startContainer, startOffset ); - range.setEnd( endContainer, endOffset ); - } + + range.setStart( startContainer, startOffset ); + range.setEnd( endContainer, endOffset ); if ( windowSelection.rangeCount > 0 ) { // If the to be added range and the live range are the same, there's no From 32ff15f6eb2883111f2c68db50073f86867f56af Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 1 Feb 2019 14:45:08 +0100 Subject: [PATCH 12/19] Update dom diff, prevent browser selection overwrite on boundary --- .../editor/src/components/rich-text/index.js | 20 +++++++++-- packages/rich-text/src/to-dom.js | 36 +++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 6076a93a0aa28..67d1259733f27 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -166,13 +166,14 @@ export class RichText extends Component { } ); } - applyRecord( record ) { + applyRecord( record, domOnly ) { apply( { value: record, current: this.editableRef, multilineTag: this.multilineTag, multilineWrapperTags: this.multilineWrapperTags, prepareEditableTree: this.props.prepareEditableTree, + domOnly, } ); } @@ -380,7 +381,22 @@ export class RichText extends Component { } this.setState( { start, end } ); - this.applyRecord( value ); + + const selection = getSelection(); + const range = selection.getRangeAt( 0 ); + + // Prevent the browser selection from being overwritten if at a zero + // width space. + if ( + range.collapsed && + range.startContainer.nodeType === window.Node.TEXT_NODE && + range.startOffset === 1 && + range.startContainer.data[ 0 ] === '\ufeff' + ) { + this.applyRecord( value, true ); + } else { + this.applyRecord( value ); + } } } diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js index a6b00e421a1ff..c68489c895028 100644 --- a/packages/rich-text/src/to-dom.js +++ b/packages/rich-text/src/to-dom.js @@ -212,6 +212,7 @@ export function apply( { multilineTag, multilineWrapperTags, prepareEditableTree, + domOnly, } ) { // Construct a new element tree in memory. const { body, selection } = toDom( { @@ -223,7 +224,7 @@ export function apply( { applyValue( body, current ); - if ( value.start !== undefined ) { + if ( value.start !== undefined && ! domOnly ) { applySelection( selection, current ); } } @@ -238,7 +239,38 @@ export function applyValue( future, current ) { if ( ! currentChild ) { current.appendChild( futureChild ); } else if ( ! currentChild.isEqualNode( futureChild ) ) { - current.replaceChild( futureChild, currentChild ); + if ( + currentChild.nodeName !== futureChild.nodeName || + ( currentChild.nodeType === TEXT_NODE && currentChild.data !== futureChild.data ) + ) { + current.replaceChild( futureChild, currentChild ); + } else { + const currentAttributes = currentChild.attributes; + const futureAttributes = futureChild.attributes; + + if ( currentAttributes ) { + for ( let ii = 0; ii < currentAttributes.length; ii++ ) { + const { name } = currentAttributes[ ii ]; + + if ( ! futureChild.getAttribute( name ) ) { + currentChild.removeAttribute( name ); + } + } + } + + if ( futureAttributes ) { + for ( let ii = 0; ii < futureAttributes.length; ii++ ) { + const { name, value } = futureAttributes[ ii ]; + + if ( currentChild.getAttribute( name ) !== value ) { + currentChild.setAttribute( name, value ); + } + } + } + + applyValue( futureChild, currentChild ); + future.removeChild( futureChild ); + } } else { future.removeChild( futureChild ); } From b6c7d2581b2ac5299a68dd4932b43617392c21c4 Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 1 Feb 2019 19:10:32 +0100 Subject: [PATCH 13/19] Update UI --- .../editor/src/components/rich-text/index.js | 15 ++++---- packages/rich-text/src/create.js | 36 ++++++++++++++----- .../src/test/__snapshots__/to-dom.js.snap | 29 ++++++++++----- packages/rich-text/src/test/helpers/index.js | 3 ++ packages/rich-text/src/to-tree.js | 19 +++++++--- 5 files changed, 74 insertions(+), 28 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 67d1259733f27..7e61363f43f10 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -149,9 +149,9 @@ export class RichText extends Component { */ getRecord() { const { formats, text } = this.formatToValue( this.props.value ); - const { start, end } = this.state; + const { start, end, selectedFormat } = this.state; - return { formats, text, start, end }; + return { formats, text, start, end, selectedFormat }; } createRecord() { @@ -370,7 +370,7 @@ export class RichText extends Component { */ onSelectionChange() { const value = this.createRecord(); - const { start, end, formats } = value; + const { start, end, formats, selectedFormat } = value; if ( start !== this.state.start || end !== this.state.end ) { const isCaretWithinFormattedText = this.props.isCaretWithinFormattedText; @@ -380,7 +380,7 @@ export class RichText extends Component { this.props.onExitFormattedText(); } - this.setState( { start, end } ); + this.setState( { start, end, selectedFormat } ); const selection = getSelection(); const range = selection.getRangeAt( 0 ); @@ -397,6 +397,9 @@ export class RichText extends Component { } else { this.applyRecord( value ); } + } else if ( selectedFormat !== this.state.selectedFormat ) { + this.setState( { start, end, selectedFormat } ); + this.applyRecord( value, true ); } } @@ -424,13 +427,13 @@ export class RichText extends Component { onChange( record, { withoutHistory } = {} ) { this.applyRecord( record ); - const { start, end } = record; + const { start, end, selectedFormat } = record; this.onChangeEditableValue( record ); this.savedContent = this.valueToFormat( record ); this.props.onChange( this.savedContent ); - this.setState( { start, end } ); + this.setState( { start, end, selectedFormat } ); if ( ! withoutHistory ) { this.onCreateUndoLevel(); diff --git a/packages/rich-text/src/create.js b/packages/rich-text/src/create.js index 6a74012979e78..b81684a973217 100644 --- a/packages/rich-text/src/create.js +++ b/packages/rich-text/src/create.js @@ -144,12 +144,13 @@ export function create( { * Helper to accumulate the value's selection start and end from the current * node and range. * - * @param {Object} accumulator Object to accumulate into. - * @param {Node} node Node to create value with. - * @param {Range} range Range to create value with. - * @param {Object} value Value that is being accumulated. + * @param {Object} accumulator Object to accumulate into. + * @param {Node} node Node to create value with. + * @param {Range} range Range to create value with. + * @param {Object} value Value that is being accumulated. + * @param {boolean} depth Depth of the format. */ -function accumulateSelection( accumulator, node, range, value ) { +function accumulateSelection( accumulator, node, range, value, depth ) { if ( ! range ) { return; } @@ -161,9 +162,24 @@ function accumulateSelection( accumulator, node, range, value ) { // Selection can be extracted from value. if ( value.start !== undefined ) { accumulator.start = currentLength + value.start; + + if ( value.selectedFormat !== undefined ) { + accumulator.selectedFormat = value.selectedFormat; + } // Range indicates that the current node has selection. } else if ( node === startContainer && node.nodeType === TEXT_NODE ) { accumulator.start = currentLength + startOffset; + + if ( + startOffset === 0 && + node.data[ startOffset ] === ZERO_WIDTH_NO_BREAK_SPACE + ) { + accumulator.selectedFormat = depth; + } + + if ( filterString( node.data ).length === startOffset ) { + accumulator.selectedFormat = depth; + } // Range indicates that the current node is selected. } else if ( parentNode === startContainer && @@ -261,6 +277,7 @@ function createFromElement( { multilineTag, multilineWrapperTags, currentWrapperTags = [], + depth = 0, } ) { const accumulator = createEmptyValue(); @@ -283,7 +300,7 @@ function createFromElement( { if ( node.nodeType === TEXT_NODE ) { const text = filterString( node.nodeValue ); range = filterRange( node, range, filterString ); - accumulateSelection( accumulator, node, range, { text } ); + accumulateSelection( accumulator, node, range, { text }, depth ); accumulator.text += text; // Create a sparse array of the same length as `text`, in which // formats can be added. @@ -296,12 +313,12 @@ function createFromElement( { } if ( node.getAttribute( 'data-rich-text-padding' ) ) { - accumulateSelection( accumulator, node, range, createEmptyValue() ); + accumulateSelection( accumulator, node, range, createEmptyValue(), depth ); continue; } if ( type === 'br' ) { - accumulateSelection( accumulator, node, range, createEmptyValue() ); + accumulateSelection( accumulator, node, range, createEmptyValue(), depth ); accumulator.text += '\n'; accumulator.formats.length += 1; continue; @@ -342,13 +359,14 @@ function createFromElement( { range, multilineTag, multilineWrapperTags, + depth: depth + 1, } ); } const text = value.text; const start = accumulator.text.length; - accumulateSelection( accumulator, node, range, value ); + accumulateSelection( accumulator, node, range, value, depth ); // Don't apply the element as formatting if it has no content. if ( isEmpty( value ) && format && ! format.attributes ) { diff --git a/packages/rich-text/src/test/__snapshots__/to-dom.js.snap b/packages/rich-text/src/test/__snapshots__/to-dom.js.snap index ca3f711de9154..3a09944707dda 100644 --- a/packages/rich-text/src/test/__snapshots__/to-dom.js.snap +++ b/packages/rich-text/src/test/__snapshots__/to-dom.js.snap @@ -2,7 +2,9 @@ exports[`recordToDom should create a value with formatting 1`] = ` - + test  @@ -27,7 +29,9 @@ Object { exports[`recordToDom should create a value with formatting for split tags 1`] = ` - + test  @@ -53,6 +57,7 @@ Object { exports[`recordToDom should create a value with formatting with attributes 1`] = ` test @@ -189,7 +194,9 @@ exports[`recordToDom should create a value with nested formatting 1`] = `  - + test  @@ -281,7 +288,9 @@ Object { exports[`recordToDom should filter format boundary attributes 1`] = ` - + test  @@ -374,9 +383,7 @@ Object { exports[`recordToDom should filter zero width space outside format 1`] = ` - + test  @@ -422,7 +429,9 @@ Object { exports[`recordToDom should handle br with formatting 1`] = ` - + 
    @@ -809,7 +818,9 @@ Object { exports[`recordToDom should preserve emoji in formatting 1`] = ` - + 🍒  diff --git a/packages/rich-text/src/test/helpers/index.js b/packages/rich-text/src/test/helpers/index.js index 5b7b702dbb230..c586538dea66c 100644 --- a/packages/rich-text/src/test/helpers/index.js +++ b/packages/rich-text/src/test/helpers/index.js @@ -539,6 +539,7 @@ export const spec = [ record: { start: 4, end: 4, + selectedFormat: 0, formats: [ , , , , ], text: 'test', }, @@ -555,6 +556,7 @@ export const spec = [ record: { start: 4, end: 4, + selectedFormat: 1, formats: [ [ em ], [ em ], [ em ], [ em ] ], text: 'test', }, @@ -571,6 +573,7 @@ export const spec = [ record: { start: 4, end: 4, + selectedFormat: 0, formats: [ [ em ], [ em ], [ em ], [ em ] ], text: 'test', }, diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index cc8fb0755e4c0..149fcb89f45d2 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -43,18 +43,29 @@ function fromFormat( { type, attributes, unregisteredAttributes, object } ) { }; } -function getDeepestActiveFormat( { formats, start } ) { +function getDeepestActiveFormat( { formats, start, selectedFormat } ) { if ( start === undefined ) { return; } - const formatsAtStart = formats[ start - 1 ]; + const formatsAtStart = formats[ start ] || []; + const formatsAtBeforeStart = formats[ start - 1 ] || []; - if ( ! formatsAtStart ) { + let f = formatsAtStart; + + if ( formatsAtBeforeStart.length > formatsAtStart.length ) { + f = formatsAtBeforeStart; + } + + if ( ! f.length ) { return; } - return formatsAtStart[ formatsAtStart.length - 1 ]; + if ( selectedFormat === undefined ) { + return f[ formatsAtStart.length - 1 ]; + } + + return f[ selectedFormat - 1 ]; } export function toTree( { From 2e3850e80219ed1493618bf2a42785c40f379e47 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 4 Feb 2019 13:42:47 +0100 Subject: [PATCH 14/19] Fix inserting format with shortcut --- packages/editor/src/components/rich-text/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 7e61363f43f10..e85e0c72b55bb 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -397,7 +397,10 @@ export class RichText extends Component { } else { this.applyRecord( value ); } - } else if ( selectedFormat !== this.state.selectedFormat ) { + } else if ( + this.state.selectedFormat !== undefined && + selectedFormat !== this.state.selectedFormat + ) { this.setState( { start, end, selectedFormat } ); this.applyRecord( value, true ); } From 5ad78e024afacff6443a211955d567abe2472c01 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 4 Feb 2019 14:00:13 +0100 Subject: [PATCH 15/19] Add check --- packages/rich-text/src/to-tree.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index 149fcb89f45d2..9a2aa9caf01f1 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -163,6 +163,10 @@ export function toTree( { lastCharacterFormats.forEach( ( format, formatIndex ) => { boundaryPointer = getLastChild( boundaryPointer ); + if ( ! boundaryPointer ) { + return; + } + if ( characterFormats && format === characterFormats[ formatIndex ] From 1a88924a8f835aefa724ddab57a27e26a7a6347b Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 4 Feb 2019 14:09:13 +0100 Subject: [PATCH 16/19] Remove nbsp inserted by TinyMCE from test --- packages/e2e-tests/specs/__snapshots__/undo.test.js.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/__snapshots__/undo.test.js.snap b/packages/e2e-tests/specs/__snapshots__/undo.test.js.snap index 49a8c19fe89fa..799e5c2c0879c 100644 --- a/packages/e2e-tests/specs/__snapshots__/undo.test.js.snap +++ b/packages/e2e-tests/specs/__snapshots__/undo.test.js.snap @@ -28,7 +28,7 @@ exports[`undo should undo typing after a pause 2`] = ` exports[`undo should undo typing after non input change 1`] = ` " -

    before keyboard after keyboard

    +

    before keyboard after keyboard

    " `; From 421876c36d67b596d8a894f8d760f7a3112f4353 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 4 Feb 2019 14:48:39 +0100 Subject: [PATCH 17/19] Do not mutate attirbutes object --- packages/rich-text/src/to-tree.js | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index 9a2aa9caf01f1..613c8a7bef2cf 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -9,14 +9,24 @@ import { ZERO_WIDTH_NO_BREAK_SPACE, } from './special-characters'; -function fromFormat( { type, attributes, unregisteredAttributes, object } ) { +function fromFormat( { type, attributes, unregisteredAttributes, object, boundaryClass } ) { const formatType = getFormatType( type ); + let elementAttributes = {}; + + if ( boundaryClass ) { + elementAttributes[ 'data-rich-text-format-boundary' ] = 'true'; + } + if ( ! formatType ) { - return { type, attributes, object }; + if ( attributes ) { + elementAttributes = { ...attributes, ...elementAttributes }; + } + + return { type, attributes: elementAttributes, object }; } - const elementAttributes = { ...unregisteredAttributes }; + elementAttributes = { ...unregisteredAttributes, ...elementAttributes }; for ( const name in attributes ) { const key = formatType.attributes ? formatType.attributes[ name ] : false; @@ -195,16 +205,14 @@ export function toTree( { return; } - const { type, attributes = {}, unregisteredAttributes, object } = format; + const { type, attributes, unregisteredAttributes, object } = format; - if ( + const boundaryClass = ( isEditableTree && ! object && character !== LINE_SEPARATOR && format === deepestActiveFormat - ) { - attributes[ 'data-rich-text-format-boundary' ] = 'true'; - } + ); const parent = getParent( pointer ); const newNode = append( parent, fromFormat( { @@ -212,6 +220,7 @@ export function toTree( { attributes, unregisteredAttributes, object, + boundaryClass, } ) ); if ( isText( pointer ) && getText( pointer ).length === 0 ) { From d2b0c7e0603aba3dc94682e19970f61c3f0b5a04 Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 4 Feb 2019 15:14:25 +0100 Subject: [PATCH 18/19] Fix link e2e tests --- packages/e2e-tests/specs/links.test.js | 4 ++-- packages/rich-text/src/get-active-format.js | 23 +++++++++++++++++-- .../rich-text/src/test/get-active-format.js | 2 +- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/e2e-tests/specs/links.test.js b/packages/e2e-tests/specs/links.test.js index 85ba99badcf24..7b0302b9400a1 100644 --- a/packages/e2e-tests/specs/links.test.js +++ b/packages/e2e-tests/specs/links.test.js @@ -6,7 +6,6 @@ import { getEditedPostContent, createNewPost, pressKeyWithModifier, - pressKeyTimes, insertBlock, } from '@wordpress/e2e-test-utils'; @@ -244,7 +243,8 @@ describe( 'Links', () => { it( 'can be edited with collapsed selection', async () => { await createAndReselectLink(); // Make a collapsed selection inside the link - await pressKeyTimes( 'ArrowRight', 3 ); + await page.keyboard.press( 'ArrowLeft' ); + await page.keyboard.press( 'ArrowRight' ); await moveMouse(); await page.click( 'button[aria-label="Edit"]' ); await waitForAutoFocus(); diff --git a/packages/rich-text/src/get-active-format.js b/packages/rich-text/src/get-active-format.js index 090a9f1d8ccbb..c11ec9bb42d02 100644 --- a/packages/rich-text/src/get-active-format.js +++ b/packages/rich-text/src/get-active-format.js @@ -15,10 +15,29 @@ import { find } from 'lodash'; * * @return {?Object} Active format object of the specified type, or undefined. */ -export function getActiveFormat( { formats, start }, formatType ) { +export function getActiveFormat( { formats, start, selectedFormat }, formatType ) { if ( start === undefined ) { return; } - return find( formats[ start ], { type: formatType } ); + const formatsAtStart = formats[ start ] || []; + const formatsAtBeforeStart = formats[ start - 1 ] || []; + + let f = formatsAtStart; + + if ( formatsAtBeforeStart.length > formatsAtStart.length ) { + f = formatsAtBeforeStart; + } + + if ( ! f.length ) { + return; + } + + f = f.slice( 0, selectedFormat ); + + if ( ! f.length ) { + return; + } + + return find( f, { type: formatType } ); } diff --git a/packages/rich-text/src/test/get-active-format.js b/packages/rich-text/src/test/get-active-format.js index bce7808040990..c4d992ccc7a18 100644 --- a/packages/rich-text/src/test/get-active-format.js +++ b/packages/rich-text/src/test/get-active-format.js @@ -26,6 +26,6 @@ describe( 'getActiveFormat', () => { end: 1, }; - expect( getActiveFormat( record, 'em' ) ).toBe( undefined ); + expect( getActiveFormat( record, 'em' ) ).toBe( em ); } ); } ); From 31968f78da8f62e085758089e3e0ae772bde393f Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 5 Feb 2019 11:21:13 +0100 Subject: [PATCH 19/19] Update isHorizontalEdge to handle empty text nodes --- packages/dom/src/dom.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/dom/src/dom.js b/packages/dom/src/dom.js index 7bfe0d101ddb9..a51a29cc8b973 100644 --- a/packages/dom/src/dom.js +++ b/packages/dom/src/dom.js @@ -130,14 +130,21 @@ export function isHorizontalEdge( container, isReverse ) { // If confirmed to be at extent, traverse up through DOM, verifying that // the node is at first or last child for reverse or forward respectively. // Continue until container is reached. - const order = isReverse ? 'first' : 'last'; + const order = isReverse ? 'previous' : 'next'; + while ( node !== container ) { - const parentNode = node.parentNode; - if ( parentNode[ `${ order }Child` ] !== node ) { + let next = node[ `${ order }Sibling` ]; + + // Skip over empty text nodes. + while ( next && next.nodeType === TEXT_NODE && next.data === '' ) { + next = next[ `${ order }Sibling` ]; + } + + if ( next ) { return false; } - node = parentNode; + node = node.parentNode; } // If reached, range is assumed to be at edge.