Skip to content

Commit fca1d7d

Browse files
committed
fix(studio): animate the implicitly-closed group too, add regression test
1 parent fdf061d commit fca1d7d

3 files changed

Lines changed: 74 additions & 18 deletions

File tree

packages/studio/src/components/editor/PropertyPanel.test.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,3 +899,50 @@ describe("PropertyPanel — fixed headers + scrollable open section (Plan 11)",
899899
RENDER_TIMEOUT_MS,
900900
);
901901
});
902+
903+
describe("PropertyPanel — flat group entrance animation scoping (fix round)", () => {
904+
it(
905+
"animates only the opening group and the implicitly-closed group on a non-adjacent toggle, never untouched siblings",
906+
async () => {
907+
const { host, root } = await renderPanel(true, sixGroupElement());
908+
// sixGroupElement() opens Text by default; jump straight to Motion
909+
// (skipping over Style/Layout) first, matching the Plan 11 worked
910+
// example, then jump back to Text — non-adjacent from Motion, again
911+
// skipping over Style/Layout. This is the exact array-slice-position-
912+
// shift scenario the justToggledIds mechanism exists to guard: Style
913+
// and Layout shift position in the before/after-open slices on both
914+
// toggles even though neither of them is the group being toggled.
915+
openFlatGroup(host, "Motion");
916+
expect(openGroupText(host)).toContain("Motion");
917+
openFlatGroup(host, "Text");
918+
expect(openGroupText(host)).toContain("Text");
919+
920+
const collapsedRowByTitle = (title: string) => {
921+
const row = Array.from(host.querySelectorAll('[data-flat-group-collapsed="true"]')).find(
922+
(el) => el.textContent?.includes(title),
923+
);
924+
if (!row) throw new Error(`expected a collapsed ${title} row`);
925+
return row;
926+
};
927+
928+
// Untouched, non-adjacent siblings must NOT receive the entrance class,
929+
// even though they shifted position in the collapsed-header list.
930+
expect(collapsedRowByTitle("Style").classList.contains("hf-flat-group-enter")).toBe(false);
931+
expect(collapsedRowByTitle("Layout").classList.contains("hf-flat-group-enter")).toBe(false);
932+
expect(collapsedRowByTitle("Grade").classList.contains("hf-flat-group-enter")).toBe(false);
933+
expect(collapsedRowByTitle("Media").classList.contains("hf-flat-group-enter")).toBe(false);
934+
935+
// Motion — open a moment ago, just implicitly closed by the click on
936+
// Text — must still play its own collapse-entrance animation (Finding 1).
937+
expect(collapsedRowByTitle("Motion").classList.contains("hf-flat-group-enter")).toBe(true);
938+
939+
// Text — the group actually clicked open — must animate too.
940+
const openWrapper = host.querySelector('[data-flat-group-open="true"]');
941+
if (!openWrapper) throw new Error("expected the open-group wrapper");
942+
expect(openWrapper.querySelector(".hf-flat-group-enter")).not.toBeNull();
943+
944+
act(() => root.unmount());
945+
},
946+
RENDER_TIMEOUT_MS,
947+
);
948+
});

packages/studio/src/components/editor/PropertyPanelFlat.tsx

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -235,17 +235,20 @@ export function PropertyPanelFlat({
235235
: "layout",
236236
);
237237

238-
// Tracks which single group is actively transitioning, so its header/body
239-
// gets the fast entrance animation (hf-flat-group-enter) and no one else's
240-
// does. Deliberately NOT derived from remounting alone: FlatGroupHeader
241-
// instances are keyed by group id and React normally preserves them across
242-
// re-renders, but toggling a non-adjacent group still shifts the untouched
243-
// collapsed siblings between the before/after-open slices below, and
244-
// Chromium restarts a CSS animation on that kind of position shift even
245-
// though nothing about the sibling actually changed. Gating on this id
246-
// (cleared shortly after the 120ms CSS animation finishes) keeps the
247-
// animation scoped to only the group that actually just toggled.
248-
const [justToggledId, setJustToggledId] = useState<string | null>(null);
238+
// Tracks which group(s) are actively transitioning this toggle cycle, so
239+
// their header/body gets the fast entrance animation (hf-flat-group-enter)
240+
// and no one else's does. Deliberately NOT derived from remounting alone:
241+
// FlatGroupHeader instances are keyed by group id and React normally
242+
// preserves them across re-renders, but toggling a non-adjacent group still
243+
// shifts the untouched collapsed siblings between the before/after-open
244+
// slices below, and Chromium restarts a CSS animation on that kind of
245+
// position shift even though nothing about the sibling actually changed.
246+
// Gating on these ids (cleared shortly after the 120ms CSS animation
247+
// finishes) keeps the animation scoped to only the groups that actually
248+
// just toggled. Two ids, not one: the clicked (newly-opening/closing) group
249+
// AND whichever group was open immediately before the click and got
250+
// implicitly closed by it — both freshly-mounted headers need to animate.
251+
const [justToggledIds, setJustToggledIds] = useState<string[]>([]);
249252
const justToggledTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
250253
useEffect(() => {
251254
return () => {
@@ -270,10 +273,16 @@ export function PropertyPanelFlat({
270273
const isTextEditable = isTextEditableSelection(element);
271274
const elementKind = sections.media ? "media" : element.textFields.length > 0 ? "text" : "other";
272275
const toggleOpen = (groupId: string) => {
276+
// Capture what was open BEFORE this click (this render's closure over
277+
// openGroupId), so the group that's about to be implicitly closed can be
278+
// tracked too — not just the one the user clicked.
279+
const previousOpenGroupId = openGroupId;
273280
setOpenGroupId((current) => (current === groupId ? "" : groupId));
274-
setJustToggledId(groupId);
281+
const implicitlyClosedId =
282+
previousOpenGroupId && previousOpenGroupId !== groupId ? previousOpenGroupId : null;
283+
setJustToggledIds(implicitlyClosedId ? [groupId, implicitlyClosedId] : [groupId]);
275284
if (justToggledTimeoutRef.current) clearTimeout(justToggledTimeoutRef.current);
276-
justToggledTimeoutRef.current = setTimeout(() => setJustToggledId(null), 200);
285+
justToggledTimeoutRef.current = setTimeout(() => setJustToggledIds([]), 200);
277286
};
278287
// Basis for the Layout keyframe gutter (X/Y/W/H/Angle + 3D Transform) —
279288
// must agree with Motion's Timing row (FlatTimingRow), which infers the
@@ -510,7 +519,7 @@ export function PropertyPanelFlat({
510519
isOpen={false}
511520
onToggleOpen={() => toggleOpen(g.id)}
512521
summary={g.summary}
513-
animateEntrance={g.id === justToggledId}
522+
animateEntrance={justToggledIds.includes(g.id)}
514523
/>
515524
))}
516525
{openGroup && (
@@ -520,10 +529,10 @@ export function PropertyPanelFlat({
520529
isOpen
521530
onToggleOpen={() => toggleOpen(openGroup.id)}
522531
accessory={openGroup.accessory}
523-
animateEntrance={openGroup.id === justToggledId}
532+
animateEntrance={justToggledIds.includes(openGroup.id)}
524533
/>
525534
<div
526-
className={`${openGroup.id === justToggledId ? "hf-flat-group-enter " : ""}min-h-0 flex-1 overflow-y-auto border-b border-panel-hairline px-4 py-3`}
535+
className={`${justToggledIds.includes(openGroup.id) ? "hf-flat-group-enter " : ""}min-h-0 flex-1 overflow-y-auto border-b border-panel-hairline px-4 py-3`}
527536
>
528537
{openGroup.content}
529538
</div>
@@ -536,7 +545,7 @@ export function PropertyPanelFlat({
536545
isOpen={false}
537546
onToggleOpen={() => toggleOpen(g.id)}
538547
summary={g.summary}
539-
animateEntrance={g.id === justToggledId}
548+
animateEntrance={justToggledIds.includes(g.id)}
540549
/>
541550
))}
542551
</div>

packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ export function FlatGroupHeader({
157157
accessory?: ReactNode;
158158
summary?: string;
159159
/** Play the fast entrance animation on this render — set only for the one
160-
* group actually transitioning (see PropertyPanelFlat's justToggledId).
160+
* group(s) actually transitioning (see PropertyPanelFlat's justToggledIds).
161161
* Not derived from `isOpen`/remounting alone: React's key-based diffing
162162
* can still shift an unrelated collapsed sibling's position in the
163163
* before/after-open arrays (e.g. when the newly opened group isn't

0 commit comments

Comments
 (0)