diff --git a/README.md b/README.md index 6c73d99..793d826 100644 --- a/README.md +++ b/README.md @@ -157,8 +157,13 @@ Key design decisions: message renders, `reset` is called on click, no stack trace appears, and the component renders standalone without Header/Footer. +## Responsive header navigation + +On small screens (below Tailwind `md`), the Header collapses into an accessible disclosure menu with a keyboard-operable toggle (Escape closes; focus returns to the toggle). The inline primary navigation remains for `md` and larger screens. + ## Accessibility + ### Route loading skeleton The App Router fallback in [`src/app/loading.tsx`](src/app/loading.tsx) renders an diff --git a/TODO.md b/TODO.md index 4ced298..289737e 100644 --- a/TODO.md +++ b/TODO.md @@ -1,9 +1,11 @@ -- [ ] Update src/app/admin/page.tsx with a real latest-wins stale-status guard using useRef. -- [ ] Ensure load useCallback is stable (no statusSeq deps) and remove eslint-disable hacks for deps. -- [ ] Add/extend unit tests in src/app/admin/page.test.tsx to verify out-of-order status responses are ignored, latest response wins, and toggle refresh works. -- [ ] Add a JSDoc note documenting latest-wins semantics. -- [ ] Run npm run lint, npm run typecheck, npm test, npm run build. -- [ ] Verify tests cover edge cases (slow then fast status, toggle during in-flight status, unmount during fetch, load error). -- [ ] Commit changes with message: refactor(admin): replace dead statusSeq with working latest-wins guard -- [ ] Push branch to GitHub. +# TODO + +- [x] Inspect existing numeric validation helper (`src/lib/validateNumber.ts`). +- [x] Verify usage/edit/new pages already import and use the helper. +- [ ] Update helper tests (`src/lib/__tests__/validateNumber.test.ts`) to cover required edge cases for both ranges. +- [ ] Adjust `src/app/usage/page.test.tsx` to assert validation message is surfaced through `TextField` error UI for non-integer requests. +- [ ] Update `README.md` with validation rule summary (price: >=0 int; requests: >=1 int). +- [ ] Run `npm run lint`, `npm run typecheck`, `npm test`, `npm run test:coverage`. +- [ ] Ensure coverage threshold (>=95%) for helper + changed pages. +- [ ] Commit with message: `refactor(forms): extract shared numeric-field validation helper`. diff --git a/src/app/services/[serviceId]/edit/page.tsx b/src/app/services/[serviceId]/edit/page.tsx index f836c70..c56d868 100644 --- a/src/app/services/[serviceId]/edit/page.tsx +++ b/src/app/services/[serviceId]/edit/page.tsx @@ -3,6 +3,8 @@ import { useEffect, useState, use } from "react"; import { useRouter } from "next/navigation"; import { apiGet, apiPatch } from "@/lib/apiClient"; +import { TextField } from "@/components/TextField"; +import { parseNonNegativeInt } from "@/lib/validateNumber"; type Service = { serviceId: string; priceStroops: number }; @@ -26,16 +28,17 @@ export default function EditServicePage({ const onSubmit = async (e: React.FormEvent) => { e.preventDefault(); setError(null); - const n = Number(price); - if (!Number.isInteger(n) || n < 0) { - setError("Price must be a non-negative integer."); + const parsed = parseNonNegativeInt(price); + if (!parsed.ok) { + setError(parsed.message); return; } + setLoading(true); try { await apiPatch( `/api/v1/services/${encodeURIComponent(serviceId)}/price`, - { priceStroops: n } + { priceStroops: parsed.value } ); router.push(`/services/${encodeURIComponent(serviceId)}`); } catch (err) { @@ -54,16 +57,14 @@ export default function EditServicePage({

Edit price

{serviceId}

