Skip to content

Commit

Permalink
RichText: only replace range and nodes if different (#12547)
Browse files Browse the repository at this point in the history
* RichText: only set range if different

* Check rangeCount

* Also compare nodes

* Add e2e test

* Simplify

* RichText: Document isRangeEqual

* Testing: RichText: Assure subscriber removal

* Unsubscribe in page.evaluate
  • Loading branch information
ellatrix authored Dec 9, 2018
1 parent 2f4317c commit f23282b
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 11 deletions.
46 changes: 35 additions & 11 deletions packages/rich-text/src/to-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,21 +226,17 @@ export function apply( {

export function applyValue( future, current ) {
let i = 0;
let futureChild;

while ( future.firstChild ) {
while ( ( futureChild = future.firstChild ) ) {
const currentChild = current.childNodes[ i ];
const futureNodeType = future.firstChild.nodeType;

if ( ! currentChild ) {
current.appendChild( future.firstChild );
} else if (
futureNodeType !== currentChild.nodeType ||
futureNodeType !== TEXT_NODE ||
future.firstChild.nodeValue !== currentChild.nodeValue
) {
current.replaceChild( future.firstChild, currentChild );
current.appendChild( futureChild );
} else if ( ! currentChild.isEqualNode( futureChild ) ) {
current.replaceChild( futureChild, currentChild );
} else {
future.removeChild( future.firstChild );
future.removeChild( futureChild );
}

i++;
Expand All @@ -251,6 +247,25 @@ export function applyValue( future, current ) {
}
}

/**
* Returns true if two ranges are equal, or false otherwise. Ranges are
* considered equal if their start and end occur in the same container and
* offset.
*
* @param {Range} a First range object to test.
* @param {Range} b First range object to test.
*
* @return {boolean} Whether the two ranges are equal.
*/
function isRangeEqual( a, b ) {
return (
a.startContainer === b.startContainer &&
a.startOffset === b.startOffset &&
a.endContainer === b.endContainer &&
a.endOffset === b.endOffset
);
}

export function applySelection( selection, current ) {
const { node: startContainer, offset: startOffset } = getNodeByPath( current, selection.startPath );
const { node: endContainer, offset: endOffset } = getNodeByPath( current, selection.endPath );
Expand Down Expand Up @@ -283,6 +298,15 @@ export function applySelection( selection, current ) {
range.setEnd( endContainer, endOffset );
}

windowSelection.removeAllRanges();
if ( windowSelection.rangeCount > 0 ) {
// If the to be added range and the live range are the same, there's no
// need to remove the live range and add the equivalent range.
if ( isRangeEqual( range, windowSelection.getRangeAt( 0 ) ) ) {
return;
}

windowSelection.removeAllRanges();
}

windowSelection.addRange( range );
}
6 changes: 6 additions & 0 deletions test/e2e/specs/__snapshots__/rich-text.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ exports[`RichText should handle change in tag name gracefully 1`] = `
<!-- /wp:heading -->"
`;

exports[`RichText should only mutate text data on input 1`] = `
"<!-- wp:paragraph -->
<p>1<strong>2</strong>34</p>
<!-- /wp:paragraph -->"
`;

exports[`RichText should transform backtick to code 1`] = `
"<!-- wp:paragraph -->
<p>A <code>backtick</code></p>
Expand Down
71 changes: 71 additions & 0 deletions test/e2e/specs/rich-text.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,75 @@ describe( 'RichText', () => {

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should only mutate text data on input', async () => {
await clickBlockAppender();
await page.keyboard.type( '1' );
await pressWithModifier( 'primary', 'b' );
await page.keyboard.type( '2' );
await pressWithModifier( 'primary', 'b' );
await page.keyboard.type( '3' );

await page.evaluate( () => {
let called;
const { body } = document;
const config = {
attributes: true,
childList: true,
characterData: true,
subtree: true,
};

const mutationObserver = new MutationObserver( ( records ) => {
if ( called || records.length > 1 ) {
throw new Error( 'Typing should only mutate once.' );
}

records.forEach( ( record ) => {
if ( record.type !== 'characterData' ) {
throw new Error(
`Typing mutated more than character data: ${ record.type }`
);
}
} );

called = true;
} );

mutationObserver.observe( body, config );

window.unsubscribes = [ () => mutationObserver.disconnect() ];

document.addEventListener( 'selectionchange', () => {
function throwMultipleSelectionChange() {
throw new Error( 'Typing should only emit one selection change event.' );
}

document.addEventListener(
'selectionchange',
throwMultipleSelectionChange,
{ once: true }
);

window.unsubscribes.push( () => {
document.removeEventListener( 'selectionchange', throwMultipleSelectionChange );
} );
}, { once: true } );
} );

await page.keyboard.type( '4' );

await page.evaluate( () => {
// The selection change event should be called once. If there's only
// one item in `window.unsubscribes`, it means that only one
// function is present to disconnect the `mutationObserver`.
if ( window.unsubscribes.length === 1 ) {
throw new Error( 'The selection change event listener was never called.' );
}

window.unsubscribes.forEach( ( unsubscribe ) => unsubscribe() );
} );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );

0 comments on commit f23282b

Please sign in to comment.