From 10b360bb54973d446689dd525b3fb93f8bcff108 Mon Sep 17 00:00:00 2001 From: Guillem Arias Date: Thu, 14 May 2026 19:38:06 +0200 Subject: [PATCH] fix(goals/funding-widget): restore DS-aligned per-account breakdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit V2 rebuilt the funding widget around per-account rows + a custom SVG sparkline, but cut visible signal and DS adherence in the process. This rebuild restores the V1 affordances and folds in the V2 sparkline as an enhancement. - Heading regression: `text-lg font-medium` (with total in `text-lg`) → `text-sm font-medium` (total inheriting `text-sm`). The section heading collapsed to body-copy size and no longer matched the Projection heading beside it. Restore both to `text-lg`. - Avatar regression: V2 hand-rolled `w-10 h-10 rounded-full … style="color: white"`. That box (40px) matches no `Goals::AvatarComponent` size (sm=24px, md=36px, lg=44px), uses `rounded-full` where the DS uses `rounded-md/lg/xl/2xl`, and hardcodes white text instead of the `text-inverse` token. Render `Goals::AvatarComponent` directly at `size: "sm"`. - Privacy regression: `row[:balance_money]` subline ("Depository · $3,000") wasn't wrapped in `privacy-sensitive`. Blur mode no longer hid the balance, while heading total and last-30d value on the same row both had the class. Add `privacy-sensitive` to the subline. - Untranslated leak: `<%= account.accountable_type %>` printed the raw "Depository" / "Investment" / "Crypto" class string with no i18n. Add `accountable_label(account)` on the component that prefers the depository subtype ("Savings", "HSA"…) via `goals.form_stepper.step1.subtypes.*`, falling back through `accounts.types.*` and finally a `titleize`. - Lost weight signal: V1 had a stacked distribution bar across the top, colored legend dots, and a 5-bar weight pill per row. Users could see "Account A contributes 60% of balance" at a glance. V2 deleted all three. Restore the distribution bar + legend + the existing `pages/dashboard/group_weight` partial in a `weight` column (skipped when only one account is linked). - Lost container framing: V1 wrapped rows in `bg-container-inset rounded-xl p-1` with `shared/ruler` dividers between rows. V2 used `space-y-3` with no container and no dividers, leaving rows floating. Restore both. - Empty state regression: V2's fallback rendered the section heading as a body paragraph (`

Funding accounts

`) inside a `p-5 rounded-xl` card — looked like an unfinished widget. Replace with a real empty state via `goals.show.funding_accounts. empty.heading` + `body` ("Edit the goal to link the depository accounts you save into."). - Row order: V2 sorted by 30-day inflow (which can flatten to ties at $0 across rows). Sort by balance instead — the column the user is comparing against anyway. - Pace alignment: drop the transfer-kind exclusion from the component's `last_30_inflow_for` and `sparkline_for` so the widget reads the same flow as `Goal#pace` (commit B). Internal transfers between linked accounts net out per-account here too, external transfers count as inflow on the receiving account. The 12-bucket sparkline still runs 12 queries per account; that N+1 lands in a follow-up commit alongside the component-level query collapse. --- ...ding_accounts_breakdown_component.html.erb | 94 +++++++++++++------ .../funding_accounts_breakdown_component.rb | 36 +++++-- config/locales/views/goals/en.yml | 4 + 3 files changed, 98 insertions(+), 36 deletions(-) diff --git a/app/components/goals/funding_accounts_breakdown_component.html.erb b/app/components/goals/funding_accounts_breakdown_component.html.erb index addddf882..c67f924e9 100644 --- a/app/components/goals/funding_accounts_breakdown_component.html.erb +++ b/app/components/goals/funding_accounts_breakdown_component.html.erb @@ -1,44 +1,80 @@ <% if rows.empty? %> -

<%= t("goals.show.funding_accounts_heading") %>

+
+

<%= t("goals.show.funding_accounts.empty.heading") %>

+

<%= t("goals.show.funding_accounts.empty.body") %>

+
<% else %>
-

+

<%= t("goals.show.funding_accounts_heading") %> · - <%= goal.current_balance_money.format %> + <%= Money.new(total, goal.currency).format(precision: 0) %>

-
- <% rows.each do |row| %> - <% account = row[:account] %> - <% spark = row[:sparkline_points] %> - <% spark_max = [ spark.max, 1.0 ].max %> -
-
- <%= account.name[0]&.upcase %> -
+ <%# Distribution bar — proportional weight of each account in this goal %> + <% if rows.size > 1 && total.positive? %> +
+ <% rows.each do |row| %> + <% next if row[:balance].to_d.zero? %> +
+ <% end %> +
-
-

<%= account.name %>

-

<%= account.accountable_type %> · <%= row[:balance_money].format %>

+
+ <% rows.each do |row| %> + <% next if row[:balance].to_d.zero? %> +
+
+

<%= row[:account].name %>

+

<%= percent_for(row[:balance]) %>%

