From e376eabcb109c69c4c2f2d14afcf7b08cefddd12 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Thu, 8 Oct 2020 19:51:48 +0800 Subject: [PATCH] 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 --- .../components/block-list-appender/index.js | 21 +++- .../src/components/block-list/index.js | 108 ++++++++++-------- .../components/block-list/insertion-point.js | 60 ++++++---- .../src/components/block-list/style.scss | 4 + .../editor/various/adding-blocks.test.js | 33 ++++++ .../blocks/widget-area/edit/inner-blocks.js | 1 + 6 files changed, 155 insertions(+), 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 index e2a4dbb0ebd6b..fae056e50dcb5 100644 --- 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 } diff --git a/packages/block-editor/src/components/block-list/index.js b/packages/block-editor/src/components/block-list/index.js index 22ede3bc48e67..c74ed80bb074b 100644 --- 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 ( - - { blockClientIds.map( ( clientId, index ) => { - const isBlockInSelection = hasMultiSelection - ? multiSelectedBlockClientIds.includes( clientId ) - : selectedBlockClientId === clientId; + + + { blockClientIds.map( ( clientId, index ) => { + const isBlockInSelection = hasMultiSelection + ? multiSelectedBlockClientIds.includes( clientId ) + : selectedBlockClientId === clientId; - const isDropTarget = - dropTargetIndex === index && isDraggingBlocks; + const isDropTarget = + dropTargetIndex === index && isDraggingBlocks; - return ( - - - - ); - } ) } - + + + ); } ) } - /> - + + + ); } diff --git a/packages/block-editor/src/components/block-list/insertion-point.js b/packages/block-editor/src/components/block-list/insertion-point.js index 8562b088bb9ee..ebfc46a22a3d0 100644 --- 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 ( { - 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 index ad60dd086b12c..77b2a30a9dab8 100644 --- 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 index d6d1496da80a7..be849af8d0125 100644 --- 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 index cf23812646eac..7b27a404a2f6c 100644 --- 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 } /> ); }