fix(web): switch dialog cards to transparent <button> overlay; add test guards

The remaining "Hydration failed" error came from passing a Card (a <div>)
as the asChild target of Radix's DialogTrigger. Radix's Slot then
injects button-specific props (type="button", aria-haspopup, …) onto
the underlying <div>, and React's SSR vs client trees diverge on those
attributes.

Same overlay pattern that already worked for the Pair card now applies
to every Dialog-card-trigger in the app:

- accounts list — Delete card per row
- account detail — Unpair card
- account detail — Delete card

The visible Card stays a <div>. A real <button type="button"> with no
children sits absolutely-positioned over the card surface and is the
DialogTrigger target. Click area is identical, HTML is valid, no Radix
prop-forwarding into the wrong element type.

Also fixed: edit-account-form.tsx had the original
  <button>...<Card>...</Card></button>
nesting (the new static guard caught it). Replaced with a Card that's
its own pressable region (onClick + onKeyDown + role=button on the
<div>; no nested button).

Test guards
-----------
+ src/test/no-render-warnings.test.tsx (6 tests)
  Renders AccountsListView, ThemeToggle, EditMessageForm via
  renderToString and asserts neither console.error nor console.warn
  was invoked. Also scans the produced HTML for any <button> region
  that contains a <div>/<p>/<h*> — invalid nesting that would cause
  a hydration mismatch in the browser.

+ src/test/no-button-wrapping-card.test.ts (2 tests)
  Walks every production .tsx file in src/ and fails if any contains
  a literal `<button` (lowercase) that wraps `<Card`/`<CardContent`/
  `<CardHeader`. Caught a real instance in edit-account-form.tsx that
  I missed in the earlier round.

Total tests: 100.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
yiekheng 2026-05-10 09:22:30 +08:00
parent 99fd2584e4
commit c8199f0bbf
6 changed files with 420 additions and 119 deletions

View File

