From e4d10a1f5f6d1a779f8b0a4ed9b9b73d70580be4 Mon Sep 17 00:00:00 2001
From: Ayke Halder <rr-it@users.noreply.github.com>
Date: Tue, 22 Feb 2022 18:40:47 +0100
Subject: [PATCH]  DEV: cleanup is-loading state of d-button component (#16012)

* DEV: remove duplicate code in button component template

* DEV: refactor is-loading state of d-button component

Before this change on initialisation `forceDisabled` is set `false` and then might change to `undefined` - depending on the use of the button component. This change ensures a boolean value for `forceDisabled`.

The added test works with and without the new change. The test is added as it represents the default use case for most buttons.
---
 .../discourse/app/components/d-button.js      |  2 +-
 .../app/templates/components/d-button.hbs     | 12 +++----
 .../integration/components/d-button-test.js   | 36 +++++++++++++++++++
 3 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/d-button.js b/app/assets/javascripts/discourse/app/components/d-button.js
index 33e70345b89..f8177d44497 100644
--- a/app/assets/javascripts/discourse/app/components/d-button.js
+++ b/app/assets/javascripts/discourse/app/components/d-button.js
@@ -25,7 +25,7 @@ export default Component.extend({
 
   isLoading: computed({
     set(key, value) {
-      this.set("forceDisabled", value);
+      this.set("forceDisabled", !!value);
       return value;
     },
   }),
diff --git a/app/assets/javascripts/discourse/app/templates/components/d-button.hbs b/app/assets/javascripts/discourse/app/templates/components/d-button.hbs
index 8d5b383d011..552394dd68f 100644
--- a/app/assets/javascripts/discourse/app/templates/components/d-button.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/d-button.hbs
@@ -1,12 +1,8 @@
-{{#if icon}}
-  {{#if isLoading}}
-    {{~d-icon "spinner" class="loading-icon"~}}
-  {{else}}
-    {{~d-icon icon~}}
-  {{/if}}
+{{#if isLoading}}
+  {{~d-icon "spinner" class="loading-icon"~}}
 {{else}}
-  {{#if isLoading}}
-    {{~d-icon "spinner" class="loading-icon"~}}
+  {{#if icon}}
+    {{~d-icon icon~}}
   {{/if}}
 {{/if}}
 
diff --git a/app/assets/javascripts/discourse/tests/integration/components/d-button-test.js b/app/assets/javascripts/discourse/tests/integration/components/d-button-test.js
index 9be51b35101..52e687148d4 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/d-button-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/d-button-test.js
@@ -99,6 +99,42 @@ discourseModule("Integration | Component | d-button", function (hooks) {
     },
   });
 
+  componentTest("button without isLoading attribute", {
+    template: hbs`{{d-button}}`,
+
+    test(assert) {
+      assert.notOk(
+        exists("button.is-loading"),
+        "it doesn't have class is-loading"
+      );
+      assert.notOk(
+        exists("button .loading-icon"),
+        "it doesn't have a spinner showing"
+      );
+      assert.notOk(exists("button[disabled]"), "it isn't disabled");
+    },
+  });
+
+  componentTest("isLoading button explicitly set to undefined state", {
+    template: hbs`{{d-button isLoading=isLoading}}`,
+
+    beforeEach() {
+      this.set("isLoading");
+    },
+
+    test(assert) {
+      assert.notOk(
+        exists("button.is-loading"),
+        "it doesn't have class is-loading"
+      );
+      assert.notOk(
+        exists("button .loading-icon"),
+        "it doesn't have a spinner showing"
+      );
+      assert.notOk(exists("button[disabled]"), "it isn't disabled");
+    },
+  });
+
   componentTest("disabled button", {
     template: hbs`{{d-button disabled=disabled}}`,