From 2c01107b814e8c94177bac952e0c60e51d4c1702 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ella=20Van=C2=A0Durpe?= <iseulde@automattic.com>
Date: Tue, 29 Jan 2019 17:07:53 +0100
Subject: [PATCH] RichText: List: Fix getParentIndex (#13562)

* RichText: List: Fix getParentIndex

* Fill out test name

* Add unit tests for getParentLineIndex

* Guard against negative lineIndex
---
 packages/rich-text/src/change-list-type.js    |  5 ++-
 .../rich-text/src/get-parent-line-index.js    | 18 +++-----
 .../src/test/get-parent-line-index.js         | 43 +++++++++++++++++++
 .../rich-text/src/test/outdent-list-items.js  | 22 ++++++++++
 4 files changed, 75 insertions(+), 13 deletions(-)
 create mode 100644 packages/rich-text/src/test/get-parent-line-index.js

diff --git a/packages/rich-text/src/change-list-type.js b/packages/rich-text/src/change-list-type.js
index 2b92dda0ab92c..1dfc040657363 100644
--- a/packages/rich-text/src/change-list-type.js
+++ b/packages/rich-text/src/change-list-type.js
@@ -21,9 +21,10 @@ import { getParentLineIndex } from './get-parent-line-index';
  */
 export function changeListType( value, newFormat ) {
 	const { text, formats, start, end } = value;
-	const startLineFormats = formats[ getLineIndex( value, start ) ] || [];
+	const startingLineIndex = getLineIndex( value, start );
+	const startLineFormats = formats[ startingLineIndex ] || [];
 	const endLineFormats = formats[ getLineIndex( value, end ) ] || [];
-	const startIndex = getParentLineIndex( value, start );
+	const startIndex = getParentLineIndex( value, startingLineIndex );
 	const newFormats = formats.slice( 0 );
 	const startCount = startLineFormats.length - 1;
 	const endCount = endLineFormats.length - 1;
diff --git a/packages/rich-text/src/get-parent-line-index.js b/packages/rich-text/src/get-parent-line-index.js
index 332764dc45bd3..bd3f72de96519 100644
--- a/packages/rich-text/src/get-parent-line-index.js
+++ b/packages/rich-text/src/get-parent-line-index.js
@@ -9,27 +9,23 @@ import { LINE_SEPARATOR } from './special-characters';
  * go through every list item until we find one with exactly one format type
  * less.
  *
- * @param {Object} value      Value to search.
- * @param {number} startIndex Index to start search at.
+ * @param {Object} value     Value to search.
+ * @param {number} lineIndex Line index of a child list item.
  *
  * @return {Array} The parent list line index.
  */
-export function getParentLineIndex( { text, formats }, startIndex ) {
-	let index = startIndex;
-	let startFormats;
+export function getParentLineIndex( { text, formats }, lineIndex ) {
+	const startFormats = formats[ lineIndex ] || [];
 
-	while ( index-- ) {
+	let index = lineIndex;
+
+	while ( index-- >= 0 ) {
 		if ( text[ index ] !== LINE_SEPARATOR ) {
 			continue;
 		}
 
 		const formatsAtIndex = formats[ index ] || [];
 
-		if ( ! startFormats ) {
-			startFormats = formatsAtIndex;
-			continue;
-		}
-
 		if ( formatsAtIndex.length === startFormats.length - 1 ) {
 			return index;
 		}
diff --git a/packages/rich-text/src/test/get-parent-line-index.js b/packages/rich-text/src/test/get-parent-line-index.js
new file mode 100644
index 0000000000000..4e6a75ffd0a6e
--- /dev/null
+++ b/packages/rich-text/src/test/get-parent-line-index.js
@@ -0,0 +1,43 @@
+/**
+ * External dependencies
+ */
+import deepFreeze from 'deep-freeze';
+
+/**
+ * Internal dependencies
+ */
+
+import { getParentLineIndex } from '../get-parent-line-index';
+import { LINE_SEPARATOR } from '../special-characters';
+
+describe( 'getParentLineIndex', () => {
+	const ul = { type: 'ul' };
+
+	it( 'should return undefined if there is only one line', () => {
+		expect( getParentLineIndex( deepFreeze( {
+			formats: [ , ],
+			text: '1',
+		} ), undefined ) ).toBe( undefined );
+	} );
+
+	it( 'should return undefined if the list is part of the first root list child', () => {
+		expect( getParentLineIndex( deepFreeze( {
+			formats: [ , ],
+			text: `1${ LINE_SEPARATOR }2`,
+		} ), 2 ) ).toBe( undefined );
+	} );
+
+	it( 'should return the line index of the parent list (1)', () => {
+		expect( getParentLineIndex( deepFreeze( {
+			formats: [ , , , [ ul ], , ],
+			text: `1${ LINE_SEPARATOR }2${ LINE_SEPARATOR }3`,
+		} ), 3 ) ).toBe( 1 );
+	} );
+
+	it( 'should return the line index of the parent list (2)', () => {
+		expect( getParentLineIndex( deepFreeze( {
+			formats: [ , [ ul ], , [ ul, ul ], , [ ul ], , ],
+			text: `1${ LINE_SEPARATOR }2${ LINE_SEPARATOR }3${ LINE_SEPARATOR }4`,
+		} ), 5 ) ).toBe( undefined );
+	} );
+} );
diff --git a/packages/rich-text/src/test/outdent-list-items.js b/packages/rich-text/src/test/outdent-list-items.js
index afe1c09d03a31..c5b75d5d39188 100644
--- a/packages/rich-text/src/test/outdent-list-items.js
+++ b/packages/rich-text/src/test/outdent-list-items.js
@@ -115,4 +115,26 @@ describe( 'outdentListItems', () => {
 		expect( result ).not.toBe( record );
 		expect( getSparseArrayLength( result.formats ) ).toBe( 2 );
 	} );
+
+	it( 'should outdent list based on parent list', () => {
+		// As we're testing list formats, the text should remain the same.
+		const text = `1${ LINE_SEPARATOR }2${ LINE_SEPARATOR }3${ LINE_SEPARATOR }4`;
+		const record = {
+			formats: [ , [ ul ], , [ ul, ul ], , [ ul ], , ],
+			text,
+			start: 6,
+			end: 6,
+		};
+		const expected = {
+			formats: [ , [ ul ], , [ ul, ul ], , , , ],
+			text,
+			start: 6,
+			end: 6,
+		};
+		const result = outdentListItems( deepFreeze( record ) );
+
+		expect( result ).toEqual( expected );
+		expect( result ).not.toBe( record );
+		expect( getSparseArrayLength( result.formats ) ).toBe( 2 );
+	} );
 } );