@ -131,31 +131,34 @@ export default async function AccountDetailPage({ params }: AccountDetailPagePro
</Card>
</Link>
{/* Unpair — entire card opens the confirm dialog */}
{/* Unpair transparent <button> overlay opens the dialog
so we don't pass button-specific props onto the Card div
(Radix asChild does that and it produces a hydration
mismatch on a div). */}
<Dialog>
<DialogTrigger asChild>
<Card
role="button"
tabIndex={0}
aria-label="Unpair WhatsApp"
className="transition-all hover:shadow-md hover:ring-amber-500/30 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
>
<CardContent className="flex items-center justify-between gap-4 py-4">
<div className="flex items-center gap-3">
<div className="flex size-9 items-center justify-center rounded-lg bg-amber-500/10">
<PowerOffIcon className="size-4 text-amber-600 dark:text-amber-400" />
</div>
<div>
<p className="text-sm font-medium">Unpair</p>
<p className="text-xs text-muted-foreground">
Disconnect from WhatsApp; keep the account so you can re-pair later
</p>
</div>
<Card className="relative transition-all hover:shadow-md hover:ring-amber-500/30 cursor-pointer">
<CardContent className="flex items-center justify-between gap-4 py-4">
<div className="flex items-center gap-3">
<div className="flex size-9 items-center justify-center rounded-lg bg-amber-500/10">
<PowerOffIcon className="size-4 text-amber-600 dark:text-amber-400" />
</div>
<ChevronRightIcon className="size-4 text-muted-foreground/60" />
</CardContent>
</Card>
</DialogTrigger>
<div>
<p className="text-sm font-medium">Unpair</p>
<p className="text-xs text-muted-foreground">
Disconnect from WhatsApp; keep the account so you can re-pair later
</p>
</div>
</div>
<ChevronRightIcon className="size-4 text-muted-foreground/60" />
</CardContent>
<DialogTrigger asChild>
<button
type="button"
aria-label="Unpair WhatsApp"
className="absolute inset-0 w-full rounded-xl bg-transparent focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
/>
</DialogTrigger>
</Card>
<DialogContent>
<DialogHeader>
<DialogTitle>Unpair this account?</DialogTitle>
@ -179,31 +182,31 @@ export default async function AccountDetailPage({ params }: AccountDetailPagePro
</>
)}
{/* Delete — entire card opens the confirm dialog */}
{/* Delete — transparent <button> overlay opens the dialog. */}
<Dialog>
<DialogTrigger asChild>
<Card
role="button"
tabIndex={0}
aria-label="Delete account"
className="transition-all hover:shadow-md hover:ring-destructive/30 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-destructive focus-visible:ring-offset-2"
>
<CardContent className="flex items-center justify-between gap-4 py-4">
<div className="flex items-center gap-3">
<div className="flex size-9 items-center justify-center rounded-lg bg-destructive/10">
<Trash2Icon className="size-4 text-destructive" />
</div>
<div>
<p className="text-sm font-medium text-destructive">Delete Account</p>
<p className="text-xs text-muted-foreground">
Remove the account and all its reminders, groups, and history
</p>
</div>
<Card className="relative transition-all hover:shadow-md hover:ring-destructive/30 cursor-pointer">
<CardContent className="flex items-center justify-between gap-4 py-4">
<div className="flex items-center gap-3">
<div className="flex size-9 items-center justify-center rounded-lg bg-destructive/10">
<Trash2Icon className="size-4 text-destructive" />
</div>
<ChevronRightIcon className="size-4 text-muted-foreground/60" />
</CardContent>
</Card>
</DialogTrigger>
<div>
<p className="text-sm font-medium text-destructive">Delete Account</p>
<p className="text-xs text-muted-foreground">
Remove the account and all its reminders, groups, and history
</p>
</div>
</div>
<ChevronRightIcon className="size-4 text-muted-foreground/60" />
</CardContent>
<DialogTrigger asChild>
<button
type="button"
aria-label="Delete account"
className="absolute inset-0 w-full rounded-xl bg-transparent focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-destructive focus-visible:ring-offset-2"
/>
</DialogTrigger>
</Card>
<DialogContent>
<DialogHeader>
<DialogTitle>Delete this account permanently?</DialogTitle>

View File

