deckhearth/.convoys/scanner-mobile-checkout/audits/a11y-20260814.md
varutasu 73424aae59
Mobile scanner checkout: scan first, commit later (#157)
* Start scanner-mobile-checkout convoy for the cart-then-commit phone flow.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Ship a cart-then-commit mobile scanner so phone sessions stay on the camera.

Scan matches enqueue locally instead of auto-writing ownership, checkout happens in a sheet, and audit fixes cover stale commit detection, returnUrl open redirects, nested Escape, and ember detection chrome.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-14 20:20:43 -05:00

146 lines
10 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# A11y audit — scanner-mobile-checkout
**Reviewer:** role-a11y-auditor
**Convoy:** `scanner-mobile-checkout`
**Surface:** uncommitted UI diff vs `origin/main` (+ untracked) — no GitHub PR
**Date:** 2026-08-14
**Model:** cursor-grok-4.5-high (`model_tier=audit`)
**Skill note:** `.cursor/skills/accessibility-audit/SKILL.md` was not present in-repo; audit follows role + convoy UX § A11y constraints (14 items) + WCAG 2.2 AA (AAA target size where convoy requires).
## Diff scope (UI)
| Path | Status |
| --- | --- |
| `components/Layout.js` | modified (`chrome="immersive"`) |
| `components/scanner/ScannerCamera.js` | modified |
| `components/scanner/ScannerCountPill.js` | modified |
| `components/scanner/ScannerToast.js` | modified |
| `components/scanner/ReviewCardItem.js` | modified |
| `components/scanner/ScannerReview.js` | modified |
| `components/scanner/ScannerCheckoutSheet.js` | **new** |
| `components/scanner/ScannerScanPeek.js` | **new** |
| `pages/scanner.js` | modified |
| `test/components/ScannerCheckoutSheet.test.js` | new (a11y-adjacent) |
Non-UI (session/hooks/login/tests) excluded from findings.
## Executive summary
- Chrome controls (flash / swap / gallery / back / Review N), peek tap target, toast live region, checkout `role="dialog"` + Escape + `useFocusTrap`, and sitewide `prefers-reduced-motion` collapse are largely in good shape.
- **One severity-3 finding:** nested List-picker `<Modal>` over open checkout sheet leaves **two Escape handlers and two focus traps** active — Escape dismisses both layers (and Tab can fight). Convoy constraint 10.
- Several severity-2 gaps vs promised constraints: missing sheet close control, no `aria-describedby` helper on disabled commit CTA, camera chrome not `inert`/`aria-hidden` under sheet, native checkbox hit target unreliable at 44×44.
- **Recommendation:** request-changes until nested-overlay Escape/trap is fixed; ship sev-2 before merge if possible.
**Counts:** 7 findings (sev ≥ 3: **1**, sev &lt; 3: **6**). Constraint checklist: **9 pass / 3 partial / 2 fail**.
## Convoy constraint checklist
| # | Constraint | Result | Notes |
| --- | --- | --- | --- |
| 1 | 44×44 targets (2.5.5) | **Partial** | ChromeIconButton, Review N, qty ±, overflow use `minWidth/Height: 44`. Native checkbox is `w-5 h-5` with style mins — hit box often stays ~20px. |
| 2 | Contrast on glass (1.4.3) | **Pass*** | Token text colors; no instrumented contrast measure (*static review). |
| 3 | Focus order open/close (2.4.3) | **Partial** | Trap focuses first focusable in panel; restore via `useFocusTrap`. No header ×; restore may miss if opener was auto-dismissed peek. |
| 4 | Keyboard Escape / peek Enter (2.1.1) | **Partial** | Escape + button peek OK; nested Escape conflict (see A1). |
| 5 | Dialog name/role (4.1.2) | **Pass** | Checkout: `role="dialog"` `aria-modal` `aria-labelledby` → "{N} cards scanned". List picker Modal title "Choose a List". |
| 6 | Required `aria-label`s | **Pass** | Leave / gallery / flash+`aria-pressed` / swap / Review N / checkbox / qty / overflow present. |
| 7 | Non-text (1.1.1) | **Pass** | Row thumbs `alt={card.name}`; peek thumb `alt=""` under named button; decorative SVGs `aria-hidden`. |
| 8 | Status messages (4.1.3) | **Pass** | Toast `role="status"` `aria-live="polite"`; scan status `aria-live="polite"`. Peek is interactive, not a second live region (toast covers identify). |
| 9 | Reduced motion (2.3.3) | **Pass** | Peek/sheet/toast transitions + spinners covered by `styles/globals.css` `@media (prefers-reduced-motion: reduce)`. |
| 10 | Focus trap all overlays | **Fail** | Sheet traps; Modal traps; both stay active when List picker opens over checkout. |
| 11 | Confidence not color-only (1.4.1) | **Partial** | Visible `{pct}% match` + ring; checkbox `aria-label` adds confidence only when &lt;85%; header says "Some matches…" not "{N} need review". |
| 12 | Gallery file input | **Pass** | `sr-only` input + labeled button; `aria-busy` while identifying. |
| 13 | Disabled CTA + describedby | **Fail** | `disabled` only; no helper text / `aria-describedby` / `aria-disabled`. |
| 14 | Hide camera under sheet | **Fail** | `isCheckoutOpen` only changes status copy; video + chrome not `aria-hidden`/`inert`. |
## Findings
| Sev | Layer | WCAG | Surface | Issue | Fix |
| --- | --- | --- | --- | --- | --- |
| **3** | Keyboard / Focus | 2.1.1, 2.4.3 | `pages/scanner.js` + `ScannerCheckoutSheet.js` | Nested List picker Modal while checkout sheet open: both register document `Escape` + Tab traps. Escape closes **picker and sheet**. | Deactivate checkout trap + Escape while `isListPickerOpen`; or close sheet Escape only when picker closed (`if (e.key === 'Escape' && !isListPickerOpen)`). Prefer `useFocusTrap(active && !nestedOpen)`. |
| **2** | Name / Instructions | 3.3.2, 4.1.2 | `ScannerCheckoutSheet.js` `CheckoutFooter` ~L4051 | Zero selection: primary disabled with no visible "Select at least one card" / `aria-describedby`. | Add helper `<p id="checkout-select-hint">…</p>`; `aria-describedby={selectedCount === 0 ? 'checkout-select-hint' : undefined}`; optional `aria-disabled` mirroring `disabled`. |
| **2** | Structure / AT | 1.3.2, 4.1.2 | `ScannerCamera.js` + `pages/scanner.js` | Sheet open does not set `aria-hidden`/`inert` on viewfinder chrome (constraint 14). | When `isCheckoutOpen` (mobile), set `inert` on camera root (or `aria-hidden` on `<video>` + `tabIndex={-1}` / hide chrome from AT). |
| **2** | Target size | 2.5.5 | `ReviewCardItem.js` ~L7279 | Checkbox visual/control ~20×20 despite `minWidth/Height: 44` on `<input type="checkbox">` (unreliable). | Wrap in `<label className="… min-w-[44px] min-h-[44px] flex items-center justify-center">` or custom hit pad. |
| **2** | Keyboard / Operable | 2.1.1, 2.4.3 | `ScannerCheckoutSheet.js` ~L211217 | No header dismiss × (convoy: close via × / Escape / scrim). Keyboard-only users depend solely on Escape. | Add `aria-label="Close cart"` button in sheet header; include in focus order as first focusable. |
| **1** | Understandable | 3.3.1 / UX | `pages/scanner.js` Leave Modal ~L152166 | Copy uses "Stay" not "Keep scanning"; description omits `{N}` count. | `description={\`${unprocessedCount} scanned cards haven't been added yet.\`}`; primary label "Keep scanning". |
| **1** | Sensory | 1.4.1 | `ScannerCheckoutSheet.js` ~L127130 | Low-confidence subline lacks count; checkbox confidence in name only when &lt;85%. | Subline: `{n} need review`; include confidence in checkbox `aria-label` whenever `confidencePct != null`. |
### Severity ≥ 3 detail
#### A1 — Nested Escape / dual focus trap (sev 3)
**Scenario:** User opens checkout sheet → taps **Add to List** → List picker Modal opens. Presses **Escape** intending to close only the picker.
**Observed behavior (static):**
1. `ScannerCheckoutSheet` always calls `useFocusTrap(true)` and adds a document `keydown` Escape → `onClose` while mounted (`ScannerCheckoutSheet.js` L171188).
2. `<Modal open={isListPickerOpen}>` independently traps focus and listens for Escape → its `onClose` (`Modal.js` L2837; `pages/scanner.js` L168173).
3. Checkout stays mounted (`pages/scanner.js` L131137) while picker is open — neither trap is suspended.
Both Escape listeners fire on one keypress → picker closes **and** cart sheet closes. Tab handlers from both traps remain registered → unpredictable focus cycling (constraint 10).
**Suggested fix (minimal):**
```js
// ScannerCheckoutSheet — accept activeTrap prop
const sheetRef = useFocusTrap(trapActive !== false);
useEffect(() => {
if (!trapActive) return undefined;
// … Escape handler only when trapActive
}, [onClose, sheetRef, trapActive]);
```
```js
// pages/scanner.js
<ScannerCheckoutSheet
trapActive={!isListPickerOpen}
/>
```
Or close List picker with Escape via Modal only, and in checkout Escape handler: `if (document.querySelector('[data-list-picker-open]')) return`.
## Layer walk (5 layers, summary)
| Layer | Verdict |
| --- | --- |
| **1 Perceivable** | Toast/status live regions OK; confidence has text+ring; peek/toast SVG icons OK; reduced-motion via global CSS OK. |
| **2 Operable** | 44px chrome generally OK; nested Escape/trap is the blocker; missing sheet ×; checkbox target weak. |
| **3 Understandable** | Labels strong on chrome; leave-modal / empty-CTA / low-confidence copy incomplete vs convoy. |
| **4 Robust** | Dialog semantics present; dual `aria-modal` when nested; camera not inert under sheet. |
| **5 Consistency** | Matches `ScannerDisambiguation` sheet shell pattern (good); nested Modal over sheet needs same trap suspension pattern as disambiguation-above-checkout (z-50 vs z-40) — disambiguation currently unlikely while checkout open because verify pauses, but List picker is the live path. |
## Suggested diffs (priority)
1. **P0** — Suspend checkout Escape + `useFocusTrap` while List picker (or any nested Modal) is open.
2. **P1** — Disabled CTA helper + `aria-describedby`.
3. **P1**`inert` / `aria-hidden` on camera when mobile checkout open.
4. **P1** — 44×44 checkbox hit area via label wrapper.
5. **P2** — Sheet close button; leave-modal copy + count; "{N} need review".
## Patterns to lift
- `ChromeIconButton` — good reusable 44px + `aria-label` / `aria-pressed` / `aria-busy` pattern (`ScannerCamera.js`).
- `ScannerDisambiguation` + `useFocusTrap` — correct shell to copy; extend with **`active` gated by nested overlays**.
- `ScannerToast` SVG + `role="status"` — keep; do not add a competing live region on peek.
## Automated checks
| Check | Result |
| --- | --- |
| axe-core / CI a11y job | Not run (static role audit) |
| Manual SR | Recommend VoiceOver pass: open cart → Add to List → Escape once |
## Approval recommendation
- [ ] approve
- [x] **request-changes** — sev ≥ 3 open (nested Escape/trap)
- [ ] comment-only
Fix A1 before merge. Sev-2 items should land in the same PR or an immediate follow-up brief tagged a11y.
## Hand-off
A11y audit complete. **7 findings** (sev ≥ 3: **1**, sev &lt; 3: **6**).
Report: `.convoys/scanner-mobile-checkout/audits/a11y-20260814.md`.
Recommend fixing sev ≥ 3 before merge.