Merge branch 'master' into i/7487-table-pasing-block-filler

docs/_snippets/examples/document-editor.html

@@ -195,12 +195,6 @@ <h3 style="text-align:center;">Welcome letter</h3>
 		margin-right: calc( 2 * var(--ck-spacing-large) );
 	}
 
-	/* Some table cells have a lot content and some not. Align them vertically
-	to make reading easier. */
-	.document-editor .ck-content table td {
-		vertical-align: middle;
-	}
-
 	@media only screen and (max-width: 960px) {
 		/* Because on mobile 2cm paddings are to big. */
 		.document-editor__editable-container .document-editor__editable.ck-editor__editable {

docs/_snippets/framework/tutorials/using-react-in-widget.html

@@ -141,10 +141,6 @@
 	animation: slideUp 0.3s ease;
 }
 
-.app .ck-content .table td {
-	vertical-align: middle;
-}
-
 @keyframes slideUp {
 	0% {
 		opacity: 0;

docs/assets/snippet-styles.css

@@ -232,6 +232,12 @@ It breaks CKEditor 5 (see how <p><code>[]</code></p> looks). */
 	display: table;
 }
 
+/* https://github.com/ckeditor/ckeditor5/issues/7310 */
+.live-snippet .ck.ck-content .table td,
+.live-snippet .ck.ck-content .table th {
+	vertical-align: middle;
+}
+
 /* https://github.com/ckeditor/ckeditor5/issues/1282 */
 .live-snippet .ck.ck-content .table p:first-child {
 	padding-top: 0;

packages/ckeditor5-core/tests/_utils-tests/assertions/attribute.js

@@ -54,7 +54,7 @@ describe( 'attribute chai assertion', () => {
 				hasAttribute: () => true,
 				getAttribute: () => 'bar'
 			} ).to.have.attribute( 'foo', 'baz' );
-		} ).to.throw( 'expected { Object (hasAttribute, getAttribute) } to have attribute \'foo\' of \'bar\', but got \'baz\'' );
+		} ).to.throw( 'expected { Object (hasAttribute, getAttribute) } to have attribute \'foo\' of \'baz\', but got \'bar\'' );
 	} );
 
 	it( 'negated, should assert for the given type the \'target.getAttribute\' returns a value different than the given one', () => {

packages/ckeditor5-core/tests/_utils/assertions/attribute.js

@@ -54,8 +54,8 @@ chai.Assertion.addMethod( 'attribute', function attributeAssertion( key, value,
 			attributeValue === value,
 			`expected #{this} to have attribute '${ key }' of #{exp}, but got #{act}`,
 			`expected #{this} to not have attribute '${ key }' of #{exp}`,
-			attributeValue,
-			value
+			value,
+			attributeValue
 		);
 	}
 } );

packages/ckeditor5-engine/src/conversion/mapper.js

@@ -104,7 +104,7 @@ export default class Mapper {
 
 			const viewContainer = this._modelToViewMapping.get( data.modelPosition.parent );
 
-			data.viewPosition = this._findPositionIn( viewContainer, data.modelPosition.offset );
+			data.viewPosition = this.findPositionIn( viewContainer, data.modelPosition.offset );
 		}, { priority: 'low' } );
 
 		// Default mapper algorithm for mapping view position to model position.
