Преглед на файлове

Change `todoListChecked` state using command when clicking on checkbox element.

Oskar Wróbel преди 6 години
родител
ревизия
919529f3cf

+ 6 - 0
packages/ckeditor5-list/src/todolistcheckcommand.js

@@ -37,6 +37,12 @@ export default class TodoListCheckCommand extends Command {
 		 * @private
 		 */
 		this._selectedElements = [];
+
+		// Refresh command before executing to be sure all values are up to date.
+		// It is needed when selection has changed before command execution, in the same change block.
+		this.on( 'execute', () => {
+			this.refresh();
+		}, { priority: 'highest' } );
 	}
 
 	/**

+ 13 - 18
packages/ckeditor5-list/src/todolistconverters.js

@@ -24,7 +24,7 @@ import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
  * @param {module:engine/model/model~Model} model Model instance.
  * @returns {Function} Returns a conversion callback.
  */
-export function modelViewInsertion( model ) {
+export function modelViewInsertion( model, onCheckboxChecked ) {
 	return ( evt, data, conversionApi ) => {
 		const consumable = conversionApi.consumable;
 
@@ -50,7 +50,7 @@ export function modelViewInsertion( model ) {
 		const viewItem = generateLiInUl( modelItem, conversionApi );
 
 		const isChecked = !!modelItem.getAttribute( 'todoListChecked' );
-		const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, model );
+		const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, onCheckboxChecked );
 
 		viewWriter.addClass( 'todo-list', viewItem.parent );
 		viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement );
@@ -228,17 +228,17 @@ export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) {
  * It is used by {@link module:engine/controller/editingcontroller~EditingController}.
  *
  * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute
- * @param {module:engine/model/model~Model} model Model instance.
+ * @param {Function} onCheckedChange Callback fired after clicking on checkmark element.
  * @returns {Function} Returns a conversion callback.
  */
-export function modelViewChangeType( model ) {
+export function modelViewChangeType( onCheckedChange ) {
 	return ( evt, data, conversionApi ) => {
 		const viewItem = conversionApi.mapper.toViewElement( data.item );
 		const viewWriter = conversionApi.writer;
 
 		if ( data.attributeNewValue == 'todo' ) {
 			const isChecked = !!data.item.getAttribute( 'todoListChecked' );
-			const checkmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, model );
+			const checkmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, onCheckedChange );
 
 			viewWriter.addClass( 'todo-list', viewItem.parent );
 			viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement );
@@ -257,10 +257,10 @@ export function modelViewChangeType( model ) {
  * It is used by {@link module:engine/controller/editingcontroller~EditingController}.
  *
  * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute
- * @param {module:engine/model/model~Model} model Model instance.
+ * @param {Function} onCheckedChange Callback fired after clicking on checkmark element.
  * @returns {Function} Returns a conversion callback.
  */
-export function modelViewChangeChecked( model ) {
+export function modelViewChangeChecked( onCheckedChange ) {
 	return ( evt, data, conversionApi ) => {
 		if ( !conversionApi.consumable.consume( data.item, 'attribute:todoListChecked' ) ) {
 			return;
@@ -271,7 +271,7 @@ export function modelViewChangeChecked( model ) {
 		const viewItem = mapper.toViewElement( data.item );
 		const itemRange = viewWriter.createRangeIn( viewItem );
 		const oldCheckmarkElement = findInRange( itemRange, item => item.is( 'uiElement' ) ? item : false );
-		const newCheckmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, model );
+		const newCheckmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, onCheckedChange );
 
 		viewWriter.insert( viewWriter.createPositionAfter( oldCheckmarkElement ), newCheckmarkElement );
 		viewWriter.remove( oldCheckmarkElement );
@@ -284,9 +284,9 @@ export function modelViewChangeChecked( model ) {
 // @param {module:engine/model/item~Item} modelItem
 // @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter
 // @param {Boolean} isChecked
-// @param {module:engine/model/model~Model} model
+// @param {Function} onChange
 // @returns {module:view/uielement~UIElement}
-function createCheckmarkElement( modelItem, viewWriter, isChecked, model ) {
+function createCheckmarkElement( modelItem, viewWriter, isChecked, onChange ) {
 	const uiElement = viewWriter.createUIElement(
 		'label',
 		{
@@ -298,14 +298,9 @@ function createCheckmarkElement( modelItem, viewWriter, isChecked, model ) {
 
 			checkbox.checked = isChecked;
 
-			checkbox.addEventListener( 'change', evt => {
-				model.change( writer => {
-					if ( evt.target.checked ) {
-						writer.setAttribute( 'todoListChecked', true, modelItem );
-					} else {
-						writer.removeAttribute( 'todoListChecked', modelItem );
-					}
-				} );
+			checkbox.addEventListener( 'mousedown', evt => {
+				evt.stopPropagation();
+				onChange( modelItem );
 			} );
 
 			const domElement = this.toDomElement( domDocument );

+ 36 - 3
packages/ckeditor5-list/src/todolistediting.js

@@ -68,13 +68,23 @@ export default class TodoListEditing extends Plugin {
 		editor.commands.add( 'todoListCheck', new TodoListCheckCommand( editor ) );
 
 		// Define converters.
-		editing.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( model ), { priority: 'high' } );
+		editing.downcastDispatcher.on(
+			'insert:listItem',
+			modelViewInsertion( model, listItem => this._handleCheckmarkChange( listItem ) ),
+			{ priority: 'high' }
+		);
 		editing.downcastDispatcher.on( 'insert:$text', modelViewTextInsertion, { priority: 'high' } );
 		data.downcastDispatcher.on( 'insert:listItem', dataModelViewInsertion( model ), { priority: 'high' } );
 		data.downcastDispatcher.on( 'insert:$text', dataModelViewTextInsertion, { priority: 'high' } );
 
-		editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) );
-		editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', modelViewChangeChecked( model ) );
+		editing.downcastDispatcher.on(
+			'attribute:listType:listItem',
+			modelViewChangeType( listItem => this._handleCheckmarkChange( listItem ) )
+		);
+		editing.downcastDispatcher.on(
+			'attribute:todoListChecked:listItem',
+			modelViewChangeChecked( listItem => this._handleCheckmarkChange( listItem ) )
+		);
 
 		data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } );
 
@@ -132,6 +142,29 @@ export default class TodoListEditing extends Plugin {
 			return hasChanged;
 		} );
 	}
+
+	/**
+	 * Handles checkmar element change, moves selection to the corresponding model item to makes possible
+	 * to toggle `todoListChecked` attribute using command and restore the selection position.
+	 *
+	 * Some say it's a hack :) Moving selection only for executing the command on a certain node and restoring it after,
+	 * it's not a clear solution. We need to design an API for using commands beyond the selection range.
+	 * See https://github.com/ckeditor/ckeditor5/issues/1954.
+	 *
+	 * @private
+	 * @param {module:engine/model/element~Element} listItem
+	 */
+	_handleCheckmarkChange( listItem ) {
+		const editor = this.editor;
+		const model = editor.model;
+		const previousSelectionRanges = Array.from( model.document.selection.getRanges() );
+
+		model.change( writer => {
+			writer.setSelection( listItem, 'end' );
+			editor.execute( 'todoListCheck' );
+			writer.setSelection( previousSelectionRanges );
+		} );
+	}
 }
 
 // Moves all uiElements in the todo list item after the checkmark element.

+ 4 - 0
packages/ckeditor5-list/tests/listediting.js

@@ -50,6 +50,10 @@ describe( 'ListEditing', () => {
 			} );
 	} );
 
+	afterEach( () => {
+		return editor.destroy();
+	} );
+
 	it( 'should be loaded', () => {
 		expect( editor.plugins.get( ListEditing ) ).to.be.instanceOf( ListEditing );
 	} );

+ 12 - 0
packages/ckeditor5-list/tests/todolistcheckcommand.js

@@ -204,5 +204,17 @@ describe( 'TodoListCheckCommand', () => {
 
 			expect( getModelData( model ) ).to.equal( '<paragraph>b[]ar</paragraph>' );
 		} );
+
+		it( 'should be up to date just before execution', () => {
+			setModelData( model,
+				'<listItem listIndent="0" listType="0">f[]oo</listItem>' +
+				'<listItem listIndent="0" listType="0">bar</listItem>'
+			);
+
+			model.change( writer => {
+				writer.setSelection( model.document.getRoot().getChild( 1 ), 'end' );
+				command.execute();
+			} );
+		} );
 	} );
 } );

+ 119 - 0
packages/ckeditor5-list/tests/todolistediting.js

@@ -12,10 +12,13 @@ import ListCommand from '../src/listcommand';
 import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 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';
 import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard';
 
+/* global Event, document */
+
 describe( 'TodoListEditing', () => {
 	let editor, model, modelDoc, modelRoot, view, viewDoc, viewRoot;
 
@@ -44,6 +47,10 @@ describe( 'TodoListEditing', () => {
 			} );
 	} );
 
+	afterEach( () => {
+		return editor.destroy();
+	} );
+
 	it( 'should load ListEditing', () => {
 		expect( TodoListEditing.requires ).to.have.members( [ ListEditing ] );
 	} );
@@ -654,6 +661,22 @@ describe( 'TodoListEditing', () => {
 				'<listItem listIndent="0" listType="todo">4.1</listItem>'
 			);
 		} );
