From c427c874211fb4019bf4eaaad10f339b1803991a Mon Sep 17 00:00:00 2001
From: Guillem Arias <accounts@gariasf.com>
Date: Wed, 27 May 2026 10:14:27 +0200
Subject: [PATCH] fix(ds): DS::Disclosure summary_class override; migrate color
 picker
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Resolves sure-design DS drift patrol findings (raw <details> on
goals/_color_picker and categories/_form). The color-icon-picker's
<summary> is a 24/28px pencil button absolutely positioned next to
the avatar — none of DS::Disclosure's existing variants
(default / card / card_inset / inline) match that trigger shape, so
the bot's suggested swap would regress the visual.

- DS::Disclosure: add optional `summary_class:` kwarg. When set, the
  caller's class string replaces the variant's hard-coded summary
  chrome; otherwise the existing variant logic is preserved (verified
  against the 8 existing callsites — none pass summary_class, all
  fall through to current behavior).
- goals/_color_picker + categories/_form: swap raw <details> for
  DS::Disclosure with summary_class carrying the pencil-button
  positioning. Stimulus data attributes (`color-icon-picker-target`
  and the outside-click handler) forwarded via **opts to tag.details
  so the controller still finds its target.

The DS::Disclosure-rendered popover content now sits inside the
component's `<div class="mt-2">` wrapper, but the popups themselves
are `position: absolute` / `position: fixed`, so the wrapper is
out-of-flow neutral.
---
 app/components/DS/disclosure.rb        |  7 +++++--
 app/views/categories/_form.html.erb    | 13 +++++++++----
 app/views/goals/_color_picker.html.erb | 13 +++++++++----
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/app/components/DS/disclosure.rb b/app/components/DS/disclosure.rb
index 5f993d6c1..1acb7dfe0 100644
--- a/app/components/DS/disclosure.rb
+++ b/app/components/DS/disclosure.rb
@@ -3,7 +3,7 @@ class DS::Disclosure < DesignSystemComponent
 
   VARIANTS = %i[default card card_inset inline].freeze
 
-  attr_reader :title, :align, :open, :variant, :opts
+  attr_reader :title, :align, :open, :variant, :summary_class_override, :opts
 
   # `:default` — bg-surface summary, no chrome on the `<details>`. Use
   # for inline expanders that sit inside a parent card (the summary
@@ -28,11 +28,12 @@ class DS::Disclosure < DesignSystemComponent
   # In card / inline variants, callers should pass their own
   # `summary_content` slot; the built-in title rendering assumes the
   # `:default` shape.
-  def initialize(title: nil, align: "right", open: false, variant: :default, **opts)
+  def initialize(title: nil, align: "right", open: false, variant: :default, summary_class: nil, **opts)
     @title = title
     @align = align.to_sym
     @open = open
     @variant = variant&.to_sym
+    @summary_class_override = summary_class
     @opts = opts
 
     raise ArgumentError, "Invalid variant: #{@variant.inspect}. Must be one of #{VARIANTS.inspect}" unless VARIANTS.include?(@variant)
@@ -59,6 +60,8 @@ class DS::Disclosure < DesignSystemComponent
   end
 
   def summary_classes
+    return summary_class_override if summary_class_override.present?
+
     case variant
     when :card, :card_inset
       # Card variants: no bg on summary — the parent details *is* the
diff --git a/app/views/categories/_form.html.erb b/app/views/categories/_form.html.erb
index 4f2979c33..a46284903 100644
--- a/app/views/categories/_form.html.erb
+++ b/app/views/categories/_form.html.erb
@@ -6,10 +6,15 @@
       <div class="w-fit mx-auto relative">
         <%= render partial: "color_avatar", locals: { category: category } %>
 
-        <details data-color-icon-picker-target="details" data-action="mousedown->color-icon-picker#handleOutsideClick">
-          <summary class="cursor-pointer absolute -bottom-2 -right-2 flex justify-center items-center bg-surface-inset hover:bg-surface-inset-hover border-2 w-7 h-7 border-subdued rounded-full text-secondary">
+        <%= render DS::Disclosure.new(
+              summary_class: "cursor-pointer absolute -bottom-2 -right-2 flex justify-center items-center bg-surface-inset hover:bg-surface-inset-hover border-2 w-7 h-7 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 %>
             <%= icon("pen", size: "sm") %>
-          </summary>
+          <% end %>
 
           <div class="fixed right-0 sm:right-auto mx-2 sm:ml-8 sm:mr-0 mt-2 z-50 bg-container p-4 border border-alpha-black-25 rounded-2xl shadow-xs h-fit" data-color-icon-picker-target="popup">
             <div class="flex gap-2 flex-col mb-4" data-color-icon-picker-target="selection" style="<%= "display:none;" if @category.subcategory? %>">
@@ -54,7 +59,7 @@
               </div>
             </div>
           </div>
-        </details>
+        <% end %>
       </div>
 
       <% if category.errors.any? %>
diff --git a/app/views/goals/_color_picker.html.erb b/app/views/goals/_color_picker.html.erb
index 72eb7ff10..4100d6d1a 100644
--- a/app/views/goals/_color_picker.html.erb
+++ b/app/views/goals/_color_picker.html.erb
@@ -14,10 +14,15 @@
       <% end %>
     </span>
 
-    <details data-color-icon-picker-target="details" data-action="mousedown->color-icon-picker#handleOutsideClick">
-      <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">
+    <%= 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 %>
         <%= icon("pen", size: "xs") %>
-      </summary>
+      <% end %>
 
       <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">
@@ -63,6 +68,6 @@
           </div>
         </div>
       </div>
-    </details>
+    <% end %>
   </div>
 </div>