diff --git a/e2e/v1427-responsive-sheet.spec.ts b/e2e/v1427-responsive-sheet.spec.ts index 0670342de..1e7a35b86 100644 --- a/e2e/v1427-responsive-sheet.spec.ts +++ b/e2e/v1427-responsive-sheet.spec.ts @@ -87,4 +87,48 @@ test.describe("v1.4.27 — ResponsiveSheet branch", () => { await expect(content).toBeVisible({ timeout: 10_000 }); await expect(content).toHaveAttribute("data-variant", "dialog"); }); + + // v1.21.1 — the desktop Dialog branch must scroll only its body and + // keep the footer (the primary action) inside the viewport when the + // form is taller than the available height. A short viewport stands in + // for the real-world triggers — browser zoom, OS display scaling, long + // locale strings — that push a form past `max-h-[calc(100dvh-2rem)]`. + test("Desktop Chrome: footer stays in viewport when the form overflows", async ({ + page, + }, testInfo) => { + test.skip( + testInfo.project.name !== "chromium-desktop", + "desktop-only branch", + ); + + // Deliberately short so the Add-Measurement form exceeds the dialog + // height cap and forces the body to scroll. + await page.setViewportSize({ width: 1280, height: 480 }); + + await page.goto("/measurements?add=WEIGHT", { + waitUntil: "domcontentloaded", + }); + + const content = page.locator('[data-slot="responsive-sheet-content"]'); + await expect(content).toBeVisible({ timeout: 10_000 }); + await expect(content).toHaveAttribute("data-variant", "dialog"); + + // The body owns the scroll — its content is taller than its box. + const body = page.locator('[data-slot="responsive-sheet-body"]'); + await expect(body).toBeVisible(); + const bodyScrolls = await body.evaluate( + (el) => el.scrollHeight > el.clientHeight + 1, + ); + expect(bodyScrolls).toBe(true); + + // The footer (primary action) is reachable without scrolling: its + // bottom edge sits within the viewport. + const footer = page.locator('[data-slot="responsive-sheet-footer"]'); + await expect(footer).toBeVisible(); + const box = await footer.boundingBox(); + expect(box).not.toBeNull(); + const viewport = page.viewportSize(); + expect(viewport).not.toBeNull(); + expect(box!.y + box!.height).toBeLessThanOrEqual(viewport!.height + 1); + }); }); diff --git a/src/components/ui/responsive-sheet.tsx b/src/components/ui/responsive-sheet.tsx index 0099f2a85..e212b3358 100644 --- a/src/components/ui/responsive-sheet.tsx +++ b/src/components/ui/responsive-sheet.tsx @@ -53,10 +53,9 @@ export interface ResponsiveSheetProps { * child and breaks that responsive stacking. This split is a * decision, not drift — don't unify the two branches. * - * The Sheet branch sticky-pins the footer to the - * bottom of the bottom-sheet so the primary CTA stays reachable - * once the body scrolls; the Dialog branch lets the footer flow - * normally beneath the body. + * Both branches keep the footer reachable once the body scrolls: + * the Sheet branch sticky-pins it to the bottom of the bottom-sheet, + * and the Dialog branch holds it `shrink-0` below the scrolling body. */ footer?: React.ReactNode; /** Class name applied to the content surface (sheet / dialog). */ @@ -100,12 +99,15 @@ const CONTENT_WIDTH_CLASS: Record< * centred `` on tablet/desktop. Picks the breakpoint via * the `useIsMobile()` hook (Tailwind `md` — 768 px). * - * On the Sheet branch we cap the content at `90dvh`, scroll the - * body on overflow, and sticky-pin the footer so Save / Cancel - * stay reachable when the keyboard pushes the bottom of the sheet - * up. The Dialog branch consumes the primitive's `max-h-[calc(100dvh-2rem)]` - * default so long forms scroll inside the dialog instead of - * spilling off-screen on tablet portrait. + * Both branches use the same layout contract: cap the surface + * height, scroll ONLY the body on overflow, and keep the footer + * reachable. The Sheet branch caps at `90dvh` and sticky-pins the + * footer so Save / Cancel stay reachable when the keyboard pushes + * the bottom of the sheet up. The Dialog branch caps at the + * primitive's `max-h-[calc(100dvh-2rem)]`, runs as a flex column, + * and pins its header + footer (`shrink-0`) so the body — not the + * whole grid — is what scrolls; the primary action never slides + * below the fold under long forms, browser zoom, or OS scaling. * * Consumers swap `` → `` and feed the * existing form markup through `children` / `footer`. The rest of @@ -194,7 +196,21 @@ export function ResponsiveSheet({ showCloseButton={showCloseButton} data-slot="responsive-sheet-content" data-variant="dialog" - className={cn(CONTENT_WIDTH_CLASS[contentWidth], className)} + // v1.21.1 — flip the desktop Dialog from the primitive's single + // `overflow-y-auto` grid (where header + body + footer all scroll + // together, so the primary CTA can slide below the fold once the + // form, a long locale string, browser zoom, or OS display scaling + // pushes the content past `max-h-[calc(100dvh-2rem)]`) to a flex + // column that scrolls ONLY the body. This brings the Dialog branch + // to parity with the Sheet branch, which already pins its footer. + // The header + footer stay shrink-0 so the body absorbs every + // height shortfall and the action row is always reachable without + // scrolling. `overflow-hidden` makes the body the lone scroll port. + className={cn( + "flex flex-col overflow-hidden", + CONTENT_WIDTH_CLASS[contentWidth], + className, + )} > {hideHeader ? ( @@ -204,7 +220,10 @@ export function ResponsiveSheet({ ) : null} ) : ( - + {title} {description ? ( {description} @@ -213,12 +232,18 @@ export function ResponsiveSheet({ )}
{children}
{footer ? ( - + {footer} ) : null}