From 2aefec3e6803a3b71c7db54422343846d718cef9 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Fri, 10 Jul 2026 10:40:56 -0700 Subject: [PATCH] fix(studio): animate the implicitly-closed group too, add regression test --- .../components/editor/PropertyPanel.test.tsx | 47 +++++++++++++++++++ .../components/editor/PropertyPanelFlat.tsx | 43 ++++++++++------- .../editor/propertyPanelFlatPrimitives.tsx | 2 +- 3 files changed, 74 insertions(+), 18 deletions(-) diff --git a/packages/studio/src/components/editor/PropertyPanel.test.tsx b/packages/studio/src/components/editor/PropertyPanel.test.tsx index 0fe2148d2..84080056a 100644 --- a/packages/studio/src/components/editor/PropertyPanel.test.tsx +++ b/packages/studio/src/components/editor/PropertyPanel.test.tsx @@ -899,3 +899,50 @@ describe("PropertyPanel — fixed headers + scrollable open section (Plan 11)", RENDER_TIMEOUT_MS, ); }); + +describe("PropertyPanel — flat group entrance animation scoping (fix round)", () => { + it( + "animates only the opening group and the implicitly-closed group on a non-adjacent toggle, never untouched siblings", + async () => { + const { host, root } = await renderPanel(true, sixGroupElement()); + // sixGroupElement() opens Text by default; jump straight to Motion + // (skipping over Style/Layout) first, matching the Plan 11 worked + // example, then jump back to Text — non-adjacent from Motion, again + // skipping over Style/Layout. This is the exact array-slice-position- + // shift scenario the justToggledIds mechanism exists to guard: Style + // and Layout shift position in the before/after-open slices on both + // toggles even though neither of them is the group being toggled. + openFlatGroup(host, "Motion"); + expect(openGroupText(host)).toContain("Motion"); + openFlatGroup(host, "Text"); + expect(openGroupText(host)).toContain("Text"); + + const collapsedRowByTitle = (title: string) => { + const row = Array.from(host.querySelectorAll('[data-flat-group-collapsed="true"]')).find( + (el) => el.textContent?.includes(title), + ); + if (!row) throw new Error(`expected a collapsed ${title} row`); + return row; + }; + + // Untouched, non-adjacent siblings must NOT receive the entrance class, + // even though they shifted position in the collapsed-header list. + expect(collapsedRowByTitle("Style").classList.contains("hf-flat-group-enter")).toBe(false); + expect(collapsedRowByTitle("Layout").classList.contains("hf-flat-group-enter")).toBe(false); + expect(collapsedRowByTitle("Grade").classList.contains("hf-flat-group-enter")).toBe(false); + expect(collapsedRowByTitle("Media").classList.contains("hf-flat-group-enter")).toBe(false); + + // Motion — open a moment ago, just implicitly closed by the click on + // Text — must still play its own collapse-entrance animation (Finding 1). + expect(collapsedRowByTitle("Motion").classList.contains("hf-flat-group-enter")).toBe(true); + + // Text — the group actually clicked open — must animate too. + const openWrapper = host.querySelector('[data-flat-group-open="true"]'); + if (!openWrapper) throw new Error("expected the open-group wrapper"); + expect(openWrapper.querySelector(".hf-flat-group-enter")).not.toBeNull(); + + act(() => root.unmount()); + }, + RENDER_TIMEOUT_MS, + ); +}); diff --git a/packages/studio/src/components/editor/PropertyPanelFlat.tsx b/packages/studio/src/components/editor/PropertyPanelFlat.tsx index 9bd267983..eee092f00 100644 --- a/packages/studio/src/components/editor/PropertyPanelFlat.tsx +++ b/packages/studio/src/components/editor/PropertyPanelFlat.tsx @@ -235,17 +235,20 @@ export function PropertyPanelFlat({ : "layout", ); - // Tracks which single group is actively transitioning, so its header/body - // gets the fast entrance animation (hf-flat-group-enter) and no one else's - // does. Deliberately NOT derived from remounting alone: FlatGroupHeader - // instances are keyed by group id and React normally preserves them across - // re-renders, but toggling a non-adjacent group still shifts the untouched - // collapsed siblings between the before/after-open slices below, and - // Chromium restarts a CSS animation on that kind of position shift even - // though nothing about the sibling actually changed. Gating on this id - // (cleared shortly after the 120ms CSS animation finishes) keeps the - // animation scoped to only the group that actually just toggled. - const [justToggledId, setJustToggledId] = useState(null); + // Tracks which group(s) are actively transitioning this toggle cycle, so + // their header/body gets the fast entrance animation (hf-flat-group-enter) + // and no one else's does. Deliberately NOT derived from remounting alone: + // FlatGroupHeader instances are keyed by group id and React normally + // preserves them across re-renders, but toggling a non-adjacent group still + // shifts the untouched collapsed siblings between the before/after-open + // slices below, and Chromium restarts a CSS animation on that kind of + // position shift even though nothing about the sibling actually changed. + // Gating on these ids (cleared shortly after the 120ms CSS animation + // finishes) keeps the animation scoped to only the groups that actually + // just toggled. Two ids, not one: the clicked (newly-opening/closing) group + // AND whichever group was open immediately before the click and got + // implicitly closed by it — both freshly-mounted headers need to animate. + const [justToggledIds, setJustToggledIds] = useState([]); const justToggledTimeoutRef = useRef | null>(null); useEffect(() => { return () => { @@ -270,10 +273,16 @@ export function PropertyPanelFlat({ const isTextEditable = isTextEditableSelection(element); const elementKind = sections.media ? "media" : element.textFields.length > 0 ? "text" : "other"; const toggleOpen = (groupId: string) => { + // Capture what was open BEFORE this click (this render's closure over + // openGroupId), so the group that's about to be implicitly closed can be + // tracked too — not just the one the user clicked. + const previousOpenGroupId = openGroupId; setOpenGroupId((current) => (current === groupId ? "" : groupId)); - setJustToggledId(groupId); + const implicitlyClosedId = + previousOpenGroupId && previousOpenGroupId !== groupId ? previousOpenGroupId : null; + setJustToggledIds(implicitlyClosedId ? [groupId, implicitlyClosedId] : [groupId]); if (justToggledTimeoutRef.current) clearTimeout(justToggledTimeoutRef.current); - justToggledTimeoutRef.current = setTimeout(() => setJustToggledId(null), 200); + justToggledTimeoutRef.current = setTimeout(() => setJustToggledIds([]), 200); }; // Basis for the Layout keyframe gutter (X/Y/W/H/Angle + 3D Transform) — // must agree with Motion's Timing row (FlatTimingRow), which infers the @@ -510,7 +519,7 @@ export function PropertyPanelFlat({ isOpen={false} onToggleOpen={() => toggleOpen(g.id)} summary={g.summary} - animateEntrance={g.id === justToggledId} + animateEntrance={justToggledIds.includes(g.id)} /> ))} {openGroup && ( @@ -520,10 +529,10 @@ export function PropertyPanelFlat({ isOpen onToggleOpen={() => toggleOpen(openGroup.id)} accessory={openGroup.accessory} - animateEntrance={openGroup.id === justToggledId} + animateEntrance={justToggledIds.includes(openGroup.id)} />
{openGroup.content}
@@ -536,7 +545,7 @@ export function PropertyPanelFlat({ isOpen={false} onToggleOpen={() => toggleOpen(g.id)} summary={g.summary} - animateEntrance={g.id === justToggledId} + animateEntrance={justToggledIds.includes(g.id)} /> ))} diff --git a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx index 5da851d2b..dfb2236ef 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx @@ -157,7 +157,7 @@ export function FlatGroupHeader({ accessory?: ReactNode; summary?: string; /** Play the fast entrance animation on this render — set only for the one - * group actually transitioning (see PropertyPanelFlat's justToggledId). + * group(s) actually transitioning (see PropertyPanelFlat's justToggledIds). * Not derived from `isOpen`/remounting alone: React's key-based diffing * can still shift an unrelated collapsed sibling's position in the * before/after-open arrays (e.g. when the newly opened group isn't