Commit Graph

4 Commits

Author SHA1 Message Date
Guillem Arias Fauste
f50c151e21 fix(design-system): DS::Alert alignment, accessibility, and hierarchy polish (#1734)
* fix(design-system): align DS::Alert icon with title

The icon was rendered at size 'sm' (w-4 h-4) and started at the very
top of the flex row (items-start without an offset), which optically
sat above the title's cap when the title was present and slightly
above the message baseline when it wasn't. The hand-rolled alerts
this PR replaced used 'w-5 h-5 mt-0.5' for exactly this reason —
restore the same combination in the component:

- size: sm -> md (w-4/h-4 -> w-5/h-5).
- class adds mt-0.5 so the icon's vertical center lines up with the
  bold title's cap-height (and with the body baseline in the title-less
  case).

No API change. Visual fix only.

Refs #1731

* fix(design-system): split DS::Alert into title-row + indented body

Replaces the items-start + margin-fudge approach with a two-row
layout that doesn't depend on icon-bounding-box vs text-cap-height
arithmetic:

- Title case: icon and bold title share a flex row with items-center,
  so the icon's vertical centre lines up with the title's line. Body
  (block content or message) renders below in a separate row, padded
  by pl-8 (= icon md width + gap-3) so it indents under the title
  text rather than under the icon.
- Block-only case (no title, no message — used by the alpha_vantage
  rate-limit alert): keeps the items-start fallback with a small mt-0.5
  on the icon so the cap of the first paragraph still sits near the
  icon centre.
- Single-line message case: items-center between icon and message, no
  fudge needed.

container_classes loses its 'flex items-start gap-3' base since the
outer div is no longer the flex container. Each branch declares its
own flex/items-* combination.

Refs #1731

* fix(design-system): a11y semantics + visual polish on DS::Alert

Builds on the title-row restructure with the items the design / a11y
review surfaced:

