From d51c88d3232dc87cc5fbbc5d272ce0fae23e2ed4 Mon Sep 17 00:00:00 2001
From: Guillem Arias <accounts@gariasf.com>
Date: Mon, 18 May 2026 21:03:55 +0200
Subject: [PATCH] fix(goals JS): listener cleanup on color/icon picker +
 debounce filter URL sync

- color_icon_picker: listeners were attached in initialize() with inline
  arrows, so they had no removable reference and no disconnect() cleanup.
  Every Turbo navigation that re-rendered the picker stacked another
  listener on the same node and left the Pickr instance alive. Move
  attachment to connect(), store bound references, and clean up in
  disconnect() (including destroyAndRemove on the Pickr).
- goals_filter: replaceState fired on every keystroke. Debounce the URL
  sync 200 ms so a typing burst collapses into a single update. The
  visible filtering stays real-time. Clear the timer in disconnect()
  so a pending sync doesn't fire on an unmounted controller.
---
 .../color_icon_picker_controller.js           | 37 ++++++++++++-------
 .../controllers/goals_filter_controller.js    | 15 +++++++-
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/app/javascript/controllers/color_icon_picker_controller.js b/app/javascript/controllers/color_icon_picker_controller.js
index 55138dd4a..a208ba969 100644
--- a/app/javascript/controllers/color_icon_picker_controller.js
+++ b/app/javascript/controllers/color_icon_picker_controller.js
@@ -22,31 +22,42 @@ export default class extends Controller {
     presetColors: Array,
   };
 
-  initialize() {
-    this.pickerBtnTarget.addEventListener("click", () => {
-      this.showPaletteSection();
-    });
-
-    this.colorInputTarget.addEventListener("input", (e) => {
-      this.picker.setColor(e.target.value);
-    });
-
-    this.detailsTarget.addEventListener("toggle", (e) => {
+  connect() {
+    // Bound references stored on the instance so disconnect() can remove
+    // them. Without this, every Turbo navigation that re-renders the
+    // picker stacks another listener on the same node.
+    this._onPickerBtnClick = () => this.showPaletteSection();
+    this._onColorInputInput = (e) => this.picker?.setColor(e.target.value);
+    this._onDetailsToggle = (e) => {
       if (!this.colorInputTarget.checkValidity()) {
         e.preventDefault();
         this.colorInputTarget.reportValidity();
         e.target.open = true;
       }
-      this.updatePopupPosition()
-    });
+      this.updatePopupPosition();
+    };
+
+    this.pickerBtnTarget.addEventListener("click", this._onPickerBtnClick);
+    this.colorInputTarget.addEventListener("input", this._onColorInputInput);
+    this.detailsTarget.addEventListener("toggle", this._onDetailsToggle);
+    document.addEventListener("mousedown", this.handleOutsideClick);
 
     this.selectedIcon = null;
 
     if (!this.presetColorsValue.includes(this.colorInputTarget.value)) {
       this.colorPickerRadioBtnTarget.checked = true;
     }
+  }
 
-    document.addEventListener("mousedown", this.handleOutsideClick);
+  disconnect() {
+    this.pickerBtnTarget.removeEventListener("click", this._onPickerBtnClick);
+    this.colorInputTarget.removeEventListener("input", this._onColorInputInput);
+    this.detailsTarget.removeEventListener("toggle", this._onDetailsToggle);
+    document.removeEventListener("mousedown", this.handleOutsideClick);
+    if (this.picker) {
+      this.picker.destroyAndRemove();
+      this.picker = null;
+    }
   }
 
   initPicker() {
diff --git a/app/javascript/controllers/goals_filter_controller.js b/app/javascript/controllers/goals_filter_controller.js
index b2479a440..5e68db6f8 100644
--- a/app/javascript/controllers/goals_filter_controller.js
+++ b/app/javascript/controllers/goals_filter_controller.js
@@ -32,6 +32,10 @@ export default class extends Controller {
     }
   }
 
+  disconnect() {
+    clearTimeout(this._urlSyncTimer);
+  }
+
   filter() {
     const query = this.hasInputTarget
       ? this.inputTarget.value.toLocaleLowerCase().trim()
@@ -60,7 +64,16 @@ export default class extends Controller {
     }
 
     this.updateEmptyState(visible, query, active);
-    this.#syncUrl();
+    this.#scheduleUrlSync();
+  }
+
+  // Debounced wrapper. Firing replaceState on every keystroke is wasteful
+  // and produced visible jank on slow CPUs; deferring 200 ms collapses a
+  // typing burst into a single URL update without losing back-button
+  // fidelity (replaceState doesn't create history entries anyway).
+  #scheduleUrlSync() {
+    clearTimeout(this._urlSyncTimer);
+    this._urlSyncTimer = setTimeout(() => this.#syncUrl(), 200);
   }
 
   #hydrateFromUrl() {