Skip to content

Commit

Permalink
Merge pull request #8354 from ckeditor/i/8160
Browse files Browse the repository at this point in the history 
Fix (list): List styles will be inherited correctly when pasting a list into another list. Closes #8160.
  • Loading branch information
@jodator
jodator authored Oct 28, 2020
2 parents 00d437e + 21b9591 commit 5dd14a3
Show file tree
Hide file tree
Showing 3 changed files with 246 additions and 26 deletions.
105 changes: 81 additions & 24 deletions packages/ckeditor5-list/src/liststyleediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,43 +490,100 @@ function fixListStyleAttributeOnListItemElements( editor ) {
} else {
writer.setAttribute( 'listStyle', DEFAULT_LIST_TYPE, item );
}

wasFixed = true;
} else {
// Adjust the `listStyle` attribute for inserted (pasted) items. See #8160.
//
// ■ List item 1. // [listStyle="square", listType="bulleted"]
// ○ List item 1.1. // [listStyle="circle", listType="bulleted"]
// ○ [] (selection is here)
//
// Then, pasting a list with different attributes (listStyle, listType):
//
// 1. First. // [listStyle="decimal", listType="numbered"]
// 2. Second // [listStyle="decimal", listType="numbered"]
//
// The `listType` attribute will be corrected by the `ListEditing` converters.
// We need to adjust the `listStyle` attribute. Expected structure:
//
// ■ List item 1. // [listStyle="square", listType="bulleted"]
// ○ List item 1.1. // [listStyle="circle", listType="bulleted"]
// ○ First. // [listStyle="circle", listType="bulleted"]
// ○ Second // [listStyle="circle", listType="bulleted"]
const previousSibling = item.previousSibling;

if ( shouldInheritListTypeFromPreviousItem( previousSibling, item ) ) {
writer.setAttribute( 'listStyle', previousSibling.getAttribute( 'listStyle' ), item );

wasFixed = true;
}
}
}

return wasFixed;
};
}

