From 76014e5a3570e0f3ab87bad0e5e961f0671f0980 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Mon, 7 Sep 2020 10:15:40 +0200 Subject: [PATCH] Fix: The list style attribute will be inherited when switching the "listType" attribue for existing the "listItem" element. --- .../ckeditor5-list/src/liststyleediting.js | 73 +++++++++++-------- .../ckeditor5-list/tests/liststyleediting.js | 69 ++++++++++++++++++ 2 files changed, 110 insertions(+), 32 deletions(-) diff --git a/packages/ckeditor5-list/src/liststyleediting.js b/packages/ckeditor5-list/src/liststyleediting.js index eff407f8cde..53fa2a82195 100644 --- a/packages/ckeditor5-list/src/liststyleediting.js +++ b/packages/ckeditor5-list/src/liststyleediting.js @@ -429,16 +429,12 @@ function fixListAfterOutdentListCommand( editor ) { function fixListStyleAttributeOnListItemElements( editor ) { return writer => { let wasFixed = false; - let insertedListItems = []; - for ( const change of editor.model.document.differ.getChanges() ) { - if ( change.type == 'insert' && change.name == 'listItem' ) { - insertedListItems.push( change.position.nodeAfter ); - } - } - - // Don't touch todo lists. - insertedListItems = insertedListItems.filter( item => item.getAttribute( 'listType' ) !== 'todo' ); + const insertedListItems = getChangedListItems( editor.model.document.differ.getChanges() ) + .filter( item => { + // Don't touch todo lists. They are handled in another post-fixer. + return item.getAttribute( 'listType' ) !== 'todo'; + } ); if ( !insertedListItems.length ) { return wasFixed; @@ -524,17 +520,11 @@ function fixListStyleAttributeOnListItemElements( editor ) { // @returns {Function} function removeListStyleAttributeFromTodoList( editor ) { return writer => { - let todoListItems = []; - - for ( const change of editor.model.document.differ.getChanges() ) { - const item = getItemFromChange( change ); - - if ( item && item.is( 'element', 'listItem' ) && item.getAttribute( 'listType' ) === 'todo' ) { - todoListItems.push( item ); - } - } - - todoListItems = todoListItems.filter( item => item.hasAttribute( 'listStyle' ) ); + const todoListItems = getChangedListItems( editor.model.document.differ.getChanges() ) + .filter( item => { + // Handle the todo lists only. The rest is handled in another post-fixer. + return item.getAttribute( 'listType' ) === 'todo' && item.hasAttribute( 'listStyle' ); + } ); if ( !todoListItems.length ) { return false; @@ -546,18 +536,6 @@ function removeListStyleAttributeFromTodoList( editor ) { return true; }; - - function getItemFromChange( change ) { - if ( change.type === 'attribute' ) { - return change.range.start.nodeAfter; - } - - if ( change.type === 'insert' ) { - return change.position.nodeAfter; - } - - return null; - } } // Restores the `listStyle` attribute after changing the list type. @@ -577,3 +555,34 @@ function restoreDefaultListStyle( editor ) { } ); }; } + +// Returns `listItem` that were inserted or changed. +// +// @private +// @param {Array.} changes The changes list returned by the differ. +// @returns {Array.} +function getChangedListItems( changes ) { + const items = []; + + for ( const change of changes ) { + const item = getItemFromChange( change ); + + if ( item && item.is( 'element', 'listItem' ) ) { + items.push( item ); + } + } + + return items; + + function getItemFromChange( change ) { + if ( change.type === 'attribute' ) { + return change.range.start.nodeAfter; + } + + if ( change.type === 'insert' ) { + return change.position.nodeAfter; + } + + return null; + } +} diff --git a/packages/ckeditor5-list/tests/liststyleediting.js b/packages/ckeditor5-list/tests/liststyleediting.js index ec31752976a..d14a05ca66e 100644 --- a/packages/ckeditor5-list/tests/liststyleediting.js +++ b/packages/ckeditor5-list/tests/liststyleediting.js @@ -522,6 +522,75 @@ describe( 'ListStyleEditing', () => { ); } ); + describe( 'modifying "listType" attribute', () => { + it( 'should inherit the list style attribute when the modified list is the same kind of the list as next sibling', () => { + setModelData( model, + 'Foo Bar.[]' + + 'Foo' + + 'Bar' + ); + + editor.execute( 'bulletedList' ); + + expect( getModelData( model ) ).to.equal( + 'Foo Bar.[]' + + 'Foo' + + 'Bar' + ); + } ); + + it( 'should inherit the list style attribute when the modified list is the same kind of the list as previous sibling', () => { + setModelData( model, + 'Foo' + + 'Bar' + + 'Foo Bar.[]' + ); + + editor.execute( 'bulletedList' ); + + expect( getModelData( model ) ).to.equal( + 'Foo' + + 'Bar' + + 'Foo Bar.[]' + ); + } ); + + it( 'should not inherit the list style attribute when the modified list already has defined it (next sibling check)', () => { + setModelData( model, + 'Foo Bar.[]' + + 'Foo' + + 'Bar' + ); + + editor.execute( 'listStyle', { type: 'disc' } ); + + expect( getModelData( model ) ).to.equal( + 'Foo Bar.[]' + + 'Foo' + + 'Bar' + ); + } ); + + it( + 'should not inherit the list style attribute when the modified list already has defined it (previous sibling check)', + () => { + setModelData( model, + 'Foo' + + 'Bar' + + 'Foo Bar.[]' + ); + + editor.execute( 'listStyle', { type: 'disc' } ); + + expect( getModelData( model ) ).to.equal( + 'Foo' + + 'Bar' + + 'Foo Bar.[]' + ); + } + ); + } ); + describe( 'indenting lists', () => { it( 'should restore the default value for the list style attribute when indenting a single item', () => { setModelData( model,