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

Jump to bottom

Fix: Update data of roots with modified content only #535

Merged
merged 2 commits into from
Sep 30, 2024
Merged

Fix: Update data of roots with modified content only #535

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
b08eccf
Fix: Update data of roots with modified content only.
f1ames Sep 24, 2024
f09915b
Chore: Apply minor code corrections.
f1ames Sep 26, 2024
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
18 changes: 9 additions & 9 deletions src/useMultiRootEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ const useMultiRootEditor = ( props: MultiRootHookProps ): MultiRootHookReturns =
data || /* istanbul ignore next -- @preserve: It should never happen, data should be always filled. */ {}
);

const hasModifiedData = dataKeys.some( rootName =>
const modifiedRoots = dataKeys.filter( rootName =>
editorData[ rootName ] !== undefined &&
JSON.stringify( editorData[ rootName ] ) !== JSON.stringify( data[ rootName ] )
);
Expand All @@ -530,12 +530,12 @@ const useMultiRootEditor = ( props: MultiRootHookProps ): MultiRootHookReturns =
} );
};

const _updateEditorData = () => {
// If any of the roots content has changed, set the editor data.
// Unfortunately, we cannot set the editor data just for one root,
// so we need to overwrite all roots (`nextProps.data` is an
// object with data for each root).
instance.data.set( data, { suppressErrorInCollaboration: true } as any );
const _updateEditorData = ( roots: Array<string> ) => {
const dataToUpdate = roots.reduce(
( result, rootName ) => ( { ...result, [ rootName ]: data[ rootName ] } ),
Object.create( null )
);
instance.data.set( dataToUpdate, { suppressErrorInCollaboration: true } as any );
};

const _updateEditorAttributes = ( writer: Writer, roots: Array<string> ) => {
Expand All @@ -555,8 +555,8 @@ const useMultiRootEditor = ( props: MultiRootHookProps ): MultiRootHookReturns =
_handleNewRoots( newRoots );
_handleRemovedRoots( removedRoots );

if ( hasModifiedData ) {
_updateEditorData();
if ( modifiedRoots.length ) {
_updateEditorData( modifiedRoots );
}

if ( rootsWithChangedAttributes.length ) {
Expand Down
91 changes: 76 additions & 15 deletions tests/useMultiRootEditor.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import turnOffDefaultErrorCatching from './_utils/turnoffdefaulterrorcatching.js
describe( 'useMultiRootEditor', () => {
const rootsContent = {
intro: '<h2>Sample</h2><p>This is an instance of the.</p>',
content: '<p>It is the custom content</p>'
content: '<p>It is the custom content</p>',
footer: '<p>Footer content</p>'
};

const rootsAttributes = {
Expand All @@ -30,6 +31,10 @@ describe( 'useMultiRootEditor', () => {
content: {
row: '1',
order: 20
},
footer: {
row: '2',
order: 30
}
};

Expand Down Expand Up @@ -232,7 +237,7 @@ describe( 'useMultiRootEditor', () => {
const { data, editableElements } = result.current;

expect( data ).to.deep.equal( rootsContent );
expect( editableElements.length ).to.equal( 2 );
expect( editableElements.length ).to.equal( 3 );
} );

it( 'should update the editor data when the state has been changed', async () => {
Expand All @@ -254,8 +259,64 @@ describe( 'useMultiRootEditor', () => {

expect( spy ).toHaveBeenCalledOnce();
expect( data.intro ).to.equal( '<p>New data</p>' );
expect( editableElements.length ).to.equal( 2 );
expect( data.content ).to.equal( rootsContent.content );
expect( data.footer ).to.equal( rootsContent.footer );
expect( spy.mock.calls[ 0 ][ 0 ] ).to.deep.equal( { 'intro': 'New data' } );
expect( editableElements.length ).to.equal( 3 );
expect( editor!.getFullData().intro ).to.equal( '<p>New data</p>' );
expect( editor!.getFullData().content ).to.equal( rootsContent.content );
expect( editor!.getFullData().footer ).to.equal( rootsContent.footer );
} );
} );

it( 'should update only editor roots which content have been changed', async () => {
const { result } = renderHook( () => useMultiRootEditor( editorProps ) );

await waitFor( () => {
expect( result.current.editor ).to.be.instanceof( TestMultiRootEditor );
} );

const { editor, setData } = result.current;
const spy = vi.spyOn( editor!.data, 'set' );

act( () => {
setData( { ...rootsContent, 'intro': '<h2>Sample</h2>', 'footer': 'Text...' } );
} );

await waitFor( () => {
const { data, editableElements } = result.current;

expect( spy ).toHaveBeenCalledOnce();
expect( data.intro ).to.equal( '<h2>Sample</h2>' );
expect( data.footer ).to.equal( '<p>Text...</p>' );
expect( spy.mock.calls[ 0 ][ 0 ] ).to.deep.equal( { 'intro': '<h2>Sample</h2>', 'footer': 'Text...' } );
expect( editableElements.length ).to.equal( 3 );
expect( editor!.getFullData().intro ).to.equal( '<h2>Sample</h2>' );
expect( editor!.getFullData().footer ).to.equal( '<p>Text...</p>' );
} );
} );

it( 'should not update editor roots when no content have been changed', async () => {
const { result } = renderHook( () => useMultiRootEditor( editorProps ) );

await waitFor( () => {
expect( result.current.editor ).to.be.instanceof( TestMultiRootEditor );
} );

const { editor, setData } = result.current;
const spy = vi.spyOn( editor!.data, 'set' );

act( () => {
setData( { ...rootsContent, 'intro': '<h2>Sample</h2><p>This is an instance of the.</p>' } );
} );

await waitFor( () => {
const { data, editableElements } = result.current;

expect( spy.mock.calls.length ).to.equal( 0 );
expect( data ).to.deep.equal( rootsContent );
expect( editableElements.length ).to.equal( 3 );
expect( editor!.getFullData() ).to.deep.equal( rootsContent );
} );
} );

Expand All @@ -281,7 +342,7 @@ describe( 'useMultiRootEditor', () => {

expect( spy ).toHaveBeenCalledOnce();
expect( data.intro ).to.be.undefined;
expect( editableElements.length ).to.equal( 1 );
expect( editableElements.length ).to.equal( 2 );
expect( editor!.getFullData().intro ).to.be.undefined;
} );
} );
Expand All @@ -306,7 +367,7 @@ describe( 'useMultiRootEditor', () => {

expect( spy ).toHaveBeenCalledOnce();
expect( data.outro ).to.be.equal( '<p>New data</p>' );
expect( editableElements.length ).to.equal( 3 );
expect( editableElements.length ).to.equal( 4 );
expect( editor!.getFullData().outro ).to.be.equal( '<p>New data</p>' );
} );
} );
Expand All @@ -328,7 +389,7 @@ describe( 'useMultiRootEditor', () => {
const { data, editableElements } = result.current;

expect( data.intro ).to.equal( '<p>New data</p>' );
expect( editableElements.length ).to.equal( 2 );
expect( editableElements.length ).to.equal( 3 );
expect( editor!.getFullData().intro ).to.equal( '<p>New data</p>' );
} );
} );
Expand All @@ -354,7 +415,7 @@ describe( 'useMultiRootEditor', () => {
expect( spy.mock.calls.length ).to.equal( editableElements.length );
expect( data.outro ).to.equal( '' );
expect( attributes.outro ).to.deep.equal( { order: null, row: null } );
expect( editableElements.length ).to.equal( 3 );
expect( editableElements.length ).to.equal( 4 );
expect( editor!.getFullData().outro ).to.equal( '' );
} );

Expand Down Expand Up @@ -396,7 +457,7 @@ describe( 'useMultiRootEditor', () => {

const { editor } = result.current;

expect( result.current.editableElements.length ).to.equal( 2 );
expect( result.current.editableElements.length ).to.equal( 3 );

act( () => {
editor!.detachRoot( 'intro' );
Expand All @@ -406,7 +467,7 @@ describe( 'useMultiRootEditor', () => {
const { data, editableElements } = result.current;

expect( data.intro ).to.be.undefined;
expect( editableElements.length ).to.equal( 1 );
expect( editableElements.length ).to.equal( 2 );
expect( editor!.getFullData().intro ).to.be.undefined;
} );
} );
Expand Down Expand Up @@ -718,7 +779,7 @@ describe( 'useMultiRootEditor', () => {
const { data, editableElements } = result.current;

expect( data.intro ).to.equal( rootsContent.intro );
expect( editableElements.length ).to.equal( 2 );
expect( editableElements.length ).to.equal( 3 );
expect( editor!.getFullData().intro ).to.equal( '<p>New data</p>' );
expect( getDataSpy ).not.toHaveBeenCalled();
} );
Expand Down Expand Up @@ -747,7 +808,7 @@ describe( 'useMultiRootEditor', () => {
expect( spy.mock.calls.length ).to.equal( editableElements.length );
expect( data.outro ).to.be.undefined;
expect( attributes.outro ).to.be.undefined;
expect( editableElements.length ).to.equal( 3 );
expect( editableElements.length ).to.equal( 4 );
expect( editor!.getFullData().outro ).to.equal( '' );
} );

Expand All @@ -763,7 +824,7 @@ describe( 'useMultiRootEditor', () => {

const { editor } = result.current;

expect( result.current.editableElements.length ).to.equal( 2 );
expect( result.current.editableElements.length ).to.equal( 3 );

act( () => {
editor!.detachRoot( 'intro' );
Expand All @@ -773,7 +834,7 @@ describe( 'useMultiRootEditor', () => {
const { data, editableElements } = result.current;

expect( data.intro ).to.be.equal( rootsContent.intro );
expect( editableElements.length ).to.equal( 1 );
expect( editableElements.length ).to.equal( 2 );
expect( editor!.getFullData().intro ).to.be.undefined;
} );
} );
Expand Down Expand Up @@ -1029,7 +1090,7 @@ describe( 'useMultiRootEditor', () => {

await waitFor( () => {
expect( renderedEditor.current ).not.to.be.null;
expect( container.getElementsByClassName( 'ck-editor__editable' ).length ).to.equal( 2 );
expect( container.getElementsByClassName( 'ck-editor__editable' ).length ).to.equal( 3 );
} );

renderedEditor.current!.model.document.roots.remove( 'intro' );
Expand Down Expand Up @@ -1104,7 +1165,7 @@ describe( 'useMultiRootEditor', () => {
const { container } = render( <Component /> );

await waitFor( () => {
expect( container.getElementsByClassName( 'ck-editor__editable' ).length ).to.equal( 2 );
expect( container.getElementsByClassName( 'ck-editor__editable' ).length ).to.equal( 3 );
} );
} );

Expand Down