Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 109 additions & 1 deletion packages/studio/src/components/editor/PropertyPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,22 @@ function inferredMotionElement() {
// A single "to" tween running from t=2 to t=5 (position 2, duration 3), with
// keyframes on "x" at 0/50/100% — enough to drive both FlatTimingRow's
// inference and the Layout "x" row's keyframe-seek gutter.
// Six-group fixture (fixed-headers + scrollable-open-section worked example):
// tagName "img" turns on both Media and Grade (resolveEditingSections), on top
// of baseElement()'s text-editable + style-editable + timing-eligible
// (data-start) defaults — yielding all six groups in a known order:
// [text, style, layout, motion, grade, media].
function sixGroupElement() {
return {
...baseElement(),
id: "six-group",
selector: "#six-group",
label: "Six Group",
tagName: "img",
dataAttributes: { start: "0", duration: "4" },
};
}

const INFERRED_TIMING_ANIMATION = {
id: "a1",
targetSelector: "#inferred-anim",
Expand Down Expand Up @@ -812,7 +828,7 @@ describe("PropertyPanel — pinning", () => {
expect(pinnedRow?.textContent).toContain("Pinned");

// The divider must appear after the pinned zone.
const container = host.querySelector(".flex-1.overflow-y-auto");
const container = host.querySelector('[data-flat-panel-body="true"]');
const children = Array.from(container?.children ?? []);
const pinnedIndex = children.indexOf(pinnedRow as Element);
const dividerIndex = children.findIndex((el) => el.textContent?.includes("one open below"));
Expand All @@ -837,3 +853,95 @@ describe("PropertyPanel — pinning", () => {
RENDER_TIMEOUT_MS,
);
});

// design_handoff scrollable-open-section: collapsed headers before/after the
// open group render in normal document flow and never move (no sticky, no
// stacking offsets) — only the open group's own body content scrolls, in a
// dedicated region between the two fixed header stacks. Worked example: 6
// groups [text, style, layout, motion, grade, media], motion open (index 3)
// -> text/style/layout render as fixed collapsed headers before it, motion
// renders as an open header + scrollable body, grade/media render as fixed
// collapsed headers after it — in exactly that DOM order, nothing sticky.
describe("PropertyPanel — fixed headers + scrollable open section (Plan 11)", () => {
it(
"renders before-open headers, the open group (header + scrollable body), then after-open headers, in that exact order, with nothing sticky",
async () => {
const { host, root } = await renderPanel(true, sixGroupElement());
// sixGroupElement() opens Text by default; open Motion (index 3) to
// match the worked example.
openFlatGroup(host, "Motion");
expect(openGroupText(host)).toContain("Motion");

const body = host.querySelector('[data-flat-panel-body="true"]');
if (!body) throw new Error("expected the flat panel body container");

// Titles in DOM order: each child is either a collapsed header button
// (before/after the open group) or the open-group wrapper div.
const titles = Array.from(body.children).map((child) => {
if (child.matches('[data-flat-group-collapsed="true"]')) return child.textContent ?? "";
if (child.matches('[data-flat-group-open="true"]')) return child.textContent ?? "";
return null;
});
// Filter to just the group entries (drop nulls from any divider, none
// expected here since no groups are pinned).
const groupTitles = titles.filter((t): t is string => t !== null);
expect(groupTitles).toHaveLength(6);
expect(groupTitles[0]).toContain("Text");
expect(groupTitles[1]).toContain("Style");
expect(groupTitles[2]).toContain("Layout");
expect(groupTitles[3]).toContain("Motion");
expect(groupTitles[4]).toContain("Grade");
expect(groupTitles[5]).toContain("Media");

// The open group (Motion, index 3) is the one wrapped in
// data-flat-group-open, sitting between the before/after collapsed
// headers — and it must contain a dedicated scrollable body.
const openWrapper = host.querySelector('[data-flat-group-open="true"]');
if (!openWrapper) throw new Error("expected the open-group wrapper");
expect(openWrapper.textContent).toContain("Motion");
expect(openWrapper.querySelector(".overflow-y-auto")).not.toBeNull();

// Nothing anywhere in the panel body carries inline sticky positioning
// — the entire sticky-stacking mechanism is gone.
const stickyEls = Array.from(body.querySelectorAll<HTMLElement>("[style]")).filter(
(el) => el.style.position === "sticky",
);
expect(stickyEls).toHaveLength(0);
act(() => root.unmount());
},
RENDER_TIMEOUT_MS,
);

it(
"renders every group as a plain collapsed header with no scrollable middle region when nothing is open",
async () => {
const { host, root } = await renderPanel(true, sixGroupElement());
// Collapse the default-open Text group so openGroupId becomes "".
const collapseButton = host.querySelector<HTMLButtonElement>(
'[data-flat-group-open="true"] button[title="Collapse"]',
);
act(() => collapseButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
expect(host.querySelector('[data-flat-group-open="true"]')).toBeNull();

const collapsedRows = Array.from(
host.querySelectorAll<HTMLButtonElement>('[data-flat-group-collapsed="true"]'),
);
expect(collapsedRows).toHaveLength(6);
const titlesInOrder = collapsedRows.map((el) => el.textContent ?? "");
expect(titlesInOrder[0]).toContain("Text");
expect(titlesInOrder[1]).toContain("Style");
expect(titlesInOrder[2]).toContain("Layout");
expect(titlesInOrder[3]).toContain("Motion");
expect(titlesInOrder[4]).toContain("Grade");
expect(titlesInOrder[5]).toContain("Media");

const body = host.querySelector('[data-flat-panel-body="true"]');
expect(body?.querySelector(".overflow-y-auto")).toBeNull();
for (const row of collapsedRows) {
expect(row.style.position).toBe("");
}
act(() => root.unmount());
},
RENDER_TIMEOUT_MS,
);
});
53 changes: 44 additions & 9 deletions packages/studio/src/components/editor/PropertyPanelFlat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { PropertyPanelProps } from "./propertyPanelHelpers";
import { formatPxMetricValue } from "./propertyPanelHelpers";
import { PropertyPanelFlatHeader } from "./PropertyPanelFlatHeader";
import { PropertyPanelFlatFooter } from "./PropertyPanelFlatFooter";
import { FlatGroup, PinnedGroupRow, PinnedZoneDivider } from "./propertyPanelFlatPrimitives";
import { FlatGroupHeader, PinnedGroupRow, PinnedZoneDivider } from "./propertyPanelFlatPrimitives";
import { FlatTextSection } from "./propertyPanelFlatTextSection";
import { FlatStyleSection } from "./propertyPanelFlatStyleSections";
import { FlatLayoutSection } from "./propertyPanelFlatLayoutSection";
Expand Down Expand Up @@ -456,6 +456,18 @@ export function PropertyPanelFlat({
const pinned = groups.filter((g) => pinnedGroupIds.includes(g.id));
const unpinned = groups.filter((g) => !pinnedGroupIds.includes(g.id));

// Fixed-headers + scrollable-open-section layout (design_handoff
// scrollable-open-section, replaces the prior sticky-stacking mechanism):
// collapsed headers before/after the open group render in normal document
// flow and never move. Only the open group's own body content scrolls, in
// a dedicated region between the two fixed header stacks. When no group is
// open, every group is just a collapsed header — there's no scrollable
// middle region at all, since nothing is expanded.
const openIndex = unpinned.findIndex((g) => g.id === openGroupId);
const beforeOpen = openIndex === -1 ? unpinned : unpinned.slice(0, openIndex);
const openGroup = openIndex === -1 ? null : unpinned[openIndex];
const afterOpen = openIndex === -1 ? [] : unpinned.slice(openIndex + 1);

return (
<div className="flex h-full min-h-0 flex-col overflow-hidden bg-panel-bg text-panel-text-1">
<PropertyPanelFlatHeader
Expand All @@ -474,7 +486,7 @@ export function PropertyPanelFlat({
onUngroup={onUngroup}
showUngroup={Boolean(onUngroup && element.dataAttributes["hf-group"] != null)}
/>
<div className="flex-1 overflow-y-auto">
<div data-flat-panel-body="true" className="flex min-h-0 flex-1 flex-col overflow-hidden">
{pinned.map((g) => (
<PinnedGroupRow
key={g.id}
Expand All @@ -486,19 +498,42 @@ export function PropertyPanelFlat({
</PinnedGroupRow>
))}
{pinned.length > 0 && unpinned.length > 0 && <PinnedZoneDivider />}
{unpinned.map((g) => (
<FlatGroup
{beforeOpen.map((g) => (
<FlatGroupHeader
key={g.id}
title={g.title}
isOpen={openGroupId === g.id}
isOpen={false}
isPinned={false}
onToggleOpen={() => toggleOpen(g.id)}
onTogglePin={() => togglePin(g.id)}
summary={g.summary}
accessory={g.accessory}
>
{g.content}
</FlatGroup>
/>
))}
{openGroup && (
<div data-flat-group-open="true" className="flex min-h-0 flex-1 flex-col">
<FlatGroupHeader
title={openGroup.title}
isOpen
isPinned={false}
onToggleOpen={() => toggleOpen(openGroup.id)}
onTogglePin={() => togglePin(openGroup.id)}
accessory={openGroup.accessory}
/>
<div className="min-h-0 flex-1 overflow-y-auto border-b border-panel-hairline px-4 py-3">
{openGroup.content}
</div>
</div>
)}
{afterOpen.map((g) => (
<FlatGroupHeader
key={g.id}
title={g.title}
isOpen={false}
isPinned={false}
onToggleOpen={() => toggleOpen(g.id)}
onTogglePin={() => togglePin(g.id)}
summary={g.summary}
/>
))}
</div>
<PropertyPanelFlatFooter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,33 @@ describe("PropertyPanelFlatFooter", () => {
expect(recordButton?.title).toBe("Stop recording 2.4s");
act(() => root.unmount());
});

// Plan 10 (sticky-footer-gap): the root must carry an opaque background —
// it previously had none at all, letting scrolled panel content show
// through. Regression coverage for the definite fix from the brief.
it("has an opaque bg-panel-bg background on its root element", () => {
const { host, root } = renderFooter({ onAskAgent: vi.fn() });
const footerRoot = host.firstElementChild as HTMLElement;
expect(footerRoot.className).toContain("bg-panel-bg");
act(() => root.unmount());
});

// Plan 11 (scrollable-open-section): the prior sticky-stacking mechanism —
// and the Plan 10 hairline-sealing hack it required at this exact boundary
// (an absolutely-positioned overlay patching a Chromium sticky-offset
// rounding gap) — is gone now that nothing above the footer is
// `position: sticky`. Live browser verification (p11 report) confirmed the
// boundary renders as a single clean hairline without it: whatever
// immediately precedes the footer (a collapsed FlatGroupHeader, the open
// group's scrollable body wrapper, or a PinnedGroupRow) already draws its
// own border-b in normal document flow, so the footer needs no border or
// seal of its own.
it("renders no seal overlay and no border of its own — the boundary line comes from whatever precedes it", () => {
const { host, root } = renderFooter({ onAskAgent: vi.fn() });
const footerRoot = host.firstElementChild as HTMLElement;
expect(footerRoot.className).not.toContain("border-t");
expect(footerRoot.className).not.toContain("border-b");
expect(host.querySelector('[data-flat-footer-seal="true"]')).toBeNull();
act(() => root.unmount());
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ export function PropertyPanelFlatFooter({
: "Record gesture (R)";

return (
<div className="flex items-center justify-between border-t border-panel-hairline px-4 py-[11px]">
// No border-t here: every possible element immediately above this footer
// in the new fixed-headers + scrollable-open-section layout (a collapsed
// FlatGroupHeader, the open group's scrollable body wrapper, or a
// PinnedGroupRow) already draws its own border-b in normal document flow
// — nothing here is `position: sticky` anymore, so there's no rounding
// seam to seal (see p11-scrollable-open-section-report.md).
<div className="flex items-center justify-between bg-panel-bg px-4 py-[11px]">
<button
type="button"
data-flat-footer-ask="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React, { act } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, it, vi } from "vitest";
import {
FlatGroup,
FlatGroupHeader,
FlatRow,
FlatSegmentedRow,
FlatSelectRow,
Expand Down Expand Up @@ -109,49 +109,78 @@ describe("FlatSegmentedRow", () => {
});
});

describe("FlatGroup", () => {
it("renders the open header (name + pin + caret) and shows children", () => {
describe("FlatGroupHeader", () => {
it("renders the open header (name + pin + caret), with no sticky-related props required", () => {
const onToggleOpen = vi.fn();
const onTogglePin = vi.fn();
const { host, root } = renderInto(
<FlatGroup
<FlatGroupHeader
title="Text"
isOpen
isPinned={false}
onToggleOpen={onToggleOpen}
onTogglePin={onTogglePin}
>
<div data-testid="body">body</div>
</FlatGroup>,
/>,
);
expect(host.querySelector('[data-testid="body"]')).not.toBeNull();
expect(host.textContent).toContain("Text");
const pin = host.querySelector<HTMLButtonElement>('[data-flat-group-pin="true"]');
act(() => pin?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
expect(onTogglePin).toHaveBeenCalledTimes(1);
const collapse = host.querySelector<HTMLButtonElement>('button[title="Collapse"]');
act(() => collapse?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
expect(onToggleOpen).toHaveBeenCalledTimes(1);
act(() => root.unmount());
});

it("renders the collapsed row (name + summary + caret-right) and no children", () => {
it("renders the collapsed row (name + summary + caret-right) with no sticky positioning", () => {
const onToggleOpen = vi.fn();
const { host, root } = renderInto(
<FlatGroup
<FlatGroupHeader
title="Style"
isOpen={false}
isPinned={false}
onToggleOpen={onToggleOpen}
onTogglePin={vi.fn()}
summary="fill none · 100%"
>
<div data-testid="body">body</div>
</FlatGroup>,
/>,
);
expect(host.querySelector('[data-testid="body"]')).toBeNull();
expect(host.textContent).toContain("fill none · 100%");
const row = host.querySelector<HTMLButtonElement>('[data-flat-group-collapsed="true"]');
expect(row?.style.position).toBe("");
act(() => row?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
expect(onToggleOpen).toHaveBeenCalledTimes(1);
act(() => root.unmount());
});

it("renders no inline position styling in either state (collapsed headers never move)", () => {
const { host: collapsedHost, root: collapsedRoot } = renderInto(
<FlatGroupHeader
title="Layout"
isOpen={false}
isPinned={false}
onToggleOpen={vi.fn()}
onTogglePin={vi.fn()}
/>,
);
const row = collapsedHost.querySelector<HTMLButtonElement>(
'[data-flat-group-collapsed="true"]',
);
expect(row?.getAttribute("style")).toBeNull();
act(() => collapsedRoot.unmount());

const { host: openHost, root: openRoot } = renderInto(
<FlatGroupHeader
title="Motion"
isOpen
isPinned={false}
onToggleOpen={vi.fn()}
onTogglePin={vi.fn()}
/>,
);
expect(openHost.textContent).toContain("Motion");
expect(openHost.querySelector("[style]")).toBeNull();
act(() => openRoot.unmount());
});
});

describe("PinnedZoneDivider", () => {
Expand Down
Loading
Loading