@ -109,20 +109,21 @@ describe("AccountsListView", () => {
expect(html).toMatch(/<strong>MyBiz<\/strong>/);
});
it("delete card is a focusable trigger element with the destructive aria-label", () => {
it("delete card uses a real <button> overlay (not a div with role=button)", () => {
const html = renderToStaticMarkup(
<AccountsListView
accounts={[mkAccount({ label: "Sales" })]}
deleteFormAction={noopAction}
/>,
);
// The trigger is a Card (rendered as <div>) acting as a button via
// role+tabIndex. Wrapping a <div> in a real <button> would be
// invalid HTML and trigger a hydration mismatch.
expect(html).toMatch(/role="button"/);
expect(html).toMatch(/tabIndex="0"|tabindex="0"/);
// The visible Card stays a <div>; a real <button type="button">
// overlays it. Putting a <div> inside a <button> would be invalid
// HTML, and Radix's `asChild` injecting button props onto a <div>
// would mismatch SSR vs client. The overlay sidesteps both.
expect(html).toMatch(
/<button[^>]*type="button"[^>]*data-testid="account-delete-card"|<button[^>]*data-testid="account-delete-card"[^>]*type="button"/,
);
expect(html).toMatch(/aria-label="Delete Sales"/);
expect(html).toMatch(/data-testid="account-delete-card"/);
// `Delete account` heading copy lives inside the card
expect(html).toContain("Delete account");
});

View File

@ -107,34 +107,34 @@ export function AccountsListView({ accounts, deleteFormAction }: AccountsListVie
</Link>
{/* Dedicated Delete card entire card is the dialog trigger.
We avoid wrapping the Card (a <div>) in a <button> because
div-inside-button is invalid HTML and the browser auto-
closes the button, breaking SSR hydration. Radix's
DialogTrigger asChild forwards the click handler to the
Card element directly; role="button"+tabIndex makes it
keyboard-focusable. */}
We use a transparent <button> overlay instead of passing
the Card as `asChild` to DialogTrigger: Radix injects
`type="button"` and other button-specific props onto its
child, which on a <div> mismatches between SSR and the
client tree. Putting a real <button> over the card keeps
the visible Card as a <div> with valid attributes. */}
<Dialog>
<DialogTrigger asChild>
<Card
role="button"
tabIndex={0}
data-testid="account-delete-card"
aria-label={`Delete ${account.label}`}
className="transition-all hover:shadow-md hover:ring-destructive/30 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-destructive focus-visible:ring-offset-2"
>
<CardContent className="flex items-center gap-3 py-3 px-4">
<div className="flex size-8 shrink-0 items-center justify-center rounded-lg bg-destructive/10">
<Trash2Icon className="size-4 text-destructive" />
</div>
<div className="min-w-0 flex-1">
<p className="text-sm font-medium text-destructive">Delete account</p>
<p className="text-xs text-muted-foreground truncate">
Remove {account.label} and its reminders & groups
</p>
</div>
</CardContent>
</Card>
</DialogTrigger>
<Card className="relative transition-all hover:shadow-md hover:ring-destructive/30 cursor-pointer">
<CardContent className="flex items-center gap-3 py-3 px-4">
<div className="flex size-8 shrink-0 items-center justify-center rounded-lg bg-destructive/10">
<Trash2Icon className="size-4 text-destructive" />
</div>
<div className="min-w-0 flex-1">
<p className="text-sm font-medium text-destructive">Delete account</p>
<p className="text-xs text-muted-foreground truncate">
Remove {account.label} and its reminders & groups
</p>
</div>
</CardContent>
<DialogTrigger asChild>
<button
type="button"
data-testid="account-delete-card"
aria-label={`Delete ${account.label}`}
className="absolute inset-0 w-full rounded-xl bg-transparent focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-destructive focus-visible:ring-offset-2"
/>
</DialogTrigger>
</Card>
<DialogContent>
<DialogHeader>
<DialogTitle>Delete this account?</DialogTitle>

View File

@ -95,51 +95,53 @@ export function EditAccountForm({
const active = account.id === selected;
const connected = account.status === "connected";
return (
<button
<Card
key={account.id}
type="button"
onClick={() => setSelected(account.id)}
role="button"
tabIndex={0}
aria-pressed={active}
aria-label={`Use ${account.label}`}
onClick={() => setSelected(account.id)}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
setSelected(account.id);
}
}}
className={cn(
"block w-full text-left rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2",
"transition-all rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2",
active
? "ring-2 ring-primary/60"
: "hover:shadow-md hover:ring-primary/30 cursor-pointer",
!connected && "opacity-70",
)}
>
<Card
className={cn(
"transition-all",
active
? "ring-2 ring-primary/60"
: "hover:shadow-md hover:ring-primary/30 cursor-pointer",
!connected && "opacity-70",
)}
>
<CardContent className="flex items-center gap-3 py-3 px-4">
<div
className={cn(
"flex size-9 shrink-0 items-center justify-center rounded-lg",
connected
? "bg-emerald-500/10 text-emerald-600 dark:text-emerald-400"
: "bg-muted text-muted-foreground",
)}
>
<SmartphoneIcon className="size-4" />
</div>
<div className="min-w-0 flex-1">
<p className="text-sm font-medium leading-snug truncate">
{account.label}
</p>
<p className="text-xs text-muted-foreground truncate">
{account.phoneNumber ?? "Not paired"}
</p>
</div>
{connected ? (
<WifiIcon className="size-4 text-emerald-500 shrink-0" />
) : (
<WifiOffIcon className="size-4 text-muted-foreground/50 shrink-0" />
<CardContent className="flex items-center gap-3 py-3 px-4">
<div
className={cn(
"flex size-9 shrink-0 items-center justify-center rounded-lg",
connected
? "bg-emerald-500/10 text-emerald-600 dark:text-emerald-400"
: "bg-muted text-muted-foreground",
)}
</CardContent>
</Card>
</button>
>
<SmartphoneIcon className="size-4" />
</div>
<div className="min-w-0 flex-1">
<p className="text-sm font-medium leading-snug truncate">
{account.label}
</p>
<p className="text-xs text-muted-foreground truncate">
{account.phoneNumber ?? "Not paired"}
</p>
</div>
{connected ? (
<WifiIcon className="size-4 text-emerald-500 shrink-0" />
) : (
<WifiOffIcon className="size-4 text-muted-foreground/50 shrink-0" />
)}
</CardContent>
</Card>
);
})}
</div>

