fix(goals): UI polish — submit validation, container-responsive cards, picker & filter fixes (#2160)

* fix(ds): disabled buttons use not-allowed cursor

Tailwind v4 preflight sets cursor:pointer on every <button>, including
disabled ones. Add disabled:cursor-not-allowed to the DS button base so
disabled buttons read as non-interactive.

* fix(goals): disable new-goal submit until required fields valid

The submit button was always enabled, and the funding-accounts checkbox
group has no native 'required', so a goal with no linked account could be
submitted and only fail server-side (422). Disable the submit until name,
a positive target amount, and >=1 account are present, and add a
submit-time guard that surfaces inline errors and focuses the first
offending field as a backstop.

* fix(goals): keep color/icon picker from shifting the form

DS::Disclosure wraps its body in an mt-2 div that sits in normal flow, so
opening the absolutely-positioned picker popover nudged the form down ~8px.
Use a raw <details> for the picker so the popover overlays without
reflowing the content beneath it.

* fix(goals): container-responsive cards, meta-line status, scrollable tabs

- Card grids + KPI strip switch from viewport breakpoints to container
  queries (@container), so columns track the actual content width when the
  account/AI sidebars are open instead of crushing three columns into a
  narrow center.
- Move the status pill from the title row to the meta line; the title now
  gets the full header width (no more 'Hou...' truncation at 3-up). Drop
  the secondary line when the pill already states it (completed / open).
- Card internals harden for narrow widths (ring shrink-0, footer wrap,
  shorter 'N days left' secondary).
- Filter tabs scroll horizontally instead of wrapping 'On track' mid-label
  or forcing the row wider than the viewport.

* fix(goals): only require a funding account on the create form

GoalsController#edit renders account checkboxes from visible accounts only,
so a goal backed solely by a now-hidden (disabled / pending_deletion)
account renders none checked. The submit-validation then wedged the edit
form — a name/notes change couldn't be saved even though #update preserves
existing links when account_ids is omitted. Gate the account requirement on
a require-account Stimulus value that is true only for the create form.

* fix(goals): give the color/icon picker trigger an accessible name

The hand-rolled <summary> is icon-only (pen), so screen readers announced an
unlabeled control. Add a localized aria-label.

* fix(goals): render form validation errors inline on server re-render

The client-side controller already surfaces name / amount / account errors
inline and disables submit, but a server 422 (JS disabled, or a race that
lets an invalid submit through) fell back to the top run-on banner
("X must be filled and Y and Z."). Make the existing inline error hints
server-aware so a failed submit shows the same per-field red text the
client path does, and drop the base-error banner — base only ever carries
the account requirement, which now renders beside the funding-accounts
list.

* fix(goals): blur projection chart under privacy mode

The projection chart's SVG axis labels and annotations (target, "$X short",
$200K/$400K ticks) rendered in cleartext while privacy mode blurred every
other number on the page. Tag the chart container `privacy-sensitive`, the
same wrapper-level treatment the net-worth and cashflow-sankey charts
already use, so it blurs with the rest.

* fix(goals): stop target-date input stretching when amount error shows

The target-amount / target-date row is a two-column grid. The amount
column carries its inline error <p> underneath, so when that error
appears the column grows and the default `items-stretch` makes the
sibling date input stretch to match — the date box visibly grew taller
than the amount box. Anchor the row with `items-start` so each input
keeps its natural height and the error just extends below its own column.

* fix(goals): make invalid submit announce errors instead of dead-ending

Review follow-ups:
- Swap the submit gate from the disabled attribute to aria-disabled: a
  truly disabled default submit also blocks Enter-key implicit
  submission, so an invalid form was a dead button with every inline
  error still hidden. The button now stays clickable and lets
  validateOnSubmit surface the errors; DS buttonish styles the
  aria-disabled state (cursor-not-allowed + opacity-50). Side effect:
  loading_button_controller submits also dim while busy, which reads as
  an upgrade.
- Focus the first funding-account checkbox when accounts are the only
  missing field (focus previously went nowhere).
- Drop the now-orphaned goals.goal_card.days_left_by locale key.
This commit is contained in:
Guillem Arias Fauste
2026-06-06 16:24:17 +02:00
committed by GitHub
parent 172301f875
commit 5d0eb7f445
9 changed files with 145 additions and 53 deletions

View File

