diff --git a/components/patterns/BetConfirmPattern.tsx b/components/patterns/BetConfirmPattern.tsx index 75a56956..d79237c5 100644 --- a/components/patterns/BetConfirmPattern.tsx +++ b/components/patterns/BetConfirmPattern.tsx @@ -189,14 +189,17 @@ export function BetConfirmPattern() { - + {/* Cancel is rendered BEFORE the primary action so DOM, + visual, and Tab order match (WCAG 2.4.3). On mobile the + primary action sits at the bottom for thumb reach. */} + ) : ( diff --git a/components/patterns/DisputeActionPattern.tsx b/components/patterns/DisputeActionPattern.tsx index 6e1cef08..53d3096d 100644 --- a/components/patterns/DisputeActionPattern.tsx +++ b/components/patterns/DisputeActionPattern.tsx @@ -88,14 +88,18 @@ export function DisputeActionPattern() { - + {/* Cancel is rendered BEFORE the destructive primary action so + DOM, visual, and Tab order match (WCAG 2.4.3). The destructive + `Submit Evidence` button keeps its red variant for visual + distinction. */} + diff --git a/components/ui/__tests__/dialog-footer.test.tsx b/components/ui/__tests__/dialog-footer.test.tsx new file mode 100644 index 00000000..b98b17c3 --- /dev/null +++ b/components/ui/__tests__/dialog-footer.test.tsx @@ -0,0 +1,176 @@ +/** + * Tests for the canonical Confirm/Cancel button order in our primitive + * `DialogFooter` and `AlertDialogFooter` (issue #474). + * + * The DOM order, the visual order, and keyboard focus order MUST all + * match — see `docs/BUTTON_ORDER.md`. + */ +import * as React from "react"; +import { describe, expect, it } from "@jest/globals"; +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { Button } from "@/components/ui/button"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + AlertDialogTrigger, +} from "@/components/ui/alert-dialog"; +import { + Dialog, + DialogContent, + DialogFooter, + DialogHeader, + DialogTitle, + DialogTrigger, +} from "@/components/ui/dialog"; + +/** + * Helper: returns all + + + + Delete this event? + + + Cancel + + Delete Event + + + + + ); +} + +function DialogHarness() { + return ( + + + + + + + Confirm Prediction + + + + + + + + ); +} + +describe("Button order — Confirm/Cancel (issue #474)", () => { + describe("AlertDialogFooter", () => { + it("renders the Cancel-equivalent before the destructive Action", () => { + render(); + + const dialog = screen.getByRole("alertdialog"); + const buttons = getFooterButtons(dialog, "alert-footer"); + + expect(buttons).toHaveLength(2); + expect(buttons[0]).toHaveTextContent(/cancel/i); + expect(buttons[1]).toHaveTextContent(/delete event/i); + }); + + it("focuses the Cancel-equivalent first when tabbing through the footer", async () => { + const user = userEvent.setup(); + render(); + + const dialog = screen.getByRole("alertdialog"); + const buttons = getFooterButtons(dialog, "alert-footer"); + + // Focus the Cancel button directly — verifying the spec rule. + buttons[0].focus(); + expect(document.activeElement).toBe(buttons[0]); + + // Tab forward once → next focusable inside the dialog must be the + // destructive Action, NOT somewhere earlier in the document. + await user.tab(); + expect(document.activeElement).toBe(buttons[1]); + }); + + it("does NOT use `flex-col-reverse` in the footer base classes", () => { + render(); + + const dialog = screen.getByRole("alertdialog"); + const footer = within(dialog).getByTestId("alert-footer"); + + // `flex-col-reverse` would invert visual order vs DOM/tab order and + // violate WCAG 2.4.3. We assert the absence of the className suffix. + expect(footer.className).not.toMatch(/flex-col-reverse/); + // The footer must still stack vertically on mobile. + expect(footer.className).toMatch(/\bflex-col\b/); + // And switch to a right-aligned row on `sm+`. + expect(footer.className).toMatch(/sm:flex-row/); + expect(footer.className).toMatch(/sm:justify-end/); + }); + }); + + describe("DialogFooter", () => { + it("renders the Cancel equivalent before the primary action", () => { + render(); + + const dialog = screen.getByRole("dialog"); + const buttons = getFooterButtons(dialog, "dialog-footer"); + + expect(buttons).toHaveLength(2); + expect(buttons[0]).toHaveTextContent(/cancel/i); + expect(buttons[1]).toHaveTextContent(/confirm prediction/i); + }); + + it("focuses the Cancel first, then the primary action", async () => { + const user = userEvent.setup(); + render(); + + const dialog = screen.getByRole("dialog"); + const buttons = getFooterButtons(dialog, "dialog-footer"); + + buttons[0].focus(); + expect(document.activeElement).toBe(buttons[0]); + + await user.tab(); + expect(document.activeElement).toBe(buttons[1]); + }); + + it("does NOT use `flex-col-reverse` in the footer base classes", () => { + render(); + + const dialog = screen.getByRole("dialog"); + const footer = within(dialog).getByTestId("dialog-footer"); + + expect(footer.className).not.toMatch(/flex-col-reverse/); + expect(footer.className).toMatch(/\bflex-col\b/); + expect(footer.className).toMatch(/sm:flex-row/); + expect(footer.className).toMatch(/sm:justify-end/); + }); + }); +}); diff --git a/components/ui/alert-dialog.tsx b/components/ui/alert-dialog.tsx index 25e7b474..2c467e9a 100644 --- a/components/ui/alert-dialog.tsx +++ b/components/ui/alert-dialog.tsx @@ -59,13 +59,24 @@ const AlertDialogHeader = ({ ) AlertDialogHeader.displayName = "AlertDialogHeader" +/** + * Footer for `AlertDialog`. Stacks children vertically on small viewports + * (Cancel on top → primary at the bottom, thumb-friendly) and horizontally + * on `sm+` viewports (Cancel on the left → primary on the right, right-aligned). + * + * The DOM order matches both the visual and Tab orders, so keyboard + * navigation follows what the user sees (WCAG 2.4.3 Focus Order). + * + * Consumers MUST place the `Cancel`-equivalent element BEFORE the primary + * (potentially destructive) `Action` element. See `docs/BUTTON_ORDER.md`. + */ const AlertDialogFooter = ({ className, ...props }: React.HTMLAttributes) => (
) => (
**Issue:** [#474 — Add accessible confirm/cancel button ordering](https://github.com/Predictify-org/predictify-frontend/issues/474) +> **Status:** Stable. All new confirm-style dialogs/drawers **MUST** follow this pattern. + +This document defines the canonical, accessible ordering for **Cancel** and **primary action** buttons inside every `Dialog`, `AlertDialog`, and `Drawer` shipped in the Predictify frontend. It exists to guarantee: + +1. **A consistent visual order** across the product, so muscle memory works everywhere. +2. **A consistent DOM/Tab order**, satisfying [WCAG 2.4.3 Focus Order](https://www.w3.org/WAI/WCAG21/Understanding/focus-order.html) — keyboard focus must move in the same direction the eye reads. +3. **A consistent thumb‑friendly mobile pattern**, with the primary action placed where the thumb naturally rests. + +If you are adding a new dialog or drawer, follow this rule **before writing any markup**. + +--- + +## TL;DR — The Rule + +> **Render the safe/Cancel element FIRST, then the primary (possibly destructive) Action element.** This applies to **both** the DOM and the markup order. Do not rely on `flex-col-reverse`, reorder CSS, or negative margins to flip visual order. + +Visual layout is then derived from CSS only — never from reordering markup. + +### Desktop (`sm:` and up — `min-width: 768px`) + +| Position | Element | Notes | +| -------- | ------- | ------------------------------------------------------------- | +| Left | Cancel | Safe / non‑committal. Outline or ghost variant. | +| Right | Action | Affirmative or destructive. Default or `destructive` variant. | + +The action is right‑aligned via `sm:justify-end`. + +### Mobile (below `sm:`) + +| Position | Element | Notes | +| -------- | -------- | ---------------------------------------------------------------------------------- | +| Top | Cancel | Less prominent, escape route. | +| Bottom | Action | Primary action — thumb‑friendly reach. Stays visually distinct via color/variant. | + +The footer uses `flex-col` (no `flex-col-reverse`) so visual order matches DOM. + +--- + +## Why this order? + +### 1. WCAG 2.1 AA compliance + +- **2.4.3 Focus Order** — The navigation order of focusable elements must preserve meaning and operability. A layout that uses `flex-col-reverse` to place the Action visually above the Cancel causes Tab to jump *downward* while advancing through the DOM, which violates this criterion. +- **2.4.7 Focus Visible** + **1.3.2 Meaningful Sequence** — Consistent visual ↔ focus order also keeps screen-reader output predictable (e.g. NVDA / VoiceOver read DOM order). + +### 2. Native platform conventions + +- **Desktop** mirrors the standard OS dialog button order: Cancel left → OK/Apply right. Users have decades of muscle memory here. +- **Mobile** mirrors Material Design and iOS HIG: secondary ("Cancel") above, primary ("Continue" / "Submit" / "Delete") at the bottom edge — where the thumb naturally rests. + +### 3. Reduced accidental destructive taps + +Putting the destructive variant at the bottom of the footer (with adequate `gap`) is safer than placing it at the top because: + +- The button is visually separated from the Cancel by literal whitespace (never adjacent). +- Variant color (`bg-destructive`) plus the spacing prevents thumb‑mistakes. +- Keyboard users still reach the destructive button via Tab in a predictable path: Cancel → Action. + +--- + +## Visual diagram + +### Desktop + +``` +┌─────────────────────────────────────────────┐ +│ Dialog title │ +│ Dialog description │ +│ …body content… │ +│ │ +│ ┌──────────┐ ┌────────────┐ │ +│ │ Cancel │ │ Action │ │ +│ └──────────┘ └────────────┘ │ +└─────────────────────────────────────────────┘ + ↑ Tab order matches this left → right order ↑ +``` + +### Mobile (drawer or stacked dialog) + +``` +┌─────────────────────────────────────────────┐ +│ Dialog / drawer title │ +│ Dialog / drawer description │ +│ …body content… │ +│ ─────────────────────────────────────────── │ +│ ┌─────────────────────────────────────────┐ │ +│ │ Cancel │ │ +│ └─────────────────────────────────────────┘ │ +│ ┌─────────────────────────────────────────┐ │ +│ │ Action (primary / destructive) │ │ +│ └─────────────────────────────────────────┘ │ +└─────────────────────────────────────────────┘ + ↑ Tab order matches this top → bottom order ↑ +``` + +--- + +## Implementation reference + +The two primitive footers in `components/ui/` already encode this rule. **You almost never need to override their className.** + +### `AlertDialogFooter` — `components/ui/alert-dialog.tsx` + +```tsx +className={cn( + "flex flex-col gap-2 sm:flex-row sm:justify-end sm:gap-3", + className, +)} +``` + +### `DialogFooter` — `components/ui/dialog.tsx` + +```tsx +className={cn( + "flex flex-col gap-2 sm:flex-row sm:justify-end sm:gap-3", + className, +)} +``` + +### Mobile `DrawerFooter` usage + +When using a `Drawer` (mobile‑first), drop the safety wrapper and put the buttons directly: + +```tsx + + {/* 1) Safe action — Cancel — first */} + + + + + {/* 2) Primary / destructive — second */} + + +``` + +### Desktop `DialogFooter` usage + +```tsx + + {/* 1) Safe action — Cancel — first */} + + + {/* 2) Primary — second */} + + +``` + +--- + +## Anti-patterns (do NOT do this) + +- ❌ Reordering markup so the Action appears first, then using `flex-col-reverse` to "fix" the order visually. This silently breaks WCAG 2.4.3. +- ❌ Using `flex-row-reverse sm:justify-start` on desktop (would put Cancel on the right). +- ❌ Swapping element order between desktop and mobile responses if it changes the DOM order — keyboard users get a different sequence than visual users. +- ❌ Wrapping the Cancel `