From cf196aa91522adc1d99038d0c0ebed46342b68b8 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 6 Apr 2018 12:13:40 +0100 Subject: [PATCH 1/8] RichText: Add a format prop to allow HTML string values to be used in RichText components --- blocks/rich-text/README.md | 14 ++- blocks/rich-text/format.js | 85 ++++++++++++++ blocks/rich-text/index.js | 83 +++++++------- .../test/__snapshots__/format.js.snap | 35 ++++++ .../test/__snapshots__/index.js.snap | 11 -- blocks/rich-text/test/format.js | 104 ++++++++++++++++++ blocks/rich-text/test/index.js | 63 ----------- blocks/rich-text/tinymce.js | 5 +- 8 files changed, 276 insertions(+), 124 deletions(-) create mode 100644 blocks/rich-text/format.js create mode 100644 blocks/rich-text/test/__snapshots__/format.js.snap delete mode 100644 blocks/rich-text/test/__snapshots__/index.js.snap create mode 100644 blocks/rich-text/test/format.js diff --git a/blocks/rich-text/README.md b/blocks/rich-text/README.md index 74db6896146a39..02f95ada8403b7 100644 --- a/blocks/rich-text/README.md +++ b/blocks/rich-text/README.md @@ -10,11 +10,17 @@ a traditional `input` field, usually when the user exits the field. ## Properties -### `value: Array` +### `format: String` -*Required.* Array of React DOM to make editable. The rendered HTML should be valid, and valid with respect to the `tagName` and `inline` property. +*Optional.* Format of the RichText provided value prop. It can be `element` or `string`. -### `onChange( value: Array ): Function` +*Default: `element`*. + +### `value: Array|String` + +*Required.* Depending on the format prop, this value could be an array of React DOM to make editable or an HTML string. The rendered HTML should be valid, and valid with respect to the `tagName` and `inline` property. + +### `onChange( value: Array|String ): Function` *Required.* Called when the value changes. @@ -31,7 +37,7 @@ a traditional `input` field, usually when the user exits the field. *Optional.* By default, a line break will be inserted on Enter. If the editable field can contain multiple paragraphs, this property can be set to `p` to create new paragraphs on Enter. -### `onSplit( before: Array, after: Array, ...blocks: Object ): Function` +### `onSplit( before: Array|String, after: Array|String, ...blocks: Object ): Function` *Optional.* Called when the content can be split with `before` and `after`. There might be blocks present, which should be inserted in between. diff --git a/blocks/rich-text/format.js b/blocks/rich-text/format.js new file mode 100644 index 00000000000000..f861c4a8be762d --- /dev/null +++ b/blocks/rich-text/format.js @@ -0,0 +1,85 @@ +/** + * External dependencies + */ +import { omitBy, map } from 'lodash'; +import { nodeListToReact } from 'dom-react'; + +/** + * WordPress dependencies + */ +import { createElement, renderToString } from '@wordpress/element'; + +/** + * Transforms a WP Element to its corresponding HTML string. + * + * @param {WPElement} value Element. + * + * @return {string} HTML. + */ +export function elementToString( value ) { + return renderToString( value ); +} + +/** + * Transforms a string HTML to its corresponding WP element. + * + * @param {string} value HTML. + * + * @return {WPElement} Element. + */ +export function stringToElement( value ) { + if ( ! value ) { + return []; + } + const domElement = document.createElement( 'div' ); + domElement.innerHTML = value; + + return domToElement( domElement.childNodes ); +} + +/** + * Strips out TinyMCE specific attributes and nodes from a WPElement + * + * @param {string} type Element type + * @param {Object} props Element Props + * @param {Array} children Element Children + * + * @return {Element} WPElement. + */ +export function createTinyMCEElement( type, props, ...children ) { + if ( props[ 'data-mce-bogus' ] === 'all' ) { + return null; + } + + if ( props.hasOwnProperty( 'data-mce-bogus' ) ) { + return children; + } + + return createElement( + type, + omitBy( props, ( _, key ) => key.indexOf( 'data-mce-' ) === 0 ), + ...children + ); +} + +/** + * Transforms an array of DOM Elements to their corresponding WP element. + * + * @param {Array} value DOM Elements. + * + * @return {WPElement} WP Element. + */ +export function domToElement( value ) { + return nodeListToReact( value || [], createTinyMCEElement ); +} + +/** + * Transforms an array of DOM Elements to their corresponding HTML string output. + * + * @param {Array} value DOM Elements. + * + * @return {string} HTML. + */ +export function domToString( value ) { + return map( value, element => element.outerHTML ).join( '' ); +} diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index e4867d969a82f7..0f7d06d52ff0ae 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -5,7 +5,6 @@ import classnames from 'classnames'; import { last, isEqual, - omitBy, forEach, merge, identity, @@ -14,13 +13,12 @@ import { noop, reject, } from 'lodash'; -import { nodeListToReact } from 'dom-react'; import 'element-closest'; /** * WordPress dependencies */ -import { createElement, Component, renderToString, Fragment, compose } from '@wordpress/element'; +import { Component, Fragment, compose } from '@wordpress/element'; import { keycodes, createBlobURL, isHorizontalEdge, getRectangleFromRange, getScrollContainer } from '@wordpress/utils'; import { withSafeTimeout, Slot } from '@wordpress/components'; import { withSelect } from '@wordpress/data'; @@ -38,25 +36,10 @@ import { pickAriaProps } from './aria'; import patterns from './patterns'; import { EVENTS } from './constants'; import { withBlockEditContext } from '../block-edit/context'; +import { domToElement, domToString, elementToString } from './format'; const { BACKSPACE, DELETE, ENTER } = keycodes; -export function createTinyMCEElement( type, props, ...children ) { - if ( props[ 'data-mce-bogus' ] === 'all' ) { - return null; - } - - if ( props.hasOwnProperty( 'data-mce-bogus' ) ) { - return children; - } - - return createElement( - type, - omitBy( props, ( value, key ) => key.indexOf( 'data-mce-' ) === 0 ), - ...children - ); -} - /** * Returns true if the node is the inline node boundary. This is used in node * filtering prevent the inline boundary from being included in the split which @@ -115,21 +98,9 @@ export function getFormatProperties( formatName, parents ) { const DEFAULT_FORMATS = [ 'bold', 'italic', 'strikethrough', 'link' ]; export class RichText extends Component { - constructor( props ) { + constructor( { value } ) { super( ...arguments ); - const { value } = props; - if ( 'production' !== process.env.NODE_ENV && undefined !== value && - ! Array.isArray( value ) ) { - // eslint-disable-next-line no-console - console.error( - `Invalid value of type ${ typeof value } passed to RichText ` + - '(expected array). Attribute values should be sourced using ' + - 'the `children` source when used with RichText.\n\n' + - 'See: https://wordpress.org/gutenberg/handbook/block-api/attributes/#children' - ); - } - this.onInit = this.onInit.bind( this ); this.getSettings = this.getSettings.bind( this ); this.onSetup = this.onSetup.bind( this ); @@ -400,7 +371,7 @@ export class RichText extends Component { onChange() { this.isEmpty = this.editor.dom.isEmpty( this.editor.getBody() ); - this.savedContent = this.isEmpty ? [] : this.getContent(); + this.savedContent = this.getContent(); this.props.onChange( this.savedContent ); } @@ -503,10 +474,12 @@ export class RichText extends Component { const index = dom.nodeIndex( selectedNode ); const beforeNodes = childNodes.slice( 0, index ); const afterNodes = childNodes.slice( index + 1 ); - const beforeElement = nodeListToReact( beforeNodes, createTinyMCEElement ); - const afterElement = nodeListToReact( afterNodes, createTinyMCEElement ); - this.restoreContentAndSplit( beforeElement, afterElement ); + const { format } = this.props; + const before = format === 'string' ? domToString( beforeNodes ) : domToElement( beforeNodes ); + const after = format === 'string' ? domToString( afterNodes ) : domToElement( afterNodes ); + + this.restoreContentAndSplit( before, after ); } else { event.preventDefault(); this.onCreateUndoLevel(); @@ -597,10 +570,12 @@ export class RichText extends Component { const beforeFragment = beforeRange.extractContents(); const afterFragment = afterRange.extractContents(); - const beforeElement = nodeListToReact( beforeFragment.childNodes, createTinyMCEElement ); - const afterElement = nodeListToReact( filterEmptyNodes( afterFragment.childNodes ), createTinyMCEElement ); + const { format } = this.props; + const before = format === 'string' ? domToString( beforeFragment.childNodes ) : domToElement( beforeFragment.childNodes ); + const filteredAfterFragment = filterEmptyNodes( afterFragment.childNodes ); + const after = format === 'string' ? domToString( filteredAfterFragment ) : domToElement( filteredAfterFragment ); - this.restoreContentAndSplit( beforeElement, afterElement, blocks ); + this.restoreContentAndSplit( before, after, blocks ); } else { this.restoreContentAndSplit( [], [], blocks ); } @@ -648,9 +623,10 @@ export class RichText extends Component { // Splitting into two blocks this.setContent( this.props.value ); + const { format } = this.props; this.restoreContentAndSplit( - nodeListToReact( before, createTinyMCEElement ), - nodeListToReact( after, createTinyMCEElement ) + format === 'string' ? domToString( before ) : domToElement( before ), + format === 'string' ? domToString( after ) : domToElement( after ) ); } @@ -702,12 +678,28 @@ export class RichText extends Component { } ); } - setContent( content = '' ) { - this.editor.setContent( renderToString( content ) ); + setContent( content ) { + const { format } = this.props; + switch ( format ) { + case 'string': + this.editor.setContent( content || '' ); + break; + default: + this.editor.setContent( elementToString( content ) ); + } } getContent() { - return nodeListToReact( this.editor.getBody().childNodes || [], createTinyMCEElement ); + const { format } = this.props; + + switch ( format ) { + case 'string': + return this.editor.getContent(); + default: + return this.editor.dom.isEmpty( this.editor.getBody() ) ? + [] : + domToElement( this.editor.getBody().childNodes || [] ); + } } componentDidUpdate( prevProps ) { @@ -806,6 +798,7 @@ export class RichText extends Component { isSelected, formatters, autocompleters, + format, } = this.props; const ariaProps = { ...pickAriaProps( this.props ), 'aria-multiline': !! MultilineTag }; @@ -849,6 +842,7 @@ export class RichText extends Component { onSetup={ this.onSetup } style={ style } defaultValue={ value } + format={ format } isPlaceholderVisible={ isPlaceholderVisible } aria-label={ placeholder } aria-autocomplete="list" @@ -886,6 +880,7 @@ RichText.contextTypes = { RichText.defaultProps = { formattingControls: DEFAULT_FORMATS, formatters: [], + format: 'element', }; export default compose( [ diff --git a/blocks/rich-text/test/__snapshots__/format.js.snap b/blocks/rich-text/test/__snapshots__/format.js.snap new file mode 100644 index 00000000000000..8997e0fe5c0e1b --- /dev/null +++ b/blocks/rich-text/test/__snapshots__/format.js.snap @@ -0,0 +1,35 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`createTinyMCEElement should render a TinyMCE element 1`] = ` +

