From 340ae480585b1d8780b8ef31bcc98df2474827ce Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 20 Sep 2022 17:01:52 +0200 Subject: [PATCH 1/4] Block Toolbar: update position when moving blocks --- .../src/components/block-popover/index.js | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-popover/index.js b/packages/block-editor/src/components/block-popover/index.js index e2b2f58d5d4e31..9aafd546b65804 100644 --- a/packages/block-editor/src/components/block-popover/index.js +++ b/packages/block-editor/src/components/block-popover/index.js @@ -8,7 +8,12 @@ import classnames from 'classnames'; */ import { useMergeRefs } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; -import { forwardRef, useMemo } from '@wordpress/element'; +import { + forwardRef, + useMemo, + useReducer, + useLayoutEffect, +} from '@wordpress/element'; /** * Internal dependencies @@ -47,8 +52,34 @@ function BlockPopover( }; }, [ selectedElement, lastSelectedElement, __unstableRefreshSize ] ); + const [ popoverAnchorRecomputeCounter, forceRecomputePopoverAnchor ] = + useReducer( ( s ) => s + 1, 0 ); + + // When blocks are moved up/down, they are animated to their new position by + // updating the `transform` property manually (i.e. without using CSS + // transitions or animations). The animation, which can also scroll the block + // editor, can sometimes cause the position of the Popover to get out of sync. + // A MutationObserver is therefore used to make sure that changes to the + // selectedElement's attribute (i.e. `transform`) can be tracked and used to + // trigger the Popover to rerender. + useLayoutEffect( () => { + if ( ! selectedElement ) { + return; + } + + const observer = new window.MutationObserver( + forceRecomputePopoverAnchor + ); + observer.observe( selectedElement, { attributes: true } ); + + return () => { + observer.disconnect(); + }; + }, [ selectedElement ] ); + const popoverAnchor = useMemo( () => { if ( + popoverAnchorRecomputeCounter < 0 || ! selectedElement || ( bottomClientId && ! lastSelectedElement ) ) { @@ -88,7 +119,12 @@ function BlockPopover( }, ownerDocument: selectedElement.ownerDocument, }; - }, [ bottomClientId, lastSelectedElement, selectedElement ] ); + }, [ + bottomClientId, + lastSelectedElement, + selectedElement, + popoverAnchorRecomputeCounter, + ] ); if ( ! selectedElement || ( bottomClientId && ! lastSelectedElement ) ) { return null; From 5b72aeefb53432db79100183976da5e9ee3752c1 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 21 Sep 2022 11:33:33 +0200 Subject: [PATCH 2/4] Make sure counter does not overflow --- .../block-editor/src/components/block-popover/index.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/block-popover/index.js b/packages/block-editor/src/components/block-popover/index.js index 9aafd546b65804..f055e0e75dd285 100644 --- a/packages/block-editor/src/components/block-popover/index.js +++ b/packages/block-editor/src/components/block-popover/index.js @@ -21,6 +21,8 @@ import { import { __unstableUseBlockElement as useBlockElement } from '../block-list/use-block-props/use-block-refs'; import usePopoverScroll from './use-popover-scroll'; +const MAX_ANCHOR_RECOMPUTE_COUNTER = Number.MAX_SAFE_INTEGER; + function BlockPopover( { clientId, @@ -53,7 +55,11 @@ function BlockPopover( }, [ selectedElement, lastSelectedElement, __unstableRefreshSize ] ); const [ popoverAnchorRecomputeCounter, forceRecomputePopoverAnchor ] = - useReducer( ( s ) => s + 1, 0 ); + useReducer( + // Module is there to make sure that the counter doesn't overflow. + ( s ) => ( s + 1 ) % MAX_ANCHOR_RECOMPUTE_COUNTER, + 0 + ); // When blocks are moved up/down, they are animated to their new position by // updating the `transform` property manually (i.e. without using CSS From 6a9bd8c8899c643bca39082557366df01c3a0828 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 21 Sep 2022 11:35:13 +0200 Subject: [PATCH 3/4] Add explainer for "< 0" check --- packages/block-editor/src/components/block-popover/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/block-editor/src/components/block-popover/index.js b/packages/block-editor/src/components/block-popover/index.js index f055e0e75dd285..578b102ed17a7f 100644 --- a/packages/block-editor/src/components/block-popover/index.js +++ b/packages/block-editor/src/components/block-popover/index.js @@ -85,6 +85,9 @@ function BlockPopover( const popoverAnchor = useMemo( () => { if ( + // popoverAnchorRecomputeCounter is by definition always equal or greater + // than 0. This check is only there to satisfy the correctness of the + // exhaustive-deps rule for the `useMemo` hook. popoverAnchorRecomputeCounter < 0 || ! selectedElement || ( bottomClientId && ! lastSelectedElement ) From cae13768cccd0a12b69cb9c1867b10605aaa9860 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 21 Sep 2022 11:42:48 +0200 Subject: [PATCH 4/4] Apply same enhancements to inbetween inserter --- .../src/components/block-popover/inbetween.js | 44 ++++++++++++++----- .../src/components/block-popover/index.js | 4 +- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/components/block-popover/inbetween.js b/packages/block-editor/src/components/block-popover/inbetween.js index 442ae06edffe08..e7e207cb733f1f 100644 --- a/packages/block-editor/src/components/block-popover/inbetween.js +++ b/packages/block-editor/src/components/block-popover/inbetween.js @@ -23,6 +23,8 @@ import { store as blockEditorStore } from '../../store'; import { __unstableUseBlockElement as useBlockElement } from '../block-list/use-block-props/use-block-refs'; import usePopoverScroll from './use-popover-scroll'; +const MAX_POPOVER_RECOMPUTE_COUNTER = Number.MAX_SAFE_INTEGER; + export const InsertionPointOpenRef = createContext(); function BlockPopoverInbetween( { @@ -34,8 +36,9 @@ function BlockPopoverInbetween( { ...props } ) { // This is a temporary hack to get the inbetween inserter to recompute properly. - const [ positionRecompute, forceRecompute ] = useReducer( - ( s ) => s + 1, + const [ popoverRecomputeCounter, forcePopoverRecompute ] = useReducer( + // Module is there to make sure that the counter doesn't overflow. + ( s ) => ( s + 1 ) % MAX_POPOVER_RECOMPUTE_COUNTER, 0 ); @@ -66,7 +69,14 @@ function BlockPopoverInbetween( { const nextElement = useBlockElement( nextClientId ); const isVertical = orientation === 'vertical'; const style = useMemo( () => { - if ( ( ! previousElement && ! nextElement ) || ! isVisible ) { + if ( + // popoverRecomputeCounter is by definition always equal or greater than 0. + // This check is only there to satisfy the correctness of the + // exhaustive-deps rule for the `useMemo` hook. + popoverRecomputeCounter < 0 || + ( ! previousElement && ! nextElement ) || + ! isVisible + ) { return {}; } @@ -102,12 +112,19 @@ function BlockPopoverInbetween( { previousElement, nextElement, isVertical, - positionRecompute, + popoverRecomputeCounter, isVisible, ] ); const popoverAnchor = useMemo( () => { - if ( ( ! previousElement && ! nextElement ) || ! isVisible ) { + if ( + // popoverRecomputeCounter is by definition always equal or greater than 0. + // This check is only there to satisfy the correctness of the + // exhaustive-deps rule for the `useMemo` hook. + popoverRecomputeCounter < 0 || + ( ! previousElement && ! nextElement ) || + ! isVisible + ) { return undefined; } @@ -161,7 +178,7 @@ function BlockPopoverInbetween( { }, [ previousElement, nextElement, - positionRecompute, + popoverRecomputeCounter, isVertical, isVisible, ] ); @@ -169,11 +186,18 @@ function BlockPopoverInbetween( { const popoverScrollRef = usePopoverScroll( __unstableContentRef ); // This is only needed for a smooth transition when moving blocks. + // When blocks are moved up/down, their position can be set by + // updating the `transform` property manually (i.e. without using CSS + // transitions or animations). The animation, which can also scroll the block + // editor, can sometimes cause the position of the Popover to get out of sync. + // A MutationObserver is therefore used to make sure that changes to the + // selectedElement's attribute (i.e. `transform`) can be tracked and used to + // trigger the Popover to rerender. useLayoutEffect( () => { if ( ! previousElement ) { return; } - const observer = new window.MutationObserver( forceRecompute ); + const observer = new window.MutationObserver( forcePopoverRecompute ); observer.observe( previousElement, { attributes: true } ); return () => { @@ -185,7 +209,7 @@ function BlockPopoverInbetween( { if ( ! nextElement ) { return; } - const observer = new window.MutationObserver( forceRecompute ); + const observer = new window.MutationObserver( forcePopoverRecompute ); observer.observe( nextElement, { attributes: true } ); return () => { @@ -199,12 +223,12 @@ function BlockPopoverInbetween( { } previousElement.ownerDocument.defaultView.addEventListener( 'resize', - forceRecompute + forcePopoverRecompute ); return () => { previousElement.ownerDocument.defaultView.removeEventListener( 'resize', - forceRecompute + forcePopoverRecompute ); }; }, [ previousElement ] ); diff --git a/packages/block-editor/src/components/block-popover/index.js b/packages/block-editor/src/components/block-popover/index.js index 578b102ed17a7f..03913faa26e421 100644 --- a/packages/block-editor/src/components/block-popover/index.js +++ b/packages/block-editor/src/components/block-popover/index.js @@ -21,7 +21,7 @@ import { import { __unstableUseBlockElement as useBlockElement } from '../block-list/use-block-props/use-block-refs'; import usePopoverScroll from './use-popover-scroll'; -const MAX_ANCHOR_RECOMPUTE_COUNTER = Number.MAX_SAFE_INTEGER; +const MAX_POPOVER_RECOMPUTE_COUNTER = Number.MAX_SAFE_INTEGER; function BlockPopover( { @@ -57,7 +57,7 @@ function BlockPopover( const [ popoverAnchorRecomputeCounter, forceRecomputePopoverAnchor ] = useReducer( // Module is there to make sure that the counter doesn't overflow. - ( s ) => ( s + 1 ) % MAX_ANCHOR_RECOMPUTE_COUNTER, + ( s ) => ( s + 1 ) % MAX_POPOVER_RECOMPUTE_COUNTER, 0 );