+ <% end %> +
+ <% end %> - - <% xs = spark.each_with_index.map { |_, i| 2 + (i / (spark.size - 1).to_f) * (spark.size * 8 - 4) } %> - <% ys = spark.map { |v| 24 - (v / spark_max) * 20 } %> - <% path = xs.zip(ys).each_with_index.map { |(x, y), i| "#{i.zero? ? "M" : "L"} #{x.round(2)} #{y.round(2)}" }.join(" ") %> - - + <%# Per-account detail — avatar / name+type / weight / sparkline / last-30d inflow %> +
+
+ <% rows.each_with_index do |row, idx| %> + <% account = row[:account] %> + <% color = Goals::AvatarComponent.color_for(account.name) %> + <% spark = row[:sparkline_points] %> + <% spark_max = [ spark.max, 1.0 ].max %> +
+ <%= render Goals::AvatarComponent.new(name: account.name, color: color, size: "sm") %> -
-

- <%= row[:last_30_money].format %> -

-

<%= t("goals.show.funding_last_30d") %>

+
+

<%= account.name %>

+

+ <%= accountable_label(account) %> · <%= row[:balance_money].format %> +

+
+ + <% if rows.size > 1 %> + <%= render "pages/dashboard/group_weight", weight: percent_for(row[:balance]), color: color %> + <% else %> + + <% end %> + + + +
+

+ <%= row[:last_30_money].format(precision: 0) %> +

+

<%= t("goals.show.funding_last_30d") %>

+
-
- <% end %> + <% if idx < rows.size - 1 %> + <%= render "shared/ruler", classes: "mx-4" %> + <% end %> + <% end %> +
<% end %> diff --git a/app/components/goals/funding_accounts_breakdown_component.rb b/app/components/goals/funding_accounts_breakdown_component.rb index 9d323a6e0..cf0c45ecb 100644 --- a/app/components/goals/funding_accounts_breakdown_component.rb +++ b/app/components/goals/funding_accounts_breakdown_component.rb @@ -9,18 +9,40 @@ class Goals::FundingAccountsBreakdownComponent < ApplicationComponent attr_reader :goal def rows - @rows ||= goal.linked_accounts.sort_by { |a| -last_30_inflow_for(a) }.map do |account| - inflow = last_30_inflow_for(account) + @rows ||= goal.linked_accounts.sort_by { |a| -a.balance.to_d }.map do |account| { account: account, + balance: account.balance.to_d, balance_money: Money.new(account.balance.to_d, goal.currency), - last_30_money: Money.new(inflow, goal.currency), - last_30_amount: inflow, + last_30_money: Money.new(last_30_inflow_for(account), goal.currency), sparkline_points: sparkline_for(account) } end end + def total + @total ||= rows.sum { |r| r[:balance].to_d } + end + + def percent_for(balance) + return 0 if total.zero? + ((balance.to_d / total) * 100).round + end + + # Label shown beneath the account name. Prefers the depository subtype + # ("Savings", "HSA"…) over the bare accountable_type ("Depository") so the + # subline carries useful signal. Falls back to the accountable type's i18n + # entry (`accounts.types.*`), and finally to a `titleize` so the row is + # never blank if a string is missing. + def accountable_label(account) + if account.subtype.present? + I18n.t("goals.form_stepper.step1.subtypes.#{account.subtype}", default: account.subtype.titleize) + else + type = account.accountable_type.to_s + I18n.t("accounts.types.#{type.underscore}", default: type.titleize) + end + end + private def last_30_inflow_for(account) @inflow_cache ||= {} @@ -28,14 +50,15 @@ class Goals::FundingAccountsBreakdownComponent < ApplicationComponent net = Entry .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") .where(account_id: account.id, date: WINDOW_DAYS.days.ago.to_date..Date.current) - .where.not(transactions: { kind: Transaction::TRANSFER_KINDS }) .where(excluded: false) .sum(:amount) (-net.to_d).clamp(0, Float::INFINITY) end end - # 12-bucket weekly sparkline of net non-transfer inflow over 90 days. + # 12-bucket weekly sparkline of net inflow over 90 days. Uses the same + # transfer-inclusive semantics as Goal#pace — transfers between linked + # accounts wash out across the goal but show on each account's sparkline. def sparkline_for(account) buckets = 12 bucket_days = (SPARK_WINDOW_DAYS / buckets.to_f).ceil @@ -46,7 +69,6 @@ class Goals::FundingAccountsBreakdownComponent < ApplicationComponent net = Entry .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") .where(account_id: account.id, date: start_at..end_at) - .where.not(transactions: { kind: Transaction::TRANSFER_KINDS }) .where(excluded: false) .sum(:amount) (-net.to_d).clamp(0, Float::INFINITY).to_f diff --git a/config/locales/views/goals/en.yml b/config/locales/views/goals/en.yml index a67facb3e..99b86223d 100644 --- a/config/locales/views/goals/en.yml +++ b/config/locales/views/goals/en.yml @@ -98,6 +98,10 @@ en: pledge_just_transferred: I just transferred… pledge_just_saved: I just saved… funding_accounts_heading: Funding accounts + funding_accounts: + empty: + heading: No funding accounts linked yet + body: Edit the goal to link the depository accounts you save into. notes: Notes funding_last_30d: last 30d pending_pledge: