Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RichText: Ensure HTML can be created from serialized object #10351

Closed
wants to merge 2 commits into from
Closed

RichText: Ensure HTML can be created from serialized object #10351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7363b63
Ensure HTML can be created from serialized object
ellatrix Oct 4, 2018
a776108
Normalise formats should handle non sparse arrays
ellatrix Oct 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/rich-text/src/normalise-formats.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function normaliseFormats( { formats, text, start, end } ) {
newFormats.forEach( ( formatsAtIndex, index ) => {
const lastFormatsAtIndex = newFormats[ index - 1 ];

if ( lastFormatsAtIndex ) {
if ( lastFormatsAtIndex && formatsAtIndex ) {
const newFormatsAtIndex = formatsAtIndex.slice( 0 );

newFormatsAtIndex.forEach( ( format, formatIndex ) => {
Expand All @@ -29,6 +29,8 @@ export function normaliseFormats( { formats, text, start, end } ) {
} );

newFormats[ index ] = newFormatsAtIndex;
} else if ( ! formatsAtIndex ) {
delete newFormats[ index ];
}
} );

Expand Down
35 changes: 26 additions & 9 deletions packages/rich-text/src/test/normalise-formats.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,36 @@ import { getSparseArrayLength } from './helpers';
describe( 'normaliseFormats', () => {
const strong = { type: 'strong' };
const em = { type: 'em' };
const record = deepFreeze( {
formats: [ , [ em ], [ { ...em }, { ...strong } ], [ em, strong ], , , ],
text: 'one two',
} );

it( 'should normalise formats', () => {
const record = {
formats: [ , [ em ], [ { ...em }, { ...strong } ], [ em, strong ] ],
text: 'one two three',
};
const result = normaliseFormats( deepFreeze( record ) );

expect( result ).toEqual( record );
expect( result ).not.toBe( record );
function isNormalised( result ) {
expect( getSparseArrayLength( result.formats ) ).toBe( 3 );
expect( result.formats[ 1 ][ 0 ] ).toBe( result.formats[ 2 ][ 0 ] );
expect( result.formats[ 1 ][ 0 ] ).toBe( result.formats[ 3 ][ 0 ] );
expect( result.formats[ 2 ][ 1 ] ).toBe( result.formats[ 3 ][ 1 ] );
}

it( 'should normalise formats', () => {
const result = normaliseFormats( record );

expect( result ).toEqual( record );
expect( result ).not.toBe( record );
isNormalised( result );
} );

it( 'should be serializable', () => {
const result = normaliseFormats( record );
expect( JSON.stringify( record ) ).toBe( JSON.stringify( result ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

} );

it( 'should normalise serialized formats', () => {
const deserialized = JSON.parse( JSON.stringify( record ) );
const result = normaliseFormats( deserialized );

expect( result ).toEqual( record );
isNormalised( result );
} );
} );
11 changes: 11 additions & 0 deletions packages/rich-text/src/test/to-html-string.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,15 @@ describe( 'toHTMLString', () => {

expect( toHTMLString( create( { element } ) ) ).toEqual( expectedHTML );
} );

it( 'should be able to work from serialized value (without references)', () => {
expect( toHTMLString( {
text: 'abc',
formats: [
null,
[ { type: 'strong' } ],
null,
],
} ) ).toEqual( 'a<strong>b</strong>c' );
} );
} );
3 changes: 2 additions & 1 deletion packages/rich-text/src/to-tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/

import { split } from './split';
import { normaliseFormats } from './normalise-formats';

export function toTree( value, multilineTag, settings ) {
if ( multilineTag ) {
Expand Down Expand Up @@ -35,7 +36,7 @@ export function toTree( value, multilineTag, settings ) {
onEndIndex,
onEmpty,
} = settings;
const { formats, text, start, end } = value;
const { formats, text, start, end } = normaliseFormats( value );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know normaliseFormats is performant at the moment but this is the kind of function that can hurt the editor's performance pretty quickly if we made a not-good-enough change. How can we ensure it doesn't happen? Any ideas.

(don't require an immediate answer)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting question, but it's valid for any change within this package.

const formatsLength = formats.length + 1;
const tree = createEmpty( tag );

Expand Down