@@ -77,7 +77,13 @@ class DS::Buttonish < DesignSystemComponent
def container_classes(override_classes = nil)
class_names(
"font-medium whitespace-nowrap",
# Tailwind v4 preflight sets `cursor: pointer` on all <button>s, which
# also applies while disabled. Override so disabled buttons read as
# non-interactive. The aria-disabled twins cover buttons that gate via
# `aria-disabled` to stay clickable/focusable (e.g. submit buttons whose
# click handler surfaces validation errors — a truly disabled default
# submit would also swallow Enter-key implicit submission).
"font-medium whitespace-nowrap disabled:cursor-not-allowed aria-disabled:cursor-not-allowed aria-disabled:opacity-50",
merged_base_classes,
full_width ? "w-full justify-center" : nil,
container_size_classes,

View File

@@ -5,20 +5,27 @@
<div class="flex items-start gap-3">
<%= render Goals::AvatarComponent.new(goal: goal, size: "lg") %>
<div class="min-w-0 flex-1">
<div class="flex items-center gap-2 mb-0.5">
<p class="text-base font-medium text-primary truncate">
<a href="<%= goal_path(goal) %>"
aria-label="<%= aria_label %>"
class="before:absolute before:inset-0 before:rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-alpha-black-100">
<%= goal.name %>
</a>
</p>
<%# Name owns its row at full width — the status pill moved to the meta
line below so it stops eating the title space on narrow cards. %>
<p class="text-base font-medium text-primary truncate">
<a href="<%= goal_path(goal) %>"
aria-label="<%= aria_label %>"
class="before:absolute before:inset-0 before:rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-alpha-black-100">
<%= goal.name %>
</a>
</p>
<div class="flex items-center gap-2 mt-0.5">
<%= render Goals::StatusPillComponent.new(goal: goal) %>
<% if secondary_line.present? %>
<p class="min-w-0 text-xs text-subdued truncate"><%= secondary_line %></p>
<% end %>
</div>
<p class="text-xs text-subdued truncate"><%= secondary_line %></p>
</div>
<%= render DS::ProgressRing.new(percent: progress_percent, tone: ring_tone) %>
<%# shrink-0 keeps the ring at full size; the name block shrinks first. %>
<div class="shrink-0">
<%= render DS::ProgressRing.new(percent: progress_percent, tone: ring_tone) %>
</div>
</div>
<div class="mt-5">
@@ -31,10 +38,13 @@
<% end %>
</div>
<div class="mt-4 flex items-center justify-between">
<div class="flex items-center gap-2">
<%# flex-wrap + gap so the account label and the footer line wrap onto
separate lines under a gap when cramped, instead of butting together
(the "1 accountcatch up" run-on at narrow card widths). %>
<div class="mt-4 flex flex-wrap items-center justify-between gap-x-3 gap-y-1">
<div class="flex items-center gap-2 min-w-0">
<%= render Goals::AccountStackComponent.new(accounts: linked_accounts, color_map: goal.account_color_map) %>
<span class="text-xs text-subdued"><%= linked_accounts_count_label %></span>
<span class="text-xs text-subdued whitespace-nowrap"><%= linked_accounts_count_label %></span>
</div>
<span class="text-xs text-subdued tabular-nums <%= "privacy-sensitive" if footer_has_money? %>">
<%= footer_line %><% if has_pending_pledge? %> · <%= t("goals.goal_card.pending_count", count: pending_pledges_count) %><% end %>

View File

@@ -51,17 +51,18 @@ class Goals::CardComponent < ApplicationComponent
end
def secondary_line
if goal.completed?
I18n.t("goals.goal_card.completed")
elsif goal.target_date.nil?
I18n.t("goals.goal_card.no_target_date")
# nil when the status pill already carries it — the pill now sits on this
# same meta line, so "Completed Completed" / "Open Open" would read twice.
return nil if goal.completed? || goal.target_date.nil?
days = (goal.target_date - Date.current).to_i
if days >= 0
# Count only ("211 days left"); the full target date lives on the show
# page. Appending "· by <long date>" here overflowed the card's single
# line and truncated to a useless "211 d…".
I18n.t("goals.goal_card.days_left", count: days)
else
days = (goal.target_date - Date.current).to_i
if days >= 0
I18n.t("goals.goal_card.days_left_by", count: days, date: I18n.l(goal.target_date, format: :long))
else
I18n.t("goals.goal_card.past_due")
end
I18n.t("goals.goal_card.past_due")
end
end

View File

@@ -18,6 +18,7 @@ export default class extends Controller {
"accountsError",
"linkedAccountCheckbox",
"suggested",
"submitButton",
];
static INVALID_INPUT_CLASSES = ["ring-2", "ring-destructive", "border-destructive"];
@@ -26,6 +27,11 @@ export default class extends Controller {
currency: { type: String, default: "USD" },
suggestedWithDate: { type: String, default: "Save {monthly}/mo across {accounts} to hit it on time." },
suggestedNoDate: { type: String, default: "Set a target date to project a finish line." },
// Only the create form must pick an account. On edit the checkboxes are
// populated from visible accounts only, so a goal backed by a now-hidden
// account renders none — and the controller preserves existing links when
// account_ids is omitted. Requiring a checkbox there would wedge the form.
requireAccount: { type: Boolean, default: true },
};
connect() {
@@ -35,12 +41,16 @@ export default class extends Controller {
this._defaultAvatarHTML = this.avatarPreviewTarget.innerHTML;
}
this.updateSuggested();
// Edit form arrives pre-filled (valid) so this clears immediately; new
// form arrives empty so the submit starts visually disabled.
this.refreshSubmitState();
}
nameChanged() {
if (this.hasNameInputTarget) {
this.clearFieldError(this.nameInputTarget, this.hasNameErrorTarget ? this.nameErrorTarget : null);
}
this.refreshSubmitState();
if (!this.hasAvatarPreviewTarget || !this.hasNameInputTarget) return;
// If the user has explicitly picked an icon, leave it alone. Name
@@ -62,6 +72,7 @@ export default class extends Controller {
if (this.hasAmountInputTarget) {
this.clearFieldError(this.amountInputTarget, this.hasAmountErrorTarget ? this.amountErrorTarget : null);
}
this.refreshSubmitState();
}
linkedAccountChanged() {
@@ -69,6 +80,64 @@ export default class extends Controller {
if (this.linkedAccountCheckboxTargets.some((cb) => cb.checked) && this.hasAccountsErrorTarget) {
this.accountsErrorTarget.classList.add("hidden");
}
this.refreshSubmitState();
}
// Required to create a goal: a name, a positive target amount, and at least
// one funding account. Mirrors the server-side Goal validations (name
// presence, target_amount > 0, must_have_at_least_one_linked_account) so the
// button only enables when a submit would actually succeed.
isValid() {
const name = this.hasNameInputTarget ? this.nameInputTarget.value.trim() : "";
const amount = this.hasAmountInputTarget ? Number.parseFloat(this.amountInputTarget.value) : Number.NaN;
const accountOk = !this.requireAccountValue || this.linkedAccountCheckboxTargets.some((cb) => cb.checked);
return name.length > 0 && Number.isFinite(amount) && amount > 0 && accountOk;
}
// `aria-disabled` instead of the `disabled` attribute: a truly disabled
// default submit also blocks Enter-key implicit submission, so an invalid
// form would be a dead button with every inline error still hidden. With
// aria-disabled the button keeps its not-allowed affordance (styled via the
// DS `aria-disabled:` variants) while clicks and Enter still reach
// validateOnSubmit, which surfaces the errors and moves focus.
refreshSubmitState() {
if (this.hasSubmitButtonTarget) {
this.submitButtonTarget.setAttribute("aria-disabled", String(!this.isValid()));
}
}
// The real gate for the submit: covers the funding-accounts group (a
// checkbox group can't carry native `required`) and everything the
// aria-disabled affordance merely hints at. Surfaces the inline errors and
// focuses the first offending field instead of a silent no-op.
validateOnSubmit(event) {
if (this.isValid()) return;
event.preventDefault();
const nameEmpty = !(this.hasNameInputTarget && this.nameInputTarget.value.trim().length > 0);
const amount = this.hasAmountInputTarget ? Number.parseFloat(this.amountInputTarget.value) : Number.NaN;
const amountInvalid = !(Number.isFinite(amount) && amount > 0);
const noAccount = this.requireAccountValue && !this.linkedAccountCheckboxTargets.some((cb) => cb.checked);
if (nameEmpty) {
this.showFieldError(this.nameInputTarget, this.hasNameErrorTarget ? this.nameErrorTarget : null);
}
if (amountInvalid) {
this.showFieldError(this.amountInputTarget, this.hasAmountErrorTarget ? this.amountErrorTarget : null);
}
if (noAccount && this.hasAccountsErrorTarget) {
this.accountsErrorTarget.classList.remove("hidden");
}
const firstInvalid = nameEmpty
? this.nameInputTarget
: amountInvalid
? this.amountInputTarget
: noAccount
? this.linkedAccountCheckboxTargets[0]
: null;
firstInvalid?.focus();
}
// Hook for any input that influences the suggested-pace hint

View File

@@ -14,15 +14,16 @@
<% end %>
</span>
<%= render DS::Disclosure.new(
summary_class: "cursor-pointer absolute -bottom-1 -right-1 flex justify-center items-center bg-surface-inset hover:bg-surface-inset-hover border-2 w-6 h-6 border-subdued rounded-full text-secondary",
data: {
color_icon_picker_target: "details",
action: "mousedown->color-icon-picker#handleOutsideClick"
}) do |d| %>
<% d.with_summary_content do %>
<%# Raw <details> instead of DS::Disclosure: the disclosure wraps its body
in an `mt-2` div that lives in normal flow, which pushed the form down
~8px every time this absolutely-positioned popover opened. %>
<details class="group"
data-color-icon-picker-target="details"
data-action="mousedown->color-icon-picker#handleOutsideClick">
<summary aria-label="<%= t("goals.color_picker.trigger_label") %>"
class="list-none cursor-pointer absolute -bottom-1 -right-1 flex justify-center items-center bg-surface-inset hover:bg-surface-inset-hover border-2 w-6 h-6 border-subdued rounded-full text-secondary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-alpha-black-300">
<%= icon("pen", size: "xs") %>
<% end %>
</summary>
<div class="absolute top-full left-1/2 -translate-x-1/2 mt-2 z-50 bg-container p-3 border border-alpha-black-25 rounded-2xl shadow-xs w-80 max-w-[calc(100vw-2rem)] max-h-[60vh] overflow-y-auto"
data-color-icon-picker-target="popup">
@@ -68,6 +69,6 @@
</div>
</div>
</div>
<% end %>
</details>
</div>
</div>

View File

@@ -2,16 +2,14 @@
<div data-controller="goal-form"
data-goal-form-currency-value="<%= Current.family.primary_currency_code %>"
data-goal-form-require-account-value="<%= !goal.persisted? %>"
data-goal-form-suggested-with-date-value="<%= t("goals.form.suggested_with_date") %>"
data-goal-form-suggested-no-date-value="<%= t("goals.form.suggested_no_date") %>">
<% if goal.errors[:base].any? %>
<%= render "shared/form_errors", model: goal %>
<% end %>
<%= styled_form_with model: goal,
url: goal.persisted? ? goal_path(goal) : goals_path,
method: goal.persisted? ? :patch : :post,
class: "space-y-5" do |f| %>
class: "space-y-5",
data: { action: "submit->goal-form#validateOnSubmit" } do |f| %>
<div class="flex justify-center">
<%= render "color_picker", form: f, colors: Goal::COLORS, icons: Goal::ICONS %>
</div>
@@ -23,17 +21,17 @@
required: true,
label: t("goals.form.fields.name"),
data: { goal_form_target: "nameInput", action: "input->goal-form#nameChanged" } %>
<p class="hidden mt-1.5 text-xs text-destructive" data-goal-form-target="nameError"><%= t("goals.form.errors.name_required") %></p>
<p class="<%= "hidden" unless goal.errors[:name].any? %> mt-1.5 text-xs text-destructive" data-goal-form-target="nameError"><%= t("goals.form.errors.name_required") %></p>
</div>
<div class="grid grid-cols-2 gap-3">
<div class="grid grid-cols-2 gap-3 items-start">
<div>
<%= f.money_field :target_amount,
label: t("goals.form.fields.target_amount"),
hide_currency: true,
required: true,
amount_data: { goal_form_target: "amountInput", action: "input->goal-form#suggestedChanged" } %>
<p class="hidden mt-1.5 text-xs text-destructive" data-goal-form-target="amountError"><%= t("goals.form.errors.amount_required") %></p>
<p class="<%= "hidden" unless goal.errors[:target_amount].any? %> mt-1.5 text-xs text-destructive" data-goal-form-target="amountError"><%= t("goals.form.errors.amount_required") %></p>
</div>
<%= f.date_field :target_date,
label: t("goals.form.fields.target_date"),
@@ -74,7 +72,7 @@
</div>
<% end %>
</div>
<p class="hidden mt-1.5 text-xs text-destructive" data-goal-form-target="accountsError"><%= t("goals.form.errors.accounts_required") %></p>
<p class="<%= "hidden" unless goal.errors[:base].any? %> mt-1.5 text-xs text-destructive" data-goal-form-target="accountsError"><%= t("goals.form.errors.accounts_required") %></p>
</div>
<%= f.text_area :notes,
@@ -83,7 +81,8 @@
placeholder: t("goals.form.fields.notes_placeholder") %>
<div class="flex justify-end pt-2">
<%= f.submit goal.persisted? ? t("goals.form.save") : t("goals.form.create") %>
<%= f.submit goal.persisted? ? t("goals.form.save") : t("goals.form.create"),
data: { goal_form_target: "submitButton" } %>
</div>
<% end %>
</div>

View File

@@ -1,4 +1,8 @@
<div class="space-y-8 pb-6 lg:pb-12">
<%# @container so the KPI strip + card grids respond to the actual content
width (which shrinks when the account / AI sidebars are open) rather than
the viewport. Viewport breakpoints kept 3 columns in a narrow center
column and crushed the cards. %>
<div class="@container space-y-8 pb-6 lg:pb-12">
<header>
<div class="flex items-center gap-2">
<h1 class="text-2xl font-semibold text-primary"><%= t(".title") %></h1>
@@ -11,7 +15,7 @@
<%= render "empty_state", linkable_account_count: @linkable_account_count %>
<% else %>
<%# KPI strip. Contributed last 30d (with prior-30d comparison) / Needs / On track %>
<section class="grid grid-cols-1 md:grid-cols-3 gap-3">
<section class="grid grid-cols-1 @3xl:grid-cols-3 gap-3">
<div class="bg-container rounded-xl shadow-border-xs px-5 py-5">
<p class="text-[11px] font-medium uppercase tracking-wide text-secondary"><%= t(".kpi.contributed_label") %></p>
<p class="text-3xl font-medium text-primary tabular-nums mt-2 privacy-sensitive">
@@ -117,7 +121,11 @@
action: "input->goals-filter#filter"
}
) %>
<div class="inline-flex items-center gap-1 p-1 bg-surface-inset rounded-xl">
<%# overflow-x-auto + nowrap chips: the segmented control scrolls
horizontally when the column is too narrow for all six chips,
rather than wrapping "On track" mid-label or forcing the row
wider than the viewport. %>
<div class="inline-flex items-center gap-1 p-1 bg-surface-inset rounded-xl overflow-x-auto max-w-full">
<% %w[all on_track behind no_target_date paused completed].each do |status| %>
<% active = status == "all" %>
<button type="button"
@@ -125,7 +133,7 @@
data-action="click->goals-filter#selectChip"
data-status="<%= status %>"
aria-pressed="<%= active %>"
class="px-2.5 py-1 text-xs font-medium rounded-lg transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-alpha-black-100 <%= active ? "bg-container shadow-border-xs text-primary" : "text-secondary" %>">
class="shrink-0 whitespace-nowrap px-2.5 py-1 text-xs font-medium rounded-lg transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-alpha-black-100 <%= active ? "bg-container shadow-border-xs text-primary" : "text-secondary" %>">
<%= t(".chips.#{status}") %>
</button>
<% end %>
@@ -139,7 +147,7 @@
<span class="text-subdued">·</span>
<span class="tabular-nums" data-goals-filter-target="count"><%= @grid_goals.size %></span>
</div>
<div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-3.5" data-goals-filter-target="grid">
<div class="grid grid-cols-1 @2xl:grid-cols-2 @5xl:grid-cols-3 gap-3.5" data-goals-filter-target="grid">
<% @grid_goals.each do |goal| %>
<%= render Goals::CardComponent.new(goal: goal) %>
<% end %>
@@ -179,7 +187,7 @@
<span class="tabular-nums"><%= @archived_goals.size %></span>
</span>
<% end %>
<div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-3.5">
<div class="grid grid-cols-1 @2xl:grid-cols-2 @5xl:grid-cols-3 gap-3.5">
<% @archived_goals.each do |goal| %>
<%= render Goals::CardComponent.new(goal: goal, filterable: false) %>
<% end %>

View File

@@ -224,7 +224,7 @@
<% end %>
</div>
</div>
<div class="flex-1 min-h-[200px]"
<div class="flex-1 min-h-[200px] privacy-sensitive"
data-controller="goal-projection-chart"
data-goal-projection-chart-data-value="<%= @goal.projection_payload.to_json %>"
data-goal-projection-chart-aria-label-value="<%= t("goals.show.projection.aria_label", name: @goal.name) %>"

View File

@@ -2,6 +2,7 @@
en:
goals:
color_picker:
trigger_label: Choose color and icon
color_heading: Color
icon_heading: Icon
poor_contrast: Poor contrast, choose darker color or
@@ -222,9 +223,6 @@ en:
days_left:
one: 1 day left
other: "%{count} days left"
days_left_by:
one: 1 day left · by %{date}
other: "%{count} days left · by %{date}"
pace_with_target: "%{avg}/mo · target %{target}/mo"
pace_no_target: "%{avg}/mo avg"
footer_paused: Paused