From 740f108ec990b932150554702d7c761c02685873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 3 Dec 2020 17:18:21 +0200 Subject: [PATCH 1/2] Block editor: rely on DOM for getting appender element --- .../components/block-list-appender/index.js | 14 +------ .../src/components/block-list/index.js | 39 +++++++------------ .../components/block-list/insertion-point.js | 12 +++--- packages/block-editor/src/utils/dom.js | 13 +++++++ 4 files changed, 35 insertions(+), 43 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 fae056e50dcb5..e01e91d68689c 100644 --- a/packages/block-editor/src/components/block-list-appender/index.js +++ b/packages/block-editor/src/components/block-list-appender/index.js @@ -7,7 +7,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { createContext, useContext } from '@wordpress/element'; +import { createContext } from '@wordpress/element'; import { withSelect } from '@wordpress/data'; import { getDefaultBlockName } from '@wordpress/blocks'; @@ -34,8 +34,6 @@ function BlockListAppender( { selectedBlockClientId, tagName: TagName = 'div', } ) { - const appenderNodesMap = useContext( AppenderNodesContext ); - if ( isLocked || CustomAppender === false ) { return null; } @@ -100,15 +98,7 @@ function BlockListAppender( { '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 || '' ); - } - } } + id={ `appender-${ rootClientId || 'root' }` } > { appender } diff --git a/packages/block-editor/src/components/block-list/index.js b/packages/block-editor/src/components/block-list/index.js index 7eb8bc69de8e2..f78c09330fb25 100644 --- a/packages/block-editor/src/components/block-list/index.js +++ b/packages/block-editor/src/components/block-list/index.js @@ -13,17 +13,10 @@ import { useRef, forwardRef } from '@wordpress/element'; * Internal dependencies */ import BlockListBlock from './block'; -import BlockListAppender, { - AppenderNodesContext, -} from '../block-list-appender'; +import BlockListAppender 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. @@ -39,22 +32,20 @@ function BlockList( const wrapperRef = ref || fallbackRef; 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 b0981ed37a492..342462b8f9447 100644 --- a/packages/block-editor/src/components/block-list/insertion-point.js +++ b/packages/block-editor/src/components/block-list/insertion-point.js @@ -11,7 +11,6 @@ import { useState, useRef, useMemo, - useContext, useEffect, useCallback, } from '@wordpress/element'; @@ -23,8 +22,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'; +import { getBlockDOMNode, getAppenderDOMNode } from '../../utils/dom'; function InsertionPointInserter( { clientId, @@ -114,18 +112,18 @@ function InsertionPointPopover( { containerRef, showInsertionPoint, } ) { - const appenderNodesMap = useContext( AppenderNodesContext ); const element = useMemo( () => { + const { ownerDocument } = containerRef.current; + if ( clientId ) { - const { ownerDocument } = containerRef.current; return getBlockDOMNode( clientId, ownerDocument ); } // 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 getAppenderDOMNode( rootClientId || '', ownerDocument ); + }, [ clientId, rootClientId ] ); return ( Date: Thu, 3 Dec 2020 19:48:06 +0200 Subject: [PATCH 2/2] Rewrite --- .../components/block-list-appender/index.js | 1 - .../components/block-list/insertion-point.js | 40 +++++++++---------- packages/block-editor/src/utils/dom.js | 13 ------ packages/components/src/popover/index.js | 4 +- packages/components/src/popover/utils.js | 22 ++++++---- 5 files changed, 38 insertions(+), 42 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 e01e91d68689c..0fc9b408ee846 100644 --- a/packages/block-editor/src/components/block-list-appender/index.js +++ b/packages/block-editor/src/components/block-list-appender/index.js @@ -98,7 +98,6 @@ function BlockListAppender( { 'wp-block', className ) } - id={ `appender-${ rootClientId || 'root' }` } > { appender } 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 342462b8f9447..ab9c010dcdc6b 100644 --- a/packages/block-editor/src/components/block-list/insertion-point.js +++ b/packages/block-editor/src/components/block-list/insertion-point.js @@ -2,18 +2,13 @@ * External dependencies */ import classnames from 'classnames'; +import { last } from 'lodash'; /** * WordPress dependencies */ import { useSelect } from '@wordpress/data'; -import { - useState, - useRef, - useMemo, - useEffect, - useCallback, -} from '@wordpress/element'; +import { useState, useRef, useEffect, useCallback } from '@wordpress/element'; import { Popover } from '@wordpress/components'; import { placeCaretAtVerticalEdge } from '@wordpress/dom'; @@ -22,7 +17,7 @@ import { placeCaretAtVerticalEdge } from '@wordpress/dom'; */ import Inserter from '../inserter'; import { getClosestTabbable } from '../writing-flow'; -import { getBlockDOMNode, getAppenderDOMNode } from '../../utils/dom'; +import { getBlockDOMNode } from '../../utils/dom'; function InsertionPointInserter( { clientId, @@ -112,31 +107,36 @@ function InsertionPointPopover( { containerRef, showInsertionPoint, } ) { - const element = useMemo( () => { - const { ownerDocument } = containerRef.current; + const element = useSelect( + ( select ) => { + const { getBlockOrder } = select( 'core/block-editor' ); + const { ownerDocument } = containerRef.current; + const targetClientId = + clientId || last( getBlockOrder( rootClientId ) ); - if ( clientId ) { - return getBlockDOMNode( clientId, ownerDocument ); - } + return getBlockDOMNode( targetClientId, ownerDocument ); + }, + [ clientId, rootClientId ] + ); - // 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 getAppenderDOMNode( rootClientId || '', ownerDocument ); - }, [ clientId, rootClientId ] ); + const position = clientId ? 'top' : 'bottom'; + const className = classnames( 'block-editor-block-list__insertion-point', { + 'is-insert-after': ! clientId, + } ); return (
{ showInsertionPoint && ( diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index 1abb07ee83af9..4f9c971ccd077 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -114,16 +114,3 @@ export function getBlockClientId( node ) { return blockNode.id.slice( 'block-'.length ); } - -/** - * Given a root client ID, returns the appender DOM node for the block list, - * if exists. - * - * @param {string} rootClientId Block client ID. - * @param {Document} doc Document to search. - * - * @return {Element?} Block DOM node. - */ -export function getAppenderDOMNode( rootClientId, doc ) { - return doc.getElementById( `appender-${ rootClientId || 'root' }` ); -} diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index 36b001ae50cbe..e2dbb12c982ff 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -272,6 +272,7 @@ const Popover = ( { __unstableSlotName = SLOT_NAME, __unstableObserveElement, __unstableBoundaryParent, + __unstableForcePosition, /* eslint-enable no-unused-vars */ ...contentProps } ) => { @@ -361,7 +362,8 @@ const Popover = ( { __unstableStickyBoundaryElement, containerRef.current, relativeOffsetTop, - boundaryElement + boundaryElement, + __unstableForcePosition ); if ( diff --git a/packages/components/src/popover/utils.js b/packages/components/src/popover/utils.js index 196b085c259a9..57c697322e821 100644 --- a/packages/components/src/popover/utils.js +++ b/packages/components/src/popover/utils.js @@ -15,6 +15,7 @@ const HEIGHT_OFFSET = 10; // used by the arrow and a bit of empty space * position. * @param {string} chosenYAxis yAxis to be used. * @param {Element} boundaryElement Boundary element. + * @param {boolean} forcePosition Don't adjust position based on anchor. * * @return {Object} Popover xAxis position and constraints. */ @@ -25,7 +26,8 @@ export function computePopoverXAxisPosition( corner, stickyBoundaryElement, chosenYAxis, - boundaryElement + boundaryElement, + forcePosition ) { const { width } = contentSize; const isRTL = document.documentElement.dir === 'rtl'; @@ -86,7 +88,7 @@ export function computePopoverXAxisPosition( let chosenXAxis = xAxis; let contentWidth = null; - if ( ! stickyBoundaryElement ) { + if ( ! stickyBoundaryElement && ! forcePosition ) { if ( xAxis === 'center' && centerAlignment.contentWidth === width ) { chosenXAxis = 'center'; } else if ( xAxis === 'left' && leftAlignment.contentWidth === width ) { @@ -143,6 +145,7 @@ export function computePopoverXAxisPosition( * @param {Element} anchorRef The anchor element. * @param {Element} relativeOffsetTop If applicable, top offset of the * relative positioned parent container. + * @param {boolean} forcePosition Don't adjust position based on anchor. * * @return {Object} Popover xAxis position and constraints. */ @@ -153,7 +156,8 @@ export function computePopoverYAxisPosition( corner, stickyBoundaryElement, anchorRef, - relativeOffsetTop + relativeOffsetTop, + forcePosition ) { const { height } = contentSize; @@ -206,7 +210,7 @@ export function computePopoverYAxisPosition( let chosenYAxis = yAxis; let contentHeight = null; - if ( ! stickyBoundaryElement ) { + if ( ! stickyBoundaryElement && ! forcePosition ) { if ( yAxis === 'middle' && middleAlignment.contentHeight === height ) { chosenYAxis = 'middle'; } else if ( yAxis === 'top' && topAlignment.contentHeight === height ) { @@ -259,6 +263,7 @@ export function computePopoverYAxisPosition( * @param {number} relativeOffsetTop If applicable, top offset of the * relative positioned parent container. * @param {Element} boundaryElement Boundary element. + * @param {boolean} forcePosition Don't adjust position based on anchor. * * @return {Object} Popover position and constraints. */ @@ -269,7 +274,8 @@ export function computePopoverPosition( stickyBoundaryElement, anchorRef, relativeOffsetTop, - boundaryElement + boundaryElement, + forcePosition ) { const [ yAxis, xAxis = 'center', corner ] = position.split( ' ' ); @@ -280,7 +286,8 @@ export function computePopoverPosition( corner, stickyBoundaryElement, anchorRef, - relativeOffsetTop + relativeOffsetTop, + forcePosition ); const xAxisPosition = computePopoverXAxisPosition( anchorRect, @@ -289,7 +296,8 @@ export function computePopoverPosition( corner, stickyBoundaryElement, yAxisPosition.yAxis, - boundaryElement + boundaryElement, + forcePosition ); return {