DEV: cleanup is-loading state of d-button component (discourse#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.
Arxnel · Feb 22, 2022 · e4d10a1 · e4d10a1
1 parent d4d3580
commit e4d10a1
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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}}`,