- live: keyword (default :none, accepts :status / :polite and
  :alert / :assertive) maps to role="status" or role="alert" on the
  outer div. Static, page-baked alerts (the migrated callsites in
  #1731) keep the default :none and stay role-less. Dynamic surfaces
  (flash, validation summaries appearing after a Turbo update) opt
  into the live role they need.
- aria-labelledby on the outer div pointing at the title <p> so AT
  picks the title as the alert's accessible name when one is set.
- Variant prefix in the title / message via an sr-only span. Screen
  reader hears 'Warning: …', 'Error: …', etc.; sighted users see no
  change. Variant labels live under ds.alert.variants.* in
  config/locales/views/components/en.yml.
- Body text inside titled alerts now defaults to text-secondary
  instead of text-primary, so hierarchy reads on weight + colour
  rather than weight alone (Refactoring UI: hierarchy needs both).
  Single-line message and block-only fallback keep text-primary
  since there is no second tier.
- Icon size goes back from md (20px) to sm (16px) — proportionally
  closer to text-sm body — and the items-center branches grow
  -mt-0.5 to compensate for the cap-centre vs line-centre offset
  that flex's items-center alone can't bridge.
- Title weight bumped from font-medium (500) to font-semibold (600)
  for clearer prominence against the now-softer body.

No API breakage: existing callers passing only message:/title:/variant:
keep working. The new live: arg defaults to the correct value for
the static migration sites.

Refs #1731

* fix(design-system): drop aria-labelledby when alert has no role; revert body to text-primary

Two corrections after numerical contrast analysis and CodeRabbit feedback:

1. aria-labelledby was being emitted on every titled alert, but the
   default live: :none leaves the outer <div> with no role. ARIA spec
   only honours the labelling relationship on elements with a host
   role, so on a generic <div> the attribute is invalid and
   accessibility validators flag it. Now only emitted when aria_role
   is set (live: :status or :alert). Static, page-baked callsites
   stay role-less and label-less; dynamic callers that opt into a
   live role get the proper accessible-name relationship.

2. text-secondary on bg-{variant}/10 in light mode lands at
   ~4.07-4.25:1 contrast — below WCAG AA's 4.5:1 for normal text.
   Reverting the body wrapper to text-primary brings it back to
   AAA (~15:1). Loses some of the Refactoring UI body-vs-title
   colour hierarchy; the title's font-semibold weight + larger
   optical mass against an otherwise plain body still reads as
   hierarchy. Single-line message and block-only fallback already
   used text-primary, so this just unifies the three branches.

The remaining contrast gap — text-success (green-600) icon on
bg-success/10 light surface at 2.77:1 — is documented in the PR
description; fixing it cleanly needs a token-level bump
(--color-success: green-600 -> green-700 in light mode) which is
out of scope for this PR.

Refs #1731

* fix(settings/providers): use DS::Alert title:+message: instead of inline content_tag

Three callsites added in #1710 passed block-level markup (`<p>`/`<h2>`)
through `message:` via `safe_join + content_tag`. The post-#1731 alert
template wraps `message:` in a `<p>`, which makes nesting a `<p>` or
`<h2>` invalid HTML — browsers auto-close the outer paragraph and the
indented body row collapses.

Each of the three is semantically a title + body pair, so swap them
to the proper `title:` + `message:` API. No new strings — the i18n
keys (`*.no_withdraw_title` / `_body`, `encryption_error.title` /
`.message`) already split that way; the inline assembly was the
artefact.

The encryption-error block loses an explicit `<h2>` wrapper around
the title; DS::Alert's title is a `<p>`. The visual hierarchy and
sr-only variant prefix are unchanged. Worth tracking heading semantics
as a follow-up against DS::Alert (a `heading_level:` arg) rather than
bringing back the manual markup.

* fix(design-system): make :destructive variant alias explicit in DS::Alert locale

Add `destructive: Error` to `ds.alert.variants` and drop the implicit
`:destructive -> :error` aliasing in `DS::Alert#variant_label`. Both the
locale file and the component now self-document the variant set; lookup
is direct, no conditional needed.

Per @jjmata review on #1734.
2026-05-11 23:29:05 +02:00
Juan José Mata
7f0569357a Defensive coding on @variant 2026-05-10 15:15:40 +00:00
Guillem Arias Fauste
57d71cd55e refactor(design-system): extend DS::Alert and migrate 9 inline alert blocks (#1731)
* feat(design-system): add info semantic color token

Mirrors success/warning/destructive: --color-info maps to blue-600 in
light mode, blue-500 in dark mode. Unblocks the DS::Alert info variant
from carrying a raw 'blue-600' literal in icon_color and lets surface
tokens use bg-info/N alpha modifiers like the rest of the system.

Refs #1715

* refactor(design-system): adopt semantic tokens and add body slot in DS::Alert

Replaces the bg-{blue,green,yellow,red}-50 / text-{...}-700 / border-{...}-200
palette block in DS::Alert with semantic alpha-modifier surfaces
(bg-{info,success,warning,destructive}/10 + matching /20 borders).
Drops the 'blue-600' literal that icon_color was returning for the
info variant; helpers#icon now accepts color: :info backed by the
new --color-info token.

Adds an optional title: kwarg and an opt-in block-content slot so
rich alerts (title + paragraph, lists, embedded actions) can render
without callers reaching for a hand-rolled flex layout. The existing
message: API stays backward-compatible — nothing in the codebase that
already calls DS::Alert.new(message: ..., variant: ...) needs to change.

Lookbook gains with_title and with_body_slot examples covering the
new shapes.

Refs #1715

* refactor(views): migrate api_keys, hostings, lunchflow alerts to DS::Alert

Cleans up nine bespoke alert blocks that hand-rolled the same
flex + icon + bordered-surface shape DS::Alert already provides:

- settings/api_keys/{new,created,created.turbo_stream}.html.erb — three
  near-identical 'Security Warning' / 'Important Security Note' boxes
  using the broken bg-warning-50 / text-warning-700 raw-palette pair.
- settings/hostings/{_alpha_vantage,_eodhd,_yahoo_finance,_twelve_data,_provider_selection}_settings.html.erb —
  five amber-50 / amber-200 warning boxes covering rate-limit notes,
  health-check failure messaging, and the env-configured override
  banner. The twelve_data plan-restriction block keeps its bullet
  list and pricing link inside the new DS::Alert body slot.
- lunchflow_items/{_api_error,_setup_required}.html.erb — two modal
  alert headers whose flex+icon scaffolding now collapses onto
  DS::Alert. The surrounding bg-surface 'Common issues' / 'Setup
  steps' info cards stay as-is; this PR only touches the alert
  shape itself.

No functional or behavioural changes. Locale keys preserved.
amber-* palette uses on the alerts disappear; remaining bg-amber-*
hits in the codebase live outside the alert pattern and stay for
follow-up sub-PRs of #1715.

Refs #1715
2026-05-10 17:14:06 +02:00
Zach Gollwitzer
ab6fdbbb68 Component namespacing (#2463)
* [claudesquad] update from 'component-namespacing' on 18 Jul 25 07:23 EDT

* [claudesquad] update from 'component-namespacing' on 18 Jul 25 07:30 EDT

* Update stimulus controller references to use namespace

* Fix remaining tests
2025-07-18 08:30:00 -04:00