From 9beaf0a0c284ef8de14001188bea4bb6d6de929f Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Thu, 21 Mar 2024 13:32:45 +1100
Subject: [PATCH] Use textContent FTW

Oh look, the test works better now too.
---
 .../controllers/popout_controller.js          |  5 ++---
 .../stimulus/popout_controller_test.js        | 19 ++++++++++++++-----
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/app/webpacker/controllers/popout_controller.js b/app/webpacker/controllers/popout_controller.js
index c896bc7e5d..30a2061187 100644
--- a/app/webpacker/controllers/popout_controller.js
+++ b/app/webpacker/controllers/popout_controller.js
@@ -61,9 +61,8 @@ export default class PopoutController extends Controller {
       }
 
       // Update button to represent any changes
-      console.log(this.updateDisplayValue);
       if (this.updateDisplayValue) {
-        this.buttonTarget.innerText = this.#displayValue();
+        this.buttonTarget.textContent = this.#displayValue();
         this.buttonTarget.innerHTML ||= "&nbsp;"; // (with default space to help with styling)
       }
       this.buttonTarget.classList.toggle("changed", this.#isChanged());
@@ -93,7 +92,7 @@ export default class PopoutController extends Controller {
     let values = this.#enabledDisplayElements().map((element) => {
       if (element.type == "checkbox") {
         if (element.checked && element.labels[0]) {
-          return element.labels[0].innerText;
+          return element.labels[0].textContent.trim();
         }
       } else {
         return element.value;
diff --git a/spec/javascripts/stimulus/popout_controller_test.js b/spec/javascripts/stimulus/popout_controller_test.js
index 28f4e49a2c..ddb9f5bb8f 100644
--- a/spec/javascripts/stimulus/popout_controller_test.js
+++ b/spec/javascripts/stimulus/popout_controller_test.js
@@ -63,14 +63,14 @@ describe("PopoutController", () => {
       input4.click();
 
       expectToBeClosed(dialog);
-      expect(button.innerText).toBe("value1");
+      expect(button.textContent).toBe("value1");
     });
 
     it("closes the dialog when focusing another field (eg with tab)", () => {
       input4.focus();
 
       expectToBeClosed(dialog);
-      expect(button.innerText).toBe("value1");
+      expect(button.textContent).toBe("value1");
     });
 
     it("doesn't close the dialog when focusing internal field", () => {
@@ -83,7 +83,8 @@ 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.
+      // and includes the checkbox label
+      expect(button.textContent).toBe("value1,label2");
     });
 
     it("doesn't close the dialog when checkbox is unchecked", () => {
@@ -102,6 +103,14 @@ describe("PopoutController", () => {
       expectToBeShown(dialog);
       // Browser will show a validation message
     });
+
+    it("only shows enabled fields in display summary", () => {
+      input1.disabled = true;
+      input2.click(); // checkbox selected
+
+      expectToBeClosed(dialog);
+      expect(button.textContent).toBe("label2");
+    });
   });
 
   describe("disable update-display", () => {
@@ -110,12 +119,12 @@ describe("PopoutController", () => {
     })
 
     it("doesn't update display value", () => {
-      expect(button.innerText).toBe("On demand");
+      expect(button.textContent).toBe("On demand");
       button.dispatchEvent(new Event("click")); // Dialog is open
       input4.click(); //close dialog
 
       expectToBeClosed(dialog);
-      expect(button.innerText).toBe("On demand");
+      expect(button.textContent).toBe("On demand");
     });
   });