From 82f3d2e0fbfa8b0a05ef547237cb7c1b865da49a Mon Sep 17 00:00:00 2001
From: Guillem Arias <accounts@gariasf.com>
Date: Thu, 14 May 2026 22:35:02 +0200
Subject: [PATCH] fix(goals/show): move Edit into kebab, match house style
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

User clarification on the "too big edit icon" finding: the header
Edit button itself (not its icon) is what felt wrong. Investigation
showed the wider Sure pattern.

Every other "Edit" affordance in Sure lives inside a DS::Menu kebab:

  - app/views/categories/_category.html.erb
  - app/views/rules/_rule.html.erb
  - app/views/family_merchants/_family_merchant.html.erb
  - app/views/chats/_chat_nav.html.erb
  - app/views/accounts/show/_menu.html.erb
  - app/views/transactions/show.html.erb

Header rows reserve top-level buttons for primary actions (e.g.
"New transaction", "Record pledge"). The goal show page was the
outlier — Edit as an outline button next to Record pledge, which
left two competing CTAs in the header.

Move `menu.with_item(text: t(".edit"), icon: "pencil", ...)` to
the top of the kebab list. Header now has a single primary CTA
(Record pledge, demoted to outline when status is :behind) + the
kebab. Matches every other Sure resource page; eliminates the
"button-too-big" framing entirely without resorting to ad-hoc
icon-size overrides (the `[&>svg]:w-4 [&>svg]:h-4` arbitrary
selector hack would have been a one-off, nobody else uses it).

Avatar pen toggle on the new-goal color picker stays reverted to
its original w-6 h-6 + border-2 form, per the user.
---
 app/views/goals/_color_picker.html.erb |  4 ++--
 app/views/goals/show.html.erb          | 12 +++++-------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/app/views/goals/_color_picker.html.erb b/app/views/goals/_color_picker.html.erb
index 83727c501..5ccba3b92 100644
--- a/app/views/goals/_color_picker.html.erb
+++ b/app/views/goals/_color_picker.html.erb
@@ -15,8 +15,8 @@
     </span>
 
     <details data-color-icon-picker-target="details" data-action="mousedown->color-icon-picker#handleOutsideClick">
-      <summary class="cursor-pointer absolute -bottom-0.5 -right-0.5 flex justify-center items-center bg-surface-inset hover:bg-surface-inset-hover w-4 h-4 rounded-full text-subdued hover:text-secondary">
-        <%= icon("pencil", size: "xs") %>
+      <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">
+        <%= icon("pen", size: "xs") %>
       </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"
diff --git a/app/views/goals/show.html.erb b/app/views/goals/show.html.erb
index 6e19a1521..6ad61838a 100644
--- a/app/views/goals/show.html.erb
+++ b/app/views/goals/show.html.erb
@@ -36,13 +36,6 @@
       </div>
     </div>
     <div class="flex items-center gap-2 flex-wrap sm:shrink-0">
-      <%= render DS::Link.new(
-        text: t(".edit"),
-        variant: "outline",
-        href: edit_goal_path(@goal),
-        icon: "pencil",
-        frame: :modal
-      ) %>
       <% unless @goal.completed? || @goal.status == :reached %>
         <%# Demote to outline when the catch-up alert below owns the primary
             action. One primary per surface. %>
@@ -55,6 +48,11 @@
         ) %>
       <% end %>
       <%= render DS::Menu.new do |menu| %>
+        <%# Edit lives in the kebab, matching the rest of Sure (accounts,
+            categories, rules, family_merchants, chats, transactions all
+            put their Edit action inside the kebab dropdown). Keeps the
+            header to one primary action. %>
+        <% menu.with_item(variant: "link", text: t(".edit"), icon: "pencil", href: edit_goal_path(@goal), data: { turbo_frame: :modal }) %>
         <% if @goal.may_pause? %>
           <% menu.with_item(variant: "button", text: t(".pause"), icon: "pause", href: pause_goal_path(@goal), method: :patch) %>
         <% end %>