Inserter: Fix 'Browse All' in Quick Inserter

Maintain the insertion point in @wordpress/block-editor state when moving from the Quick Inserter to the Inserter. This fixes a bug where the insertion point is wrong after clicking a block appender and selecting 'Browse All'. Care is taken to not regress a previous bug where the insertion point is wrong after clicking an inline insertion button and selecting 'Browse ALl'.
WordPress · Oct 26, 2020 · 6cbae2e · 6cbae2e
1 parent 07b3ab2
commit 6cbae2e
Show file tree

Hide file tree

Showing 11 changed files with 260 additions and 103 deletions.
diff --git a/docs/designers-developers/developers/data/data-core-block-editor.md b/docs/designers-developers/developers/data/data-core-block-editor.md
@@ -160,16 +160,22 @@ _Returns_
 
 <a name="getBlockInsertionPoint" href="#getBlockInsertionPoint">#</a> **getBlockInsertionPoint**
 
-Returns the insertion point, the index at which the new inserted block would
-be placed. Defaults to the last index.
+Returns the insertion point. This will be:
+
+1) The insertion point manually set using setInsertionPoint() or
+   showInsertionPoint(); or
+2) The point after the current block selection, if there is a selection; or
+3) The point at the end of the block list.
+
+Components like <Inserter> will default to inserting blocks at this point.
 
 _Parameters_
 
--   _state_ `Object`: Editor state.
+-   _state_ `Object`: Global application state.
 
 _Returns_
 
--   `Object`: Insertion point object with `rootClientId`, `index`.
+-   `Object`: Insertion point object with `rootClientId`, `index`, `isVisible`.
 
 <a name="getBlockListSettings" href="#getBlockListSettings">#</a> **getBlockListSettings**
 
@@ -812,15 +818,16 @@ _Returns_
 
 <a name="isBlockInsertionPointVisible" href="#isBlockInsertionPointVisible">#</a> **isBlockInsertionPointVisible**
 
-Returns true if we should show the block insertion point.
+Whether or not the insertion point should be shown to users. This is set
+using showInsertionPoint() or hideInsertionPoint().
 
 _Parameters_
 
 -   _state_ `Object`: Global application state.
 
 _Returns_
 
--   `?boolean`: Whether the insertion point is visible or not.
+-   `?boolean`: Whether the insertion point should be shown.
 
 <a name="isBlockMultiSelected" href="#isBlockMultiSelected">#</a> **isBlockMultiSelected**
 
@@ -1048,7 +1055,7 @@ _Parameters_
 
 <a name="hideInsertionPoint" href="#hideInsertionPoint">#</a> **hideInsertionPoint**
 
-Returns an action object hiding the insertion point.
+Hides the insertion point for users.
 
 _Returns_
 
@@ -1351,6 +1358,21 @@ _Parameters_
 -   _clientId_ `string`: The block's clientId.
 -   _hasControlledInnerBlocks_ `boolean`: True if the block's inner blocks are controlled.
 
+<a name="setInsertionPoint" href="#setInsertionPoint">#</a> **setInsertionPoint**
+
+Sets the insertion point without showing it to users.
+
+Components like <Inserter> will default to inserting blocks at this point.
+
+_Parameters_
+
+-   _rootClientId_ `?string`: Optional root client ID of block list on which to insert.
+-   _index_ `?number`: Index at which block should be inserted.
+
+_Returns_
+
+-   `Object`: Action object.
+
 <a name="setNavigationMode" href="#setNavigationMode">#</a> **setNavigationMode**
 
 Generators that triggers an action used to enable or disable the navigation mode.
@@ -1373,8 +1395,9 @@ _Returns_
 
 <a name="showInsertionPoint" href="#showInsertionPoint">#</a> **showInsertionPoint**
 
-Returns an action object used in signalling that the insertion point should
-be shown.
+Sets the insertion point and shows it to users.
+
+Components like <Inserter> will default to inserting blocks at this point.
 
 _Parameters_
 

diff --git a/packages/block-editor/src/components/inserter/hooks/use-insertion-point.js b/packages/block-editor/src/components/inserter/hooks/use-insertion-point.js
@@ -1,8 +1,3 @@
-/**
- * External dependencies
- */
-import { pick } from 'lodash';
-
 /**
  * WordPress dependencies
  */
