Files
sure/app/components/DS/dialog.rb
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

153 lines
4.6 KiB
Ruby

class DS::Dialog < DesignSystemComponent
renders_one :header, ->(title: nil, subtitle: nil, custom_header: false, **opts, &block) do
content_tag(:header, class: "px-4 flex flex-col gap-2", **opts) do
title_div = content_tag(:div, class: "flex items-center justify-between gap-2") do
# `id: title_id` lets the host `<dialog>` reference the title via
# `aria-labelledby` so AT users hear the title when focus lands
# in the dialog. `content_tag("h#{heading_level}", ...)` builds an
# h2/h3/etc based on the caller's `heading_level:`.
title = content_tag("h#{heading_level}", title, id: title_id, class: class_names("font-medium text-primary", drawer? ? "text-lg" : "")) if title
close_icon = close_button unless custom_header
safe_join([ title, close_icon ].compact)
end
subtitle = content_tag(:p, subtitle, class: "text-sm text-secondary") if subtitle
block_content = capture(&block) if block
safe_join([ title_div, subtitle, block_content ].compact)
end
end
renders_one :body
renders_many :actions, ->(cancel_action: false, **button_opts) do
merged_opts = if cancel_action
button_opts.merge(type: "button", data: { action: "DS--dialog#close" })
else
button_opts
end
render DS::Button.new(**merged_opts)
end
renders_many :sections, ->(title:, **disclosure_opts, &block) do
render DS::Disclosure.new(title: title, align: :right, **disclosure_opts) do
block.call
end
end
attr_reader :variant, :auto_open, :reload_on_close, :width, :disable_frame, :content_class, :disable_click_outside, :opts, :responsive, :scrollable, :heading_level, :title_id
VARIANTS = %w[modal drawer].freeze
WIDTHS = {
sm: "lg:max-w-[300px]",
md: "lg:max-w-[550px]",
lg: "lg:max-w-[700px]",
full: "lg:max-w-full"
}.freeze
VALID_HEADING_LEVELS = (1..6).freeze
def initialize(variant: "modal", auto_open: true, reload_on_close: false, width: "md", frame: nil, disable_frame: false, content_class: nil, disable_click_outside: false, responsive: false, scrollable: true, heading_level: 2, **opts)
unless heading_level.is_a?(Integer) && VALID_HEADING_LEVELS.cover?(heading_level)
raise ArgumentError, "heading_level must be an Integer between 1 and 6, got: #{heading_level.inspect}"
end
@variant = variant.to_sym
@auto_open = auto_open
@reload_on_close = reload_on_close
@width = width.to_sym
@frame = frame
@disable_frame = disable_frame
@content_class = content_class
@disable_click_outside = disable_click_outside
@responsive = responsive
@scrollable = scrollable
@heading_level = heading_level
@title_id = "dialog-title-#{SecureRandom.hex(4)}"
@opts = opts
end
def frame
@frame || variant
end
# Caller must "opt-out" of using the default turbo-frame based on the variant
def wrapper_element(&block)
if disable_frame
content_tag(:div, &block)
else
content_tag("turbo-frame", id: frame, &block)
end
end
def dialog_outer_classes
variant_classes = if responsive?
"items-center justify-center lg:items-end lg:justify-end"
elsif drawer?
"items-end justify-end"
else
"items-center justify-center"
end
class_names(
"flex h-full w-full",
variant_classes
)
end
def dialog_inner_classes
variant_classes = if responsive?
"max-h-full lg:h-full lg:w-[550px]"
elsif drawer?
"lg:w-[550px] h-full"
else
class_names(
"max-h-full",
WIDTHS[width]
)
end
class_names(
"flex flex-col bg-container rounded-xl shadow-border-xs mx-3 lg:mx-0 w-full",
variant_classes,
content_class
)
end
def merged_opts
merged_opts = opts.dup
data = merged_opts.delete(:data) || {}
data[:controller] = [ "DS--dialog", "hotkey", data[:controller] ].compact.join(" ")
data[:DS__dialog_auto_open_value] = auto_open
data[:DS__dialog_reload_on_close_value] = reload_on_close
data[:DS__dialog_disable_click_outside_value] = disable_click_outside
data[:action] = [ "click->DS--dialog#clickOutside", data[:action] ].compact.join(" ")
data[:hotkey] = "esc:DS--dialog#close"
merged_opts[:data] = data
merged_opts
end
def drawer?
variant == :drawer
end
def responsive?
@responsive
end
def close_button
classes = responsive? ? "ml-auto hidden lg:flex" : "ml-auto"
render DS::Button.new(
variant: "icon",
class: classes,
icon: "x",
title: I18n.t("ds.dialog.close"),
aria_label: I18n.t("ds.dialog.close"),
data: { action: "DS--dialog#close" }
)
end
end