Files
sure/app/components/DS/dialog.html.erb
Guillem Arias Fauste 7a0cafd6ba fix(design-system): DS::Dialog a11y — role, aria-modal, aria-labelledby, heading_level (#1846)
* fix(design-system): DS::Dialog a11y — role, aria-modal, aria-labelledby, heading_level

Closes #1740. The savings-goals audit captured the dialog rendering
without `role`, `aria-modal`, or `aria-labelledby` — AT users
landing focus inside the dialog hear no title and no modal-mode
hint. Affects every modal/drawer surface in the app (transfer
matches, valuations, trades, imports, settings, etc. — 30+ views).

Fixes:

1. `role="dialog"` + `aria-modal="true"` on the `<dialog>` element.
   Native `<dialog>` already maps to these implicitly in modern
   browsers, but Safari and pre-2024 mappings benefit from the
   explicit role.

2. `aria-labelledby` wired to a stable `dialog-title-<8-char hex>`
   id minted in initialize. The header slot's `<h*>` carries the
   matching id; AT now announces the title on focus-in. If the
   caller passes `custom_header: true` (no title), the
   `aria-labelledby` reference resolves to nothing and AT
   gracefully falls back to the first focusable.

3. New `heading_level:` kwarg (default `2`). Lets callers nest
   dialogs inside surfaces that already have an `<h2>` heading
   without breaking outline order. The existing `<h2>` baseline
   stays as the default.

API is additive; existing 30+ DS::Dialog callsites work without
modification.

Out of scope (own issues):
- Drawer modal-vs-non-modal split (`<dialog>` is currently always
  opened via `showModal()`). Browser behavior is correct for both
  variants today; non-modal drawer is a separate UX call.
- Reduced-motion audit — no CSS transitions on `dialog` open/close.
- Explicit focus-on-open (title vs first input) — browser-native
  `showModal()` already focuses the first focusable; caller can
  override with `autofocus`. Not changing the default here.
- `en.common.close` missing translation — separate bug, filed.

* fix(review): gate aria-labelledby + validate heading_level

Only emit aria-labelledby when the header slot rendered an auto-title
so the id reference never dangles (custom_header: true and body-only
dialogs like the global confirm dialog no longer expose a broken
label). Validate heading_level is an Integer 1..6 in the initializer
to prevent invalid <h0>/<h7> markup. Update stale comment that
referenced tag.public_send instead of content_tag.

* fix(ds-dialog): always emit aria-labelledby (slot lambda is lazy)

The previous fix gated `aria-labelledby` on `@has_auto_title`, set
inside the `renders_one :header` slot lambda. ViewComponent v3
evaluates slot lambdas lazily at slot-render time (after the parent
template's `tag.dialog` opening attributes are computed), so the
flag was always `false` when the `aria-labelledby` attribute was
read.

Verified end-to-end via Playwright on `/design-system/preview/dialog/{modal,drawer}`:
the rendered `<dialog>` is missing `aria-labelledby` even when
`with_header(title: ...)` is set, despite the matching `<h2 id="dialog-title-...">`
being present in the DOM. AT therefore announces "dialog" with no
title — the exact regression the PR set out to fix on slot-driven
callers (which is every dialog in the app).

Always emitting `aria-labelledby="dialog-title-<hex>"` is safe per
the WAI-ARIA spec: a dangling reference (e.g. `custom_header: true`
or body-only dialogs) is silently ignored, and callers can override
via `**opts` (last-wins). This matches the intent stated in the PR
body of #1740.

- Drop now-dead `@has_auto_title` ivar + `has_auto_title?` predicate.
- Update template comment to explain the slot-lambda timing trap.
2026-05-20 18:18:38 +02:00

2.4 KiB