RichText: remove DOM logic from list toolbar (WordPress#14840)

* RichText: remove DOM logic from list toolbar * Fix mobile side * Add e2e test for ordered split
mchowning · Apr 15, 2019 · 5c9d8f8 · 5c9d8f8
1 parent e4e2742
commit 5c9d8f8
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 111 deletions.
diff --git a/packages/block-editor/src/components/rich-text/list-edit.js b/packages/block-editor/src/components/rich-text/list-edit.js
@@ -9,6 +9,8 @@ import {
 	__unstableIndentListItems as indentListItems,
 	__unstableOutdentListItems as outdentListItems,
 	__unstableChangeListType as changeListType,
+	__unstableIsListRootSelected as isListRootSelected,
+	__unstableIsActiveListType as isActiveListType,
 } from '@wordpress/rich-text';
 
 /**
@@ -18,72 +20,6 @@ import {
 import { RichTextShortcut } from './shortcut';
 import BlockFormatControls from '../block-format-controls';
 
-const { TEXT_NODE, ELEMENT_NODE } = window.Node;
-
-/**
- * Gets the selected list node, which is the closest list node to the start of
- * the selection.
- *
- * @return {?Element} The selected list node, or undefined if none is selected.
- */
-function getSelectedListNode() {
-	const selection = window.getSelection();
-
-	if ( selection.rangeCount === 0 ) {
-		return;
-	}
-
-	let { startContainer } = selection.getRangeAt( 0 );
-
-	if ( startContainer.nodeType === TEXT_NODE ) {
-		startContainer = startContainer.parentNode;
-	}
-
-	if ( startContainer.nodeType !== ELEMENT_NODE ) {
-		return;
-	}
-
-	const rootNode = startContainer.closest( '*[contenteditable]' );
-
-	if ( ! rootNode || ! rootNode.contains( startContainer ) ) {
-		return;
-	}
-
-	return startContainer.closest( 'ol,ul' );
-}
-
-/**
- * Whether or not the root list is selected.
- *
- * @return {boolean} True if the root list or nothing is selected, false if an
- *                   inner list is selected.
- */
-function isListRootSelected() {
-	const listNode = getSelectedListNode();
-
-	// Consider the root list selected if nothing is selected.
-	return ! listNode || listNode.contentEditable === 'true';
-}
-
-/**
- * Wether or not the selected list has the given tag name.
- *
- * @param {string}  tagName     The tag name the list should have.
- * @param {string}  rootTagName The current root tag name, to compare with in
- *                              case nothing is selected.
- *
- * @return {boolean}             [description]
- */
-function isActiveListType( tagName, rootTagName ) {
-	const listNode = getSelectedListNode();
-
-	if ( ! listNode ) {
-		return tagName === rootTagName;
-	}
-
-	return listNode.nodeName.toLowerCase() === tagName;
-}
-
 export const ListEdit = ( {
 	onTagNameChange,
 	tagName,
@@ -125,23 +61,23 @@ export const ListEdit = ( {
 					onTagNameChange && {
 						icon: 'editor-ul',
 						title: __( 'Convert to unordered list' ),
-						isActive: isActiveListType( 'ul', tagName ),
+						isActive: isActiveListType( value, 'ul', tagName ),
 						onClick() {
 							onChange( changeListType( value, { type: 'ul' } ) );
 
-							if ( isListRootSelected() ) {
+							if ( isListRootSelected( value ) ) {
 								onTagNameChange( 'ul' );
 							}
 						},
 					},
 					onTagNameChange && {
 						icon: 'editor-ol',
 						title: __( 'Convert to ordered list' ),
-						isActive: isActiveListType( 'ol', tagName ),
+						isActive: isActiveListType( value, 'ol', tagName ),
 						onClick() {
 							onChange( changeListType( value, { type: 'ol' } ) );
 
-							if ( isListRootSelected() ) {
+							if ( isListRootSelected( value ) ) {
 								onTagNameChange( 'ol' );
 							}
 						},

diff --git a/packages/block-editor/src/components/rich-text/list-edit.native.js b/packages/block-editor/src/components/rich-text/list-edit.native.js
@@ -6,8 +6,8 @@ import { Toolbar } from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
 import {
 	changeListType,
-	__unstableIsListRootSelected,
-	__unstableIsActiveListType,
+	__unstableIsListRootSelected as isListRootSelected,
+	__unstableIsActiveListType as isActiveListType,
 } from '@wordpress/rich-text';
 
 /**
@@ -28,23 +28,23 @@ export const ListEdit = ( {
 				onTagNameChange && {
 					icon: 'editor-ul',
 					title: __( 'Convert to unordered list' ),
-					isActive: __unstableIsActiveListType( 'ul', tagName, value ),
+					isActive: isActiveListType( value, 'ul', tagName ),
 					onClick() {
 						onChange( changeListType( value, { type: 'ul' } ) );
 
-						if ( __unstableIsListRootSelected( value ) ) {
+						if ( isListRootSelected( value ) ) {
 							onTagNameChange( 'ul' );
 						}
 					},
 				},
 				onTagNameChange && {
 					icon: 'editor-ol',
 					title: __( 'Convert to ordered list' ),
-					isActive: __unstableIsActiveListType( 'ol', tagName, value ),
+					isActive: isActiveListType( value, 'ol', tagName ),
 					onClick() {
 						onChange( changeListType( value, { type: 'ol' } ) );
 
-						if ( __unstableIsListRootSelected( value ) ) {
+						if ( isListRootSelected( value ) ) {
 							onTagNameChange( 'ol' );
 						}
 					},

diff --git a/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap b/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap
@@ -218,6 +218,20 @@ exports[`List should split indented list item 1`] = `
 <!-- /wp:list -->"
 `;
 
+exports[`List should split into two ordered lists with paragraph 1`] = `
+"<!-- wp:list {\\"ordered\\":true} -->
+<ol><li>one</li></ol>
+<!-- /wp:list -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:list {\\"ordered\\":true} -->
+<ol><li>two</li></ol>
+<!-- /wp:list -->"
+`;
+
 exports[`List should split into two with paragraph and merge lists 1`] = `
 "<!-- wp:list -->
 <ul><li>one</li><li></li><li>two</li></ul>

diff --git a/packages/e2e-tests/specs/blocks/list.test.js b/packages/e2e-tests/specs/blocks/list.test.js
@@ -191,6 +191,18 @@ describe( 'List', () => {
 		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
 
+	it( 'should split into two ordered lists with paragraph', async () => {
+		await clickBlockAppender();
+		await page.keyboard.type( '1. one' );
+		await page.keyboard.press( 'Enter' );
+		await page.keyboard.type( 'two' );
+		await page.keyboard.press( 'ArrowUp' );
+		await page.keyboard.press( 'Enter' );
+		await page.keyboard.press( 'Enter' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
+
 	it( 'should split indented list item', async () => {
 		await insertBlock( 'List' );
 		await page.keyboard.type( 'one' );

diff --git a/packages/rich-text/src/get-line-list-formats.js b/packages/rich-text/src/get-line-list-formats.js
diff --git a/packages/rich-text/src/is-active-list-type.js b/packages/rich-text/src/is-active-list-type.js
@@ -2,25 +2,28 @@
  * Internal dependencies
  */
 
-import { getLineListFormats } from './get-line-list-formats';
+import { getLineIndex } from './get-line-index';
 
 /**
  * Wether or not the selected list has the given tag name.
  *
- * @param {string}  tagName     The tag name the list should have.
- * @param {string}  rootTagName The current root tag name, to compare with in
- *                              case nothing is selected.
- * @param {Object}  value       The internal rich-text value.
+ * @param {Object}  value    The value to check.
+ * @param {string}  type     The tag name the list should have.
+ * @param {string}  rootType The current root tag name, to compare with in case
+ *                           nothing is selected.
  *
- * @return {boolean}             [description]
+ * @return {boolean} True if the current list type matches `type`, false if not.
  */
-export function isActiveListType( tagName, rootTagName, value ) {
-	const startLineFormats = getLineListFormats( value );
-	const [ deepestListFormat ] = startLineFormats.slice( -1 );
+export function isActiveListType( value, type, rootType ) {
+	const { replacements, start } = value;
+	const lineIndex = getLineIndex( value, start );
+	const replacement = replacements[ lineIndex ];
 
-	if ( ! deepestListFormat || ! deepestListFormat.type ) {
-		return tagName === rootTagName;
+	if ( ! replacement || replacement.length === 0 ) {
+		return type === rootType;
 	}
 
-	return deepestListFormat.type.toLowerCase() === tagName;
+	const lastFormat = replacement[ replacement.length - 1 ];
+
+	return lastFormat.type === type;
 }
diff --git a/packages/rich-text/src/is-list-root-selected.js b/packages/rich-text/src/is-list-root-selected.js
@@ -2,17 +2,20 @@
  * Internal dependencies
  */
 
-import { getLineListFormats } from './get-line-list-formats';
+import { getLineIndex } from './get-line-index';
 
 /**
  * Whether or not the root list is selected.
  *
- * @param {Object}   value  The internal rich-text value.
+ * @param {Object} value The value to check.
  *
  * @return {boolean} True if the root list or nothing is selected, false if an
  *                   inner list is selected.
  */
 export function isListRootSelected( value ) {
-	const startLineFormats = getLineListFormats( value );
-	return startLineFormats.length < 1;
+	const { replacements, start } = value;
+	const lineIndex = getLineIndex( value, start );
+	const replacement = replacements[ lineIndex ];
+
+	return ! replacement || replacement.length < 1;
 }