refactor(settings/providers): finish design-review cleanup pass

Picks up the remaining items from Claude Design's review of #1710
that the previous review-feedback commit didn't cover.

DS / casing
- Sentence-case the page title ("Bank Sync" -> "Bank sync") and
  align the nav label.
- Drop the card hover-lift (shadow-border-sm) in favour of
  bg-container-hover; per the DS, card hover is colour-only.
- Whole-tile click target on each provider card — the inner
  "Connect ->" link was a hit-target inversion.
- Set Sync all to whitespace-nowrap so the label stops wrapping at
  narrow viewport widths.

UX simplifications
- Drop the four health-summary tiles (per-row warn/err pills already
  surface the signal at the scale this app sees). Removes
  Settings::HealthSummary, the @health_counts controller block, and
  the now-unused health.* locale keys.
- Hide "Your connections" heading + empty-state line when no
  providers are connected — the lede already invites a connect.
- Drop the redundant "Free" tier from per-card meta lines (printed
  10x for one fact); "Paid" still surfaces on Plaid.

Tests updated to drop the obsolete tiles assertion and switch the
provider-card click selector to look up the (now whole-card) anchor
by provider name.
This commit is contained in:
Guillem Arias
2026-05-09 11:26:55 +02:00
committed by Guillem Arias
parent 60b2f2b1ce
commit bf73e3a1e3
9 changed files with 18 additions and 107 deletions

View File

@@ -1,8 +0,0 @@
<div class="grid grid-cols-2 sm:grid-cols-4 gap-2">
<% tiles.each do |tile| %>
<div class="bg-container shadow-border-xs rounded-xl p-3 text-center">
<div class="text-2xl font-bold tabular-nums <%= tile[:color_class] %>"><%= tile[:count] %></div>
<div class="text-xs text-secondary mt-0.5"><%= tile[:label] %></div>
</div>
<% end %>
</div>

View File

@@ -1,38 +0,0 @@
class Settings::HealthSummary < ApplicationComponent
def initialize(counts:)
@counts = counts
end
private
attr_reader :counts
def connected_count = counts[:connected]
def needs_attention_count = counts[:needs_attention]
def errors_count = counts[:errors]
def accounts_synced_count = counts[:accounts_synced]
def tiles
[
{
count: connected_count,
label: t("settings.providers.health.connected"),
color_class: connected_count > 0 ? "text-success" : "text-subdued"
},
{
count: needs_attention_count,
label: t("settings.providers.health.needs_attention"),
color_class: needs_attention_count > 0 ? "text-warning" : "text-subdued"
},
{
count: errors_count,
label: t("settings.providers.health.errors"),
color_class: errors_count > 0 ? "text-destructive" : "text-subdued"
},
{
count: accounts_synced_count,
label: t("settings.providers.health.accounts_synced"),
color_class: "text-primary"
}
]
end
end

View File

@@ -1,4 +1,6 @@
<div class="bg-container shadow-border-xs rounded-xl p-4 flex flex-col gap-3 hover:shadow-border-sm transition-shadow">
<%= link_to connect_path,
class: "bg-container shadow-border-xs rounded-xl p-4 flex flex-col gap-3 text-primary hover:bg-container-hover transition-colors",
data: { turbo_frame: "drawer", turbo_prefetch: "false" } do %>
<div class="flex items-start gap-3">
<div class="w-9 h-9 rounded-lg flex items-center justify-center shrink-0 <%= logo_bg %>">
<span class="text-xs font-bold text-inverse"><%= logo_text %></span>
@@ -18,12 +20,8 @@
<% if tagline.present? %>
<p class="text-sm text-secondary grow"><%= tagline %></p>
<% end %>
<div class="flex justify-end">
<%= link_to connect_path,
class: "inline-flex items-center gap-1.5 text-sm font-medium text-primary hover:text-primary/70 transition-colors",
data: { turbo_frame: "drawer", turbo_prefetch: "false" } do %>
<%= t("settings.providers.connect") %>
<%= helpers.icon "arrow-right", class: "w-4 h-4" %>
<% end %>
<div class="flex items-center justify-end gap-1.5 text-sm font-medium text-primary">
<%= t("settings.providers.connect") %>
<%= helpers.icon "arrow-right", class: "w-4 h-4" %>
</div>
</div>
<% end %>

