Block Toolbar: Prevent position shifts when using mover control

[shrivastavanolo]

What?

Closes #61435

Stops the block toolbar from jumping around when using the mover controls.

Why?

When a block is moved, useMovingAnimation writes transform on every spring tick. The MutationObserver was firing synchronously on each mutation, racing with Floating UI's own autoUpdate frame loop — causing the toolbar to visibly jump mid-animation.

How?

Coalesced the MutationObserver callbacks via requestAnimationFrame so at most one recompute is triggered per frame, matching the cadence of the spring animation. Also added proper cleanup via cancelAnimationFrame on unmount to prevent stale updates.
The observer itself can't simply be removed: without it, Floating UI's animationFrame mode alone causes the toolbar to trail the block by ~1 frame throughout the animation because the spring's rAF and autoUpdate's rAF are independently scheduled.

Testing Instructions

1. Open a post with many blocks
2. Select a block and scroll upwards till it is partially in view then use the mover arrows up and down
3. Verify the toolbar doesn't jump during animation (Also check for regressions in #44220, #44281)
4. Click the mover arrows rapidly (overlapping animations) and verify no jump
5. Test in the Site Editor (Appearance → Editor) with a template containing many blocks
6. Enable prefers-reduced-motion in OS accessibility settings and repeat

Screenshots or screencast

Before

before.mov

After (Post editor)

post-editor.mov

After (Site editor)

site-ediotr.mov

After (reduced-motion enabled)

reduced-motion.mov

Use of AI Tools

Claude (Anthropic) was used to help draft this PR. All changes were reviewed manually.

note: please check this for further explanation

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: shrivastavanolo <shreya0shrivastava@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[Mamaduka]

The new debouncedRecompute shouldn't be needed; we could just inline rAF and call it directly, no redundant memoization or dependencies.

cc @ciampo, it looks like you worked on previous optimizaton (#44301), any suggestion here?

[shrivastavanolo]

Thanks for the feedback! Removed the useCallback wrapper and inlined the requestAnimationFrame without the extra memoization.

[Copilot]

Pull request overview

This PR aims to stabilize the block toolbar’s positioning during block move animations by deferring popover dimension recomputation so it doesn’t react mid-frame to frequent transform attribute updates.

Changes:

• Adjusted the MutationObserver handler in BlockPopover to defer forceRecomputePopoverDimensions via requestAnimationFrame.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ciampo]

Thank you for working on this!

While this PR seems to fix the issue, it's likely fixing a symptom, rather than curing the root cause.

Here is a more detailed explanation of what's going on IMO

The MutationObserver was added in #44301 to make the toolbar follow blocks during the mover animation. At that point, the Popover component had already been switched to Floating UI's autoUpdate({ animationFrame: true }) in #43617 — and that PR explicitly stated that the previous __unstableObservedElement/MutationObserver mechanism was "not necessary anymore" precisely because animationFrame: true polls getBoundingClientRect() every frame.

What's happening on trunk today:

• useMovingAnimation writes style.transform on every spring tick.
• The MutationObserver here fires synchronously after each mutation and bumps popoverDimensionsRecomputeCounter.
• That counter is in the useMemo deps for popoverAnchor, so a new virtual element object is returned on every frame.
• Popover has a useLayoutEffect that calls refs.setReference( ... ) whenever anchor changes identity. Floating UI's useFloating then runs its whileElementsMounted cleanup and re-creates autoUpdate from scratch (cancels the running animation-frame loop, re-sets up ResizeObserver and ancestor listeners, calls update() once).
• Meanwhile, the existing autoUpdate frameLoop (which is already calling our anchor's getBoundingClientRect() every frame and calling update() whenever the rect differs) gets torn down and rebuilt on every animation tick.

That continuous teardown/rebuild fighting with the in-flight frameLoop is, I believe, what produces the visible jump.

If that analysis is right, then the right fix is most likely to remove the MutationObserver entirely and rely on Floating UI's per-frame autoUpdate, which is already doing the job of calling our anchor's custom getBoundingClientRect() (which returns rectUnion( selectedElement, lastSelectedElement ) already). No invalidation step needed.

Can we try just removing the MutationObserver and the reducer/counter, and re-test the original mover scenario plus the cases the observer was introduced for (#44220, #44281)? That feels like the real fix to me.

Example code changes

 import {
     forwardRef,
     useMemo,
-    useReducer,
-    useLayoutEffect,
 } from '@wordpress/element';
 ...
-const MAX_POPOVER_RECOMPUTE_COUNTER = Number.MAX_SAFE_INTEGER;
-
 function BlockPopover( ... ) {
-    const [
-        popoverDimensionsRecomputeCounter,
-        forceRecomputePopoverDimensions,
-    ] = useReducer(
-        ( s ) => ( s + 1 ) % MAX_POPOVER_RECOMPUTE_COUNTER,
-        0
-    );
-
-    useLayoutEffect( () => {
-        if ( ! selectedElement ) {
-            return;
-        }
-        const observer = new window.MutationObserver( () =>
-            window.requestAnimationFrame( () =>
-                forceRecomputePopoverDimensions()
-            )
-        );
-        observer.observe( selectedElement, { attributes: true } );
-        return () => observer.disconnect();
-    }, [ selectedElement ] );

     const popoverAnchor = useMemo( () => {
         if (
-            popoverDimensionsRecomputeCounter < 0 ||
             ! selectedElement ||
             ( bottomClientId && ! lastSelectedElement )
         ) {
             return undefined;
         }
         return {
             getBoundingClientRect() { ... },
             contextElement: selectedElement,
         };
-    }, [ popoverDimensionsRecomputeCounter, selectedElement, bottomClientId, lastSelectedElement ] );
+    }, [ selectedElement, bottomClientId, lastSelectedElement ] );

If removing it does regress some scenario, the next-best alternative is to keep the observer but stop having it change popoverAnchor's identity. We should not be calling setReference on every animation frame; that's the expensive operation. Triggering a position-only update (e.g. via the floating UI update function exposed by the Popover consumer, or by reading selectedElement.getBoundingClientRect() directly) would avoid the rebuild churn even if we keep an observer.

[shrivastavanolo]

@ciampo Thanks for the detailed breakdown!

Confirmed: removing the MutationObserver entirely does regress things including the changes of this PR. In the original mover scenario, the toolbar drifts to overlap the block during the spring animation rather than tracking above it. I've attached a video showing the regression.

Screen.Recording.2026-05-04.at.3.43.48.PM.mov

So I'd like to explore your secondary suggestion: keep the observer but avoid changing popoverAnchor's identity, and instead trigger a position-only update.

The idea would be something like:

const updateRef = useRef();

// Expose Floating UI's update() via a callback ref on the Popover
// so the observer can call it directly without touching the anchor object.

useLayoutEffect( () => {
    if ( ! selectedElement ) return;

    const observer = new window.MutationObserver( () => {
        updateRef.current?.();
    } );
    observer.observe( selectedElement, { attributes: true } );
    return () => observer.disconnect();
}, [ selectedElement ] );

The anchor identity stays stable, so setReference() is never called during animation. The observer just pokes Floating UI to re-run computePosition() on the existing anchor — which already returns rectUnion( selectedElement, lastSelectedElement ) — so position stays correct without the rebuild churn.

I checked the current Popover source — update is destructured from useFloating() internally but isn't forwarded to consumers via useImperativeHandle or otherwise. So the update() path would require a change to @wordpress/components first.

Alternatively, rAF coalescing on the existing counter approach might also work — keeping the observer and the counter but limiting setReference() to at most once per frame, which would match the cadence of the spring animation and avoid the visible jump without needing any changes to Popover. Something like this:

useLayoutEffect( () => {
		if ( ! selectedElement ) {
			return;
		}

		let rafId;
		const observer = new window.MutationObserver( () => {
			if ( rafId ) {
				return;
			}
			rafId = window.requestAnimationFrame( () => {
				rafId = null;
				forceRecomputePopoverDimensions();
			} );
		} );
		observer.observe( selectedElement, { attributes: true } );

		return () => {
			observer.disconnect();
			if ( rafId ) {
				window.cancelAnimationFrame( rafId );
			}
		};
	}, [ selectedElement ] );

Happy to explore either path.

cc: @Mamaduka

[ciampo]

Thanks for the regression video and the two alternatives — that nails down what the observer is actually buying us.

Quick framing on why we still need it: the drift you captured is a one-frame ordering issue between the spring's rAF (in useMovingAnimation) and Floating UI's own frameLoop rAF. When frameLoop runs before the spring tick within the same frame, it reads the previous rect and skips update(), so the toolbar trails the block for the duration of the animation. The MutationObserver short-circuits that by forcing a same-tick reposition. So removing it is off the table.

Between your two options, let's go with your rAF-coalesce variant (the second one). I'd skip exposing update() from Popover:

• Even as __unstable*, it's a one-consumer escape hatch that bleeds implementation details into Popover's public surface.
• The underlying cost we want to avoid is setReference() being called every frame — and your coalesced variant already addresses that without changing Popover.
• It also hard-caps work at one recompute per frame (vs. the current diff, which queues a fresh rAF per MO callback — and useMovingAnimation mutates transformOrigin, transform, and zIndex separately per spring tick), and it cancels the pending rAF on cleanup.

Two small things to add on top of your snippet:

1. A short comment explaining why we keep the observer + why we coalesce, so the next maintainer doesn't try to "simplify" it again. Something like:

   // `useMovingAnimation` writes the block's `transform` on every spring tick.
   // Reacting synchronously to each mutation would race with Floating UI's own
   // autoUpdate frame loop and cause the toolbar to visibly jump. Coalescing
   // to one recompute per animation frame avoids that. The observer can't
   // simply be removed: with autoUpdate's animationFrame mode alone, the
   // toolbar trails the block by ~1 frame because the spring's rAF and
   // autoUpdate's rAF are independently scheduled.

2. Before merging, a quick manual pass on:

   • the original reproducers from Mover control: Block toolbar no longer sticks to block that's being moved until you scroll #44220 and Block toolbar sometimes positioned incorrectly after dragging and dropping a block #44281,
   • the Site Editor (iframe) mover scenario on a template with many blocks,
   • rapid repeated mover clicks (overlapping springs),
   • prefers-reduced-motion: reduce.

Happy to take another look once it's pushed.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[shrivastavanolo]

Pushed the changes with the rAF coalescing approach. Tested the following scenarios and attached videos:

Post editor - mover controls and rapid clicks

post-editor.mov

Site Editor - mover controls and rapid clicks

site-ediotr.mov

prefers-reduced-motion

reduced-motion.mov

[ciampo]

LGTM 🚀

@Mamaduka does it look good for you, too?

@shrivastavanolo , do you mind updating the PR description so that it reflect the latest changes, including the latest screen captures?

[Mamaduka]

Sorry for the late review. The change looks good. Thanks, @shrivastavanolo!