// Checks whether the `listStyle` attribute should be copied from the `baseItem` element.
//
// The attribute should be copied if the inserted element does not have defined it and
// the value for the element is other than default in the base element.
//
// @param {module:engine/model/element~Element|null} baseItem
// @param {module:engine/model/element~Element} itemToChange
// @returns {Boolean}
function shouldInheritListType( baseItem, itemToChange ) {
if ( !baseItem ) {
return false;
}
// Checks whether the `listStyle` attribute should be copied from the `baseItem` element.
//
// The attribute should be copied if the inserted element does not have defined it and
// the value for the element is other than default in the base element.
//
// @param {module:engine/model/element~Element|null} baseItem
// @param {module:engine/model/element~Element} itemToChange
// @returns {Boolean}
function shouldInheritListType( baseItem, itemToChange ) {
if ( !baseItem ) {
return false;
}

const baseListStyle = baseItem.getAttribute( 'listStyle' );
const baseListStyle = baseItem.getAttribute( 'listStyle' );

if ( !baseListStyle ) {
return false;
}
if ( !baseListStyle ) {
return false;
}

if ( baseListStyle === DEFAULT_LIST_TYPE ) {
return false;
}
if ( baseListStyle === DEFAULT_LIST_TYPE ) {
return false;
}

if ( baseItem.getAttribute( 'listType' ) !== itemToChange.getAttribute( 'listType' ) ) {
return false;
}
if ( baseItem.getAttribute( 'listType' ) !== itemToChange.getAttribute( 'listType' ) ) {
return false;
}

return true;
return true;
}

// Checks whether the `listStyle` attribute should be copied from previous list item.
//
// The attribute should be copied if there's a mismatch of styles of the pasted list into a nested list.
// Top-level lists are not normalized as we allow side-by-side list of different types.
//
// @param {module:engine/model/element~Element|null} previousItem
// @param {module:engine/model/element~Element} itemToChange
// @returns {Boolean}
function shouldInheritListTypeFromPreviousItem( previousItem, itemToChange ) {
if ( !previousItem || !previousItem.is( 'element', 'listItem' ) ) {
return false;
}

if ( itemToChange.getAttribute( 'listType' ) !== previousItem.getAttribute( 'listType' ) ) {
return false;
}

const previousItemIndent = previousItem.getAttribute( 'listIndent' );

if ( previousItemIndent < 1 || previousItemIndent !== itemToChange.getAttribute( 'listIndent' ) ) {
return false;
}

const previousItemListStyle = previousItem.getAttribute( 'listStyle' );

if ( !previousItemListStyle || previousItemListStyle === itemToChange.getAttribute( 'listStyle' ) ) {
return false;
}

return true;
}

// Removes the `listStyle` attribute from "todo" list items.
Expand Down
4 changes: 2 additions & 2 deletions packages/ckeditor5-list/tests/liststylecommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe( 'ListStyleCommand', () => {
it( 'should return the value of `listStyle` attribute for the selection inside a nested list', () => {
setData( model,
'<listItem listIndent="0" listType="bulleted" listStyle="square">1.</listItem>' +
'<listItem listIndent="0" listType="bulleted" listStyle="disc">1.1.[]</listItem>' +
'<listItem listIndent="1" listType="bulleted" listStyle="disc">1.1.[]</listItem>' +
'<listItem listIndent="0" listType="bulleted" listStyle="square">2.</listItem>'
);

Expand All @@ -106,7 +106,7 @@ describe( 'ListStyleCommand', () => {
it( 'should return the value of `listStyle` attribute from a list where the selection starts (selection over nested list)', () => {
setData( model,
'<listItem listIndent="0" listType="bulleted" listStyle="square">1.</listItem>' +
'<listItem listIndent="0" listType="bulleted" listStyle="disc">1.1.[</listItem>' +
'<listItem listIndent="1" listType="bulleted" listStyle="disc">1.1.[</listItem>' +
'<listItem listIndent="0" listType="bulleted" listStyle="square">2.]</listItem>'
);

Expand Down
163 changes: 163 additions & 0 deletions packages/ckeditor5-list/tests/liststyleediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* global document */

import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Typing from '@ckeditor/ckeditor5-typing/src/typing';
import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard';
import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';

Expand Down Expand Up @@ -1459,5 +1463,164 @@ describe( 'ListStyleEditing', () => {
} );
}
} );

// #8160
describe( 'pasting a list into another list', () => {
let element;

beforeEach( () => {
element = document.createElement( 'div' );
document.body.append( element );

return ClassicTestEditor
.create( element, {
plugins: [ Paragraph, Clipboard, ListStyleEditing, UndoEditing ]
} )
.then( newEditor => {
editor = newEditor;
model = editor.model;
} );
} );

afterEach( () => {
return editor.destroy()
.then( () => {
element.remove();
} );
} );

it( 'should inherit attributes from the previous sibling element (collapsed selection)', () => {
setModelData( model,
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">Foo Bar</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">[]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);

pasteHtml( editor,
'<ul style="list-style-type: square">' +
'<li>Foo 1</li>' +
'<li>Foo 2</li>' +
'</ul>'
);

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

it( 'should inherit attributes from the previous sibling element (non-collapsed selection)', () => {
setModelData( model,
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">Foo Bar</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">[Foo]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);

pasteHtml( editor,
'<ul style="list-style-type: square">' +
'<li>Foo 1</li>' +
'<li>Foo 2</li>' +
'</ul>'
);

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

it( 'should inherit attributes from the previous sibling element (non-collapsed selection over a few elements)', () => {
setModelData( model,
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">Foo Bar</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">[Foo 1.</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">Foo 2.</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">Foo 3.]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);

pasteHtml( editor,
'<ul style="list-style-type: square">' +
'<li>Foo 1</li>' +
'<li>Foo 2</li>' +
'</ul>'
);

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

it( 'should do nothing when pasting the similar list', () => {
setModelData( model,
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">Foo Bar</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">[]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);

pasteHtml( editor,
'<ol style="list-style-type: decimal">' +
'<li>Foo</li>' +
'</ol>'
);

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

it( 'should replace the entire list if selected', () => {
setModelData( model,
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Foo</listItem>' +
'<listItem listIndent="1" listStyle="decimal" listType="numbered">[Foo Bar]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar</listItem>'
);

pasteHtml( editor,
'<ul style="list-style-type: square">' +
'<li>Foo</li>' +
'</ul>'
);

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

function pasteHtml( editor, html ) {
editor.editing.view.document.fire( 'paste', {
dataTransfer: createDataTransfer( { 'text/html': html } ),
stopPropagation() {},
preventDefault() {}
} );
}

function createDataTransfer( data ) {
return {
getData( type ) {
return data[ type ];
},
setData() {}
};
}
} );
} );
} );

0 comments on commit 5dd14a3

Please sign in to comment.