Fix blue line indicator not showing at the end (#25849)

* Fix insertion blue line not showing at the end * Adjust margin-top * Add a e2e test * Use a different approach with rootClientId and query selector * Use appender map and context * Rename to AppenderNodes
WordPress · Oct 8, 2020 · e376eab · e376eab
1 parent 1a34926
commit e376eab
Show file tree

Hide file tree

Showing 6 changed files with 155 additions and 72 deletions.
diff --git a/packages/block-editor/src/components/block-list-appender/index.js b/packages/block-editor/src/components/block-list-appender/index.js
@@ -7,6 +7,7 @@ import classnames from 'classnames';
 /**
  * WordPress dependencies
  */
+import { createContext, useContext } from '@wordpress/element';
 import { withSelect } from '@wordpress/data';
 import { getDefaultBlockName } from '@wordpress/blocks';
 
@@ -16,6 +17,9 @@ import { getDefaultBlockName } from '@wordpress/blocks';
 import DefaultBlockAppender from '../default-block-appender';
 import ButtonBlockAppender from '../button-block-appender';
 
+// A Context to store the map of the appender map.
+export const AppenderNodesContext = createContext();
+
 function stopPropagation( event ) {
 	event.stopPropagation();
 }
@@ -30,6 +34,8 @@ function BlockListAppender( {
 	selectedBlockClientId,
 	tagName: TagName = 'div',
 } ) {
+	const appenderNodesMap = useContext( AppenderNodesContext );
+
 	if ( isLocked || CustomAppender === false ) {
 		return null;
 	}
@@ -89,7 +95,20 @@ function BlockListAppender( {
 			// Prevent the block from being selected when the appender is
 			// clicked.
 			onFocus={ stopPropagation }
-			className={ classnames( 'block-list-appender', className ) }
+			className={ classnames(
+				'block-list-appender',
+				'wp-block',
+				className
+			) }
+			ref={ ( ref ) => {
+				if ( ref ) {
+					// Set the reference of the "Appender" with `rootClientId` as key.
+					appenderNodesMap.set( rootClientId || '', ref );
+				} else {
+					// If it un-mounts, cleanup the map.
+					appenderNodesMap.delete( rootClientId || '' );
+				}
+			} }
 		>
 			{ appender }
 		</TagName>

diff --git a/packages/block-editor/src/components/block-list/index.js b/packages/block-editor/src/components/block-list/index.js
@@ -13,10 +13,17 @@ import { useRef, forwardRef } from '@wordpress/element';
  * Internal dependencies
  */
 import BlockListBlock from './block';
-import BlockListAppender from '../block-list-appender';
+import BlockListAppender, {
+	AppenderNodesContext,
+} from '../block-list-appender';
 import RootContainer from './root-container';
 import useBlockDropZone from '../use-block-drop-zone';
 
+/**
+ * A map to store the reference of each "Appenders" rendered with `rootClientId` as key.
+ */
+const appenderNodesMap = new Map();
+
 /**
  * If the block count exceeds the threshold, we disable the reordering animation
  * to avoid laginess.
@@ -82,57 +89,60 @@ function BlockList(
 		dropTargetIndex === blockClientIds.length && isDraggingBlocks;
 
 	return (
-		<Container
-			{ ...__experimentalPassedProps }
-			ref={ element }
-			className={ classnames(
-				'block-editor-block-list__layout',
-				className,
-				__experimentalPassedProps.className
-			) }
-		>
-			{ blockClientIds.map( ( clientId, index ) => {
-				const isBlockInSelection = hasMultiSelection
-					? multiSelectedBlockClientIds.includes( clientId )
-					: selectedBlockClientId === clientId;
+		<AppenderNodesContext.Provider value={ appenderNodesMap }>
+			<Container
+				{ ...__experimentalPassedProps }
+				ref={ element }
+				className={ classnames(
+					'block-editor-block-list__layout',
+					className,
+					__experimentalPassedProps.className
+				) }
+			>
+				{ blockClientIds.map( ( clientId, index ) => {
+					const isBlockInSelection = hasMultiSelection
+						? multiSelectedBlockClientIds.includes( clientId )
+						: selectedBlockClientId === clientId;
 
-				const isDropTarget =
-					dropTargetIndex === index && isDraggingBlocks;
+					const isDropTarget =
+						dropTargetIndex === index && isDraggingBlocks;
 
-				return (
-					<AsyncModeProvider
-						key={ clientId }
-						value={ ! isBlockInSelection }
-					>
-						<BlockListBlock
-							rootClientId={ rootClientId }
-							clientId={ clientId }
-							// This prop is explicitely computed and passed down
-							// to avoid being impacted by the async mode
-							// otherwise there might be a small delay to trigger the animation.
-							index={ index }
-							enableAnimation={ enableAnimation }
-							className={ classnames( {
-								'is-drop-target': isDropTarget,
-								'is-dropping-horizontally':
-									isDropTarget &&
-									orientation === 'horizontal',
-							} ) }
-						/>
-					</AsyncModeProvider>
-				);
-			} ) }
-			<BlockListAppender
-				tagName={ __experimentalAppenderTagName }
-				rootClientId={ rootClientId }
-				renderAppender={ renderAppender }
-				className={ classnames( {
-					'is-drop-target': isAppenderDropTarget,
-					'is-dropping-horizontally':
-						isAppenderDropTarget && orientation === 'horizontal',
+					return (
+						<AsyncModeProvider
+							key={ clientId }
+							value={ ! isBlockInSelection }
+						>
+							<BlockListBlock
+								rootClientId={ rootClientId }
+								clientId={ clientId }
+								// This prop is explicitely computed and passed down
+								// to avoid being impacted by the async mode
+								// otherwise there might be a small delay to trigger the animation.
+								index={ index }
+								enableAnimation={ enableAnimation }
+								className={ classnames( {
+									'is-drop-target': isDropTarget,
+									'is-dropping-horizontally':
+										isDropTarget &&
+										orientation === 'horizontal',
+								} ) }
+							/>
+						</AsyncModeProvider>
+					);
 				} ) }
-			/>
-		</Container>
+				<BlockListAppender
+					tagName={ __experimentalAppenderTagName }
+					rootClientId={ rootClientId }
+					renderAppender={ renderAppender }
+					className={ classnames( {
+						'is-drop-target': isAppenderDropTarget,
+						'is-dropping-horizontally':
+							isAppenderDropTarget &&
+							orientation === 'horizontal',
+					} ) }
+				/>
+			</Container>
+		</AppenderNodesContext.Provider>
 	);
 }
 

diff --git a/packages/block-editor/src/components/block-list/insertion-point.js b/packages/block-editor/src/components/block-list/insertion-point.js
@@ -7,7 +7,7 @@ import classnames from 'classnames';
  * WordPress dependencies
  */
 import { useSelect } from '@wordpress/data';
-import { useState, useRef } from '@wordpress/element';
+import { useState, useRef, useMemo, useContext } from '@wordpress/element';
 import { Popover } from '@wordpress/components';
 import { placeCaretAtVerticalEdge } from '@wordpress/dom';
 
@@ -17,6 +17,7 @@ import { placeCaretAtVerticalEdge } from '@wordpress/dom';
 import Inserter from '../inserter';
 import { getClosestTabbable } from '../writing-flow';
 import { getBlockDOMNode } from '../../utils/dom';
+import { AppenderNodesContext } from '../block-list-appender';
 
 function InsertionPointInserter( {
 	clientId,
@@ -98,13 +99,24 @@ function InsertionPointInserter( {
 
 function InsertionPointPopover( {
 	clientId,
+	rootClientId,
 	isInserterShown,
 	isInserterForced,
 	setIsInserterForced,
 	containerRef,
 	showInsertionPoint,
 } ) {
-	const element = getBlockDOMNode( clientId );
+	const appenderNodesMap = useContext( AppenderNodesContext );
+	const element = useMemo( () => {
+		if ( clientId ) {
+			return getBlockDOMNode( clientId );
+		}
+
+		// Can't find the element, might be at the end of the block list, or inside an empty block list.
+		// We instead try to find the "Appender" and place the indicator above it.
+		// `rootClientId` could be null or undefined when there's no parent block, we normalize it to an empty string.
+		return appenderNodesMap.get( rootClientId || '' );
+	}, [ clientId, rootClientId, appenderNodesMap ] );
 
 	return (
 		<Popover
@@ -139,26 +151,29 @@ export default function InsertionPoint( { children, containerRef } ) {
 	const [ isInserterShown, setIsInserterShown ] = useState( false );
 	const [ isInserterForced, setIsInserterForced ] = useState( false );
 	const [ inserterClientId, setInserterClientId ] = useState( null );
-	const { isMultiSelecting, isInserterVisible, selectedClientId } = useSelect(
-		( select ) => {
-			const {
-				isMultiSelecting: _isMultiSelecting,
-				isBlockInsertionPointVisible,
-				getBlockInsertionPoint,
-				getBlockOrder,
-			} = select( 'core/block-editor' );
-
-			const insertionPoint = getBlockInsertionPoint();
-			const order = getBlockOrder( insertionPoint.rootClientId );
-
-			return {
-				isMultiSelecting: _isMultiSelecting(),
-				isInserterVisible: isBlockInsertionPointVisible(),
-				selectedClientId: order[ insertionPoint.index ],
-			};
-		},
-		[]
-	);
+	const {
+		isMultiSelecting,
+		isInserterVisible,
+		selectedClientId,
+		selectedRootClientId,
+	} = useSelect( ( select ) => {
+		const {
+			isMultiSelecting: _isMultiSelecting,
+			isBlockInsertionPointVisible,
+			getBlockInsertionPoint,
+			getBlockOrder,
+		} = select( 'core/block-editor' );
+
+		const insertionPoint = getBlockInsertionPoint();
+		const order = getBlockOrder( insertionPoint.rootClientId );
+
+		return {
+			isMultiSelecting: _isMultiSelecting(),
+			isInserterVisible: isBlockInsertionPointVisible(),
+			selectedClientId: order[ insertionPoint.index ],
+			selectedRootClientId: insertionPoint.rootClientId,
+		};
+	}, [] );
 
 	function onMouseMove( event ) {
 		if (
@@ -215,6 +230,7 @@ export default function InsertionPoint( { children, containerRef } ) {
 					clientId={
 						isInserterVisible ? selectedClientId : inserterClientId
 					}
+					rootClientId={ selectedRootClientId }
 					isInserterShown={ isInserterShown }
 					isInserterForced={ isInserterForced }
 					setIsInserterForced={ setIsInserterForced }

diff --git a/packages/block-editor/src/components/block-list/style.scss b/packages/block-editor/src/components/block-list/style.scss
@@ -365,6 +365,10 @@
 	position: relative;
 	z-index: z-index(".block-editor-block-list__insertion-point");
 	margin-top: -$block-padding * 2;
+
+	&.is-insert-after {
+		margin-top: $block-padding;
+	}
 }
 
 .block-editor-block-list__insertion-point-indicator {

diff --git a/packages/e2e-tests/specs/editor/various/adding-blocks.test.js b/packages/e2e-tests/specs/editor/various/adding-blocks.test.js
@@ -8,6 +8,7 @@ import {
 	pressKeyTimes,
 	setBrowserViewport,
 	closeGlobalBlockInserter,
+	searchForBlock,
 } from '@wordpress/e2e-test-utils';
 
 /** @typedef {import('puppeteer').ElementHandle} ElementHandle */
@@ -262,4 +263,36 @@ describe( 'adding blocks', () => {
 		await coverBlock.click();
 		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
+
+	// Check for regression of https://github.com/WordPress/gutenberg/issues/25785
+	it( 'inserts a block should show a blue line indicator', async () => {
+		// First insert a random Paragraph.
+		await insertBlock( 'Paragraph' );
+		await page.keyboard.type( 'First paragraph' );
+
+		// Open the global inserter and search for the Heading block.
+		await searchForBlock( 'Heading' );
+
+		const headingButton = (
+			await page.$x( `//button//span[contains(text(), 'Heading')]` )
+		 )[ 0 ];
+
+		// Hover over the block should show the blue line indicator.
+		await headingButton.hover();
+
+		// Should show the blue line indicator somewhere.
+		const indicator = await page.$(
+			'.block-editor-block-list__insertion-point-indicator'
+		);
+		const indicatorRect = await indicator.boundingBox();
+
+		const paragraphBlock = await page.$(
+			'p[aria-label="Paragraph block"]'
+		);
+		const paragraphRect = await paragraphBlock.boundingBox();
+
+		// The blue line indicator should be below the last block.
+		expect( indicatorRect.x ).toBe( paragraphRect.x );
+		expect( indicatorRect.y > paragraphRect.y ).toBe( true );
+	} );
 } );
diff --git a/packages/edit-widgets/src/blocks/widget-area/edit/inner-blocks.js b/packages/edit-widgets/src/blocks/widget-area/edit/inner-blocks.js
@@ -15,6 +15,7 @@ export default function WidgetAreaInnerBlocks() {
 			onInput={ onInput }
 			onChange={ onChange }
 			templateLock={ false }
+			renderAppender={ InnerBlocks.DefaultBlockAppender }
 		/>
 	);
 }