Kaynağa Gözat

Changed: improved IndentCommand to properly handle changing items type when indenting and outdenting.

Szymon Cofalik 8 yıl önce
ebeveyn
işleme
0f63a827fc

+ 48 - 15
packages/ckeditor5-list/src/indentcommand.js

@@ -82,22 +82,14 @@ export default class IndentCommand extends Command {
 				}
 				}
 				// If indent is >= 0, change the attribute value.
 				// If indent is >= 0, change the attribute value.
 				else {
 				else {
-					// Check whether list item's type should not be fixed.
-					// First, find previous sibling with same indent - if it exists.
-					let prev = item.previousSibling;
-
-					while ( prev && prev.is( 'listItem' ) && prev.getAttribute( 'indent' ) > indent ) {
-						prev = prev.previousSibling;
-					}
-
-					// Then check if that sibling has same type. If not, change type of this item.
-					if ( prev && prev.getAttribute( 'type' ) != item.getAttribute( 'type' ) ) {
-						batch.setAttribute( item, 'type', prev.getAttribute( 'type' ) );
-					}
-
 					batch.setAttribute( item, 'indent', indent );
 					batch.setAttribute( item, 'indent', indent );
 				}
 				}
 			}
 			}
+
+			// Check whether some of changed list items' type should not be fixed.
+			for ( let item of itemsToChange ) {
+				_fixType( item, batch );
+			}
 		} );
 		} );
 	}
 	}
 
 
@@ -115,13 +107,13 @@ export default class IndentCommand extends Command {
 
 
 		if ( this._indentBy > 0 ) {
 		if ( this._indentBy > 0 ) {
 			// Cannot indent first item in it's list. Check if before `listItem` is a list item that is in same list.
 			// Cannot indent first item in it's list. Check if before `listItem` is a list item that is in same list.
-			// To be in the same list, the item has to have same attributes.
+			// To be in the same list, the item has to have same attributes and cannot be "split" by an item with lower indent.
 			const indent = listItem.getAttribute( 'indent' );
 			const indent = listItem.getAttribute( 'indent' );
 			const type = listItem.getAttribute( 'type' );
 			const type = listItem.getAttribute( 'type' );
 
 
 			let prev = listItem.previousSibling;
 			let prev = listItem.previousSibling;
 
 
-			while ( prev && prev.is( 'listItem' ) ) {
+			while ( prev && prev.is( 'listItem' ) && prev.getAttribute( 'indent' ) >= indent ) {
 				if ( prev.getAttribute( 'indent' ) == indent && prev.getAttribute( 'type' ) == type ) {
 				if ( prev.getAttribute( 'indent' ) == indent && prev.getAttribute( 'type' ) == type ) {
 					return true;
 					return true;
 				}
 				}
@@ -137,3 +129,44 @@ export default class IndentCommand extends Command {
 		return true;
 		return true;
 	}
 	}
 }
 }
+
+// Fixes type of `item` element after it was indented/outdented. Looks for a sibling of `item` that has the same
+// indent and sets `item`'s type to the same as that sibling.
+function _fixType( item, batch ) {
+	// Find a preceding sibling of `item` that is a list item of the same list as `item`.
+	const prev = _seekListItem( item, false );
+
+	// If found, fix type.
+	if ( prev ) {
+		batch.setAttribute( item, 'type', prev.getAttribute( 'type' ) );
+
+		return;
+	}
+
+	// If not found, find a following sibling of `item` that is a list item of the same list as `item`.
+	const next = _seekListItem( item, true );
+
+	// If found, fix type.
+	if ( next ) {
+		batch.setAttribute( item, 'type', next.getAttribute( 'type' ) );
+	}
+}
+
+// Seeks for a list item that has same indent as given `item`. May look through next siblings (`seekForward = true`) or
+// previous siblings (`seekForward = false`). Returns found list item or `null` if item has not been found.
+function _seekListItem( item, seekForward ) {
+	let result = item[ seekForward ? 'nextSibling' : 'previousSibling' ];
+
+	// Look for the previous/next sibling that has same indent and is before a list item element with lower indent.
+	// If elements are split by an element with lower indent, they are on different lists.
+	while ( result && result.is( 'listItem' ) && result.getAttribute( 'indent' ) >= item.getAttribute( 'indent' ) ) {
+		if ( result.getAttribute( 'indent' ) == item.getAttribute( 'indent' ) ) {
+			// We found sibling that is on the same list.
+			return result;
+		}
+
+		result = result[ seekForward ? 'nextSibling' : 'previousSibling' ];
+	}
+
+	return null;
+}

+ 83 - 2
packages/ckeditor5-list/tests/indentcommand.js

@@ -67,6 +67,20 @@ describe( 'IndentCommand', () => {
 				expect( command.isEnabled ).to.be.false;
 				expect( command.isEnabled ).to.be.false;
 			} );
 			} );
 
 
