From 8f8d4612da8c50f97e58df72f1484b53bd16d4d9 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Tue, 3 Nov 2026 23:15:35 +0000 Subject: [PATCH 1/2] ui/motion: SlidingIndicator re-places a resize that lands during a slide The ResizeObserver skips a change while a slide runs; the slide's finish now re-reads the selected item's box and places the highlight there with no motion. Fixes #33770. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?id=aid-e5afe752a9bef9377fa3 --- webviews/src/ui/motion/SlidingIndicator.tsx | 20 ++++++++--- webviews/src/ui/motion/motion.test.tsx | 40 ++++++++++++++++++++- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/webviews/src/ui/motion/SlidingIndicator.tsx b/webviews/src/ui/motion/SlidingIndicator.tsx index f02488205eb5..f47f9f9dab20 100644 --- a/webviews/src/ui/motion/SlidingIndicator.tsx +++ b/webviews/src/ui/motion/SlidingIndicator.tsx @@ -15,7 +15,8 @@ // Stop conditions: no timer and no frame loop. The slide is one finite Web Animation (at most // --cmux-motion-large, 240 ms) that ends on its own; the next change and unmount cancel it. One // ResizeObserver on the host and its items re-places the highlight when a size changes (a label, a -// font load); it is disconnected on unmount. +// font load); it is disconnected on unmount. A size change during a slide is held until the slide +// finishes, then re-placed with no motion, so the highlight never ends off its item. import { type CSSProperties, type RefObject, useLayoutEffect, useRef } from "react"; import { MOTION_EASE_OUT, MOTION_MS, MOTION_SPRING, MOTION_SPRING_STEP } from "../../motion/scale.generated"; import { cx } from "../cx"; @@ -73,6 +74,8 @@ export function useSlidingIndicator( ) { const rest = useRef(null); const slide = useRef(null); + // Re-places it on the selected item with no motion, if its box changed; set while mounted. + const settle = useRef<(() => void) | null>(null); // Place it on every selection change, sliding from where it is drawn. // perf-hot: one read and one write per selection change; no per-frame work. @@ -117,6 +120,7 @@ export function useSlidingIndicator( ); slide.current.onfinish = () => { slide.current = null; + settle.current?.(); }; }, [mark, selected, travel, selector]); @@ -125,19 +129,27 @@ export function useSlidingIndicator( const element = mark.current; const parent = element?.parentElement; if (!parent || !element || typeof ResizeObserver === "undefined") return; - const observer = new ResizeObserver(() => { + const place = () => { const item = parent.querySelector(selector); - if (!item || slide.current) return; + if (!item) return; const target = boxIn(parent, item); if (rest.current && same(rest.current, target)) return; rest.current = target; element.style.width = `${target.w}px`; element.style.height = `${target.h}px`; element.style.transform = `translate(${target.x}px, ${target.y}px)`; + }; + settle.current = place; + // During a slide the box is read again when it finishes (onfinish calls settle). + const observer = new ResizeObserver(() => { + if (!slide.current) place(); }); observer.observe(parent); for (const child of parent.children) if (child !== element) observer.observe(child); - return () => observer.disconnect(); + return () => { + observer.disconnect(); + settle.current = null; + }; }, [mark, selector]); useLayoutEffect( diff --git a/webviews/src/ui/motion/motion.test.tsx b/webviews/src/ui/motion/motion.test.tsx index 6b61fa39084d..4dee3ecbfcb5 100644 --- a/webviews/src/ui/motion/motion.test.tsx +++ b/webviews/src/ui/motion/motion.test.tsx @@ -16,6 +16,7 @@ const saved = new Map(); type Played = { element: Element; frames: Keyframe[]; timing: KeyframeAnimationOptions }; let played: Played[] = []; +let animations: Animation[] = []; function installDom(options: { reduced?: boolean; noLinear?: boolean } = {}): HTMLElement { dom = new JSDOM("
", { url: "http://localhost/" }); @@ -37,7 +38,9 @@ function installDom(options: { reduced?: boolean; noLinear?: boolean } = {}): HT timing: KeyframeAnimationOptions, ) { played.push({ element: this, frames, timing }); - return { cancel() {}, playState: "running", onfinish: null } as unknown as Animation; + const animation = { cancel() {}, playState: "running", onfinish: null } as unknown as Animation; + animations.push(animation); + return animation; } as never; for (const key of ["window", "document", "navigator", "Node", "HTMLElement", "Element", "IS_REACT_ACT_ENVIRONMENT"]) if (!saved.has(key)) saved.set(key, scope[key]); @@ -57,6 +60,7 @@ afterEach(async () => { if (root) await act(async () => root!.unmount()); root = undefined; played = []; + animations = []; dom?.window.close(); dom = undefined; for (const [key, value] of saved) { @@ -121,6 +125,40 @@ test("a selection change places it on the new tab at once and slides on transfor expect(timing).toEqual({ duration: MOTION_MS.large, easing: MOTION_SPRING }); }); +test("a size change during a slide re-places the highlight when the slide finishes", async () => { + const mount = installDom(); + const observed: (() => void)[] = []; + const saved = scope.ResizeObserver; + scope.ResizeObserver = class { + constructor(callback: () => void) { + observed.push(callback); + } + observe() {} + disconnect() {} + }; + try { + await act(async () => { + root = createRoot(mount); + root.render(); + }); + await act(async () => select(1)); + const mark = mount.querySelector(".ui-slide-indicator")!; + // A font load widens the new tab mid-slide; the slide keeps its course. + mount.querySelectorAll(".ui-slide-item")[1]!.dataset.box = "48,4,96,24"; + for (const callback of observed) callback(); + expect(mark.style.width).toBe("80px"); + // It lands on the tab's new box, with no second slide. + const [slide] = animations as [Animation]; + await act(async () => void slide.onfinish?.call(slide, new dom!.window.Event("finish") as AnimationPlaybackEvent)); + expect(mark.style.width).toBe("96px"); + expect(mark.style.transform).toBe("translate(48px, 4px)"); + expect(played).toHaveLength(1); + } finally { + if (saved === undefined) delete scope.ResizeObserver; + else scope.ResizeObserver = saved; + } +}); + test("an engine without linear() easing slides on the ease-out instead of throwing", async () => { const mount = installDom({ noLinear: true }); await act(async () => { From 6ad85e276d01b7974e239f57274ec3999dd62b3f Mon Sep 17 00:00:00 2001 From: Leo Li Date: Tue, 3 Nov 2026 23:16:35 +0000 Subject: [PATCH 2/2] test: name the saved ResizeObserver apart from the globals map Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?id=aid-e5afe752a9bef9377fa3 --- webviews/src/ui/motion/motion.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/webviews/src/ui/motion/motion.test.tsx b/webviews/src/ui/motion/motion.test.tsx index 4dee3ecbfcb5..08f51e06376d 100644 --- a/webviews/src/ui/motion/motion.test.tsx +++ b/webviews/src/ui/motion/motion.test.tsx @@ -128,7 +128,7 @@ test("a selection change places it on the new tab at once and slides on transfor test("a size change during a slide re-places the highlight when the slide finishes", async () => { const mount = installDom(); const observed: (() => void)[] = []; - const saved = scope.ResizeObserver; + const before = scope.ResizeObserver; scope.ResizeObserver = class { constructor(callback: () => void) { observed.push(callback); @@ -154,8 +154,8 @@ test("a size change during a slide re-places the highlight when the slide finish expect(mark.style.transform).toBe("translate(48px, 4px)"); expect(played).toHaveLength(1); } finally { - if (saved === undefined) delete scope.ResizeObserver; - else scope.ResizeObserver = saved; + if (before === undefined) delete scope.ResizeObserver; + else scope.ResizeObserver = before; } });