View File

@@ -261,13 +261,6 @@ class Settings::ProvidersController < ApplicationController
@connected = entries.select { |e| e[:summary][:status] == :ok }
@needs_attention = entries.select { |e| [ :warn, :err ].include?(e[:summary][:status]) }
@available = entries.select { |e| e[:summary][:status] == :off }
@health_counts = {
connected: @connected.size + @needs_attention.size,
needs_attention: @needs_attention.size,
errors: @needs_attention.count { |e| e[:summary][:status] == :err },
accounts_synced: Current.family.accounts.joins(:account_providers).distinct.count
}
end
# Returns a hash mapping provider key → { error:, last_synced_at:, stale: }

View File

@@ -4,7 +4,6 @@ class Provider
simplefin: {
region: "US",
kind: "Bank",
tier: "Free",
maturity: :stable,
logo_bg: "bg-blue-600",
logo_text: "SF"
@@ -12,7 +11,6 @@ class Provider
lunchflow: {
region: "US",
kind: "Lunch",
tier: "Free",
maturity: :stable,
logo_bg: "bg-orange-500",
logo_text: "LF"
@@ -20,7 +18,6 @@ class Provider
enable_banking: {
region: "EU",
kind: "Bank",
tier: "Free",
maturity: :beta,
logo_bg: "bg-purple-600",
logo_text: "EB"
@@ -28,7 +25,6 @@ class Provider
coinstats: {
region: "Global",
kind: "Crypto",
tier: "Free",
maturity: :beta,
logo_bg: "bg-yellow-500",
logo_text: "CS"
@@ -36,7 +32,6 @@ class Provider
mercury: {
region: "US",
kind: "Bank",
tier: "Free",
maturity: :beta,
logo_bg: "bg-cyan-600",
logo_text: "ME"
@@ -44,7 +39,6 @@ class Provider
coinbase: {
region: "Global",
kind: "Crypto",
tier: "Free",
maturity: :beta,
logo_bg: "bg-blue-500",
logo_text: "CB"
@@ -52,7 +46,6 @@ class Provider
binance: {
region: "Global",
kind: "Crypto",
tier: "Free",
maturity: :beta,
logo_bg: "bg-yellow-400",
logo_text: "BI"
@@ -60,7 +53,6 @@ class Provider
snaptrade: {
region: "US / CA",
kind: "Investment",
tier: "Free",
maturity: :beta,
logo_bg: "bg-green-600",
logo_text: "ST"
@@ -68,7 +60,6 @@ class Provider
indexa_capital: {
region: "ES",
kind: "Investment",
tier: "Free",
maturity: :alpha,
logo_bg: "bg-red-600",
logo_text: "IC"
@@ -76,7 +67,6 @@ class Provider
sophtron: {
region: "US",
kind: "Bank",
tier: "Free",
maturity: :alpha,
logo_bg: "bg-teal-600",
logo_text: "SO"

View File

@@ -20,23 +20,19 @@
method: :post,
disabled: sync_all_disabled,
title: sync_all_disabled ? t("settings.providers.sync_all_recently") : nil,
class: "inline-flex items-center gap-2 shrink-0 px-3 py-1.5 text-sm font-medium text-secondary hover:text-primary border border-secondary rounded-lg hover:border-primary transition-colors disabled:opacity-50 disabled:cursor-not-allowed" do %>
class: "inline-flex items-center gap-2 shrink-0 whitespace-nowrap px-3 py-1.5 text-sm font-medium text-secondary hover:text-primary border border-secondary rounded-lg hover:border-primary transition-colors disabled:opacity-50 disabled:cursor-not-allowed" do %>
<%= icon "refresh-cw", class: "w-4 h-4" %>
<%= t("settings.providers.sync_all") %>
<% end %>
<% end %>
</div>
<%= render Settings::HealthSummary.new(counts: @health_counts) %>
<% all_connections = @needs_attention + @connected %>
<%= render "settings/providers/group_heading",
title: t("settings.providers.groups.your_connections"),
count: all_connections.size %>
<% if all_connections.empty? %>
<p class="text-sm text-secondary px-1 py-2"><%= t("settings.providers.groups.empty_connected") %></p>
<% if all_connections.any? %>
<%= render "settings/providers/group_heading",
title: t("settings.providers.groups.your_connections"),
count: all_connections.size %>
<% end %>
<% all_connections.each do |entry| %>

View File

@@ -176,7 +176,7 @@ en:
whats_new_label: What's new
api_keys_label: API Key
appearance_label: Appearance
bank_sync_label: Bank Sync
bank_sync_label: Bank sync
settings_nav_link_large:
next: Next
previous: Back
@@ -188,7 +188,7 @@ en:
providers:
not_authorized: Not authorized
bank_sync:
page_title: Bank Sync
page_title: Bank sync
lede: Connect external accounts so transactions, balances and holdings flow into Sure automatically.
status:
ok: Connected
@@ -204,13 +204,7 @@ en:
connected: Connected
needs_attention: Action needed
available: Available
empty_connected: Nothing connected yet — pick a provider below to get started.
empty_available: All available providers are connected.
health:
connected: Connected
needs_attention: Action needed
errors: Errors
accounts_synced: Accounts synced
meta:
sync_error: Sync error
no_recent_sync: Sync overdue

View File

@@ -81,17 +81,6 @@ class Settings::ProvidersTest < ApplicationSystemTestCase
assert_operator available_y, :<, available_grid_top, "Available heading should appear above the card grid"
end
test "health strip shows four tiles with correct counts" do
SimplefinItem.create!(family: @family, name: "Test SimpleFIN", access_url: "https://bridge.simplefin.org/simplefin/access")
visit settings_providers_path
assert_text "Connected"
assert_text "Action needed"
assert_text "Errors"
assert_text "Accounts synced"
end
test "action needed group is absent when no providers have issues" do
SimplefinItem.create!(family: @family, name: "Test SimpleFIN", access_url: "https://bridge.simplefin.org/simplefin/access")
@@ -149,14 +138,11 @@ class Settings::ProvidersTest < ApplicationSystemTestCase
end
end
test "clicking a provider card connect link opens the connect drawer" do
test "clicking a provider card opens the connect drawer" do
visit settings_providers_path
within available_provider_cards_container do
card = find(:xpath, ".//div[contains(concat(' ', normalize-space(@class), ' '), ' bg-container ')][.//span[normalize-space()='SimpleFIN']]")
within(card) do
find("a[data-turbo-frame='drawer']", text: "Connect").click
end
find("a[data-turbo-frame='drawer']", text: "SimpleFIN").click
end
assert_selector "dialog[open]"

View File

@@ -9,7 +9,7 @@ class SettingsTest < ApplicationSystemTestCase
[ "Accounts", accounts_path ]
]
@settings_links << [ "Bank Sync", settings_providers_path ] if @user.admin?
@settings_links << [ "Bank sync", settings_providers_path ] if @user.admin?
@settings_links += [
[ "Preferences", settings_preferences_path ],
@@ -91,7 +91,7 @@ class SettingsTest < ApplicationSystemTestCase
# Assert that admin-only settings are not present in the navigation
assert_no_selector "li", text: "AI Prompts"
assert_no_selector "li", text: "API Key"
assert_no_selector "li", text: "Bank Sync"
assert_no_selector "li", text: "Bank sync"
end
end