View File

@ -0,0 +1,103 @@
import { describe, it, expect } from "vitest";
import { readdirSync, readFileSync, statSync } from "node:fs";
import { join, relative } from "node:path";
/**
* Static guard: no `.tsx` file may contain a literal `<button` element
* wrapping a `<Card`, `<CardContent`, or `<CardHeader` (the shadcn
* primitives that render `<div>`).
*
* That nesting is invalid HTML `<button>`'s content model is phrasing
* content, and `<div>` is flow content. Browsers auto-close the button
* when they hit the inner div; React 19 SSR doesn't, and the resulting
* tree mismatch raised "Hydration failed" in production. We use Card
* directly as the trigger now (DialogTrigger asChild forwards the
* click) or place a transparent submit button as a sibling.
*
* We scan for the lowercase `<button` only `<Button>` (shadcn's
* styled component) is fine; it just renders a real `<button>` with no
* children. `<button>` followed by `<Card>` within a few hundred
* characters is the smell.
*/
const SRC_ROOT = join(__dirname, "..");
function listTsxFiles(dir: string): string[] {
const out: string[] = [];
for (const entry of readdirSync(dir)) {
const full = join(dir, entry);
const st = statSync(full);
if (st.isDirectory()) {
out.push(...listTsxFiles(full));
} else if (entry.endsWith(".tsx")) {
out.push(full);
}
}
return out;
}
interface Hit {
file: string;
line: number;
excerpt: string;
}
function findHits(content: string): Array<{ line: number; excerpt: string }> {
const hits: Array<{ line: number; excerpt: string }> = [];
// For every opening `<button` (lowercase), grab everything up to its
// matching `</button>` and look for a Card primitive opener inside.
const cardOpener = /<(?:Card|CardContent|CardHeader)\b/;
let cursor = 0;
while (cursor < content.length) {
const open = content.indexOf("<button", cursor);
if (open === -1) break;
// Skip JSX comments, attribute-substring matches like `<buttonish`, etc.
const next = content.charAt(open + "<button".length);
if (next && /[a-zA-Z0-9]/.test(next)) {
cursor = open + 1;
continue;
}
const close = content.indexOf("</button>", open);
if (close === -1) break;
const segment = content.slice(open, close);
if (cardOpener.test(segment)) {
const line = content.slice(0, open).split("\n").length;
hits.push({ line, excerpt: segment.slice(0, 160).replace(/\s+/g, " ") });
}
cursor = close + "</button>".length;
}
return hits;
}
describe("static guard: no <button> wrapping a Card primitive", () => {
const files = listTsxFiles(SRC_ROOT)
// Test files describe and document patterns; they may legitimately
// contain regex strings that mention `<button>` and `<Card>` together.
.filter((f) => !/\.test\.tsx?$/.test(f));
it("scans at least one source file (sanity)", () => {
expect(files.length).toBeGreaterThan(0);
});
it("finds no <button>…<Card> nesting in any production .tsx file", () => {
const allHits: Hit[] = [];
for (const file of files) {
const content = readFileSync(file, "utf8");
for (const h of findHits(content)) {
allHits.push({ file: relative(SRC_ROOT, file), ...h });
}
}
if (allHits.length > 0) {
const message = allHits
.map((h) => ` ${h.file}:${h.line}${h.excerpt}`)
.join("\n");
throw new Error(
`Invalid HTML nesting detected — <button> wraps a Card primitive:\n${message}\n` +
`Card renders a <div>, which is flow content and not allowed inside <button>.\n` +
`Use Card directly as DialogTrigger asChild's child, or place a transparent\n` +
`<button type="submit"> as a sibling that overlays the card.`,
);
}
expect(allHits).toEqual([]);
});
});