@@ -510,25 +510,24 @@ export default class Mapper {
 	 *
 	 *		<p>fo<b>bar</b>bom</p> -> expected offset: 4
 	 *
-	 *		_findPositionIn( p, 4 ):
+	 *		findPositionIn( p, 4 ):
 	 *		<p>|fo<b>bar</b>bom</p> -> expected offset: 4, actual offset: 0
 	 *		<p>fo|<b>bar</b>bom</p> -> expected offset: 4, actual offset: 2
 	 *		<p>fo<b>bar</b>|bom</p> -> expected offset: 4, actual offset: 5 -> we are too far
 	 *
-	 *		_findPositionIn( b, 4 - ( 5 - 3 ) ):
+	 *		findPositionIn( b, 4 - ( 5 - 3 ) ):
 	 *		<p>fo<b>|bar</b>bom</p> -> expected offset: 2, actual offset: 0
 	 *		<p>fo<b>bar|</b>bom</p> -> expected offset: 2, actual offset: 3 -> we are too far
 	 *
-	 *		_findPositionIn( bar, 2 - ( 3 - 3 ) ):
+	 *		findPositionIn( bar, 2 - ( 3 - 3 ) ):
 	 *		We are in the text node so we can simple find the offset.
 	 *		<p>fo<b>ba|r</b>bom</p> -> expected offset: 2, actual offset: 2 -> position found
 	 *
-	 * @private
 	 * @param {module:engine/view/element~Element} viewParent Tree view element in which we are looking for the position.
 	 * @param {Number} expectedOffset Expected offset.
 	 * @returns {module:engine/view/position~Position} Found position.
 	 */
-	_findPositionIn( viewParent, expectedOffset ) {
+	findPositionIn( viewParent, expectedOffset ) {
 		// Last scanned view node.
 		let viewNode;
 		// Length of the last scanned view node.
@@ -560,7 +559,7 @@ export default class Mapper {
 		else {
 			// ( modelOffset - lastLength ) is the offset to the child we enter,
 			// so we subtract it from the expected offset to fine the offset in the child.
-			return this._findPositionIn( viewNode, expectedOffset - ( modelOffset - lastLength ) );
+			return this.findPositionIn( viewNode, expectedOffset - ( modelOffset - lastLength ) );
 		}
 	}
 

packages/ckeditor5-engine/tests/conversion/mapper.js

@@ -536,7 +536,7 @@ describe( 'Mapper', () => {
 			it( 'should transform viewTextFOO 3', () => createToModelTest( viewTextFOO, 3, modelCaption, 3 ) );
 		} );
 
-		describe( 'toViewPosition', () => {
+		describe( 'toViewPosition and findPositionIn', () => {
 			it( 'should transform modelDiv 0', () => createToViewTest( modelDiv, 0, viewTextX, 0 ) );
 			it( 'should transform modelDiv 1', () => createToViewTest( modelDiv, 1, viewTextX, 1 ) );
 			it( 'should transform modelDiv 2', () => createToViewTest( modelDiv, 2, viewTextZZ, 0 ) );
@@ -553,7 +553,11 @@ describe( 'Mapper', () => {
 
 		function createToViewTest( modelElement, modelOffset, viewElement, viewOffset ) {
 			const modelPosition = ModelPosition._createAt( modelElement, modelOffset );
-			const viewPosition = mapper.toViewPosition( modelPosition );
+			let viewPosition = mapper.toViewPosition( modelPosition );
+			expect( viewPosition.parent ).to.equal( viewElement );
+			expect( viewPosition.offset ).to.equal( viewOffset );
+
+			viewPosition = mapper.findPositionIn( mapper.toViewElement( modelElement ), modelOffset );
 			expect( viewPosition.parent ).to.equal( viewElement );
 			expect( viewPosition.offset ).to.equal( viewOffset );
 		}

packages/ckeditor5-list/src/todolistconverters.js

@@ -9,7 +9,7 @@
 
 /* global document */
 
-import { generateLiInUl, injectViewList } from './utils';
+import { generateLiInUl, injectViewList, positionAfterUiElements, findNestedList } from './utils';
 import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
 
 /**
@@ -22,6 +22,7 @@ import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
  *
  * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert
  * @param {module:engine/model/model~Model} model Model instance.
+ * @param {Function} onCheckboxChecked Callback function.
  * @returns {Function} Returns a conversion callback.
  */
 export function modelViewInsertion( model, onCheckboxChecked ) {
@@ -52,8 +53,13 @@ export function modelViewInsertion( model, onCheckboxChecked ) {
 		const isChecked = !!modelItem.getAttribute( 'todoListChecked' );
 		const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, onCheckboxChecked );
 
+		const span = viewWriter.createContainerElement( 'span', {
+			class: 'todo-list__label__description'
+		} );
+
 		viewWriter.addClass( 'todo-list', viewItem.parent );
 		viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement );
+		viewWriter.insert( viewWriter.createPositionAfter( checkmarkElement ), span );
 
 		injectViewList( modelItem, viewItem, conversionApi, model );
 	};
@@ -83,17 +89,19 @@ export function dataModelViewInsertion( model ) {
 			return;
 		}
 
-		consumable.consume( data.item, 'insert' );
-		consumable.consume( data.item, 'attribute:listType' );
-		consumable.consume( data.item, 'attribute:listIndent' );
+		const modelItem = data.item;
+
+		consumable.consume( modelItem, 'insert' );
+		consumable.consume( modelItem, 'attribute:listType' );
+		consumable.consume( modelItem, 'attribute:listIndent' );
+		consumable.consume( modelItem, 'attribute:todoListChecked' );
 
 		const viewWriter = conversionApi.writer;
-		const modelItem = data.item;
 		const viewItem = generateLiInUl( modelItem, conversionApi );
 
 		viewWriter.addClass( 'todo-list', viewItem.parent );
 
-		const label = viewWriter.createAttributeElement( 'label', {
+		const label = viewWriter.createContainerElement( 'label', {
 			class: 'todo-list__label'
 		} );
 
@@ -102,51 +110,22 @@ export function dataModelViewInsertion( model ) {
 			disabled: 'disabled'
 		} );
 
-		if ( data.item.getAttribute( 'todoListChecked' ) ) {
+		const span = viewWriter.createContainerElement( 'span', {
+			class: 'todo-list__label__description'
+		} );
+
+		if ( modelItem.getAttribute( 'todoListChecked' ) ) {
 			viewWriter.setAttribute( 'checked', 'checked', checkbox );
-			viewWriter.addClass( 'todo-list__label', label );
 		}
 
-		viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkbox );
-		viewWriter.wrap( viewWriter.createRangeOn( checkbox ), label );
+		viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), label );
+		viewWriter.insert( viewWriter.createPositionAt( label, 0 ), checkbox );
+		viewWriter.insert( viewWriter.createPositionAfter( checkbox ), span );
 
 		injectViewList( modelItem, viewItem, conversionApi, model );
 	};
 }
 
-/**
- * A model-to-view converter for the model `$text` element inside a to-do list item.
- *
- * It is used by {@link module:engine/controller/datacontroller~DataController}.
- *
- * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert
- * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event.
- * @param {Object} data Additional information about the change.
- * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface.
- */
-export function dataModelViewTextInsertion( evt, data, conversionApi ) {
-	const parent = data.range.start.parent;
-
-	if ( parent.name != 'listItem' || parent.getAttribute( 'listType' ) != 'todo' ) {
-		return;
-	}
-
-	if ( !conversionApi.consumable.consume( data.item, 'insert' ) ) {
-		return;
-	}
-
-	const viewWriter = conversionApi.writer;
-	const viewPosition = conversionApi.mapper.toViewPosition( data.range.start );
-	const viewText = viewWriter.createText( data.item.data );
-
-	const span = viewWriter.createAttributeElement( 'span', { class: 'todo-list__label__description' } );
-	const label = viewPosition.parent.getChild( 0 );
-
-	viewWriter.insert( viewWriter.createPositionAt( viewPosition.parent, 'end' ), viewText );
-	viewWriter.wrap( viewWriter.createRangeOn( viewText ), span );
-	viewWriter.wrap( viewWriter.createRangeOn( viewText.parent ), label );
-}
-
 /**
  * A view-to-model converter for the checkbox element inside a view list item.
  *
@@ -198,22 +177,42 @@ export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) {
  *
  * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute
  * @param {Function} onCheckedChange Callback fired after clicking the checkbox UI element.
+ * @param {module:engine/view/view~View} view Editing view controller.
  * @returns {Function} Returns a conversion callback.
  */
 export function modelViewChangeType( onCheckedChange, view ) {
 	return ( evt, data, conversionApi ) => {
 		const viewItem = conversionApi.mapper.toViewElement( data.item );
 		const viewWriter = conversionApi.writer;
 
+		const labelElement = findLabel( viewItem, view );
+
 		if ( data.attributeNewValue == 'todo' ) {
 			const isChecked = !!data.item.getAttribute( 'todoListChecked' );
 			const checkmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, onCheckedChange );
 
+			const span = viewWriter.createContainerElement( 'span', {
+				class: 'todo-list__label__description'
+			} );
+
+			const itemRange = viewWriter.createRangeIn( viewItem );
+			const nestedList = findNestedList( viewItem );
+
+			const descriptionStart = positionAfterUiElements( itemRange.start );
+			const descriptionEnd = nestedList ? viewWriter.createPositionBefore( nestedList ) : itemRange.end;
+			const descriptionRange = viewWriter.createRange( descriptionStart, descriptionEnd );
+
 			viewWriter.addClass( 'todo-list', viewItem.parent );
+			viewWriter.move( descriptionRange, viewWriter.createPositionAt( span, 0 ) );
 			viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement );
+			viewWriter.insert( viewWriter.createPositionAfter( checkmarkElement ), span );
 		} else if ( data.attributeOldValue == 'todo' ) {
+			const descriptionSpan = findDescription( viewItem, view );
+
 			viewWriter.removeClass( 'todo-list', viewItem.parent );
-			viewWriter.remove( findLabel( viewItem, view ) );
+			viewWriter.remove( labelElement );
+			viewWriter.move( viewWriter.createRangeIn( descriptionSpan ), viewWriter.createPositionBefore( descriptionSpan ) );
+			viewWriter.remove( descriptionSpan );
 		}
 	};
 }
@@ -261,34 +260,22 @@ export function modelViewChangeChecked( onCheckedChange ) {
  * It only handles the position at the beginning of a list item as other positions are properly mapped be the default mapper.
  *
  * @param {module:engine/view/view~View} view
- * @param {module:engine/conversion/mapper~Mapper} mapper
  * @return {Function}
  */
-export function mapModelToViewZeroOffsetPosition( view, mapper ) {
+export function mapModelToViewPosition( view ) {
 	return ( evt, data ) => {
 		const modelPosition = data.modelPosition;
 		const parent = modelPosition.parent;
 
-		// Handle only position at the beginning of a todo list item.
-		if ( !parent.is( 'listItem' ) || parent.getAttribute( 'listType' ) != 'todo' || modelPosition.offset !== 0 ) {
+		if ( !parent.is( 'listItem' ) || parent.getAttribute( 'listType' ) != 'todo' ) {
 			return;
 		}
 
-		const viewLi = mapper.toViewElement( parent );
-		const label = findLabel( viewLi, view );
+		const viewLi = data.mapper.toViewElement( parent );
+		const descSpan = findDescription( viewLi, view );
 
-		// If there is no label then most probably the default converter was overridden.
-		if ( !label ) {
-			return;
-		}
-
-		// Map the position to the next sibling (if it is not a marker) - most likely it will be a text node...
-		if ( label.nextSibling && !label.nextSibling.is( 'uiElement' ) ) {
-			data.viewPosition = view.createPositionAt( label.nextSibling, 0 );
-		}
-		// ... otherwise return position after the label.
-		else {
-			data.viewPosition = view.createPositionAfter( label );
+		if ( descSpan ) {
+			data.viewPosition = data.mapper.findPositionIn( descSpan, modelPosition.offset );
 		}
 	};
 }
@@ -338,3 +325,13 @@ function findLabel( viewItem, view ) {
 		}
 	}
 }
+
+function findDescription( viewItem, view ) {
+	const range = view.createRangeIn( viewItem );
+
+	for ( const value of range ) {
+		if ( value.item.is( 'containerElement', 'span' ) && value.item.hasClass( 'todo-list__label__description' ) ) {
+			return value.item;
+		}
+	}
+}