Skip to content

Commit

Permalink
Fix: The list style attribute will be inherited when switching the "l…
Browse files Browse the repository at this point in the history 
…istType" attribue for existing the "listItem" element.
  • Loading branch information
@pomek
pomek committed Sep 7, 2020
1 parent d0b7dc1 commit 76014e5
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 32 deletions.
73 changes: 41 additions & 32 deletions packages/ckeditor5-list/src/liststyleediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -577,3 +555,34 @@ function restoreDefaultListStyle( editor ) {
} );
};
}

// Returns `listItem` that were inserted or changed.
//
// @private
// @param {Array.<Object>} changes The changes list returned by the differ.
// @returns {Array.<module:engine/model/element~Element>}
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;
}
}
69 changes: 69 additions & 0 deletions packages/ckeditor5-list/tests/liststyleediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
'<listItem listIndent="0" listStyle="default" listType="numbered">Foo Bar.[]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);

editor.execute( 'bulletedList' );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo Bar.[]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);
} );

it( 'should inherit the list style attribute when the modified list is the same kind of the list as previous sibling', () => {
setModelData( model,
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>' +
'<listItem listIndent="0" listStyle="default" listType="numbered">Foo Bar.[]</listItem>'
);

editor.execute( 'bulletedList' );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo Bar.[]</listItem>'
);
} );

it( 'should not inherit the list style attribute when the modified list already has defined it (next sibling check)', () => {
setModelData( model,
'<listItem listIndent="0" listStyle="default" listType="bulleted">Foo Bar.[]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);

editor.execute( 'listStyle', { type: 'disc' } );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="disc" listType="bulleted">Foo Bar.[]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);
} );

it(
'should not inherit the list style attribute when the modified list already has defined it (previous sibling check)',
() => {
setModelData( model,
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>' +
'<listItem listIndent="0" listStyle="default" listType="bulleted">Foo Bar.[]</listItem>'
);

editor.execute( 'listStyle', { type: 'disc' } );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>' +
'<listItem listIndent="0" listStyle="disc" listType="bulleted">Foo Bar.[]</listItem>'
);
}
);
} );

describe( 'indenting lists', () => {
it( 'should restore the default value for the list style attribute when indenting a single item', () => {
setModelData( model,
Expand Down

0 comments on commit 76014e5

Please sign in to comment.