+
+		it( 'should be overwritable', () => {
+			editor.data.upcastDispatcher.on( 'element:input', ( evt, data, conversionApi ) => {
+				conversionApi.consumable.consume( data.viewItem, { name: true } );
+				conversionApi.writer.setAttribute( 'listType', 'numbered', data.modelCursor.parent );
+				data.modelRange = conversionApi.writer.createRange( data.modelCursor );
+			}, { priority: 'highest' } );
+
+			editor.setData(
+				'<ul>' +
+					'<li><input type="checkbox">foo</li>' +
+				'</ul>'
+			);
+
+			expect( getModelData( model ) ).to.equal( '<listItem listIndent="0" listType="numbered">[]foo</listItem>' );
+		} );
 	} );
 
 	describe( 'selection view post-fixer', () => {
@@ -864,3 +887,99 @@ describe( 'TodoListEditing', () => {
 		} );
 	} );
 } );
+
+describe( 'TodoListEditing', () => {
+	let editorElement, editor, model, view, viewDoc, viewRoot;
+
+	beforeEach( () => {
+		editorElement = document.createElement( 'div' );
+		document.body.appendChild( editorElement );
+
+		return ClassicTestEditor
+			.create( editorElement, {
+				plugins: [ TodoListEditing ]
+			} )
+			.then( newEditor => {
+				editor = newEditor;
+
+				model = editor.model;
+
+				view = editor.editing.view;
+				viewDoc = view.document;
+				viewRoot = viewDoc.getRoot();
+			} );
+	} );
+
+	afterEach( () => {
+		editorElement.remove();
+
+		return editor.destroy();
+	} );
+
+	it( 'should render checkbox inside a checkmark UIElement', () => {
+		setModelData( model, '<listItem listIndent="0" listType="todo">foo</listItem>' );
+
+		const checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 );
+
+		expect( checkmarkViewElement.is( 'uiElement' ) ).to.equal( true );
+
+		const checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement );
+		const checkboxElement = checkmarkDomElement.children[ 0 ];
+
+		expect( checkboxElement.tagName ).to.equal( 'INPUT' );
+		expect( checkboxElement.checked ).to.equal( false );
+	} );
+
+	it( 'should render checked checkbox inside a checkmark UIElement', () => {
+		setModelData( model, '<listItem listIndent="0" listType="todo" todoListChecked="true">foo</listItem>' );
+
+		const checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 );
+		const checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement );
+		const checkboxElement = checkmarkDomElement.children[ 0 ];
+
+		expect( checkboxElement.checked ).to.equal( true );
+	} );
+
+	it( 'should toggle `todoListChecked` state using command when click on checkbox element', () => {
+		setModelData( model,
+			'<listItem listIndent="0" listType="todo">foo</listItem>' +
+			'<paragraph>b[a]r</paragraph>'
+		);
+
+		const command = editor.commands.get( 'todoListCheck' );
+
+		sinon.spy( command, 'execute' );
+
+		let checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 );
+		let checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement );
+		let checkboxElement = checkmarkDomElement.children[ 0 ];
+
+		expect( checkboxElement.checked ).to.equal( false );
+
+		checkboxElement.dispatchEvent( new Event( 'mousedown' ) );
+
+		checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 );
+		checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement );
+		checkboxElement = checkmarkDomElement.children[ 0 ];
+
+		sinon.assert.calledOnce( command.execute );
+		expect( checkboxElement.checked ).to.equal( true );
+		expect( getModelData( model ) ).to.equal(
+			'<listItem listIndent="0" listType="todo" todoListChecked="true">foo</listItem>' +
+			'<paragraph>b[a]r</paragraph>'
+		);
+
+		checkboxElement.dispatchEvent( new Event( 'mousedown' ) );
+
+		checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 );
+		checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement );
+		checkboxElement = checkmarkDomElement.children[ 0 ];
+
+		sinon.assert.calledTwice( command.execute );
+		expect( checkboxElement.checked ).to.equal( false );
+		expect( getModelData( model ) ).to.equal(
+			'<listItem listIndent="0" listType="todo">foo</listItem>' +
+			'<paragraph>b[a]r</paragraph>'
+		);
+	} );
+} );