View File

@ -0,0 +1,192 @@
import { describe, it, expect, vi } from "vitest";
import { renderToString } from "react-dom/server";
import type { ReactNode, ReactElement } from "react";
/**
* Guard: server-rendering critical surfaces must not produce ANY
* console.error or console.warn from React. The most common sources are:
*
* - Invalid HTML nesting (e.g. <div> inside <button>) this is exactly
* what produced the recent "Hydration failed" runtime error.
* - Two children with the same `key`.
* - "Encountered a script tag while rendering React component" a
* React 19 warning when a `<script>` appears inside the React tree
* rather than `<head>`.
*
* We can't fully reproduce *runtime* hydration in a Node test, but
* `renderToString` runs React's SSR-time validation, which catches
* these structural issues during the same render that would later
* mismatch in the browser.
*/
// --- Mocks for next/link + the radix primitives we use --------------------
// These are deliberately TRANSPARENT (render children) so the underlying
// element tree we care about (button vs div nesting, etc.) reaches React's
// validator unchanged.
vi.mock("next/link", () => ({
default: ({ href, children, ...rest }: { href: string; children: ReactNode } & Record<string, unknown>) => (
<a href={href} {...rest}>
{children}
</a>
),
}));
vi.mock("@/components/ui/dialog", () => ({
Dialog: ({ children }: { children: ReactNode }) => <>{children}</>,
DialogTrigger: ({ children }: { children: ReactNode; asChild?: boolean }) => <>{children}</>,
DialogContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DialogHeader: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DialogTitle: ({ children }: { children: ReactNode }) => <h2>{children}</h2>,
DialogDescription: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DialogFooter: ({ children }: { children: ReactNode }) => <div>{children}</div>,
}));
import { AccountsListView, type AccountsListAccount } from "@/components/accounts-list-view";
import { EditMessageForm } from "@/components/reminder-edit/edit-message-form";
// next-themes / radix dropdown mocks for ThemeToggle.
const useThemeReturn: { theme: string | undefined; setTheme: () => void } = {
theme: "system",
setTheme: () => {},
};
vi.mock("next-themes", () => ({
useTheme: () => useThemeReturn,
}));
vi.mock("@/components/ui/dropdown-menu", () => ({
DropdownMenu: ({ children }: { children: ReactNode }) => <>{children}</>,
DropdownMenuTrigger: ({ children }: { children: ReactNode; asChild?: boolean }) => <>{children}</>,
DropdownMenuContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DropdownMenuItem: ({ children }: { children: ReactNode }) => (
<div role="menuitem">{children}</div>
),
}));
// next/navigation is touched by the edit form's useRouter().
vi.mock("next/navigation", () => ({
useRouter: () => ({ push: () => {} }),
}));
// Mock the server action import inside EditMessageForm so importing it
// doesn't try to wire up the real action machinery.
vi.mock("@/actions/reminders", () => ({
updateReminderAction: () => Promise.resolve({ ok: true, reminderId: "r-1" }),
}));
import { ThemeToggle } from "@/components/theme-toggle";
// Helper: render with console.error / console.warn captured. Restores the
// originals via vi.restoreAllMocks at end of each test.
function renderQuiet(node: ReactElement): { html: string; errors: unknown[][]; warns: unknown[][] } {
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
const html = renderToString(node);
const errors = errorSpy.mock.calls;
const warns = warnSpy.mock.calls;
errorSpy.mockRestore();
warnSpy.mockRestore();
return { html, errors, warns };
}
const account: AccountsListAccount = {
id: "a-1",
label: "Personal",
status: "connected",
phoneNumber: "+60123456789",
lastConnectedAt: new Date("2026-05-01T10:00:00Z"),
};
describe("SSR render — no React errors or warnings", () => {
it("AccountsListView (populated) renders cleanly", () => {
const { errors, warns } = renderQuiet(
<AccountsListView accounts={[account]} deleteFormAction={() => {}} />,
);
expect(errors).toEqual([]);
expect(warns).toEqual([]);
});
it("AccountsListView (empty) renders cleanly", () => {
const { errors, warns } = renderQuiet(
<AccountsListView accounts={[]} deleteFormAction={() => {}} />,
);
expect(errors).toEqual([]);
expect(warns).toEqual([]);
});
it("ThemeToggle renders cleanly across all theme values", () => {
for (const t of ["light", "dark", "system", undefined]) {
useThemeReturn.theme = t;
const { errors, warns } = renderQuiet(<ThemeToggle />);
expect(errors, `console.error during theme=${t}`).toEqual([]);
expect(warns, `console.warn during theme=${t}`).toEqual([]);
}
});
it("EditMessageForm renders cleanly (text-only and media-attached)", () => {
const baseProps = {
reminderId: "r-1",
accountId: "acc-1",
groupIds: ["g-1"],
scheduledAtIso: "2026-05-13T09:00:00.000+08:00",
rrule: "FREQ=DAILY",
timezone: "Asia/Kuala_Lumpur",
initialText: "Hello",
initialMediaId: null as string | null,
initialCaption: "",
};
const a = renderQuiet(<EditMessageForm {...baseProps} />);
expect(a.errors).toEqual([]);
expect(a.warns).toEqual([]);
const b = renderQuiet(
<EditMessageForm
{...baseProps}
initialMediaId="m-1"
initialCaption="caption text"
/>,
);
expect(b.errors).toEqual([]);
expect(b.warns).toEqual([]);
});
});
describe("SSR markup — no <div> inside <button> (the bug we just fixed)", () => {
it("AccountsListView never nests a div inside a button", () => {
const { html } = renderQuiet(
<AccountsListView accounts={[account]} deleteFormAction={() => {}} />,
);
// Naive but effective: scan each <button>...</button> region for any
// <div, <p, or <h-tag inside. Those are flow content and not allowed
// inside a button per the HTML spec — browsers auto-close the button
// and the SSR/client trees diverge.
const buttons = [...html.matchAll(/<button\b[^>]*>([\s\S]*?)<\/button>/g)];
for (const m of buttons) {
const inner = m[1] ?? "";
expect(
/<(?:div|p|h[1-6])\b/.test(inner),
`Invalid nesting inside <button>: ${inner.slice(0, 200)}`,
).toBe(false);
}
});
it("EditMessageForm never nests a div inside a button", () => {
const { html } = renderQuiet(
<EditMessageForm
reminderId="r-1"
accountId="acc-1"
groupIds={["g-1"]}
scheduledAtIso="2026-05-13T09:00:00.000+08:00"
rrule={null}
timezone="Asia/Kuala_Lumpur"
initialText="Hello"
initialMediaId={null}
initialCaption=""
/>,
);
const buttons = [...html.matchAll(/<button\b[^>]*>([\s\S]*?)<\/button>/g)];
for (const m of buttons) {
const inner = m[1] ?? "";
expect(/<(?:div|p|h[1-6])\b/.test(inner)).toBe(false);
}
});
});