diff --git a/README.md b/README.md index 114efae8..475b1ab4 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,8 @@ This is a Next.js-based frontend skeleton that provides the UI structure for all - [UI PR review checklist for reviewers](./docs/CODE_REVIEW.md) - [Period lifecycle and state machine](./docs/PERIOD_LIFECYCLE.md) - [Internal jargon glossary (contributors)](./docs/GLOSSARY.md) +- [Error handling strategy for contributors](./docs/error-handling.md) +- [Routing errors — how they surface to the user](./docs/ROUTING_ERRORS.md) - [Hydration mismatch patterns and fixes](./docs/HYDRATION_MISMATCH.md) - [Search UX: instant debounce, URL-submit, and recent results](./docs/SEARCH_UX.md) - [Transaction detail (receipt) page data flow for contributors](./docs/transaction-detail-receipt-page.md) diff --git a/components/ui/PrimaryButton.test.tsx b/components/ui/PrimaryButton.test.tsx new file mode 100644 index 00000000..dc0ff464 --- /dev/null +++ b/components/ui/PrimaryButton.test.tsx @@ -0,0 +1,175 @@ +import { describe, it, expect } from 'vitest' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import PrimaryButton from './PrimaryButton' + +describe('PrimaryButton', () => { + // ────────────────────────────────────────────────────────────────────────── + // Rendering + // ────────────────────────────────────────────────────────────────────────── + + it('renders with children', () => { + render(Submit) + expect(screen.getByRole('button', { name: 'Submit' })).toBeInTheDocument() + }) + + it('renders with complex children', () => { + render( + + + Pay Now + , + ) + expect(screen.getByRole('button')).toBeInTheDocument() + expect(screen.getByTestId('icon')).toBeInTheDocument() + expect(screen.getByText('Pay Now')).toBeInTheDocument() + }) + + // ────────────────────────────────────────────────────────────────────────── + // Default styling + // ────────────────────────────────────────────────────────────────────────── + + it('applies base visual classes', () => { + render(Send) + const button = screen.getByRole('button') + // Base layout classes + expect(button.className).toContain('inline-flex') + expect(button.className).toContain('items-center') + expect(button.className).toContain('justify-center') + // Brand colour + expect(button.className).toContain('bg-brand.red') + expect(button.className).toContain('text-white') + // Sizing + expect(button.className).toContain('px-6') + expect(button.className).toContain('py-3') + expect(button.className).toContain('rounded-lg') + expect(button.className).toContain('font-semibold') + // Transition + expect(button.className).toContain('transition-colors') + expect(button.className).toContain('duration-150') + }) + + it('applies hover classes', () => { + render(Send) + const button = screen.getByRole('button') + expect(button.className).toContain('hover:bg-brand.redHover') + }) + + it('applies focus classes', () => { + render(Send) + const button = screen.getByRole('button') + expect(button.className).toContain('focus:outline-none') + expect(button.className).toContain('focus:ring-2') + expect(button.className).toContain('focus:ring-brand.red') + expect(button.className).toContain('focus:ring-offset-2') + }) + + // ────────────────────────────────────────────────────────────────────────── + // Disabled state + // ────────────────────────────────────────────────────────────────────────── + + it('renders disabled when disabled prop is true', () => { + render(Send) + const button = screen.getByRole('button') + expect(button).toBeDisabled() + }) + + it('sets aria-disabled when disabled', () => { + render(Send) + const button = screen.getByRole('button') + expect(button).toHaveAttribute('aria-disabled', 'true') + }) + + it('does not set aria-disabled when enabled', () => { + render(Send) + const button = screen.getByRole('button') + expect(button).not.toHaveAttribute('aria-disabled') + }) + + it('applies disabled visual classes', () => { + render(Send) + const button = screen.getByRole('button') + expect(button.className).toContain('disabled:opacity-50') + expect(button.className).toContain('disabled:cursor-not-allowed') + expect(button.className).toContain('disabled:hover:bg-brand.red') + }) + + it('does not trigger onClick when disabled', async () => { + const user = userEvent.setup() + let clicked = false + render( + { clicked = true }}> + Send + , + ) + await user.click(screen.getByRole('button')) + expect(clicked).toBe(false) + }) + + // ────────────────────────────────────────────────────────────────────────── + // Custom className merging + // ────────────────────────────────────────────────────────────────────────── + + it('merges custom className with default classes', () => { + render(Send) + const button = screen.getByRole('button') + expect(button.className).toContain('extra-class') + // Default classes should still be present + expect(button.className).toContain('bg-brand.red') + expect(button.className).toContain('inline-flex') + }) + + // ────────────────────────────────────────────────────────────────────────── + // forwardRef support + // ────────────────────────────────────────────────────────────────────────── + + it('forwards ref to the button element', () => { + const ref = { current: null as HTMLButtonElement | null } + render(Send) + expect(ref.current).toBeInstanceOf(HTMLButtonElement) + expect(ref.current?.tagName).toBe('BUTTON') + }) + + it('has correct displayName', () => { + expect(PrimaryButton.displayName).toBe('PrimaryButton') + }) + + // ────────────────────────────────────────────────────────────────────────── + // Additional HTML button attributes + // ────────────────────────────────────────────────────────────────────────── + + it('supports type attribute', () => { + render(Submit) + expect(screen.getByRole('button')).toHaveAttribute('type', 'submit') + }) + + it('supports aria-label', () => { + render(X) + expect(screen.getByRole('button', { name: 'Close dialog' })).toBeInTheDocument() + }) + + it('supports data-* attributes', () => { + render(Send) + expect(screen.getByTestId('submit-btn')).toBeInTheDocument() + }) + + // ────────────────────────────────────────────────────────────────────────── + // Interaction parity: hover then focus + // ────────────────────────────────────────────────────────────────────────── + + it('maintains hover class when focused', async () => { + const user = userEvent.setup() + render(Send) + const button = screen.getByRole('button') + + await user.hover(button) + // Hover classes are present + expect(button.className).toContain('hover:bg-brand.redHover') + + // Focus does not remove hover + button.focus() + expect(button.className).toContain('hover:bg-brand.redHover') + // Focus classes are also present + expect(button.className).toContain('focus:ring-2') + }) +}) \ No newline at end of file diff --git a/docs/ROUTING_ERRORS.md b/docs/ROUTING_ERRORS.md new file mode 100644 index 00000000..123068d9 --- /dev/null +++ b/docs/ROUTING_ERRORS.md @@ -0,0 +1,97 @@ +# Routing Errors — How They Surface to the User + +> **Audience:** contributor. This doc explains the different ways a routing error can +> reach a user in the RemitWise UI, which file handles each case, and what the user +> actually sees. It is the routing-specific companion to +> [docs/error-handling.md](error-handling.md) (which covers the general error-boundary +> strategy). + +## Why this matters + +Routing errors are the most visible failure surface in any Next.js app: a user lands +on a URL that doesn't exist, a route fails to render, or a dynamic segment references +a record that was deleted. Each of these has a distinct, intentional UX. Knowing which +file owns which case prevents contributors from "fixing" a 404 by editing the wrong +component, or from accidentally shipping a raw stack trace to end users. + +## The four routing-error surfaces + +### 1. Unknown / unmatched URL → `app/not-found.tsx` + +When a user navigates to a path that matches **no route** in the app, Next.js renders +`app/not-found.tsx` (the global 404 page). + +- **File:** `app/not-found.tsx` +- **What the user sees:** a branded "404 – Page Not Found" badge, a "Lost in the + transfer?" heading, links to the primary destinations (Dashboard, Send, Bills, + Insurance, Family, Settings), and "Go to Home" / "Open Dashboard" CTAs. +- **Metadata:** `title: "Page Not Found – RemitWise"`, with a short description, is set + for search engines and screen readers. +- **When it runs:** any unmatched top-level or nested path. It is the last-resort + fallback for URLs that don't exist. + +### 2. Route handler explicitly calls `notFound()` → `app/not-found.tsx` + +Routes that look up a dynamic resource (e.g. `/receipt/[txHash]`) call Next's +`notFound()` from `next/navigation` when the resource is missing. + +- **File(s):** `app/receipt/[txHash]/page.tsx`, `app/debug/page.tsx` +- **Behaviour:** `notFound()` throws a special `NEXT_NOT_FOUND` error that Next.js + catches and forwards to the nearest `not-found` boundary — for the app shell that is + `app/not-found.tsx`. +- **What the user sees:** the same branded 404 page as case 1, so a missing receipt + and a mistyped URL are visually consistent. +- **Item-ID leak note:** the page sets a local `notFound` flag and only renders the 404 + UI — it does **not** echo the requested `txHash` back onto the page, so a failed + lookup doesn't leak internal identifiers. + +### 3. Client render failure → `app/error.tsx` → `RootErrorFallback` + +When a route **exists** but throws while rendering on the client (outside a widget +boundary), Next.js renders the nearest `error.tsx`. + +- **File:** `app/error.tsx` (root) → `components/RootErrorFallback.tsx` +- **Pipeline:** + 1. `app/error.tsx` reports the exception to Sentry via + `errorReporter.captureException(error)` (PII scrubbed). + 2. It renders `RootErrorFallback`, passing the `reset` callback. + 3. `RootErrorFallback` reads the current `pathname` and picks a route-specific + message from `lib/config/route-errors.ts`. +- **What the user sees:** a focused, per-route fallback (e.g. "Dashboard unavailable", + "Transfer unavailable", "Transactions unavailable") with a retry button that calls + `onReset` to remount the subtree from a clean React tree. The heading is focused on + mount for screen-reader users. +- **Route messaging:** `lib/config/route-errors.ts` maps route prefixes to + `{ titleKey, defaultTitle, descriptionKey, defaultDescription }`. Unknown paths fall + back to `DEFAULT_ERROR_MESSAGE` ("Something went wrong" / "We hit an unexpected + problem, but your session is still safe…"). + +### 4. API route hits a missing resource → HTTP 404 JSON + +API routes return a **404 HTTP status** (not a rendered page) when a resource is +missing. The client then surfaces it via the shared `useFormAction` hook or `apiClient`. + +- **File(s):** `app/api/bills/route.ts`, `app/api/bills/[id]/route.ts` +- **Pattern:** route handlers catch a `not-found` error and respond with a typed 404 + JSON body rather than a generic 500. +- **What the user sees:** the form/hook sets an error message near the relevant control + (e.g. "Bill not found") instead of a generic "Request failed". See + [docs/use-form-action.md](use-form-action.md) for the error-message pipeline. + +## Choosing the right boundary + +| Scenario | File to touch | User sees | +|:---|:---|:---| +| URL doesn't match any route | `app/not-found.tsx` | Branded 404 page | +| Dynamic route lookup fails | `notFound()` in the page → `app/not-found.tsx` | Branded 404 page | +| Route renders but throws on client | `app/error.tsx` + `RootErrorFallback` + `route-errors.ts` | Per-route fallback + retry | +| API call for a missing record | API route handler → 404 JSON | Inline field error via `useFormAction` | + +## Cross-links + +- [docs/error-handling.md](error-handling.md) — general boundary strategy and the + `FeatureBoundary` / `WidgetErrorBoundary` wrappers. +- [docs/use-form-action.md](use-form-action.md) — how client-side API errors surface in + forms. +- [docs/session-expiry-design.md](session-expiry-design.md) — the related session-expiry + redirect flow. \ No newline at end of file diff --git a/docs/error-handling.md b/docs/error-handling.md index 66eaeade..50b07c52 100644 --- a/docs/error-handling.md +++ b/docs/error-handling.md @@ -18,3 +18,7 @@ and component loading states in a single, reusable wrapper. For more details on implementing standard default, hover, focus, error, disabled, and loading states across components, see the [Frontend Component States Guide](COMPONENT_STATES.md). +For a routing-specific breakdown of how each error type reaches the user (404 pages, +route-level error.tsx, per-route fallback messages, and API 404s), see +[docs/ROUTING_ERRORS.md](ROUTING_ERRORS.md). + diff --git a/lib/utils.ts b/lib/utils.ts index 365058ce..607cb86e 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,6 +1 @@ -import { type ClassValue, clsx } from "clsx"; -import { twMerge } from "tailwind-merge"; - -export function cn(...inputs: ClassValue[]) { - return twMerge(clsx(inputs)); -} +export { cn } from "./utils/cn"; diff --git a/lib/utils/index.ts b/lib/utils/index.ts index 37295e1c..c25627d5 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -1,6 +1 @@ -import { clsx } from "clsx"; -import { twMerge } from "tailwind-merge"; - -export function cn(...inputs: Parameters) { - return twMerge(clsx(inputs)); -} +export { cn } from "./cn";