From 093831a6e51edb92b6e97e734e48cd6f182f05f0 Mon Sep 17 00:00:00 2001 From: Guillem Arias Date: Mon, 11 May 2026 16:58:17 +0200 Subject: [PATCH] fix(savings_goals): neutral ring percent, chart start vertical-line, contribution select wrapper, deterministic account colors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ring percentage no longer takes the warning yellow tint when behind — the colored ring stroke + status pill + catch-up alert already signal the state, doubling it on the percent number was noise. Reached stays green (celebratory), everything else uses text-primary (white/dark). Chart vertical line at the left edge was the (start_date, $0) point the controller prepended to the saved series. When start_date equals the first contribution date (now common after the earlier earliest- contribution fix), this drew a vertical jump from $0 to first contribution at x=start. Skip the prepend when there's no temporal gap so the line starts at the first real point. Add Contribution modal — wrap the source-account select in the styled form-field via f.select instead of label_tag + bare select_tag. Match the rest of Sure's form controls. Also pass hide_currency on the amount field so single-currency families don't see a redundant USD dropdown. Account avatar colors — replace Ruby String#hash (randomized per process by Ruby for DoS protection) with a deterministic MD5-based pick from Savings::GoalAvatarComponent::PALETTE. Same account name now resolves to the same color across processes and across components. Apply via a new Savings::GoalAvatarComponent.color_for helper used by both the form stepper account list and the goal-card AccountStackComponent (which was hardcoding blue-500 for every avatar in the stack, hence Chase + Ally looking identical on the wedding card). --- .../savings/account_stack_component.html.erb | 2 +- app/components/savings/goal_avatar_component.rb | 10 ++++++++++ .../savings/progress_ring_component.html.erb | 2 +- app/components/savings/progress_ring_component.rb | 7 +++---- .../savings_goal_projection_chart_controller.js | 15 +++++++++++---- app/views/savings_contributions/new.html.erb | 13 +++++-------- app/views/savings_goals/_form_stepper.html.erb | 2 +- 7 files changed, 32 insertions(+), 19 deletions(-) diff --git a/app/components/savings/account_stack_component.html.erb b/app/components/savings/account_stack_component.html.erb index 3a3ee6f87..349c316f3 100644 --- a/app/components/savings/account_stack_component.html.erb +++ b/app/components/savings/account_stack_component.html.erb @@ -1,7 +1,7 @@ <% shown.each_with_index do |account, i| %> 0 %>" + style="background-color: <%= Savings::GoalAvatarComponent.color_for(account.name) %>; <%= "margin-left: -6px;" if i > 0 %>" title="<%= account.name %>"> <%= initial_for(account) %> diff --git a/app/components/savings/goal_avatar_component.rb b/app/components/savings/goal_avatar_component.rb index 01d2af314..4842d4d27 100644 --- a/app/components/savings/goal_avatar_component.rb +++ b/app/components/savings/goal_avatar_component.rb @@ -6,6 +6,16 @@ class Savings::GoalAvatarComponent < ApplicationComponent "xl" => { box: "w-16 h-16", text: "text-2xl", radius: "rounded-2xl" } }.freeze + PALETTE = SavingsGoal::COLORS + + # Deterministic color pick from the palette so the same string maps to + # the same color across processes (Ruby's String#hash is randomized per + # boot for DoS protection — not stable enough for visual identity). + def self.color_for(name) + return PALETTE.first if name.blank? + PALETTE[Digest::MD5.hexdigest(name).to_i(16) % PALETTE.size] + end + def initialize(goal: nil, name: nil, color: nil, size: "md") @goal = goal @name = name || goal&.name diff --git a/app/components/savings/progress_ring_component.html.erb b/app/components/savings/progress_ring_component.html.erb index aa494b34a..3cd21c1de 100644 --- a/app/components/savings/progress_ring_component.html.erb +++ b/app/components/savings/progress_ring_component.html.erb @@ -7,7 +7,7 @@
<%= t("savings_goals.show.ring.saved") %> - <%= percent %>% + <%= percent %>% <%= amount_label %> of <%= target_label %>
diff --git a/app/components/savings/progress_ring_component.rb b/app/components/savings/progress_ring_component.rb index 7f6d288c8..aca6523ff 100644 --- a/app/components/savings/progress_ring_component.rb +++ b/app/components/savings/progress_ring_component.rb @@ -22,11 +22,10 @@ class Savings::ProgressRingComponent < ApplicationComponent goal.remaining_amount_money.format end - def percent_text_color + def percent_text_class case goal.status - when :reached then "var(--color-green-600)" - when :behind then "var(--color-yellow-600)" - else "var(--text-primary)" + when :reached then "text-success" + else "text-primary" end end end diff --git a/app/javascript/controllers/savings_goal_projection_chart_controller.js b/app/javascript/controllers/savings_goal_projection_chart_controller.js index 07b24042f..e2084bcbd 100644 --- a/app/javascript/controllers/savings_goal_projection_chart_controller.js +++ b/app/javascript/controllers/savings_goal_projection_chart_controller.js @@ -51,10 +51,17 @@ export default class extends Controller { const endDate = target || new Date(today.getTime() + 30 * 24 * 60 * 60 * 1000); - const savedSeries = [{ date: start, value: 0 }].concat( - (data.saved_series || []).map((p) => ({ date: new Date(p.date), value: p.value })), - ); - if (savedSeries[savedSeries.length - 1].date < today) { + const rawSavedSeries = (data.saved_series || []).map((p) => ({ date: new Date(p.date), value: p.value })); + const firstContribDate = rawSavedSeries[0]?.date; + const savedSeries = []; + // Only seed a (start, 0) point when start_date predates the first + // contribution. Otherwise the line draws a vertical jump up at the + // chart's left edge. + if (!firstContribDate || firstContribDate.getTime() > start.getTime()) { + savedSeries.push({ date: start, value: 0 }); + } + savedSeries.push(...rawSavedSeries); + if (savedSeries.length && savedSeries[savedSeries.length - 1].date < today) { savedSeries.push({ date: today, value: currentAmount }); } diff --git a/app/views/savings_contributions/new.html.erb b/app/views/savings_contributions/new.html.erb index 2b5a9b702..9ed4cff47 100644 --- a/app/views/savings_contributions/new.html.erb +++ b/app/views/savings_contributions/new.html.erb @@ -10,18 +10,15 @@ class: "space-y-3" do |f| %> <%= f.money_field :amount, label: t(".amount"), - required: true, + hide_currency: true, autofocus: true %> - <%= label_tag "savings_contribution[account_id]", t(".source_account"), class: "block text-sm text-secondary" %> - <%= select_tag "savings_contribution[account_id]", - options_from_collection_for_select(@savings_goal.linked_accounts, :id, :name), - class: "w-full", - include_blank: t(".select_account") %> + <%= f.select :account_id, + options_from_collection_for_select(@savings_goal.linked_accounts, :id, :name, @contribution.account_id), + { label: t(".source_account"), include_blank: t(".select_account") } %> <%= f.date_field :contributed_at, - label: t(".contributed_at"), - required: true %> + label: t(".contributed_at") %> <%= f.text_area :notes, label: t(".notes"), rows: 2 %> diff --git a/app/views/savings_goals/_form_stepper.html.erb b/app/views/savings_goals/_form_stepper.html.erb index 765d2bc48..86832e858 100644 --- a/app/views/savings_goals/_form_stepper.html.erb +++ b/app/views/savings_goals/_form_stepper.html.erb @@ -72,7 +72,7 @@ account_subtype: account.subtype || subtype, account_balance: account.balance } %> - <%= render Savings::GoalAvatarComponent.new(name: account.name, color: Category::COLORS[account.name.hash.abs % Category::COLORS.size], size: "md") %> + <%= render Savings::GoalAvatarComponent.new(name: account.name, color: Savings::GoalAvatarComponent.color_for(account.name), size: "md") %>

<%= account.name %>

<%= (account.subtype || subtype).titleize %>