From 8ff7d19206e4b805782f387a314972d278144a32 Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 6 Nov 2018 16:57:19 +0100 Subject: [PATCH] Fix format parsing --- packages/format-library/src/bold/index.js | 5 +- packages/format-library/src/code/index.js | 5 +- packages/format-library/src/image/index.js | 5 +- packages/format-library/src/italic/index.js | 5 +- packages/format-library/src/link/index.js | 5 +- .../format-library/src/strikethrough/index.js | 5 +- packages/rich-text/src/create.js | 26 ++-- .../rich-text/src/register-format-type.js | 49 ++++++++ packages/rich-text/src/store/selectors.js | 34 ++++++ packages/rich-text/src/test/create.js | 26 +++- packages/rich-text/src/test/helpers/index.js | 77 ++++++++++++ .../src/test/register-format-type.js | 112 ++++++++++++++++++ packages/rich-text/src/test/to-html-string.js | 25 ++++ packages/rich-text/src/to-tree.js | 26 ++-- 14 files changed, 366 insertions(+), 39 deletions(-) create mode 100644 packages/rich-text/src/test/register-format-type.js diff --git a/packages/format-library/src/bold/index.js b/packages/format-library/src/bold/index.js index 72c4133f4591d2..dd236edf66798d 100644 --- a/packages/format-library/src/bold/index.js +++ b/packages/format-library/src/bold/index.js @@ -11,9 +11,8 @@ const name = 'core/bold'; export const bold = { name, title: __( 'Bold' ), - match: { - tagName: 'strong', - }, + tagName: 'strong', + className: null, edit( { isActive, value, onChange } ) { const onToggle = () => onChange( toggleFormat( value, { type: name } ) ); diff --git a/packages/format-library/src/code/index.js b/packages/format-library/src/code/index.js index a0e5fb2f891e07..d5e0b2705f245b 100644 --- a/packages/format-library/src/code/index.js +++ b/packages/format-library/src/code/index.js @@ -10,9 +10,8 @@ const name = 'core/code'; export const code = { name, title: __( 'Code' ), - match: { - tagName: 'code', - }, + tagName: 'code', + className: null, edit( { value, onChange } ) { const onToggle = () => onChange( toggleFormat( value, { type: name } ) ); diff --git a/packages/format-library/src/image/index.js b/packages/format-library/src/image/index.js index 01016cf7ac8ff3..7deae883d3b3b4 100644 --- a/packages/format-library/src/image/index.js +++ b/packages/format-library/src/image/index.js @@ -16,9 +16,8 @@ export const image = { title: __( 'Image' ), keywords: [ __( 'photo' ), __( 'media' ) ], object: true, - match: { - tagName: 'img', - }, + tagName: 'img', + className: null, attributes: { className: 'class', style: 'style', diff --git a/packages/format-library/src/italic/index.js b/packages/format-library/src/italic/index.js index 6b8bd50dd4240f..3677c6e82acc9a 100644 --- a/packages/format-library/src/italic/index.js +++ b/packages/format-library/src/italic/index.js @@ -11,9 +11,8 @@ const name = 'core/italic'; export const italic = { name, title: __( 'Italic' ), - match: { - tagName: 'em', - }, + tagName: 'em', + className: null, edit( { isActive, value, onChange } ) { const onToggle = () => onChange( toggleFormat( value, { type: name } ) ); diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index a5ab91c076bbbf..a3ffc2c6b27f09 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -23,9 +23,8 @@ const name = 'core/link'; export const link = { name, title: __( 'Link' ), - match: { - tagName: 'a', - }, + tagName: 'a', + className: null, attributes: { url: 'href', target: 'target', diff --git a/packages/format-library/src/strikethrough/index.js b/packages/format-library/src/strikethrough/index.js index 3d3310434a21ab..eba22e7a64f329 100644 --- a/packages/format-library/src/strikethrough/index.js +++ b/packages/format-library/src/strikethrough/index.js @@ -11,9 +11,8 @@ const name = 'core/strikethrough'; export const strikethrough = { name, title: __( 'Strikethrough' ), - match: { - tagName: 'del', - }, + tagName: 'del', + className: null, edit( { isActive, value, onChange } ) { const onToggle = () => onChange( toggleFormat( value, { type: name } ) ); diff --git a/packages/rich-text/src/create.js b/packages/rich-text/src/create.js index 39028ddc2ff8d8..f41483e0615e2a 100644 --- a/packages/rich-text/src/create.js +++ b/packages/rich-text/src/create.js @@ -1,8 +1,7 @@ /** - * External dependencies + * WordPress dependencies */ - -import { find } from 'lodash'; +import { select } from '@wordpress/data'; /** * Internal dependencies @@ -11,7 +10,6 @@ import { find } from 'lodash'; import { isEmpty } from './is-empty'; import { isFormatEqual } from './is-format-equal'; import { createElement } from './create-element'; -import { getFormatTypes } from './get-format-types'; import { LINE_SEPARATOR, OBJECT_REPLACEMENT_CHARACTER, @@ -36,9 +34,23 @@ function simpleFindKey( object, value ) { } function toFormat( { type, attributes } ) { - const formatType = find( getFormatTypes(), ( { match } ) => - type === match.tagName - ); + let formatType; + + if ( attributes && attributes.class ) { + formatType = select( 'core/rich-text' ).getFormatTypeForClassName( attributes.class ); + + if ( formatType ) { + attributes.class = ` ${ attributes.class } `.replace( ` ${ formatType.className } `, ' ' ).trim(); + + if ( ! attributes.class ) { + delete attributes.class; + } + } + } + + if ( ! formatType ) { + formatType = select( 'core/rich-text' ).getFormatTypeForBareElement( type ); + } if ( ! formatType ) { return attributes ? { type, attributes } : { type }; diff --git a/packages/rich-text/src/register-format-type.js b/packages/rich-text/src/register-format-type.js index 1760f8023ddf8f..1b475bf5d89d95 100644 --- a/packages/rich-text/src/register-format-type.js +++ b/packages/rich-text/src/register-format-type.js @@ -52,6 +52,55 @@ export function registerFormatType( name, settings ) { return; } + if ( + typeof settings.tagName !== 'string' || + settings.tagName === '' + ) { + window.console.error( + 'Format tag names must be a string.' + ); + return; + } + + if ( + ( typeof settings.className !== 'string' || settings.className === '' ) && + settings.className !== null + ) { + window.console.error( + 'Format class names must be a string, or null to handle bare elements.' + ); + return; + } + + if ( ! /^[_a-zA-Z]+[a-zA-Z0-9-]*$/.test( settings.className ) ) { + window.console.error( + 'A class name must begin with a letter, followed by any number of hyphens, letters, or numbers.' + ); + return; + } + + if ( settings.className === null ) { + const formatTypeForBareElement = select( 'core/rich-text' ) + .getFormatTypeForBareElement( settings.tagName ); + + if ( formatTypeForBareElement ) { + window.console.error( + `Format "${ formatTypeForBareElement.name }" is already registered to handle bare tag name "${ settings.tagName }".` + ); + return; + } + } else { + const formatTypeForClassName = select( 'core/rich-text' ) + .getFormatTypeForClassName( settings.className ); + + if ( formatTypeForClassName ) { + window.console.error( + `Format "${ formatTypeForClassName.name }" is already registered to handle class name "${ settings.className }".` + ); + return; + } + } + if ( ! ( 'title' in settings ) || settings.title === '' ) { window.console.error( 'The format "' + settings.name + '" must have a title.' diff --git a/packages/rich-text/src/store/selectors.js b/packages/rich-text/src/store/selectors.js index c5a28baf0abc22..518bb7d90800a5 100644 --- a/packages/rich-text/src/store/selectors.js +++ b/packages/rich-text/src/store/selectors.js @@ -2,6 +2,7 @@ * External dependencies */ import createSelector from 'rememo'; +import { find } from 'lodash'; /** * Returns all the available format types. @@ -28,3 +29,36 @@ export const getFormatTypes = createSelector( export function getFormatType( state, name ) { return state.formatTypes[ name ]; } + +/** + * Gets the format type, if any, that can handle a bare element (without a + * data-format-type attribute), given the tag name of this element. + * + * @param {Object} state Data state. + * @param {string} bareElementTagName The tag name of the element to find a + * format type for. + * @return {?Object} Format type. + */ +export function getFormatTypeForBareElement( state, bareElementTagName ) { + return find( getFormatTypes( state ), ( { tagName } ) => { + return bareElementTagName === tagName; + } ); +} + +/** + * Gets the format type, if any, that can handle an element, given its classes. + * + * @param {Object} state Data state. + * @param {string} elementClassName The classes of the element to find a format + * type for. + * @return {?Object} Format type. + */ +export function getFormatTypeForClassName( state, elementClassName ) { + return find( getFormatTypes( state ), ( { className } ) => { + if ( className === null ) { + return false; + } + + return ` ${ elementClassName } `.indexOf( ` ${ className } ` ) >= 0; + } ); +} diff --git a/packages/rich-text/src/test/create.js b/packages/rich-text/src/test/create.js index 79d11f23134f5e..d9ad8138c44523 100644 --- a/packages/rich-text/src/test/create.js +++ b/packages/rich-text/src/test/create.js @@ -9,7 +9,9 @@ import { JSDOM } from 'jsdom'; */ import { create } from '../create'; import { createElement } from '../create-element'; -import { getSparseArrayLength, spec } from './helpers'; +import { registerFormatType } from '../register-format-type'; +import { unregisterFormatType } from '../unregister-format-type'; +import { getSparseArrayLength, spec, specWithRegistration } from './helpers'; const { window } = new JSDOM(); const { document } = window; @@ -54,6 +56,28 @@ describe( 'create', () => { } ); } ); + specWithRegistration.forEach( ( { + description, + formatName, + formatType, + html, + value, + } ) => { + it( description, () => { + if ( formatName ) { + registerFormatType( formatName, formatType ); + } + + const result = create( { html } ); + + if ( formatName ) { + unregisterFormatType( formatName ); + } + + expect( result ).toEqual( value ); + } ); + } ); + it( 'should reference formats', () => { const value = create( { html: 'test' } ); diff --git a/packages/rich-text/src/test/helpers/index.js b/packages/rich-text/src/test/helpers/index.js index a220d4ac705610..0b4a7b6e64b704 100644 --- a/packages/rich-text/src/test/helpers/index.js +++ b/packages/rich-text/src/test/helpers/index.js @@ -709,3 +709,80 @@ export const spec = [ }, }, ]; + +export const specWithRegistration = [ + { + description: 'should create format by matching the class', + formatName: 'my-plugin/link', + formatType: { + title: 'Custom Link', + tagName: 'a', + className: 'custom-format', + edit() {}, + }, + html: 'a', + value: { + formats: [ [ { + type: 'my-plugin/link', + attributes: {}, + unregisteredAttributes: {}, + } ] ], + text: 'a', + }, + }, + { + description: 'should retain class names', + formatName: 'my-plugin/link', + formatType: { + title: 'Custom Link', + tagName: 'a', + className: 'custom-format', + edit() {}, + }, + html: 'a', + value: { + formats: [ [ { + type: 'my-plugin/link', + attributes: {}, + unregisteredAttributes: { + class: 'test', + }, + } ] ], + text: 'a', + }, + }, + { + description: 'should create base format', + formatName: 'core/link', + formatType: { + title: 'Link', + tagName: 'a', + className: null, + edit() {}, + }, + html: 'a', + value: { + formats: [ [ { + type: 'core/link', + attributes: {}, + unregisteredAttributes: { + class: 'custom-format', + }, + } ] ], + text: 'a', + }, + }, + { + description: 'should create fallback format', + html: 'a', + value: { + formats: [ [ { + type: 'a', + attributes: { + class: 'custom-format', + }, + } ] ], + text: 'a', + }, + }, +]; diff --git a/packages/rich-text/src/test/register-format-type.js b/packages/rich-text/src/test/register-format-type.js new file mode 100644 index 00000000000000..5dadffb050abf6 --- /dev/null +++ b/packages/rich-text/src/test/register-format-type.js @@ -0,0 +1,112 @@ +/** + * WordPress dependencies + */ +import { select } from '@wordpress/data'; + +/** + * Internal dependencies + */ + +import { registerFormatType } from '../register-format-type'; +import { unregisterFormatType } from '../unregister-format-type'; + +describe( 'registerFormatType', () => { + beforeAll( () => { + // Initialize the rich-text store. + require( '../store' ); + } ); + + afterEach( () => { + select( 'core/rich-text' ).getFormatTypes().forEach( ( { name } ) => { + unregisterFormatType( name ); + } ); + } ); + + const validName = 'plugin/test'; + const validSettings = { + tagName: 'test', + className: null, + title: 'Test', + edit() {}, + }; + + it( 'should register format', () => { + const settings = registerFormatType( validName, validSettings ); + expect( settings ).toEqual( { ...validSettings, name: validName } ); + expect( console ).not.toHaveErrored(); + } ); + + it( 'should error without arguments', () => { + registerFormatType(); + expect( console ).toHaveErroredWith( 'Format names must be strings.' ); + } ); + + it( 'should error on invalid name', () => { + registerFormatType( 'test', validSettings ); + expect( console ).toHaveErroredWith( 'Format names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-format' ); + } ); + + it( 'should error on already registered name', () => { + registerFormatType( validName, validSettings ); + registerFormatType( validName, validSettings ); + expect( console ).toHaveErroredWith( 'Format "plugin/test" is already registered.' ); + } ); + + it( 'should error on undefined edit property', () => { + registerFormatType( 'plugin/test', { + ...validSettings, + edit: undefined, + } ); + expect( console ).toHaveErroredWith( 'The "edit" property must be specified and must be a valid function.' ); + } ); + + it( 'should error on empty tagName property', () => { + registerFormatType( validName, { + ...validSettings, + tagName: '', + } ); + expect( console ).toHaveErroredWith( 'Format tag names must be a string.' ); + } ); + + it( 'should error on invalid empty className property', () => { + registerFormatType( validName, { + ...validSettings, + className: '', + } ); + expect( console ).toHaveErroredWith( 'Format class names must be a string, or null to handle bare elements.' ); + } ); + + it( 'should error on invalid className property', () => { + registerFormatType( validName, { + ...validSettings, + className: 'invalid class name', + } ); + expect( console ).toHaveErroredWith( 'A class name must begin with a letter, followed by any number of hyphens, letters, or numbers.' ); + } ); + + it( 'should error on already registered tagName', () => { + registerFormatType( validName, validSettings ); + registerFormatType( 'plugin/second', validSettings ); + expect( console ).toHaveErroredWith( 'Format "plugin/test" is already registered to handle bare tag name "test".' ); + } ); + + it( 'should error on already registered className', () => { + registerFormatType( validName, { + ...validSettings, + className: 'test', + } ); + registerFormatType( 'plugin/second', { + ...validSettings, + className: 'test', + } ); + expect( console ).toHaveErroredWith( 'Format "plugin/test" is already registered to handle class name "test".' ); + } ); + + it( 'should error on empty title property', () => { + registerFormatType( validName, { + ...validSettings, + title: '', + } ); + expect( console ).toHaveErroredWith( 'The format "plugin/test" must have a title.' ); + } ); +} ); diff --git a/packages/rich-text/src/test/to-html-string.js b/packages/rich-text/src/test/to-html-string.js index 975663acdadbb2..cdb491b21dedfb 100644 --- a/packages/rich-text/src/test/to-html-string.js +++ b/packages/rich-text/src/test/to-html-string.js @@ -10,6 +10,9 @@ import { JSDOM } from 'jsdom'; import { create } from '../create'; import { toHTMLString } from '../to-html-string'; +import { registerFormatType } from '../register-format-type'; +import { unregisterFormatType } from '../unregister-format-type'; +import { specWithRegistration } from './helpers'; const { window } = new JSDOM(); const { document } = window; @@ -26,6 +29,28 @@ describe( 'toHTMLString', () => { require( '../store' ); } ); + specWithRegistration.forEach( ( { + description, + formatName, + formatType, + html, + value, + } ) => { + it( description, () => { + if ( formatName ) { + registerFormatType( formatName, formatType ); + } + + const result = toHTMLString( { value } ); + + if ( formatName ) { + unregisterFormatType( formatName ); + } + + expect( result ).toEqual( html ); + } ); + } ); + it( 'should extract recreate HTML 1', () => { const HTML = 'one two 🍒 three'; const element = createNode( `

${ HTML }

` ); diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js index fa8810b8e17ea7..68fd23f0669efc 100644 --- a/packages/rich-text/src/to-tree.js +++ b/packages/rich-text/src/to-tree.js @@ -9,21 +9,14 @@ import { ZERO_WIDTH_NO_BREAK_SPACE, } from './special-characters'; -function fromFormat( { type, attributes, object } ) { +function fromFormat( { type, attributes, unregisteredAttributes, object } ) { const formatType = getFormatType( type ); if ( ! formatType ) { return { type, attributes, object }; } - if ( ! attributes ) { - return { - type: formatType.match.tagName, - object: formatType.object, - }; - } - - const elementAttributes = {}; + const elementAttributes = unregisteredAttributes ? { ...unregisteredAttributes } : {}; for ( const name in attributes ) { const key = formatType.attributes[ name ]; @@ -35,8 +28,16 @@ function fromFormat( { type, attributes, object } ) { } } + if ( formatType.className ) { + if ( elementAttributes.class ) { + elementAttributes.class = `${ formatType.className } ${ elementAttributes.class }`; + } else { + elementAttributes.class = formatType.className; + } + } + return { - type: formatType.match.tagName, + type: formatType.tagName, object: formatType.object, attributes: elementAttributes, }; @@ -145,15 +146,14 @@ export function toTree( { return; } - const { type, attributes, object } = format; const parent = getParent( pointer ); - const newNode = append( parent, fromFormat( { type, attributes, object } ) ); + const newNode = append( parent, fromFormat( format ) ); if ( isText( pointer ) && getText( pointer ).length === 0 ) { remove( pointer ); } - pointer = append( object ? parent : newNode, '' ); + pointer = append( format.object ? parent : newNode, '' ); } ); }