+

+ Child +

+

+`; + +exports[`domToElement should return the corresponding element 1`] = ` +Array [ +
+ + content + +
, +] +`; + +exports[`stringToElement should return the corresponding element 1`] = ` +Array [ +
+ + content + +
, +] +`; diff --git a/blocks/rich-text/test/__snapshots__/index.js.snap b/blocks/rich-text/test/__snapshots__/index.js.snap deleted file mode 100644 index 55995e3ad8730c..00000000000000 --- a/blocks/rich-text/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,11 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`createTinyMCEElement should render a TinyMCE element 1`] = ` -

-

- Child -

-

-`; diff --git a/blocks/rich-text/test/format.js b/blocks/rich-text/test/format.js new file mode 100644 index 00000000000000..2bdb905a2896e7 --- /dev/null +++ b/blocks/rich-text/test/format.js @@ -0,0 +1,104 @@ +/** + * External dependencies + */ +import { shallow } from 'enzyme'; + +/** + * WordPress dependencies + */ +import { createElement } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { + createTinyMCEElement, + elementToString, + stringToElement, + domToElement, + domToString, +} from '../format'; + +describe( 'createTinyMCEElement', () => { + const type = 'p'; + const children =

Child

; + + test( 'should return null', () => { + const props = { + 'data-mce-bogus': 'all', + }; + + expect( createTinyMCEElement( type, props, children ) ).toBeNull(); + } ); + + test( 'should return children', () => { + const props = { + 'data-mce-bogus': '', + }; + + const wrapper = createTinyMCEElement( type, props, children ); + expect( wrapper ).toEqual( [ children ] ); + } ); + + test( 'should render a TinyMCE element', () => { + const props = { + 'a-prop': 'hi', + }; + + const wrapper = shallow( createTinyMCEElement( type, props, children ) ); + expect( wrapper ).toMatchSnapshot(); + } ); +} ); + +describe( 'elementToString', () => { + test( 'should return an empty string for null element', () => { + expect( elementToString( null ) ).toBe( '' ); + } ); + + test( 'should return an empty string for an empty array', () => { + expect( elementToString( [] ) ).toBe( '' ); + } ); + + test( 'should return the HTML content ', () => { + const element = createElement( 'div', { className: 'container' }, + createElement( 'strong', {}, 'content' ) + ); + expect( elementToString( element ) ).toBe( '
content
' ); + } ); +} ); + +describe( 'stringToElement', () => { + test( 'should return an empty array', () => { + expect( stringToElement( '' ) ).toEqual( [] ); + } ); + + test( 'should return the corresponding element ', () => { + expect( stringToElement( '
content
' ) ).toMatchSnapshot(); + } ); +} ); + +describe( 'domToElement', () => { + test( 'should return an empty array', () => { + expect( domToElement( [] ) ).toEqual( [] ); + } ); + + test( 'should return the corresponding element ', () => { + const domElement = document.createElement( 'div' ); + domElement.innerHTML = '
content
'; + expect( domToElement( domElement.childNodes ) ).toMatchSnapshot(); + } ); +} ); + +describe( 'domToString', () => { + test( 'should return an empty string', () => { + expect( domToString( [] ) ).toEqual( '' ); + } ); + + test( 'should return the HTML ', () => { + const domElement = document.createElement( 'div' ); + const content = '
content
'; + domElement.innerHTML = content; + expect( domToString( domElement.childNodes ) ).toBe( content ); + } ); +} ); + diff --git a/blocks/rich-text/test/index.js b/blocks/rich-text/test/index.js index f1fe726cf973ab..f34c7b5fd9e8e0 100644 --- a/blocks/rich-text/test/index.js +++ b/blocks/rich-text/test/index.js @@ -8,7 +8,6 @@ import { shallow } from 'enzyme'; */ import { RichText, - createTinyMCEElement, isEmptyInlineBoundary, isEmptyNode, filterEmptyNodes, @@ -16,37 +15,6 @@ import { } from '../'; import { diffAriaProps, pickAriaProps } from '../aria'; -describe( 'createTinyMCEElement', () => { - const type = 'p'; - const children =

Child

; - - test( 'should return null', () => { - const props = { - 'data-mce-bogus': 'all', - }; - - expect( createTinyMCEElement( type, props, children ) ).toBeNull(); - } ); - - test( 'should return children', () => { - const props = { - 'data-mce-bogus': '', - }; - - const wrapper = createTinyMCEElement( type, props, children ); - expect( wrapper ).toEqual( [ children ] ); - } ); - - test( 'should render a TinyMCE element', () => { - const props = { - 'a-prop': 'hi', - }; - - const wrapper = shallow( createTinyMCEElement( type, props, children ) ); - expect( wrapper ).toMatchSnapshot(); - } ); -} ); - describe( 'isEmptyInlineBoundary', () => { describe( 'link', () => { const node = document.createElement( 'a' ); @@ -234,37 +202,6 @@ describe( 'RichText', () => { } ); } ); - describe( '.propTypes', () => { - /* eslint-disable no-console */ - let consoleError; - beforeEach( () => { - consoleError = console.error; - console.error = jest.fn(); - } ); - - afterEach( () => { - console.error = consoleError; - } ); - - it( 'should warn when rendered with string value', () => { - shallow( ); - - expect( console.error ).toHaveBeenCalled(); - } ); - - it( 'should not warn when rendered with undefined value', () => { - shallow( ); - - expect( console.error ).not.toHaveBeenCalled(); - } ); - - it( 'should not warn when rendered with array value', () => { - shallow( ); - - expect( console.error ).not.toHaveBeenCalled(); - } ); - /* eslint-enable no-console */ - } ); describe( 'pickAriaProps()', () => { it( 'should should filter all properties to only those begining with "aria-"', () => { expect( pickAriaProps( { diff --git a/blocks/rich-text/tinymce.js b/blocks/rich-text/tinymce.js index 6dbf1499071dd3..f5572de516e74f 100644 --- a/blocks/rich-text/tinymce.js +++ b/blocks/rich-text/tinymce.js @@ -14,6 +14,7 @@ import { Component, Children, createElement } from '@wordpress/element'; * Internal dependencies */ import { diffAriaProps, pickAriaProps } from './aria'; +import { stringToElement } from './format'; const IS_PLACEHOLDER_VISIBLE_ATTR_NAME = 'data-is-placeholder-visible'; export default class TinyMCE extends Component { @@ -96,7 +97,7 @@ export default class TinyMCE extends Component { } render() { - const { tagName = 'div', style, defaultValue, className, isPlaceholderVisible } = this.props; + const { tagName = 'div', style, defaultValue, className, isPlaceholderVisible, format } = this.props; const ariaProps = pickAriaProps( this.props ); if ( [ 'ul', 'ol', 'table' ].indexOf( tagName ) === -1 ) { ariaProps.role = 'textbox'; @@ -107,7 +108,7 @@ export default class TinyMCE extends Component { // us to show and focus the content before it's truly ready to edit. let children; if ( defaultValue ) { - children = Children.toArray( defaultValue ); + children = format === 'string' ? stringToElement( defaultValue ) : Children.toArray( defaultValue ); } return createElement( tagName, { From d85b71803f6da8998eae81e139a1a1fbb44ab68e Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 11 Apr 2018 09:17:46 +0100 Subject: [PATCH 2/8] Don't align params and returns docs --- blocks/rich-text/format.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/blocks/rich-text/format.js b/blocks/rich-text/format.js index f861c4a8be762d..ae403afff3f53d 100644 --- a/blocks/rich-text/format.js +++ b/blocks/rich-text/format.js @@ -14,7 +14,7 @@ import { createElement, renderToString } from '@wordpress/element'; * * @param {WPElement} value Element. * - * @return {string} HTML. + * @return {string} HTML. */ export function elementToString( value ) { return renderToString( value ); @@ -23,9 +23,9 @@ export function elementToString( value ) { /** * Transforms a string HTML to its corresponding WP element. * - * @param {string} value HTML. + * @param {string} value HTML. * - * @return {WPElement} Element. + * @return {WPElement} Element. */ export function stringToElement( value ) { if ( ! value ) { @@ -44,7 +44,7 @@ export function stringToElement( value ) { * @param {Object} props Element Props * @param {Array} children Element Children * - * @return {Element} WPElement. + * @return {Element} WPElement. */ export function createTinyMCEElement( type, props, ...children ) { if ( props[ 'data-mce-bogus' ] === 'all' ) { @@ -65,9 +65,9 @@ export function createTinyMCEElement( type, props, ...children ) { /** * Transforms an array of DOM Elements to their corresponding WP element. * - * @param {Array} value DOM Elements. + * @param {Array} value DOM Elements. * - * @return {WPElement} WP Element. + * @return {WPElement} WP Element. */ export function domToElement( value ) { return nodeListToReact( value || [], createTinyMCEElement ); @@ -76,9 +76,9 @@ export function domToElement( value ) { /** * Transforms an array of DOM Elements to their corresponding HTML string output. * - * @param {Array} value DOM Elements. + * @param {Array} value DOM Elements. * - * @return {string} HTML. + * @return {string} HTML. */ export function domToString( value ) { return map( value, element => element.outerHTML ).join( '' ); From a3f4726a3f45de4eaa78309a21bff3c623bcb238 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 11 Apr 2018 09:27:13 +0100 Subject: [PATCH 3/8] Add domToFormat function to factorize ternaries --- blocks/rich-text/format.js | 17 +++++++++++++++++ blocks/rich-text/index.js | 17 ++++++++--------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/blocks/rich-text/format.js b/blocks/rich-text/format.js index ae403afff3f53d..190f6f4c73a17f 100644 --- a/blocks/rich-text/format.js +++ b/blocks/rich-text/format.js @@ -83,3 +83,20 @@ export function domToElement( value ) { export function domToString( value ) { return map( value, element => element.outerHTML ).join( '' ); } + +/** + * Transforms an array of DOM Elements to the given format. + * + * @param {Array} value DOM Elements. + * @param {string} format Output format (string or element) + * + * @return {*} Output. + */ +export function domToFormat( value, format ) { + switch ( format ) { + case 'string': + return domToString( value ); + default: + return domToElement( value ); + } +} diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index 0f7d06d52ff0ae..9b5d8e4307faa7 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -36,7 +36,7 @@ import { pickAriaProps } from './aria'; import patterns from './patterns'; import { EVENTS } from './constants'; import { withBlockEditContext } from '../block-edit/context'; -import { domToElement, domToString, elementToString } from './format'; +import { domToString, elementToString } from './format'; const { BACKSPACE, DELETE, ENTER } = keycodes; @@ -476,8 +476,8 @@ export class RichText extends Component { const afterNodes = childNodes.slice( index + 1 ); const { format } = this.props; - const before = format === 'string' ? domToString( beforeNodes ) : domToElement( beforeNodes ); - const after = format === 'string' ? domToString( afterNodes ) : domToElement( afterNodes ); + const before = domToFormat( beforeNodes, format ); + const after = domToFormat( afterNodes, format ); this.restoreContentAndSplit( before, after ); } else { @@ -571,9 +571,8 @@ export class RichText extends Component { const afterFragment = afterRange.extractContents(); const { format } = this.props; - const before = format === 'string' ? domToString( beforeFragment.childNodes ) : domToElement( beforeFragment.childNodes ); - const filteredAfterFragment = filterEmptyNodes( afterFragment.childNodes ); - const after = format === 'string' ? domToString( filteredAfterFragment ) : domToElement( filteredAfterFragment ); + const before = domToFormat( beforeFragment.childNodes, format ); + const after = domToFormat( filterEmptyNodes( afterFragment.childNodes ), format ); this.restoreContentAndSplit( before, after, blocks ); } else { @@ -625,8 +624,8 @@ export class RichText extends Component { const { format } = this.props; this.restoreContentAndSplit( - format === 'string' ? domToString( before ) : domToElement( before ), - format === 'string' ? domToString( after ) : domToElement( after ) + domToFormat( before, format ), + domToFormat( after, format ) ); } @@ -698,7 +697,7 @@ export class RichText extends Component { default: return this.editor.dom.isEmpty( this.editor.getBody() ) ? [] : - domToElement( this.editor.getBody().childNodes || [] ); + domToFormat( this.editor.getBody().childNodes || [], 'element' ); } } From 0bec1082c97b0b4fd258ae6c7d932cffc788ae8d Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 11 Apr 2018 09:32:54 +0100 Subject: [PATCH 4/8] Remove useless stringToElement --- blocks/rich-text/format.js | 17 ----------------- .../rich-text/test/__snapshots__/format.js.snap | 12 ------------ blocks/rich-text/test/format.js | 11 ----------- blocks/rich-text/tinymce.js | 12 +++++++----- 4 files changed, 7 insertions(+), 45 deletions(-) diff --git a/blocks/rich-text/format.js b/blocks/rich-text/format.js index 190f6f4c73a17f..95e0d72f24a5b4 100644 --- a/blocks/rich-text/format.js +++ b/blocks/rich-text/format.js @@ -20,23 +20,6 @@ export function elementToString( value ) { return renderToString( value ); } -/** - * Transforms a string HTML to its corresponding WP element. - * - * @param {string} value HTML. - * - * @return {WPElement} Element. - */ -export function stringToElement( value ) { - if ( ! value ) { - return []; - } - const domElement = document.createElement( 'div' ); - domElement.innerHTML = value; - - return domToElement( domElement.childNodes ); -} - /** * Strips out TinyMCE specific attributes and nodes from a WPElement * diff --git a/blocks/rich-text/test/__snapshots__/format.js.snap b/blocks/rich-text/test/__snapshots__/format.js.snap index 8997e0fe5c0e1b..43973d9dc188bc 100644 --- a/blocks/rich-text/test/__snapshots__/format.js.snap +++ b/blocks/rich-text/test/__snapshots__/format.js.snap @@ -21,15 +21,3 @@ Array [ , ] `; - -exports[`stringToElement should return the corresponding element 1`] = ` -Array [ -
- - content - -
, -] -`; diff --git a/blocks/rich-text/test/format.js b/blocks/rich-text/test/format.js index 2bdb905a2896e7..30e28e162a59b9 100644 --- a/blocks/rich-text/test/format.js +++ b/blocks/rich-text/test/format.js @@ -14,7 +14,6 @@ import { createElement } from '@wordpress/element'; import { createTinyMCEElement, elementToString, - stringToElement, domToElement, domToString, } from '../format'; @@ -67,16 +66,6 @@ describe( 'elementToString', () => { } ); } ); -describe( 'stringToElement', () => { - test( 'should return an empty array', () => { - expect( stringToElement( '' ) ).toEqual( [] ); - } ); - - test( 'should return the corresponding element ', () => { - expect( stringToElement( '
content
' ) ).toMatchSnapshot(); - } ); -} ); - describe( 'domToElement', () => { test( 'should return an empty array', () => { expect( domToElement( [] ) ).toEqual( [] ); diff --git a/blocks/rich-text/tinymce.js b/blocks/rich-text/tinymce.js index f5572de516e74f..845d4816e927e2 100644 --- a/blocks/rich-text/tinymce.js +++ b/blocks/rich-text/tinymce.js @@ -14,7 +14,6 @@ import { Component, Children, createElement } from '@wordpress/element'; * Internal dependencies */ import { diffAriaProps, pickAriaProps } from './aria'; -import { stringToElement } from './format'; const IS_PLACEHOLDER_VISIBLE_ATTR_NAME = 'data-is-placeholder-visible'; export default class TinyMCE extends Component { @@ -106,9 +105,11 @@ export default class TinyMCE extends Component { // 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. - let children; - if ( defaultValue ) { - children = format === 'string' ? stringToElement( defaultValue ) : Children.toArray( defaultValue ); + let extraProps = {}; + if ( defaultValue && format === 'string' ) { + extraProps = { dangerouslySetInnerHTML: { __html: defaultValue } }; + } else if ( defaultValue ) { + extraProps = { children: Children.toArray( defaultValue ) }; } return createElement( tagName, { @@ -119,6 +120,7 @@ export default class TinyMCE extends Component { ref: ( node ) => this.editorNode = node, style, suppressContentEditableWarning: true, - }, children ); + ...extraProps, + } ); } } From dd9e0992fe03ce9a5769a5d45545b228dbbcb2da Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 11 Apr 2018 09:38:12 +0100 Subject: [PATCH 5/8] Add valueToString to avoid ternaries when converting to strings --- blocks/rich-text/format.js | 17 +++++++++++++++++ blocks/rich-text/index.js | 10 ++-------- blocks/rich-text/tinymce.js | 11 +++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/blocks/rich-text/format.js b/blocks/rich-text/format.js index 95e0d72f24a5b4..ae63550268ccdf 100644 --- a/blocks/rich-text/format.js +++ b/blocks/rich-text/format.js @@ -20,6 +20,23 @@ export function elementToString( value ) { return renderToString( value ); } +/** + * Transforms a value in a given format into string. + * + * @param {Array|string?} value DOM Elements. + * @param {string} format Output format (string or element) + * + * @return {string} HTML output as string. + */ +export function valueToString( value, format ) { + switch ( format ) { + case 'string': + return value || ''; + default: + return elementToString( value ); + } +} + /** * Strips out TinyMCE specific attributes and nodes from a WPElement * diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index 9b5d8e4307faa7..aaf4d02e4fb499 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -36,7 +36,7 @@ import { pickAriaProps } from './aria'; import patterns from './patterns'; import { EVENTS } from './constants'; import { withBlockEditContext } from '../block-edit/context'; -import { domToString, elementToString } from './format'; +import { domToFormat, valueToString } from './format'; const { BACKSPACE, DELETE, ENTER } = keycodes; @@ -679,13 +679,7 @@ export class RichText extends Component { setContent( content ) { const { format } = this.props; - switch ( format ) { - case 'string': - this.editor.setContent( content || '' ); - break; - default: - this.editor.setContent( elementToString( content ) ); - } + this.editor.setContent( valueToString( content, format ) ); } getContent() { diff --git a/blocks/rich-text/tinymce.js b/blocks/rich-text/tinymce.js index 845d4816e927e2..986040ea8006e6 100644 --- a/blocks/rich-text/tinymce.js +++ b/blocks/rich-text/tinymce.js @@ -8,12 +8,13 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { Component, Children, createElement } from '@wordpress/element'; +import { Component, createElement } from '@wordpress/element'; /** * Internal dependencies */ import { diffAriaProps, pickAriaProps } from './aria'; +import { valueToString } from './format'; const IS_PLACEHOLDER_VISIBLE_ATTR_NAME = 'data-is-placeholder-visible'; export default class TinyMCE extends Component { @@ -105,12 +106,6 @@ export default class TinyMCE extends Component { // 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. - let extraProps = {}; - if ( defaultValue && format === 'string' ) { - extraProps = { dangerouslySetInnerHTML: { __html: defaultValue } }; - } else if ( defaultValue ) { - extraProps = { children: Children.toArray( defaultValue ) }; - } return createElement( tagName, { ...ariaProps, @@ -120,7 +115,7 @@ export default class TinyMCE extends Component { ref: ( node ) => this.editorNode = node, style, suppressContentEditableWarning: true, - ...extraProps, + dangerouslySetInnerHTML: { __html: valueToString( defaultValue, format ) }, } ); } } From 2963b65c30cab44da7844e9a21f32c94199a7c66 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 13 Apr 2018 14:54:20 +0100 Subject: [PATCH 6/8] Avoid double isEmpty check --- blocks/rich-text/format.js | 17 +++++++++++++++++ blocks/rich-text/index.js | 12 ++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/blocks/rich-text/format.js b/blocks/rich-text/format.js index ae63550268ccdf..08e0a7c14f089b 100644 --- a/blocks/rich-text/format.js +++ b/blocks/rich-text/format.js @@ -100,3 +100,20 @@ export function domToFormat( value, format ) { return domToElement( value ); } } + +/** + * Checks whether the value is empty or not + * + * @param {Array|string} value Value. + * @param {string} format Format (string or element) + * + * @return {boolean} Is value empty. + */ +export function isEmpty( value, format ) { + switch ( format ) { + case 'string': + return value === ''; + default: + return ! value.length; + } +} diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index aaf4d02e4fb499..8ab31ef5090018 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -36,7 +36,7 @@ import { pickAriaProps } from './aria'; import patterns from './patterns'; import { EVENTS } from './constants'; import { withBlockEditContext } from '../block-edit/context'; -import { domToFormat, valueToString } from './format'; +import { domToFormat, valueToString, isEmpty } from './format'; const { BACKSPACE, DELETE, ENTER } = keycodes; @@ -264,7 +264,7 @@ export class RichText extends Component { if ( item && ! HTML ) { const blob = item.getAsFile ? item.getAsFile() : item; const rootNode = this.editor.getBody(); - const isEmpty = this.editor.dom.isEmpty( rootNode ); + const isEmptyEditor = this.editor.dom.isEmpty( rootNode ); const content = rawHandler( { HTML: ``, mode: 'BLOCKS', @@ -274,7 +274,7 @@ export class RichText extends Component { // Allows us to ask for this information when we get a report. window.console.log( 'Received item:\n\n', blob ); - if ( isEmpty && this.props.onReplace ) { + if ( isEmptyEditor && this.props.onReplace ) { // Necessary to allow the paste bin to be removed without errors. this.props.setTimeout( () => this.props.onReplace( content ) ); } else if ( this.props.onSplit ) { @@ -328,11 +328,11 @@ export class RichText extends Component { } const rootNode = this.editor.getBody(); - const isEmpty = this.editor.dom.isEmpty( rootNode ); + const isEmptyEditor = this.editor.dom.isEmpty( rootNode ); let mode = 'INLINE'; - if ( isEmpty && this.props.onReplace ) { + if ( isEmptyEditor && this.props.onReplace ) { mode = 'BLOCKS'; } else if ( this.props.onSplit ) { mode = 'AUTO'; @@ -370,8 +370,8 @@ export class RichText extends Component { */ onChange() { - this.isEmpty = this.editor.dom.isEmpty( this.editor.getBody() ); this.savedContent = this.getContent(); + this.isEmpty = isEmpty( this.savedContent, this.props.format ); this.props.onChange( this.savedContent ); } From 665c59e7e5227d6a5921eb023f0761a3079f338e Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 13 Apr 2018 14:57:54 +0100 Subject: [PATCH 7/8] Use valid HTML in unit tests --- blocks/rich-text/test/__snapshots__/format.js.snap | 6 +++--- blocks/rich-text/test/format.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/blocks/rich-text/test/__snapshots__/format.js.snap b/blocks/rich-text/test/__snapshots__/format.js.snap index 43973d9dc188bc..731d9765c970d5 100644 --- a/blocks/rich-text/test/__snapshots__/format.js.snap +++ b/blocks/rich-text/test/__snapshots__/format.js.snap @@ -1,13 +1,13 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`createTinyMCEElement should render a TinyMCE element 1`] = ` -

Child

-

+ `; exports[`domToElement should return the corresponding element 1`] = ` diff --git a/blocks/rich-text/test/format.js b/blocks/rich-text/test/format.js index 30e28e162a59b9..1a42aa9e39f051 100644 --- a/blocks/rich-text/test/format.js +++ b/blocks/rich-text/test/format.js @@ -19,7 +19,7 @@ import { } from '../format'; describe( 'createTinyMCEElement', () => { - const type = 'p'; + const type = 'div'; const children =

Child

; test( 'should return null', () => { @@ -41,7 +41,7 @@ describe( 'createTinyMCEElement', () => { test( 'should render a TinyMCE element', () => { const props = { - 'a-prop': 'hi', + 'data-prop': 'hi', }; const wrapper = shallow( createTinyMCEElement( type, props, children ) ); From db265218feaffc8aef3e2e4bda6b29fedb19b7bd Mon Sep 17 00:00:00 2001 From: iseulde Date: Mon, 16 Apr 2018 15:45:06 +0200 Subject: [PATCH 8/8] Fix splitting --- blocks/rich-text/format.js | 20 ++++++++++++++------ blocks/rich-text/index.js | 14 +++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/blocks/rich-text/format.js b/blocks/rich-text/format.js index 08e0a7c14f089b..595bff937aa8d7 100644 --- a/blocks/rich-text/format.js +++ b/blocks/rich-text/format.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { omitBy, map } from 'lodash'; +import { omitBy } from 'lodash'; import { nodeListToReact } from 'dom-react'; /** @@ -76,12 +76,19 @@ export function domToElement( value ) { /** * Transforms an array of DOM Elements to their corresponding HTML string output. * - * @param {Array} value DOM Elements. + * @param {Array} value DOM Elements. + * @param {Editor} editor TinyMCE editor instance. * * @return {string} HTML. */ -export function domToString( value ) { - return map( value, element => element.outerHTML ).join( '' ); +export function domToString( value, editor ) { + const doc = document.implementation.createHTMLDocument( '' ); + + Array.from( value ).forEach( ( child ) => { + doc.body.appendChild( child ); + } ); + + return editor ? editor.serializer.serialize( doc.body ) : doc.body.innerHTML; } /** @@ -89,13 +96,14 @@ export function domToString( value ) { * * @param {Array} value DOM Elements. * @param {string} format Output format (string or element) + * @param {Editor} editor TinyMCE editor instance. * * @return {*} Output. */ -export function domToFormat( value, format ) { +export function domToFormat( value, format, editor ) { switch ( format ) { case 'string': - return domToString( value ); + return domToString( value, editor ); default: return domToElement( value ); } diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index 8ab31ef5090018..9f3d982dc16787 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -476,8 +476,8 @@ export class RichText extends Component { const afterNodes = childNodes.slice( index + 1 ); const { format } = this.props; - const before = domToFormat( beforeNodes, format ); - const after = domToFormat( afterNodes, format ); + const before = domToFormat( beforeNodes, format, this.editor ); + const after = domToFormat( afterNodes, format, this.editor ); this.restoreContentAndSplit( before, after ); } else { @@ -571,8 +571,8 @@ export class RichText extends Component { const afterFragment = afterRange.extractContents(); const { format } = this.props; - const before = domToFormat( beforeFragment.childNodes, format ); - const after = domToFormat( filterEmptyNodes( afterFragment.childNodes ), format ); + const before = domToFormat( beforeFragment.childNodes, format, this.editor ); + const after = domToFormat( filterEmptyNodes( afterFragment.childNodes ), format, this.editor ); this.restoreContentAndSplit( before, after, blocks ); } else { @@ -624,8 +624,8 @@ export class RichText extends Component { const { format } = this.props; this.restoreContentAndSplit( - domToFormat( before, format ), - domToFormat( after, format ) + domToFormat( before, format, this.editor ), + domToFormat( after, format, this.editor ) ); } @@ -691,7 +691,7 @@ export class RichText extends Component { default: return this.editor.dom.isEmpty( this.editor.getBody() ) ? [] : - domToFormat( this.editor.getBody().childNodes || [], 'element' ); + domToFormat( this.editor.getBody().childNodes || [], 'element', this.editor ); } }