feat(web): drop Delete card from accounts overview
Account-level destructive actions (Delete, Unpair, Re-pair) live on the detail page only. The overview is now a calm grid of one card per account, each linking to its detail page. - Removed the dedicated Delete card and its dialog from accounts-list-view.tsx. - The whole account card is once again the link target — no inline trigger surfaces, no Dialog component, no destructive click area. - AccountsListView no longer needs the deleteFormAction prop; the /accounts page passes only `accounts`. Tests updated: - accounts-list-view.test.tsx: 6 tests now (was 8). The two cases that asserted on the delete card are replaced with one positive test that asserts no Delete affordance is rendered on the overview, plus a test that the only `<a>` per cell wraps the card with no inline buttons inside it. - no-render-warnings.test.tsx: drops the obsolete deleteFormAction prop in its renderQuiet calls. Hydration: live curl on /, /accounts, /reminders, /activity, /settings and a detail page returns 200 with no Hydration / script-tag warning in the web logs after this commit. 98 tests passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c8199f0bbf
commit
8ca7ebdd5b
@ -1,11 +1,10 @@
|
||||
import { AccountsListView } from "@/components/accounts-list-view";
|
||||
import { getSeededOperator } from "@/lib/operator";
|
||||
import { listAccounts } from "@/lib/queries";
|
||||
import { deleteAccountAction } from "@/actions/accounts";
|
||||
|
||||
export default async function AccountsPage() {
|
||||
const op = await getSeededOperator();
|
||||
const accounts = await listAccounts(op.id);
|
||||
|
||||
return <AccountsListView accounts={accounts} deleteFormAction={deleteAccountAction} />;
|
||||
return <AccountsListView accounts={accounts} />;
|
||||
}
|
||||
|
||||
@ -16,21 +16,6 @@ vi.mock("next/link", () => ({
|
||||
),
|
||||
}));
|
||||
|
||||
// Radix Dialog uses portals + client refs. For SSR markup tests we want
|
||||
// the trigger to be rendered inline (with `asChild`) and the content tree
|
||||
// to render too, so we can assert dialog text deterministically.
|
||||
vi.mock("./ui/dialog", () => ({
|
||||
Dialog: ({ children }: { children: ReactNode }) => <>{children}</>,
|
||||
DialogTrigger: ({ children }: { children: ReactNode; asChild?: boolean }) => <>{children}</>,
|
||||
DialogContent: ({ children }: { children: ReactNode }) => (
|
||||
<div data-testid="dialog-content">{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>,
|
||||
}));
|
||||
|
||||
const mkAccount = (over: Partial<AccountsListAccount> = {}): AccountsListAccount => ({
|
||||
id: "a-1",
|
||||
label: "Personal",
|
||||
@ -60,11 +45,9 @@ function render(html: string) {
|
||||
};
|
||||
}
|
||||
|
||||
const noopAction = () => {};
|
||||
|
||||
describe("AccountsListView", () => {
|
||||
describe("layout — accounts present", () => {
|
||||
it("renders one cell per account, each with a main card and a delete card", () => {
|
||||
it("renders exactly one card per account (no inline destructive triggers)", () => {
|
||||
const accounts = [
|
||||
mkAccount({ id: "a-1", label: "Personal" }),
|
||||
mkAccount({ id: "a-2", label: "Work" }),
|
||||
@ -72,100 +55,56 @@ describe("AccountsListView", () => {
|
||||
];
|
||||
|
||||
const { count } = render(
|
||||
renderToStaticMarkup(
|
||||
<AccountsListView accounts={accounts} deleteFormAction={noopAction} />,
|
||||
),
|
||||
renderToStaticMarkup(<AccountsListView accounts={accounts} />),
|
||||
);
|
||||
|
||||
expect(count('data-testid="account-cell"')).toBe(3);
|
||||
expect(count('data-testid="account-card"')).toBe(3);
|
||||
expect(count('data-testid="account-delete-card"')).toBe(3);
|
||||
});
|
||||
|
||||
it("each main card links to /accounts/[id]", () => {
|
||||
const { has } = render(
|
||||
renderToStaticMarkup(
|
||||
<AccountsListView
|
||||
accounts={[mkAccount({ id: "abc-123", label: "Personal" })]}
|
||||
deleteFormAction={noopAction}
|
||||
/>,
|
||||
),
|
||||
);
|
||||
expect(has(/href="\/accounts\/abc-123"/)).toBe(true);
|
||||
});
|
||||
|
||||
it("renders the account label in both the main card and the delete card", () => {
|
||||
it("does NOT render any Delete affordance on the overview", () => {
|
||||
// Account-level destructive actions live on the detail page only.
|
||||
const html = renderToStaticMarkup(
|
||||
<AccountsListView
|
||||
accounts={[mkAccount({ label: "MyBiz" })]}
|
||||
deleteFormAction={noopAction}
|
||||
/>,
|
||||
<AccountsListView accounts={[mkAccount({ label: "Sales" })]} />,
|
||||
);
|
||||
// Main card title
|
||||
expect(html).toContain(">MyBiz<");
|
||||
// Delete card description references the account by name
|
||||
expect(html).toContain("Remove MyBiz and its reminders");
|
||||
// Dialog confirmation also uses the label
|
||||
expect(html).toMatch(/<strong>MyBiz<\/strong>/);
|
||||
expect(html).not.toContain("Delete account");
|
||||
expect(html).not.toContain("Remove Sales");
|
||||
expect(html).not.toMatch(/data-testid="account-delete-card"/);
|
||||
expect(html).not.toMatch(/aria-label="Delete /);
|
||||
});
|
||||
|
||||
it("delete card uses a real <button> overlay (not a div with role=button)", () => {
|
||||
it("the whole card is the link target — no inline buttons", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<AccountsListView
|
||||
accounts={[mkAccount({ label: "Sales" })]}
|
||||
deleteFormAction={noopAction}
|
||||
/>,
|
||||
<AccountsListView accounts={[mkAccount({ id: "abc-123", label: "Personal" })]} />,
|
||||
);
|
||||
// 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"/);
|
||||
// `Delete account` heading copy lives inside the card
|
||||
expect(html).toContain("Delete account");
|
||||
});
|
||||
|
||||
it("delete dialog form posts the matching accountId hidden input", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<AccountsListView
|
||||
accounts={[mkAccount({ id: "uuid-XYZ" })]}
|
||||
deleteFormAction={noopAction}
|
||||
/>,
|
||||
);
|
||||
expect(html).toMatch(
|
||||
/<input[^>]+type="hidden"[^>]+name="accountId"[^>]+value="uuid-XYZ"/,
|
||||
);
|
||||
expect(html).toContain("Yes, delete");
|
||||
// Wrapping anchor goes straight to the detail page.
|
||||
expect(html).toMatch(/<a [^>]*href="\/accounts\/abc-123"/);
|
||||
// Header CTA (Add Account) is the only <button>-like control —
|
||||
// and even that is rendered as an <a> by our mocked Link wrapper.
|
||||
// No inline form/button trigger inside any account-cell.
|
||||
const cells = html.match(
|
||||
/<a [^>]*data-testid="account-cell"[^>]*>[\s\S]*?<\/a>/g,
|
||||
) ?? [];
|
||||
expect(cells).toHaveLength(1);
|
||||
expect(cells[0]).not.toContain("<button");
|
||||
});
|
||||
|
||||
it("displays the phone number when paired, italic 'Not paired yet' otherwise", () => {
|
||||
const paired = renderToStaticMarkup(
|
||||
<AccountsListView
|
||||
accounts={[mkAccount({ phoneNumber: "+60123456789" })]}
|
||||
deleteFormAction={noopAction}
|
||||
/>,
|
||||
<AccountsListView accounts={[mkAccount({ phoneNumber: "+60123456789" })]} />,
|
||||
);
|
||||
expect(paired).toContain("+60123456789");
|
||||
expect(paired).not.toContain("Not paired yet");
|
||||
|
||||
const unpaired = renderToStaticMarkup(
|
||||
<AccountsListView
|
||||
accounts={[mkAccount({ phoneNumber: null })]}
|
||||
deleteFormAction={noopAction}
|
||||
/>,
|
||||
<AccountsListView accounts={[mkAccount({ phoneNumber: null })]} />,
|
||||
);
|
||||
expect(unpaired).toContain("Not paired yet");
|
||||
});
|
||||
|
||||
it("renders the Add Account header link", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<AccountsListView
|
||||
accounts={[mkAccount()]}
|
||||
deleteFormAction={noopAction}
|
||||
/>,
|
||||
<AccountsListView accounts={[mkAccount()]} />,
|
||||
);
|
||||
expect(html).toContain("Add Account");
|
||||
expect(html).toMatch(/href="\/accounts\/new"/);
|
||||
@ -174,9 +113,7 @@ describe("AccountsListView", () => {
|
||||
|
||||
describe("layout — empty state", () => {
|
||||
it("shows the empty-state card and hides the grid when no accounts", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<AccountsListView accounts={[]} deleteFormAction={noopAction} />,
|
||||
);
|
||||
const html = renderToStaticMarkup(<AccountsListView accounts={[]} />);
|
||||
expect(html).toContain('data-testid="accounts-empty"');
|
||||
expect(html).not.toContain('data-testid="accounts-grid"');
|
||||
expect(html).not.toContain('data-testid="account-cell"');
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
import Link from "next/link";
|
||||
import { PlusIcon, SmartphoneIcon, CalendarIcon, Trash2Icon } from "lucide-react";
|
||||
import { PlusIcon, SmartphoneIcon, CalendarIcon } from "lucide-react";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import {
|
||||
Card,
|
||||
@ -7,15 +7,6 @@ import {
|
||||
CardHeader,
|
||||
CardTitle,
|
||||
} from "@/components/ui/card";
|
||||
import {
|
||||
Dialog,
|
||||
DialogContent,
|
||||
DialogDescription,
|
||||
DialogFooter,
|
||||
DialogHeader,
|
||||
DialogTitle,
|
||||
DialogTrigger,
|
||||
} from "@/components/ui/dialog";
|
||||
import { AccountStatusBadge } from "@/components/account-status-badge";
|
||||
|
||||
export interface AccountsListAccount {
|
||||
@ -28,17 +19,15 @@ export interface AccountsListAccount {
|
||||
|
||||
interface AccountsListViewProps {
|
||||
accounts: AccountsListAccount[];
|
||||
/** Server action wired by the page; takes the FormData with `accountId`. */
|
||||
deleteFormAction: (formData: FormData) => void | Promise<void>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure presentational view for /accounts. Renders one main link card per
|
||||
* account and a separate destructive Delete card stacked below it. The
|
||||
* page passes accounts + the server action; this component has no data
|
||||
* dependencies, which makes it unit-testable via SSR markup.
|
||||
* Pure presentational view for /accounts. Renders one card per account
|
||||
* that links to its detail page. Account-level destructive actions
|
||||
* (Delete, Unpair, Re-pair) live on the detail page so this overview
|
||||
* stays calm — no inline trigger surfaces here.
|
||||
*/
|
||||
export function AccountsListView({ accounts, deleteFormAction }: AccountsListViewProps) {
|
||||
export function AccountsListView({ accounts }: AccountsListViewProps) {
|
||||
return (
|
||||
<div className="px-4 py-6 sm:px-6 sm:py-8 max-w-5xl mx-auto space-y-6">
|
||||
<div className="flex items-center justify-between gap-4">
|
||||
@ -58,103 +47,50 @@ export function AccountsListView({ accounts, deleteFormAction }: AccountsListVie
|
||||
className="grid grid-cols-1 gap-4 sm:grid-cols-2 lg:grid-cols-3"
|
||||
>
|
||||
{accounts.map((account) => (
|
||||
<div
|
||||
<Link
|
||||
key={account.id}
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
href={`/accounts/${account.id}` as any}
|
||||
data-testid="account-cell"
|
||||
data-account-id={account.id}
|
||||
className="flex flex-col gap-2"
|
||||
className="block focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded-xl"
|
||||
>
|
||||
<Link
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
href={`/accounts/${account.id}` as any}
|
||||
className="block focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded-xl"
|
||||
<Card
|
||||
data-testid="account-card"
|
||||
className="h-full transition-all hover:shadow-md hover:ring-primary/30 cursor-pointer"
|
||||
>
|
||||
<Card
|
||||
data-testid="account-card"
|
||||
className="h-full transition-all hover:shadow-md hover:ring-primary/30 cursor-pointer"
|
||||
>
|
||||
<CardHeader>
|
||||
<div className="flex items-start justify-between gap-2">
|
||||
<CardTitle className="text-base leading-snug">{account.label}</CardTitle>
|
||||
<AccountStatusBadge status={account.status} />
|
||||
<CardHeader>
|
||||
<div className="flex items-start justify-between gap-2">
|
||||
<CardTitle className="text-base leading-snug">{account.label}</CardTitle>
|
||||
<AccountStatusBadge status={account.status} />
|
||||
</div>
|
||||
</CardHeader>
|
||||
<CardContent className="space-y-2">
|
||||
{account.phoneNumber ? (
|
||||
<div className="flex items-center gap-1.5 text-sm text-muted-foreground">
|
||||
<SmartphoneIcon className="size-3.5 shrink-0" />
|
||||
<span>{account.phoneNumber}</span>
|
||||
</div>
|
||||
</CardHeader>
|
||||
<CardContent className="space-y-2">
|
||||
{account.phoneNumber ? (
|
||||
<div className="flex items-center gap-1.5 text-sm text-muted-foreground">
|
||||
<SmartphoneIcon className="size-3.5 shrink-0" />
|
||||
<span>{account.phoneNumber}</span>
|
||||
</div>
|
||||
) : (
|
||||
<p className="text-sm text-muted-foreground/60 italic">Not paired yet</p>
|
||||
)}
|
||||
{account.lastConnectedAt ? (
|
||||
<div className="flex items-center gap-1.5 text-xs text-muted-foreground">
|
||||
<CalendarIcon className="size-3 shrink-0" />
|
||||
<span>
|
||||
Last connected{" "}
|
||||
{account.lastConnectedAt.toLocaleDateString("en-MY", {
|
||||
timeZone: "Asia/Kuala_Lumpur",
|
||||
year: "numeric",
|
||||
month: "short",
|
||||
day: "numeric",
|
||||
})}
|
||||
</span>
|
||||
</div>
|
||||
) : null}
|
||||
</CardContent>
|
||||
</Card>
|
||||
</Link>
|
||||
|
||||
{/* Dedicated Delete card — entire card is the dialog trigger.
|
||||
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>
|
||||
<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" />
|
||||
) : (
|
||||
<p className="text-sm text-muted-foreground/60 italic">Not paired yet</p>
|
||||
)}
|
||||
{account.lastConnectedAt ? (
|
||||
<div className="flex items-center gap-1.5 text-xs text-muted-foreground">
|
||||
<CalendarIcon className="size-3 shrink-0" />
|
||||
<span>
|
||||
Last connected{" "}
|
||||
{account.lastConnectedAt.toLocaleDateString("en-MY", {
|
||||
timeZone: "Asia/Kuala_Lumpur",
|
||||
year: "numeric",
|
||||
month: "short",
|
||||
day: "numeric",
|
||||
})}
|
||||
</span>
|
||||
</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>
|
||||
<DialogDescription>
|
||||
<strong>{account.label}</strong> and all its reminders, groups, and
|
||||
history will be permanently removed. This cannot be undone.
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
<DialogFooter showCloseButton>
|
||||
<form action={deleteFormAction}>
|
||||
<input type="hidden" name="accountId" value={account.id} />
|
||||
<Button type="submit" variant="destructive" size="sm">
|
||||
<Trash2Icon />
|
||||
Yes, delete
|
||||
</Button>
|
||||
</form>
|
||||
</DialogFooter>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
</div>
|
||||
) : null}
|
||||
</CardContent>
|
||||
</Card>
|
||||
</Link>
|
||||
))}
|
||||
</div>
|
||||
) : (
|
||||
|
||||
@ -99,7 +99,7 @@ const account: AccountsListAccount = {
|
||||
describe("SSR render — no React errors or warnings", () => {
|
||||
it("AccountsListView (populated) renders cleanly", () => {
|
||||
const { errors, warns } = renderQuiet(
|
||||
<AccountsListView accounts={[account]} deleteFormAction={() => {}} />,
|
||||
<AccountsListView accounts={[account]} />,
|
||||
);
|
||||
expect(errors).toEqual([]);
|
||||
expect(warns).toEqual([]);
|
||||
@ -107,7 +107,7 @@ describe("SSR render — no React errors or warnings", () => {
|
||||
|
||||
it("AccountsListView (empty) renders cleanly", () => {
|
||||
const { errors, warns } = renderQuiet(
|
||||
<AccountsListView accounts={[]} deleteFormAction={() => {}} />,
|
||||
<AccountsListView accounts={[]} />,
|
||||
);
|
||||
expect(errors).toEqual([]);
|
||||
expect(warns).toEqual([]);
|
||||
@ -153,7 +153,7 @@ describe("SSR render — no React errors or warnings", () => {
|
||||
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={() => {}} />,
|
||||
<AccountsListView accounts={[account]} />,
|
||||
);
|
||||
// Naive but effective: scan each <button>...</button> region for any
|
||||
// <div, <p, or <h-tag inside. Those are flow content and not allowed
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user