Commit Graph

4 Commits

Author SHA1 Message Date
Guillem Arias Fauste
77dda53ffb feat(ds): one canonical focus ring across primitives (#2140)
* feat(ds): one canonical focus ring across primitives (#2136)

Replaces the grab-bag of per-primitive focus indicators (neutral
ring-alpha-black/white, outline-gray-900/white, faint form-field ring-4)
with a single recipe — the #1737 accessibility follow-up.

- New --color-focus-ring token: blue-600 (light) / blue-500 (dark),
  >=4:1 against both surfaces.
- Canonical .focus-ring / .focus-ring-within in components.css: a 2px
  outline + 2px offset on :focus-visible only. Outline (not a box-shadow
  ring) so the offset gap is transparent on any surface with no layout
  shift; :focus-visible so it never shows for mouse/touch.
- Applied to every focusable DS primitive: Button (had none), Link,
  Disclosure summary, Tabs nav, MenuItem (replaces the browser-default
  box), SearchInput, Tooltip trigger, Popover trigger, Select panel
  (focus-within), Toggle (peer-driven outline-focus-ring). .form-field
  adopts it via :focus-within, replacing the ~1:1 ring-4.
- Dialog close button is a DS::Button icon variant, so it inherits the
  focus-visible-only ring and keeps no resting border (fixes "stuck ring").

Verified in-browser, light+dark: focus-visible ring on button, input, and
full-width menu row — consistent blue 2px+offset, legible on both surfaces.

Remaining follow-up: >=44px touch targets (disclosure trigger, composer
send); bespoke notification / account-new close buttons that still carry a
permanent border.

* fix(ds): #2136 interactive-state follow-ups — touch target + close-button chrome

- Disclosure default trigger: add min-h-11 (44px) so the standalone disclosure
  summary clears the touch-target minimum (was px-3 py-2 ~36px). Composer send +
  the coming-soon icons are already DS::Button icon/md (w-11 h-11).
- Notification close buttons (sync_toast, notice): drop the resting
  border-alpha-black-50 box ("frame shouts, glyph muted"); keep a bg-container +
  shadow-xs chip so the corner control stays visible over the page, and brighten
  the muted glyph on hover (text-subdued -> hover:text-primary).

* refactor(ds): focus ring -> neutral hugging box-shadow (was blue outline)

Per design feedback: the blue 2px outline + 2px offset read as a loud,
detached frame on the otherwise-neutral UI. Switch the canonical .focus-ring
to a soft box-shadow ring that hugs the control (follows border-radius, no
gap), in the theme-aware neutral focus-ring token (alpha-black/white-400).
Transparent outline kept as a forced-colors fallback; toggle peer-driver
switched from outline-* to ring-* to match. Still one token, :focus-visible
only. Strength is tunable (currently subtle ~1.5:1).

* fix(ds): focus ring vanished on shadowed controls — outline, not box-shadow

The neutral box-shadow ring lived in the components layer, so any utility-layer
shadow-* (or .form-field's focus-within:shadow-none) on the same element
overrode it and the ring silently disappeared on shadowed buttons/inputs. Draw
the same subtle neutral ring with a hugging `outline` (outline-offset: 0)
instead — a separate property with no box-shadow conflict, and it doubles as the
forced-colors indicator. Toggle peer-driver switched ring-* -> outline-* to
match. Look is unchanged (neutral, hugging, subtle); it just no longer vanishes.

* fix(a11y): enlarge sync-toast close-button touch target (p-0.5 -> p-1.5)

The hover-revealed close button had ~2px padding around a 20px icon (~24px
total), at the WCAG 2.5.8 AAA boundary. p-1.5 brings the interactive area to
~32px. Addresses CodeRabbit review on #2140.

* fix(ds): keep form-field's resting halo; stop the outline color flash

Two testing findings:

- .form-field reverts to its original always-on soft ring
  (focus-within ring-4 at low alpha, theme-aware) instead of adopting
  the keyboard-only outline. It's a resting decoration, not a focus
  indicator, and the lower-opacity halo was the better look. The
  canonical block's comment documents the deliberate opt-out.

- .focus-ring/.focus-ring-within now carry a base transparent 2px
  outline so consumers with transition-all (form-field had it) animate
  transparent -> token on focus instead of passing through
  currentColor, which flashed as a black border appearing and then
  fading out.

* feat(ds): focus-ring token clears WCAG 3:1 non-text contrast

alpha-black-400 (20%) measured ~1.6:1 against white — visible but below
the AA bar for focus indicators. Bump to the 700 stop (50%): ~3.95:1 on
light containers, ~4.6:1 on dark. Recipe unchanged; one token edit via
tokens:build.

* fix(ds): ring the hand-rolled privacy toggle too

The header pair showed two different focus treatments: panel-right (a
DS::Button) got the new token ring while the hand-rolled privacy
toggle next to it fell back to the browser-default ring — the sweep
covered DS primitives but not bespoke buttons. Both privacy toggles
(mobile + desktop) now carry .focus-ring.

Also documents the transition interplay on the focused-state rule:
consumers with transition-colors fade the ring in over 150ms because
Tailwind v4's color transition list includes outline-color. Verified
settled value at the intended 50% alpha via Playwright.

* fix(ds): ring the sidebar and settings nav links

The reshoot caught both nav species falling back to the browser's blue
default ring — main sidebar items and settings nav items are bespoke
link_to markup the primitive sweep missed, and they're the primary
keyboard path in the app. Both adopt .focus-ring (main nav adds
rounded-lg so the outline follows a shape).

* fix(ds): retire the legacy base-layer button ring for the canonical outline

The @layer base button rule still painted a ring-2 ring-offset-2
box-shadow on :focus-visible. Box-shadow and outline are independent
properties, so .focus-ring (an outline) could never clear it and every
button-tag primitive double-painted both indicators on keyboard focus.

Apply the canonical recipe to the base button rule itself: every
<button> now gets the transparent resting outline + focus-ring token on
:focus-visible by default.

Two bespoke buttons suppressed the outline with focus:outline-none and
relied on the base ring for their keyboard indicator (category dropdown
rows, the sign-up password toggle). Drop the suppression so they pick
up the canonical outline — :focus-visible keeps it keyboard-only, which
is what the suppression was protecting against anyway.

* fix(ds): segmented control adopts the canonical focus recipe

The segment rule inlined its own focus-visible outline (offset 2,
alpha-400 colors) with a comment noting it was temporary until the
canonical token landed — this branch is that token. Drop the inlined
utilities: button segments get the outline from the base button rule,
and link segments now carry .focus-ring.
2026-06-11 16:05:13 +02:00
Guillem Arias Fauste
7a0665a8f6 fix(ds): one height rail for icon and text buttons (#2202)
* fix(ds): put icon buttons on the text-button height rail

Icon-only DS::Button containers were 32/44/48px squares while text
buttons of the same nominal size render ~28/36/48px tall, so every
mixed header row (icon menu trigger next to text buttons — the
transactions index, account pages, the app header) sat misaligned.

- buttonish SIZES: icon containers now share the text rail
  (sm w-7, md w-9, lg w-12 unchanged).
- DS::Popover's hand-rolled w-11 trigger joins the rail at w-9.
- The layout's hand-rolled privacy toggle (mobile + desktop) matches
  the md icon-button chrome: w-9, rounded-lg, container-inset hover —
  it sat at w-8 with a different hover next to a DS icon button.
- DS::Select's panel adopts shadow-border-lg, the elevation Menu and
  Popover already use, replacing the weaker shadow-lg+border-xs combo.

Measured on the transactions header after the change: 36/38/36px.

* fix(ds): keep the 44px touch target on coarse pointers

The height rail trades icon-button size for row alignment, which is a
pointer-precision tradeoff: WCAG 2.5.5's 44x44 minimum is about
fingers, not mice. sm/md icon containers (and the two off-rail
consumers: the popover trigger and the layout privacy toggles) gain
pointer-coarse:w-11/h-11, so touch devices keep the full target while
fine-pointer layouts get the aligned 36px row.

Measured via Playwright: desktop 36x36, iPhone emulation 44x44 on both
the menu trigger and the privacy toggle.
2026-06-06 17:00:54 +02:00
Guillem Arias Fauste
1742f4ef1e feat(ds): elevate dropdown overlays and stabilize selection check gutter (#2161)
* feat(ds): elevate dropdown overlays and stabilize selection check gutter

Menus and popovers floated at the same elevation as inline cards
(shadow-border-xs), so dropdowns blended into the content beneath them.
Bump DS::Menu and DS::Popover panels to shadow-border-lg.

DS::MenuItem rendered its leading icon only when present, so a selection
check shifted the row's text out of alignment with the unselected rows.
Add a `selected:` param that reserves a fixed-width check gutter (check
when selected, empty otherwise) so row text stays aligned. Apply the same
reserved gutter to the bespoke category dropdown row, and add a
`selectable` menu preview.

* fix(ds): expose menu selection via menuitemradio + aria-checked

Selectable DS::MenuItem rows conveyed selection only visually. Render them
as role="menuitemradio" with aria-checked so assistive tech gets the
selection state of single-select lists, merging the menu ARIA contract with
any caller-supplied aria. Addresses CodeRabbit review feedback.

* fix(ds): include selectable roles in menu roving-focus query

DS::MenuItem selectable rows render as role=menuitemradio, but the menu
controller built its roving-focus list from [role=menuitem] only, leaving
single-select menus with no keyboard focus/arrow handling. Query the
menuitemradio/menuitemcheckbox roles too. Addresses Codex review feedback.
2026-06-04 11:55:57 +02:00
Guillem Arias Fauste
12785754c8 feat(design-system): split DS::Menu into strict action-list + new DS::Popover (#1850)
* feat(design-system): split DS::Menu into strict action-list + new DS::Popover for mixed content

Closes #1743.

DS::Menu used to absorb both action-list dropdowns (row context menus,
"more actions") AND mixed-content panels (user-account dropdown,
filter forms, picker pop-ups). The two shapes carry incompatible a11y
contracts:

- **Action list**: `role="menu"` container, `role="menuitem"` children,
  Up/Down arrow nav per WAI-ARIA APG.
- **Mixed content**: NO menu role — `role="menu"` restricts AT users
  to menuitem-only navigation and breaks any panel with forms,
  headings, or generic groupings.

This PR splits the component:

## DS::Menu (tightened)

Strict action-list primitive. Variants reduced to `:icon` and
`:button` (no `:avatar`). `custom_content` slot removed. Bakes in:

- `role="menu"` on the panel, `aria-haspopup="menu"` +
  `aria-expanded` + `aria-controls` on the trigger.
- `role="menuitem"` + `tabindex="-1"` on every DS::MenuItem; the
  controller installs roving tabindex (first item gets `tabindex="0"`
  when the menu opens) and handles ArrowUp/Down/Home/End +
  Escape + Enter/Space activation.
- `role="separator"` on the divider variant.
- Stable per-instance `menu-<8-char hex>` id so the trigger's
  `aria-controls` resolves correctly.

`DS::Menu.new(variant: :avatar, ...)` now raises ArgumentError
pointing at DS::Popover.

## DS::Popover (new)

Positioned panel for **mixed**, **non-action-list** content: account
menus, picker forms, filter forms, embedded controls. Slots: `button`,
`header`, `custom_content`. Variants: `:icon`, `:button`, `:avatar`.
NO `role="menu"` — the panel announces as a generic dialog-popup
(`aria-haspopup="dialog"`, `aria-expanded`, `aria-controls`).
Mirrors DS::Menu's floating-ui positioning + Escape/outside-click
lifecycle in its own Stimulus controller (`DS--popover`). Avatar
variant ships a focus ring + bumped touch target (44×44 via `w-11
h-11` per #1738).

## Migrated callsites (7 → DS::Popover)

- `app/views/users/_user_menu.html.erb` — avatar trigger + profile
  header + nav links (items kept as DS::MenuItem inside
  `custom_content` for visual parity)
- `app/views/categories/_menu.html.erb` — turbo-framed category picker
- `app/views/budgets/_budget_header.html.erb` — budget picker
- `app/views/reports/index.html.erb` — period picker
- `app/views/holdings/_cost_basis_cell.html.erb` — cost-basis edit form
- `app/views/transactions/searches/_form.html.erb` — filter form
- `app/components/UI/account/activity_feed.html.erb:70` — status
  checkboxes (the row-level "new" menu on line 9 stays as DS::Menu)

The other 33 DS::Menu callsites stay as-is — pure action lists.

Locale: `ds.popover.avatar_default_label` + `users.user_menu.aria_label`
keys added (en only; other locales handled in a separate i18n pass).

* fix(test): update sidebar user-menu selector for Menu→Popover migration

The user-menu now renders as `DS::Popover` (variant: :avatar) instead
of `DS::Menu` after the menu split, so its trigger carries
`data-DS--popover-target="button"` rather than the old
`data-DS--menu-target`. Update the sidebar-driven settings test helper
to match — every system test that drives Settings via the sidebar
gates on this selector.

* fix(review): DS::Popover/Menu trigger a11y + caller-attr preservation

- popover.rb / menu.rb: button slot now merges (not overwrites) caller-
  provided data and aria hashes, sets aria-haspopup/expanded/controls on
  the :button variant, defaults type="button" on block-rendered buttons.
- menu.rb / menu.html.erb: drop renders_one :header (strict-menu API
  shouldn't expose an arbitrary-markup escape hatch); preview updated.
- menu_controller.js: handle Enter/Space activation on focused menuitem
  so keyboard navigation matches the ARIA menu pattern.
- cost_basis_cell / transactions/searches/_menu: retarget cancel button
  data-action from DS--menu#close to DS--popover#close (host controller
  changed in the migration).

* fix: apply CodeRabbit auto-fixes

Fixed 1 file(s) based on 1 unresolved review comment.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>

* fix(review): MenuItem roving: false for DS::Popover usage

Codex P1 on #1850: \`DS::MenuItem\` hard-codes \`tabindex=\"-1\"\` and
\`role=\"menuitem\"\` for both link and button variants — correct
inside \`DS::Menu\` (which provides arrow-key roving and announces
\`role=\"menu\"\`), but breaks every \`DS::MenuItem\` rendered inside
\`DS::Popover\` (\`app/views/users/_user_menu.html.erb\`). Popover has
no roving handler, so Tab skips every item — Settings, Changelog,
Feedback, Contact, Log out become keyboard-unreachable.

Add a \`roving:\` keyword (default \`true\`) to \`DS::MenuItem\` that
gates both \`tabindex=\"-1\"\` and \`role=\"menuitem\"\`. \`DS::Menu\`
callers keep the default (roving menu semantics intact). Pass
\`roving: false\` from \`_user_menu.html.erb\` so user-menu items land
in the normal Tab order. Existing \`menu.with_item(...)\` callers in
the design system still default to \`true\`, so no behavior change for
\`DS::Menu\` consumers.

* fix(review): make menuitem_attrs authoritative on roving

CodeRabbit Major on #1850: \`merged_opts\` was splatted AFTER
\`menuitem_attrs\` in \`DS::MenuItem#wrapper\`, so a stray
\`role: :button\` or \`tabindex: 0\` from a \`menu.with_item(..., role: …)\`
caller could silently downgrade the \`DS::Menu\` ARIA contract that
\`menuitem_attrs\` enforces.

Strip \`:role\` and \`:tabindex\` from \`merged_opts\` whenever
\`roving\` is enabled, then splat \`menuitem_attrs\` last. When
\`roving: false\` (popover usage in \`_user_menu.html.erb\`) callers
keep full control — Tab order and explicit ARIA stay tunable by the
caller.

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
2026-05-20 18:30:25 +02:00