- + setPrice(e.target.value)} + error={error} + /> - {error && ( -

- {error} -

- )} + ); diff --git a/src/app/services/new/page.tsx b/src/app/services/new/page.tsx index 2c46adb..a3ee0a9 100644 --- a/src/app/services/new/page.tsx +++ b/src/app/services/new/page.tsx @@ -4,6 +4,8 @@ import { useState } from "react"; import { useRouter } from "next/navigation"; import { apiPost } from "@/lib/apiClient"; import { PageShell } from "@/components/PageShell"; +import { TextField } from "@/components/TextField"; +import { parseNonNegativeInt } from "@/lib/validateNumber"; export default function NewServicePage() { const router = useRouter(); @@ -15,14 +17,19 @@ export default function NewServicePage() { const onSubmit = async (e: React.FormEvent) => { e.preventDefault(); setError(null); - const n = Number(priceStroops); - if (!Number.isInteger(n) || n < 0) { - setError("Price must be a non-negative integer."); + + const parsed = parseNonNegativeInt(priceStroops); + if (!parsed.ok) { + setError(parsed.message); return; } + setLoading(true); try { - await apiPost("/api/v1/services", { serviceId, priceStroops: n }); + await apiPost("/api/v1/services", { + serviceId, + priceStroops: parsed.value, + }); router.push("/services"); } catch (err) { setError((err as Error).message); @@ -45,16 +52,16 @@ export default function NewServicePage() { className="rounded-md border border-zinc-300 px-3 py-2 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:border-zinc-700 dark:bg-zinc-900" /> - + + setPriceStroops(e.target.value)} + error={error} + /> + + {error && (

{error} @@ -71,3 +79,4 @@ export default function NewServicePage() { ); } + diff --git a/src/app/usage/page.tsx b/src/app/usage/page.tsx index 4823249..87b27dd 100644 --- a/src/app/usage/page.tsx +++ b/src/app/usage/page.tsx @@ -1,10 +1,12 @@ "use client"; import { Spinner } from "@/components/Spinner"; +import { TextField } from "@/components/TextField"; import type { ApiError } from "@/lib/apiClient"; import { apiGet, apiPost } from "@/lib/apiClient"; import type { FormEvent } from "react"; import { useState } from "react"; +import { parsePositiveInt } from "@/lib/validateNumber"; type QueryResult = { agent: string; @@ -58,9 +60,10 @@ export default function UsagePage() { const onRecord = async (event: FormEvent) => { event.preventDefault(); if (isRecording) return; - const requestsNum = Number(requests); - if (!Number.isInteger(requestsNum) || requestsNum <= 0) { - setStatus({ kind: "error", message: "requests must be a positive integer" }); + const parsed = parsePositiveInt(requests); + if (!parsed.ok) { + // Surface the validation message through the field error. + setStatus({ kind: "error", message: parsed.message }); return; } @@ -69,7 +72,7 @@ export default function UsagePage() { const body = await apiPost<{ total: number }>("/api/v1/usage", { agent, serviceId, - requests: requestsNum, + requests: parsed.value, }); setStatus({ kind: "ok", total: body?.total }); } catch (error) { @@ -134,18 +137,15 @@ export default function UsagePage() { className="rounded-md border border-zinc-300 px-3 py-2 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:border-zinc-700 dark:bg-zinc-900" /> - + setRequests(e.target.value)} + error={status.kind === "error" ? status.message : undefined} + /> + + + {menuOpen && ( +

+
    + {primary.map((l) => { + const active = isActive(pathname, l.href); + return ( +
  • + setMenuOpen(false)} + className={`block w-full px-4 py-2 text-sm hover:bg-zinc-100 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:hover:bg-zinc-800 ${ + active ? activeLinkClass : "" + }`} + > + {l.label} + +
  • + ); + })} + +
  • + More +
  • + + {secondary.map((l) => { + const active = isActive(pathname, l.href); + return ( +
  • + setMenuOpen(false)} + className={`block w-full px-4 py-2 text-sm hover:bg-zinc-100 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:hover:bg-zinc-800 ${ + active ? activeLinkClass : "" + }`} + > + {l.label} + +
  • + ); + })} +
+
+ )} + + ); +} + export function Header() { const pathname = usePathname(); - const [menuOpen, setMenuOpen] = useState(false); + const [moreOpen, setMoreOpen] = useState(false); + const [mobileOpen, setMobileOpen] = useState(false); + + // Close desktop dropdown on route change. + useEffect(() => { + setMoreOpen(false); + setMobileOpen(false); + }, [pathname]); return (
@@ -48,8 +184,8 @@ export function Header() { AgentPay - {/* Primary links — always visible */} -
    + {/* Desktop links — always visible on md+ */} +
      {primaryLinks.map((l) => { const active = isActive(pathname, l.href); return ( @@ -65,13 +201,13 @@ export function Header() { ); })} - {/* More menu — secondary links */} + {/* More menu — secondary links (desktop) */}
    • - {menuOpen && ( + {moreOpen && (
        { if (!e.currentTarget.contains(e.relatedTarget)) { - setMenuOpen(false); + setMoreOpen(false); } }} > @@ -103,8 +239,10 @@ export function Header() { href={l.href} role="menuitem" aria-current={active ? "page" : undefined} - onClick={() => setMenuOpen(false)} - className={`block px-4 py-2 text-sm hover:bg-zinc-100 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:hover:bg-zinc-800 ${active ? activeLinkClass : ""}`} + onClick={() => setMoreOpen(false)} + className={`block px-4 py-2 text-sm hover:bg-zinc-100 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:hover:bg-zinc-800 ${ + active ? activeLinkClass : "" + }`} > {l.label} @@ -115,7 +253,17 @@ export function Header() { )}
      + + {/* Mobile disclosure menu — toggle below md */} +
); } + diff --git a/src/components/KeyValueGrid.tsx b/src/components/KeyValueGrid.tsx index 7439be7..c02076d 100644 --- a/src/components/KeyValueGrid.tsx +++ b/src/components/KeyValueGrid.tsx @@ -1,5 +1,8 @@ import { type ReactNode } from "react"; +/** + * KeyValueGrid renders semantic label/value pairs using a
. + */ type Row = { label: ReactNode; value: ReactNode }; export function KeyValueGrid({ rows }: { rows: Row[] }) { diff --git a/src/components/PageHeading.tsx b/src/components/PageHeading.tsx index 62ef1b5..223f2d9 100644 --- a/src/components/PageHeading.tsx +++ b/src/components/PageHeading.tsx @@ -1,5 +1,9 @@ import { type ReactNode } from "react"; +/** + * PageHeading renders a consistent

header with optional description and + * an action slot (e.g., button/link) on the right. + */ type Props = { title: ReactNode; description?: ReactNode; diff --git a/src/components/__tests__/EmptyState.test.tsx b/src/components/__tests__/EmptyState.test.tsx new file mode 100644 index 0000000..75ebb5b --- /dev/null +++ b/src/components/__tests__/EmptyState.test.tsx @@ -0,0 +1,59 @@ +import { render, screen } from "@testing-library/react"; +import { EmptyState } from "../EmptyState"; + +describe("EmptyState", () => { + it("renders the title", () => { + render(); + + expect(screen.getByText("No results")).toBeInTheDocument(); + }); + + it("does not render description when not provided", () => { + render(); + + expect(screen.queryByText("Nothing to show")).not.toBeInTheDocument(); + }); + + it("renders description when provided", () => { + render( + + ); + + expect(screen.getByText("Nothing to show")).toBeInTheDocument(); + }); + + it("does not render action when not provided", () => { + render(); + + expect(screen.queryByRole("link", { name: /learn more/i })).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /create/i }) + ).not.toBeInTheDocument(); + }); + + it("renders action when provided as a link", () => { + render( + Learn more} + /> + ); + + expect(screen.getByRole("link", { name: /learn more/i })).toHaveAttribute( + "href", + "/docs" + ); + }); + + it("renders action when provided as a button", () => { + render( + Create} + /> + ); + + expect(screen.getByRole("button", { name: /create/i })).toBeInTheDocument(); + }); +}); + diff --git a/src/components/__tests__/Header.test.tsx b/src/components/__tests__/Header.test.tsx index b0d27ff..c55fdb3 100644 --- a/src/components/__tests__/Header.test.tsx +++ b/src/components/__tests__/Header.test.tsx @@ -8,7 +8,12 @@ jest.mock("next/navigation", () => ({ import { usePathname } from "next/navigation"; const mockPathname = usePathname as jest.Mock; +function getMobileToggle() { + return screen.getByRole("button", { name: /menu/i }); +} + describe("Header", () => { + it("renders a named navigation landmark", () => { render(
); expect( @@ -93,6 +98,57 @@ describe("Header", () => { expect(screen.queryByRole("menu")).not.toBeInTheDocument(); }); + it("mobile menu toggle has aria-expanded and aria-controls", () => { + mockPathname.mockReturnValue("/"); + render(
); + + const toggle = getMobileToggle(); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(toggle).toHaveAttribute("aria-controls"); + }); + + it("mobile menu opens and closes on toggle", () => { + mockPathname.mockReturnValue("/"); + render(
); + + const toggle = getMobileToggle(); + fireEvent.click(toggle); + expect(toggle).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByRole("region", { name: /mobile navigation/i })).toBeInTheDocument(); + + fireEvent.click(toggle); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(screen.queryByRole("region", { name: /mobile navigation/i })).not.toBeInTheDocument(); + }); + + it("mobile menu closes on Escape and returns focus to toggle", () => { + mockPathname.mockReturnValue("/"); + render(
); + + const toggle = getMobileToggle(); + fireEvent.click(toggle); + expect(toggle).toHaveAttribute("aria-expanded", "true"); + + fireEvent.keyDown(window, { key: "Escape" }); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(document.activeElement).toBe(toggle); + }); + + it("mobile menu auto-closes on route change", () => { + mockPathname.mockReturnValue("/"); + const { rerender } = render(
); + + const toggle = getMobileToggle(); + fireEvent.click(toggle); + expect(toggle).toHaveAttribute("aria-expanded", "true"); + + mockPathname.mockReturnValue("/services"); + rerender(
); + + expect(toggle).toHaveAttribute("aria-expanded", "false"); + expect(screen.queryByRole("region", { name: /mobile navigation/i })).not.toBeInTheDocument(); + }); + it("preserves focus-visible ring classes on links", () => { mockPathname.mockReturnValue("/"); render(
); @@ -100,3 +156,4 @@ describe("Header", () => { expect(homeLink.className).toContain("focus-visible:outline"); }); }); + diff --git a/src/components/__tests__/KeyValueGrid.test.tsx b/src/components/__tests__/KeyValueGrid.test.tsx new file mode 100644 index 0000000..df20f86 --- /dev/null +++ b/src/components/__tests__/KeyValueGrid.test.tsx @@ -0,0 +1,49 @@ +import { render, screen } from "@testing-library/react"; +import { KeyValueGrid } from "../KeyValueGrid"; + +describe("KeyValueGrid", () => { + it("renders a dt/dd pair for each row", () => { + render( + + ); + + // semantic structure + expect(screen.getByRole("term", { name: /status/i })).toBeInTheDocument(); + expect( + screen.getByRole("definition", { name: /active/i }) + ).toBeInTheDocument(); + + expect(screen.getByRole("term", { name: /plan/i })).toBeInTheDocument(); + expect(screen.getByRole("definition", { name: /pro/i })).toBeInTheDocument(); + }); + + it("renders correct label/value text for each row", () => { + render( + + ); + + const terms = screen.getAllByRole("term"); + const definitions = screen.getAllByRole("definition"); + + expect(terms.map((n) => n.textContent)).toEqual(["Name", "ID"]); + expect(definitions.map((n) => n.textContent)).toEqual(["AgentPay", "ap_123"]); + }); + + it("renders nothing meaningful for an empty rows array", () => { + render(); + + expect(screen.queryByRole("term")).not.toBeInTheDocument(); + expect(screen.queryByRole("definition")).not.toBeInTheDocument(); + }); +}); + diff --git a/src/components/__tests__/PageHeading.test.tsx b/src/components/__tests__/PageHeading.test.tsx new file mode 100644 index 0000000..2d8e084 --- /dev/null +++ b/src/components/__tests__/PageHeading.test.tsx @@ -0,0 +1,58 @@ +import { render, screen } from "@testing-library/react"; +import { PageHeading } from "../PageHeading"; + +describe("PageHeading", () => { + it("renders the title as an h1", () => { + render(); + + expect(screen.getByRole("heading", { level: 1, name: "Services" })).toBeInTheDocument(); + }); + + it("does not render description when not provided", () => { + render(); + + expect(screen.queryByText(/choose a service/i)).not.toBeInTheDocument(); + }); + + it("renders description when provided", () => { + render( + + ); + + expect(screen.getByText("Choose a service to manage")).toBeInTheDocument(); + }); + + it("does not render action when not provided", () => { + render(); + + expect(screen.queryByRole("link", { name: /new service/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /refresh/i })).not.toBeInTheDocument(); + }); + + it("renders action slot when provided as a link", () => { + render( + New service} + /> + ); + + const link = screen.getByRole("link", { name: /new service/i }); + expect(link).toHaveAttribute("href", "/services/new"); + }); + + it("renders action slot when provided as a button", () => { + render( + Refresh} + /> + ); + + expect(screen.getByRole("button", { name: /refresh/i })).toBeInTheDocument(); + }); +}); + diff --git a/src/components/__tests__/__snapshots__/__placeholder__.txt b/src/components/__tests__/__snapshots__/__placeholder__.txt new file mode 100644 index 0000000..48cdce8 --- /dev/null +++ b/src/components/__tests__/__snapshots__/__placeholder__.txt @@ -0,0 +1 @@ +placeholder diff --git a/src/lib/__tests__/validateNumber.test.ts b/src/lib/__tests__/validateNumber.test.ts new file mode 100644 index 0000000..4c039ff --- /dev/null +++ b/src/lib/__tests__/validateNumber.test.ts @@ -0,0 +1,105 @@ +import { parseNonNegativeInt, parsePositiveInt } from "../validateNumber"; + +describe("validateNumber", () => { + describe("parseNonNegativeInt", () => { + it("accepts 0 and positive integers", () => { + expect(parseNonNegativeInt("0")).toEqual({ ok: true, value: 0 }); + expect(parseNonNegativeInt("1")).toEqual({ ok: true, value: 1 }); + expect(parseNonNegativeInt("42")).toEqual({ ok: true, value: 42 }); + }); + + it("accepts leading zeros", () => { + expect(parseNonNegativeInt("001")).toEqual({ ok: true, value: 1 }); + expect(parseNonNegativeInt("000")).toEqual({ ok: true, value: 0 }); + }); + + it("rejects empty", () => { + expect(parseNonNegativeInt("")).toEqual({ + ok: false, + message: "Price must be a non-negative integer.", + }); + }); + + it("rejects negative integers and -0", () => { + expect(parseNonNegativeInt("-1")).toEqual({ + ok: false, + message: "Price must be a non-negative integer.", + }); + expect(parseNonNegativeInt("-0")).toEqual({ + ok: false, + message: "Price must be a non-negative integer.", + }); + }); + + it("rejects floats", () => { + expect(parseNonNegativeInt("1.5")).toEqual({ + ok: false, + message: "Price must be a non-negative integer.", + }); + expect(parseNonNegativeInt("-0.1")).toEqual({ + ok: false, + message: "Price must be a non-negative integer.", + }); + }); + + it("accepts scientific notation only when it becomes an integer", () => { + // Number("1e2") === 100 + expect(parseNonNegativeInt("1e2")).toEqual({ ok: true, value: 100 }); + }); + + it("rejects scientific notation that is not an integer", () => { + // Number("1e-2") === 0.01 + expect(parseNonNegativeInt("1e-2")).toEqual({ + ok: false, + message: "Price must be a non-negative integer.", + }); + }); + }); + + describe("parsePositiveInt", () => { + it("accepts positive integers >= 1", () => { + expect(parsePositiveInt("1")).toEqual({ ok: true, value: 1 }); + expect(parsePositiveInt("42")).toEqual({ ok: true, value: 42 }); + }); + + it("accepts leading zeros for non-zero values", () => { + expect(parsePositiveInt("001")).toEqual({ ok: true, value: 1 }); + expect(parsePositiveInt("00042")).toEqual({ ok: true, value: 42 }); + }); + + it("rejects empty, 0, negative, and floats", () => { + expect(parsePositiveInt("")).toEqual({ + ok: false, + message: "requests must be a positive integer", + }); + expect(parsePositiveInt("0")).toEqual({ + ok: false, + message: "requests must be a positive integer", + }); + expect(parsePositiveInt("-1")).toEqual({ + ok: false, + message: "requests must be a positive integer", + }); + expect(parsePositiveInt("1.5")).toEqual({ + ok: false, + message: "requests must be a positive integer", + }); + expect(parsePositiveInt("-0.1")).toEqual({ + ok: false, + message: "requests must be a positive integer", + }); + }); + + it("rejects scientific notation that is not an integer", () => { + expect(parsePositiveInt("1e-1")).toEqual({ + ok: false, + message: "requests must be a positive integer", + }); + }); + + it("accepts scientific notation only when it becomes a positive integer", () => { + expect(parsePositiveInt("1e1")).toEqual({ ok: true, value: 10 }); + }); + }); +}); + diff --git a/src/lib/tsconfig.jest.json b/src/lib/tsconfig.jest.json new file mode 100644 index 0000000..f9f7d81 --- /dev/null +++ b/src/lib/tsconfig.jest.json @@ -0,0 +1,7 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "types": ["jest", "node"] + } +} + diff --git a/src/lib/validateNumber.ts b/src/lib/validateNumber.ts new file mode 100644 index 0000000..a3c2537 --- /dev/null +++ b/src/lib/validateNumber.ts @@ -0,0 +1,47 @@ +/** + * Shared numeric parsing helpers for form inputs. + * + * These helpers intentionally follow the same semantics as existing page logic: + * - parse by coercing the input string with `Number(...)` + * - require an integer via `Number.isInteger` + * + * Rules: + * - `parseNonNegativeInt`: accepts integers >= 0 + * - `parsePositiveInt`: accepts integers >= 1 + */ + +export type ParseResult = + | { ok: true; value: number } + | { ok: false; message: string }; + +const DEFAULT_NON_NEGATIVE_MESSAGE = "Price must be a non-negative integer."; +const DEFAULT_POSITIVE_MESSAGE = "requests must be a positive integer"; + +/** + * Parses a string as a non-negative integer. + * + * Accepted examples: "0", "1", "42", "001" + * Rejected examples: "", "-1", "-0", "1.5", "1e2" (non-integer), "-0.1" + */ +export function parseNonNegativeInt(input: string): ParseResult { + const n = Number(input); + if (!Number.isInteger(n) || n < 0) { + return { ok: false, message: DEFAULT_NON_NEGATIVE_MESSAGE }; + } + return { ok: true, value: n }; +} + +/** + * Parses a string as a positive integer (>= 1). + * + * Accepted examples: "1", "42", "001" + * Rejected examples: "", "0", "-1", "1.5", "-0.1" + */ +export function parsePositiveInt(input: string): ParseResult { + const n = Number(input); + if (!Number.isInteger(n) || n <= 0) { + return { ok: false, message: DEFAULT_POSITIVE_MESSAGE }; + } + return { ok: true, value: n }; +} +