Files
sure/app/views/settings/providers/_enable_banking_panel.html.erb
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

193 lines
10 KiB
Plaintext

<div class="space-y-4">
<%
eb_link = link_to("Enable Banking", "https://enablebanking.com", target: "_blank", rel: "noopener noreferrer", class: "text-primary font-medium underline")
%>
<%= render "settings/providers/setup_steps",
steps: [
t("settings.providers.enable_banking_panel.step_1_html", link: eb_link),
t("settings.providers.enable_banking_panel.step_2"),
t("settings.providers.enable_banking_panel.step_3"),
t("settings.providers.enable_banking_panel.callback_url_instruction", callback_url: enable_banking_callback_url)
] %>
<% error_msg = local_assigns[:error_message] || @error_message %>
<% if error_msg.present? %>
<%= render DS::Alert.new(message: error_msg, variant: :error) %>
<% end %>
<%
# Use local family variable if available (e.g., from Sidekiq broadcast), otherwise fall back to Current.family (HTTP requests)
family = local_assigns[:family] || Current.family
enable_banking_item = family.enable_banking_items.first_or_initialize(name: "Enable Banking Connection")
is_new_record = enable_banking_item.new_record?
# Check if there are any authenticated connections (have session_id)
has_authenticated_connections = family.enable_banking_items.where.not(session_id: nil).exists?
%>
<%= styled_form_with model: enable_banking_item,
url: is_new_record ? enable_banking_items_path : enable_banking_item_path(enable_banking_item),
scope: :enable_banking_item,
method: is_new_record ? :post : :patch,
data: { turbo: true },
class: "space-y-3" do |form| %>
<%= form.select :country_code,
options_for_select([
["Austria (AT)", "AT"],
["Belgium (BE)", "BE"],
["Bulgaria (BG)", "BG"],
["Croatia (HR)", "HR"],
["Cyprus (CY)", "CY"],
["Czech Republic (CZ)", "CZ"],
["Denmark (DK)", "DK"],
["Estonia (EE)", "EE"],
["Finland (FI)", "FI"],
["France (FR)", "FR"],
["Germany (DE)", "DE"],
["Greece (GR)", "GR"],
["Hungary (HU)", "HU"],
["Iceland (IS)", "IS"],
["Ireland (IE)", "IE"],
["Italy (IT)", "IT"],
["Latvia (LV)", "LV"],
["Liechtenstein (LI)", "LI"],
["Lithuania (LT)", "LT"],
["Luxembourg (LU)", "LU"],
["Malta (MT)", "MT"],
["Netherlands (NL)", "NL"],
["Norway (NO)", "NO"],
["Poland (PL)", "PL"],
["Portugal (PT)", "PT"],
["Romania (RO)", "RO"],
["Slovakia (SK)", "SK"],
["Slovenia (SI)", "SI"],
["Spain (ES)", "ES"],
["Sweden (SE)", "SE"],
["United Kingdom (GB)", "GB"]
], enable_banking_item.country_code),
{ label: true, include_blank: "Select country..." },
{ label: "Country", class: "form-field__input" } %>
<% if has_authenticated_connections && !is_new_record %>
<%= render DS::Alert.new(
variant: :warning,
title: "Configuration locked",
message: "Disconnect all linked banks before changing these credentials."
) %>
<% end %>
<%= form.text_field :application_id,
label: "Application ID",
placeholder: is_new_record ? "Enter application ID" : "Enter new ID to update",
value: enable_banking_item.application_id,
disabled: has_authenticated_connections && !is_new_record %>
<%= form.text_area :client_certificate,
label: "Client Certificate (with Private Key)",
placeholder: "-----BEGIN PRIVATE KEY-----\n...\n-----END PRIVATE KEY-----\n-----BEGIN CERTIFICATE-----\n...\n-----END CERTIFICATE-----",
rows: 6,
class: "form-field__input font-mono text-xs",
disabled: has_authenticated_connections && !is_new_record %>
<div class="flex justify-end">
<%= form.submit is_new_record ? "Save and connect" : "Update connection",
class: "inline-flex items-center justify-center rounded-lg px-4 py-2 text-sm font-medium text-inverse button-bg-primary hover:button-bg-primary-hover focus:outline-none focus:ring-2 focus:ring-gray-900 theme-dark:focus:ring-white focus:ring-offset-2 transition-colors" %>
</div>
<% end %>
<% items = local_assigns[:enable_banking_items] || @enable_banking_items || family.enable_banking_items.where.not(client_certificate: nil) %>
<% if items&.any? %>
<%
# Find the first item with valid session to use for "Add Connection" button
item_for_new_connection = items.find(&:session_valid?)
# Check if any item needs initial connection (configured but no session yet)
item_needing_connection = items.find { |i| !i.session_valid? && !i.session_expired? }
%>
<div class="border-t border-primary pt-4 space-y-3">
<% items.each do |item| %>
<div class="flex items-center justify-between p-3 rounded-lg bg-container border border-primary">
<div class="flex items-center gap-3">
<% if item.syncing? %>
<div class="w-2 h-2 bg-inverse rounded-full animate-pulse"></div>
<div>
<p class="text-sm font-medium text-primary"><%= item.aspsp_name || t("settings.providers.enable_banking_panel.syncing", default: "Syncing") %></p>
<p class="text-xs text-secondary"><%= t("settings.providers.enable_banking_panel.syncing", default: "Syncing") %></p>
</div>
<% elsif item.sync_error.present? %>
<div class="w-2 h-2 bg-destructive rounded-full"></div>
<div>
<p class="text-sm font-medium text-primary"><%= item.aspsp_name || t("settings.providers.enable_banking_panel.connection_error") %></p>
<p class="text-xs text-destructive" title="<%= item.sync_error %>"><%= item.sync_error.truncate(50) %></p>
</div>
<% elsif item.session_valid? %>
<div class="w-2 h-2 bg-success rounded-full"></div>
<div>
<p class="text-sm font-medium text-primary"><%= item.aspsp_name || "Connected Bank" %></p>
<p class="text-xs text-secondary">
Session expires: <%= item.session_expires_at&.strftime("%b %d, %Y") || "Unknown" %>
</p>
</div>
<% elsif item.session_expired? %>
<div class="w-2 h-2 bg-warning rounded-full"></div>
<div>
<p class="text-sm font-medium text-primary"><%= item.aspsp_name || "Connection" %></p>
<p class="text-xs text-destructive">Session expired - reconnect</p>
</div>
<% else %>
<div class="w-2 h-2 bg-secondary rounded-full"></div>
<div>
<p class="text-sm font-medium text-primary">Configured</p>
<p class="text-xs text-secondary">Ready to link accounts</p>
</div>
<% end %>
</div>
<div class="flex items-center gap-2">
<% if item.session_valid? %>
<%= button_to sync_enable_banking_item_path(item),
method: :post,
class: "inline-flex items-center justify-center rounded-lg px-3 py-1.5 text-xs font-medium text-primary bg-container border border-primary hover:bg-surface-inset transition-colors",
data: { turbo: false } do %>
Sync
<% end %>
<% elsif item.session_expired? %>
<%= button_to reauthorize_enable_banking_item_path(item),
method: :post,
class: "inline-flex items-center justify-center rounded-lg px-3 py-1.5 text-xs font-medium text-white bg-warning hover:opacity-90 transition-colors",
data: { turbo: false } do %>
Reconnect
<% end %>
<% else %>
<%= link_to select_bank_enable_banking_item_path(item),
class: "inline-flex items-center justify-center rounded-lg px-3 py-1.5 text-xs font-medium text-inverse button-bg-primary hover:button-bg-primary-hover transition-colors",
data: { turbo_frame: "modal" } do %>
Connect bank
<% end %>
<% end %>
<%= button_to enable_banking_item_path(item),
method: :delete,
class: "inline-flex items-center justify-center rounded-lg px-3 py-1.5 text-xs font-medium text-destructive hover:bg-destructive/10 transition-colors",
data: { turbo_confirm: "Are you sure you want to remove this connection?" } do %>
Remove
<% end %>
</div>
</div>
<% end %>
<%# Add Connection button below the list - only show if we have a valid session to copy credentials from %>
<% if item_for_new_connection %>
<div class="flex justify-center pt-2">
<%= button_to new_connection_enable_banking_item_path(item_for_new_connection),
method: :post,
class: "inline-flex items-center gap-2 justify-center rounded-lg px-4 py-2 text-sm font-medium text-inverse button-bg-primary hover:button-bg-primary-hover transition-colors",
data: { turbo_frame: "modal" } do %>
<%= icon "plus", size: "sm" %>
Add Connection
<% end %>
</div>
<% end %>
</div>
<% end %>
</div>