+			// #53.
+			it( 'should be false if selection starts in first list item #2', () => {
+				setData(
+					doc,
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="bulleted">b</listItem>' +
+					'<listItem indent="0" type="bulleted">c</listItem>' +
+					'<listItem indent="1" type="bulleted">[]d</listItem>' +
+					'<listItem indent="2" type="bulleted">e</listItem>'
+				);
+
+				expect( command.isEnabled ).to.be.false;
+			} );
+
 			it( 'should be false if selection starts in first list item of top level list with different type than previous list', () => {
 			it( 'should be false if selection starts in first list item of top level list with different type than previous list', () => {
 				setData(
 				setData(
 					doc,
 					doc,
@@ -154,7 +168,7 @@ describe( 'IndentCommand', () => {
 				);
 				);
 			} );
 			} );
 
 
-			it( 'should fix list type when item is intended (if needed)', () => {
+			it( 'should fix list type when item is intended #1', () => {
 				setData(
 				setData(
 					doc,
 					doc,
 					'<listItem indent="0" type="bulleted">a</listItem>' +
 					'<listItem indent="0" type="bulleted">a</listItem>' +
@@ -172,6 +186,30 @@ describe( 'IndentCommand', () => {
 					'<listItem indent="2" type="numbered">d</listItem>'
 					'<listItem indent="2" type="numbered">d</listItem>'
 				);
 				);
 			} );
 			} );
+
+			// Not only boundary list items has to be fixed, but also items in the middle of selection.
+			it( 'should fix list type when item is intended #2', () => {
+				setData(
+					doc,
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="numbered">b</listItem>' +
+					'<listItem indent="1" type="numbered">[c</listItem>' +
+					'<listItem indent="0" type="bulleted">d</listItem>' +
+					'<listItem indent="1" type="numbered">e]</listItem>' +
+					'<listItem indent="0" type="bulleted">f</listItem>'
+				);
+
+				command._doExecute();
+
+				expect( getData( doc, { withoutSelection: true } ) ).to.equal(
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="numbered">b</listItem>' +
+					'<listItem indent="2" type="numbered">c</listItem>' +
+					'<listItem indent="1" type="numbered">d</listItem>' +
+					'<listItem indent="2" type="numbered">e</listItem>' +
+					'<listItem indent="0" type="bulleted">f</listItem>'
+				);
+			} );
 		} );
 		} );
 	} );
 	} );
 
 
@@ -290,7 +328,7 @@ describe( 'IndentCommand', () => {
 				);
 				);
 			} );
 			} );
 
 
-			it( 'should fix list type when item is outdented (if needed)', () => {
+			it( 'should fix list type when item is outdented #1', () => {
 				setData(
 				setData(
 					doc,
 					doc,
 					'<listItem indent="0" type="bulleted">a</listItem>' +
 					'<listItem indent="0" type="bulleted">a</listItem>' +
@@ -307,6 +345,26 @@ describe( 'IndentCommand', () => {
 				);
 				);
 			} );
 			} );
 
 
+			// Look at next siblings if, after outdenting, a list item is a first item in it's list (list item "c").
+			it( 'should fix list type when item is outdented #2', () => {
+				setData(
+					doc,
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="numbered">[]b</listItem>' +
+					'<listItem indent="2" type="bulleted">c</listItem>' +
+					'<listItem indent="1" type="numbered">d</listItem>'
+				);
+
+				command._doExecute();
+
+				expect( getData( doc, { withoutSelection: true } ) ).to.equal(
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="0" type="bulleted">b</listItem>' +
+					'<listItem indent="1" type="numbered">c</listItem>' +
+					'<listItem indent="1" type="numbered">d</listItem>'
+				);
+			} );
+
 			it( 'should fix list type when item is outdented to top level (if needed)', () => {
 			it( 'should fix list type when item is outdented to top level (if needed)', () => {
 				setData(
 				setData(
 					doc,
 					doc,
@@ -325,6 +383,29 @@ describe( 'IndentCommand', () => {
 					'<listItem indent="0" type="numbered">c</listItem>'
 					'<listItem indent="0" type="numbered">c</listItem>'
 				);
 				);
 			} );
 			} );
+
+			it( 'should not fix list type when not needed', () => {
+				// There was a bug that the nested sub-list changed it's type because for a while, it was on same indent
+				// level as the originally outdented element.
+				setData(
+					doc,
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="numbered">b</listItem>' +
+					'<listItem indent="1" type="numbered">[]c</listItem>' +
+					'<listItem indent="2" type="bulleted">d</listItem>' +
+					'<listItem indent="2" type="bulleted">e</listItem>'
+				);
+
+				command._doExecute();
+
+				expect( getData( doc, { withoutSelection: true } ) ).to.equal(
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="numbered">b</listItem>' +
+					'<listItem indent="0" type="bulleted">c</listItem>' +
+					'<listItem indent="1" type="bulleted">d</listItem>' +
+					'<listItem indent="1" type="bulleted">e</listItem>'
+				);
+			} );
 		} );
 		} );
 	} );
 	} );
 } );
 } );