@@ -14,10 +9,17 @@ import { speak } from '@wordpress/a11y';
 /**
  * @typedef WPInserterConfig
  *
- * @property {string=} rootClientId        Inserter Root Client ID.
- * @property {string=} clientId            Inserter Client ID.
- * @property {boolean} isAppender          Whether the inserter is an appender or not.
- * @property {boolean} selectBlockOnInsert Whether the block should be selected on insert.
+ * @property {string=}   rootClientId        If set, insertion will be into the
+ *                                           block with this ID.
+ * @property {number=}   insertionIndex      If set, insertion will be into this
+ *                                           explciit position.
+ * @property {string=}   clientId            If set, insertion will be after the
+ *                                           block with this ID.
+ * @property {boolean=}  isAppender          Whether the inserter is an appender
+ *                                           or not.
+ * @property {boolean=}  selectBlockOnInsert Whether the block should be
+ *                                           selected on insert.
+ * @property {Function=} onSelect            Called after insertion.
  */
 
 /**
@@ -27,77 +29,75 @@ import { speak } from '@wordpress/a11y';
  * @return {Array} Insertion Point State (rootClientID, onInsertBlocks and onToggle).
  */
 function useInsertionPoint( {
-	onSelect,
 	rootClientId,
+	insertionIndex,
 	clientId,
 	isAppender,
 	selectBlockOnInsert,
-	insertionIndex,
+	onSelect,
 } ) {
 	const {
+		selectedBlock,
 		destinationRootClientId,
-		getSelectedBlock,
-		getBlockIndex,
-		getBlockSelectionEnd,
-		getBlockOrder,
+		destinationIndex,
 	} = useSelect(
 		( select ) => {
 			const {
-				getSettings,
-				getBlockRootClientId,
-				getBlockSelectionEnd: _getBlockSelectionEnd,
+				getSelectedBlock,
+				getBlockIndex,
+				getBlockOrder,
+				getBlockInsertionPoint,
 			} = select( 'core/block-editor' );
 
-			let destRootClientId = rootClientId;
-			if ( ! destRootClientId && ! clientId && ! isAppender ) {
-				const end = _getBlockSelectionEnd();
-				if ( end ) {
-					destRootClientId = getBlockRootClientId( end );
+			let _destinationRootClientId, _destinationIndex;
+
+			if ( rootClientId || insertionIndex || clientId || isAppender ) {
+				// If any of these arguments are set, we're in "manual mode"
+				// meaning the insertion point is set by the caller.
+
+				_destinationRootClientId = rootClientId;
+
+				if ( insertionIndex ) {
+					// Insert into a specific index.
+					_destinationIndex = insertionIndex;
+				} else if ( clientId ) {
+					// Insert after a specific client ID.
+					_destinationIndex = getBlockIndex(
+						clientId,
+						_destinationRootClientId
+					);
+				} else {
+					// Insert at the end of the list.
+					_destinationIndex = getBlockOrder(
+						_destinationRootClientId
+					).length;
 				}
+			} else {
+				// Otherwise, we're in "auto mode" where the insertion point is
+				// decided by getBlockInsertionPoint().
+
+				_destinationRootClientId = getBlockInsertionPoint()
+					.rootClientId;
+				_destinationIndex = getBlockInsertionPoint().index;
 			}
+
 			return {
-				hasPatterns: !! getSettings().__experimentalBlockPatterns
-					?.length,
-				destinationRootClientId: destRootClientId,
-				...pick( select( 'core/block-editor' ), [
-					'getSelectedBlock',
-					'getBlockIndex',
-					'getBlockSelectionEnd',
-					'getBlockOrder',
-				] ),
+				selectedBlock: getSelectedBlock(),
+				destinationRootClientId: _destinationRootClientId,
+				destinationIndex: _destinationIndex,
 			};
 		},
-		[ isAppender, clientId, rootClientId ]
+		[ rootClientId, insertionIndex, clientId, isAppender ]
 	);
+
 	const {
 		replaceBlocks,
 		insertBlocks,
 		showInsertionPoint,
 		hideInsertionPoint,
 	} = useDispatch( 'core/block-editor' );
 
-	function getInsertionIndex() {
-		if ( insertionIndex !== undefined ) {
-			return insertionIndex;
-		}
-
-		// If the clientId is defined, we insert at the position of the block.
-		if ( clientId ) {
-			return getBlockIndex( clientId, destinationRootClientId );
-		}
-
-		// If there's a selected block, and the selected block is not the destination root block, we insert after the selected block.
-		const end = getBlockSelectionEnd();
-		if ( ! isAppender && end ) {
-			return getBlockIndex( end, destinationRootClientId ) + 1;
-		}
-
-		// Otherwise, we insert at the end of the current rootClientId
-		return getBlockOrder( destinationRootClientId ).length;
-	}
-
 	const onInsertBlocks = ( blocks, meta ) => {
-		const selectedBlock = getSelectedBlock();
 		if (
 			! isAppender &&
 			selectedBlock &&
@@ -107,7 +107,7 @@ function useInsertionPoint( {
 		} else {
 			insertBlocks(
 				blocks,
-				getInsertionIndex(),
+				destinationIndex,
 				destinationRootClientId,
 				selectBlockOnInsert,
 				meta
@@ -131,8 +131,7 @@ function useInsertionPoint( {
 
 	const onToggleInsertionPoint = ( show ) => {
 		if ( show ) {
-			const index = getInsertionIndex();
-			showInsertionPoint( destinationRootClientId, index );
+			showInsertionPoint( destinationRootClientId, destinationIndex );
 		} else {
 			hideInsertionPoint();
 		}

diff --git a/packages/block-editor/src/components/inserter/quick-inserter.js b/packages/block-editor/src/components/inserter/quick-inserter.js
@@ -154,26 +154,22 @@ export default function QuickInserter( {
 		[ filterValue, patterns ]
 	);
 
-	const setInserterIsOpened = useSelect(
-		( select ) =>
-			select( 'core/block-editor' ).getSettings()
+	const { setInserterIsOpened, blockIndex } = useSelect( ( select ) => {
+		const { getSettings, getBlockIndex } = select( 'core/block-editor' );
+		return {
+			setInserterIsOpened: getSettings()
 				.__experimentalSetIsInserterOpened,
-		[]
-	);
-
-	const previousBlockClientId = useSelect(
-		( select ) =>
-			select( 'core/block-editor' ).getPreviousBlockClientId( clientId ),
-		[ clientId ]
-	);
+			blockIndex: getBlockIndex( clientId, rootClientId ),
+		};
+	}, [] );
 
 	useEffect( () => {
 		if ( setInserterIsOpened ) {
 			setInserterIsOpened( false );
 		}
 	}, [ setInserterIsOpened ] );
 
-	const { selectBlock } = useDispatch( 'core/block-editor' );
+	const { setInsertionPoint } = useDispatch( 'core/block-editor' );
 
 	// Announce search results on change
 	useEffect( () => {
@@ -189,17 +185,9 @@ export default function QuickInserter( {
 		debouncedSpeak( resultsFoundMessage );
 	}, [ filterValue, debouncedSpeak ] );
 
-	// When clicking Browse All select the appropriate block so as
-	// the insertion point can work as expected
 	const onBrowseAll = () => {
-		// We have to select the previous block because the menu inserter
-		// inserts the new block after the selected one.
-		// Ideally, this selection shouldn't focus the block to avoid the setTimeout.
-		selectBlock( previousBlockClientId );
-		// eslint-disable-next-line @wordpress/react-no-unsafe-timeout
-		setTimeout( () => {
-			setInserterIsOpened( true );
-		} );
+		setInsertionPoint( rootClientId, blockIndex );
+		setInserterIsOpened( true );
 	};
 
 	// Disable reason (no-autofocus): The inserter menu is a modal display, not one which

diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js
@@ -523,8 +523,28 @@ export function* insertBlocks(
 }
 
 /**
- * Returns an action object used in signalling that the insertion point should
- * be shown.
+ * Sets the insertion point without showing it to users.
+ *
+ * Components like <Inserter> will default to inserting blocks at this point.
+ *
+ * @param {?string} rootClientId Optional root client ID of block list on
+ *                               which to insert.
+ * @param {?number} index        Index at which block should be inserted.
+ *
+ * @return {Object} Action object.
+ */
+export function setInsertionPoint( rootClientId, index ) {
+	return {
+		type: 'SET_INSERTION_POINT',
+		rootClientId,
+		index,
+	};
+}
+
+/**
+ * Sets the insertion point and shows it to users.
+ *
+ * Components like <Inserter> will default to inserting blocks at this point.
  *
  * @param {?string} rootClientId Optional root client ID of block list on
  *                               which to insert.
@@ -541,7 +561,7 @@ export function showInsertionPoint( rootClientId, index ) {
 }
 
 /**
- * Returns an action object hiding the insertion point.
+ * Hides the insertion point for users.
  *
  * @return {Object} Action object.
  */

diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js
@@ -1379,11 +1379,25 @@ export function blocksMode( state = {}, action ) {
  */
 export function insertionPoint( state = null, action ) {
 	switch ( action.type ) {
-		case 'SHOW_INSERTION_POINT':
+		case 'SET_INSERTION_POINT': {
 			const { rootClientId, index } = action;
-			return { rootClientId, index };
+			return { rootClientId, index, isVisible: false };
+		}
+
+		case 'SHOW_INSERTION_POINT': {
+			const { rootClientId, index } = action;
+			return { rootClientId, index, isVisible: true };
+		}
 
 		case 'HIDE_INSERTION_POINT':
+			return state ? { ...state, isVisible: false } : state;
+
+		case 'CLEAR_SELECTED_BLOCK':
+		case 'SELECT_BLOCK':
+		case 'REPLACE_INNER_BLOCKS':
+		case 'INSERT_BLOCKS':
+		case 'REMOVE_BLOCKS':
+		case 'REPLACE_BLOCKS':
 			return null;
 	}