Merge pull request #16785 from ckeditor/ck/15602

Fix (list): A to-do list should preserve the state of the checked items on the data load. Closes #15602.
ckeditor · Aug 5, 2024 · 6316589 · 6316589
2 parents e58a5c6 + 39dd863
commit 6316589
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 3 deletions.
diff --git a/packages/ckeditor5-list/src/todolist/todolistediting.ts b/packages/ckeditor5-list/src/todolist/todolistediting.ts
@@ -11,15 +11,15 @@ import {
 	Matcher,
 	type UpcastElementEvent,
 	type Model,
-	type Element,
 	type MatcherPattern,
 	type ViewElement,
 	type ViewDocumentKeyDownEvent,
 	type ViewDocumentArrowKeyEvent,
 	type MapperViewToModelPositionEvent,
 	type ViewDocumentFragment,
 	type SelectionChangeRangeEvent,
-	type DocumentFragment
+	type DocumentFragment,
+	type Element
 } from 'ckeditor5/src/engine.js';
 
 import {
@@ -32,7 +32,7 @@ import {
 
 import { Plugin } from 'ckeditor5/src/core.js';
 
-import { isFirstBlockOfListItem, isListItemBlock } from '../list/utils/model.js';
+import { getAllListItemBlocks, isFirstBlockOfListItem, isListItemBlock } from '../list/utils/model.js';
 import ListEditing, {
 	type ListEditingCheckElementEvent,
 	type ListEditingPostFixerEvent
@@ -98,6 +98,11 @@ export default class TodoListEditing extends Plugin {
 			// Upcast of to-do list item is based on a checkbox at the beginning of a <li> to keep compatibility with markdown input.
 			dispatcher.on( 'element:input', todoItemInputConverter() );
 
+			// Priority is set to low to allow generic list item converter to run first.
+			dispatcher.on( 'element:li', todoListItemUpcastConverter(), {
+				priority: 'low'
+			} );
+
 			// Consume other elements that are normally generated in data downcast, so they won't get captured by GHS.
 			dispatcher.on( 'element:label', elementUpcastConsumingConverter(
 				{ name: 'label', classes: 'todo-list__label' }
@@ -108,6 +113,7 @@ export default class TodoListEditing extends Plugin {
 			dispatcher.on( 'element:span', elementUpcastConsumingConverter(
 				{ name: 'span', classes: 'todo-list__label__description' }
 			) );
+
 			dispatcher.on( 'element:ul', attributeUpcastConsumingConverter(
 				{ name: 'ul', classes: 'todo-list' }
 			) );
@@ -389,6 +395,47 @@ export default class TodoListEditing extends Plugin {
 	}
 }
 
+/**
+ * Returns an upcast converter for to-do list items.
+ */
+function todoListItemUpcastConverter(): GetCallback<UpcastElementEvent> {
+	return ( evt, data, conversionApi ) => {
+		const { writer, schema } = conversionApi;
+
+		if ( !data.modelRange ) {
+			return;
+		}
+
+		// Group to-do list items by their listItemId attribute to ensure that all items of the same list item have the same checked state.
+		const groupedItems = Array
+			.from( data.modelRange.getItems( { shallow: true } ) )
+			.filter( ( item ): item is Element =>
+				item.getAttribute( 'listType' ) === 'todo' && schema.checkAttribute( item, 'listItemId' )
+			)
+			.reduce( ( acc, item ) => {
+				const listItemId = item.getAttribute( 'listItemId' ) as string;
+
+				if ( !acc.has( listItemId ) ) {
+					acc.set( listItemId, getAllListItemBlocks( item ) );
+				}
+
+				return acc;
+			}, new Map<string, Array<Element>>() );
+
+		// During the upcast, we need to ensure that all items of the same list have the same checked state. From time to time
+		// the checked state of the items can be different when the user pastes content from the clipboard with <input type="checkbox">
+		// that has checked state set to true. In such cases, we need to ensure that all items of the same list have the same checked state.
+		// See more: https://github.com/ckeditor/ckeditor5/issues/15602
+		for ( const [ , items ] of groupedItems.entries() ) {
+			if ( items.some( item => item.getAttribute( 'todoListChecked' ) ) ) {
+				for ( const item of items ) {
+					writer.setAttribute( 'todoListChecked', true, item );
+				}
+			}
+		}
+	};
+}
+
 /**
  * Returns an upcast converter that detects a to-do list checkbox and marks the list item as a to-do list.
  */

diff --git a/packages/ckeditor5-list/tests/todolist/todolistediting.js b/packages/ckeditor5-list/tests/todolist/todolistediting.js
@@ -203,6 +203,53 @@ describe( 'TodoListEditing', () => {
 			);
 		} );
 
+		it( 'should convert li with a checkbox and a paragraph ( when checked )', () => {
+			testUpcast(
+				'<ul>' +
+					'<li>' +
+						'<input type="checkbox" checked="checked">' +
+						'<p>foo</p>' +
+					'</li>' +
+				'</ul>',
+				'<paragraph listIndent="0" listItemId="a00" listType="todo" todoListChecked="true">foo</paragraph>'
+			);
+		} );
+
+		it( 'should convert nested li with a checkbox and a paragraph ( when checked )', () => {
+			testUpcast(
+				'<ul>' +
+					'<li>' +
+						'<input type="checkbox">' +
+						'<ul>' +
+							'<li>' +
+								'<input type="checkbox" checked="checked">' +
+								'<p>foo</p>' +
+							'</li>' +
+						'</ul>' +
+					'</li>' +
+				'</ul>',
+				'<paragraph listIndent="0" listItemId="a01" listType="todo"></paragraph>' +
+				'<paragraph listIndent="1" listItemId="a00" listType="todo" todoListChecked="true">foo</paragraph>'
+			);
+		} );
+
+		it( 'should not convert nested li if it was already consumed', () => {
+			editor.data.upcastDispatcher.on( 'element:li', ( evt, data, conversionApi ) => {
+				conversionApi.consumable.consume( data.viewItem, { name: true } );
+			}, { priority: 'highest' } );
+
+			editor.setData(
+				'<ul>' +
+					'<li>' +
+						'<input type="checkbox" checked="checked">' +
+						'<p>foo</p>' +
+					'</li>' +
+				'</ul>'
+			);
+
+			expect( getModelData( model, { withoutSelection: true } ) ).to.equal( '<paragraph></paragraph>' );
+		} );
+
 		it( 'should convert li with a checkbox and two paragraphs', () => {
 			testUpcast(
 				'<ul>' +