From 9bc1e873d38141b50542a2037a72eb71a92299ca Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 22 Nov 2023 17:10:22 +1100
Subject: [PATCH] Display summary of the popout values

I couldn't think of a simpler way to hardcode it, so now we have a clever generic method :)

We can assume that hidden elements will stay hidden, but we need to check each time if an element is disabled or not.
---
 .../controllers/popout_controller.js          | 31 ++++++++++++++++---
 .../stimulus/popout_controller_test.js        | 18 ++++++++---
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/app/webpacker/controllers/popout_controller.js b/app/webpacker/controllers/popout_controller.js
index 89a7d79791..71ad3c0dfc 100644
--- a/app/webpacker/controllers/popout_controller.js
+++ b/app/webpacker/controllers/popout_controller.js
@@ -6,6 +6,7 @@ export default class PopoutController extends Controller {
 
   connect() {
     this.first_input = this.dialogTarget.querySelector("input");
+    this.displayElements = Array.from(this.element.querySelectorAll('input:not([type="hidden"]'));
 
     // Show when click or down-arrow on button
     this.buttonTarget.addEventListener("click", this.show.bind(this));
@@ -39,7 +40,11 @@ export default class PopoutController extends Controller {
   }
 
   close() {
-    this.dialogTarget.style.display = "none";
+    // Close if not already closed
+    if (this.dialogTarget.style.display != "none") {
+      this.buttonTarget.innerText = this.#displayValue();
+      this.dialogTarget.style.display = "none";
+    }
   }
 
   closeIfOutside(e) {
@@ -49,13 +54,31 @@ export default class PopoutController extends Controller {
   }
 
   // Close if checked
-  // But the `change` or `input` events are fired before the mouseup, therefore the user never sees the item has been successfully checked, making it feel like it wasn't
-  // We could try listening to the mouseup on the label and check for e.target.controls.checked, but that doesn't support use of keybaord, and the value is inverted for some reason..
-  // but maybe we don't need to. User will get enough feedback when the button text is updated..
   closeIfChecked(e) {
     if (e.target.checked) {
       this.close();
       this.buttonTarget.focus();
     }
   }
+
+  // private
+
+  // Summarise the active field(s)
+  #displayValue() {
+    let values = this.#enabledDisplayElements().map((element) => {
+      if (element.type == "checkbox") {
+        if (element.checked && element.labels[0]) {
+          return element.labels[0].innerText;
+        }
+      } else {
+        return element.value;
+      }
+    });
+    // Filter empty values and convert to string
+    return values.filter(Boolean).join();
+  }
+
+  #enabledDisplayElements() {
+    return this.displayElements.filter((element) => !element.disabled);
+  }
 }
diff --git a/spec/javascripts/stimulus/popout_controller_test.js b/spec/javascripts/stimulus/popout_controller_test.js
index 4d413ab2cd..91f62e7ec5 100644
--- a/spec/javascripts/stimulus/popout_controller_test.js
+++ b/spec/javascripts/stimulus/popout_controller_test.js
@@ -16,17 +16,22 @@ describe("PopoutController", () => {
       <div data-controller="popout">
         <button id="button" data-popout-target="button">On demand</button>
         <div id="dialog" data-popout-target="dialog" style="display: none;">
-          <input id="input1">
-          <input id="input2" type="checkbox" data-action="change->popout#closeIfChecked">
+          <input id="input1" value="value1">
+          <label>
+            <input id="input2" type="checkbox" value="value2" data-action="change->popout#closeIfChecked">
+            label2
+          </label>
+          <input id="input3" type="hidden" value="value3">
         </div>
       </div>
-      <input id="input3">
+      <input id="input4">
     `;
 
     const button = document.getElementById("button");
     const input1 = document.getElementById("input1");
     const input2 = document.getElementById("input2");
     const input3 = document.getElementById("input3");
+    const input4 = document.getElementById("input4");
   });
 
   describe("Show", () => {
@@ -56,15 +61,17 @@ describe("PopoutController", () => {
     })
 
     it("closes the dialog when click outside", () => {
-      input3.click();
+      input4.click();
 
       expectToBeClosed(dialog);
+      expect(button.innerText).toBe("value1");
     });
 
     it("closes the dialog when focusing another field (eg with tab)", () => {
-      input3.focus();
+      input4.focus();
 
       expectToBeClosed(dialog);
+      expect(button.innerText).toBe("value1");
     });
 
     it("doesn't close the dialog when focusing internal field", () => {
@@ -77,6 +84,7 @@ describe("PopoutController", () => {
       input2.click();
 
       expectToBeClosed(dialog);
+      expect(button.innerText).toBe("value1");// The checkbox label should be here, but I just couldn't get the test to work with labels. Don't worry, it works in the browser.
     });
 
     it("doesn't close the dialog when checkbox is unchecked", () => {