DEV: Convert DButton to a Glimmer Component (#17767)

This change is intended to be backwards-compatible with all the previous arguments to `DButton`.

A deprecation warning will be triggered when a string is passed to the `@action` argument. This kind of action bubbling has been deprecated in Ember for some time, and should be updated to use closure actions.

Co-authored-by: Dan Gebhardt <dan@cerebris.com>
Co-authored-by: David Taylor <david@taylorhq.com>

app/assets/javascripts/discourse/app/components/bulk-select-all.js

@@ -1,7 +0,0 @@
-import DButton from "discourse/components/d-button";
-
-export default DButton.extend({
-  click() {
-    $("input.bulk-select:not(checked)").click();
-  },
-});

app/assets/javascripts/discourse/app/components/composer-save-button.hbs

@@ -0,0 +1,9 @@
+<DButton
+  @translatedTitle={{this.translatedTitle}}
+  @label={{@label}}
+  @action={{@action}}
+  @icon={{@icon}}
+  @forwardEvent={{@forwardEvent}}
+  class="btn-primary create {{if @disabledSubmit "disabled"}}"
+  ...attributes
+/>

app/assets/javascripts/discourse/app/components/composer-save-button.js

@@ -1,10 +1,9 @@
-import Button from "discourse/components/d-button";
+import Component from "@glimmer/component";
 import I18n from "I18n";
 import { translateModKey } from "discourse/lib/utilities";
 
-export default Button.extend({
-  classNameBindings: [":btn-primary", ":create", "disableSubmit:disabled"],
-  translatedTitle: I18n.t("composer.title", {
-    modifier: translateModKey("Meta+"),
-  }),
-});
+export default class ComposerSaveButton extends Component {
+  get translatedTitle() {
+    return I18n.t("composer.title", { modifier: translateModKey("Meta+") });
+  }
+}

app/assets/javascripts/discourse/app/components/d-button.js

@@ -1,161 +1,137 @@
 import { inject as service } from "@ember/service";
+import { action } from "@ember/object";
 import { empty, equal, notEmpty } from "@ember/object/computed";
-import Component from "@ember/component";
+import GlimmerComponentWithDeprecatedParentView from "discourse/components/glimmer-component-with-deprecated-parent-view";
+import deprecated from "discourse-common/lib/deprecated";
 import DiscourseURL from "discourse/lib/url";
 import I18n from "I18n";
-import { computed } from "@ember/object";
-import discourseComputed from "discourse-common/utils/decorators";
 
-export default Component.extend({
-  tagName: "button",
-  // subclasses need this
-  layoutName: "components/d-button",
-  form: null,
-  type: "button",
-  title: null,
-  translatedTitle: null,
-  label: null,
-  translatedLabel: null,
-  ariaLabel: null,
-  ariaExpanded: null,
-  ariaControls: null,
-  translatedAriaLabel: null,
-  forwardEvent: false,
-  preventFocus: false,
-  onKeyDown: null,
-  router: service(),
+const ACTION_AS_STRING_DEPRECATION_ARGS = [
+  "DButton no longer supports @action as a string. Please refactor to use an closure action instead.",
+  { id: "discourse.d-button-action-string" },
+];
 
-  isLoading: computed({
-    set(key, value) {
-      this.set("forceDisabled", !!value);
-      return value;
-    },
-  }),
+export default class DButton extends GlimmerComponentWithDeprecatedParentView {
+  @service router;
 
-  classNameBindings: [
-    "isLoading:is-loading",
-    "btnLink::btn",
-    "btnLink",
-    "noText",
-    "btnType",
-  ],
-  attributeBindings: [
-    "form",
-    "isDisabled:disabled",
-    "computedTitle:title",
-    "computedAriaLabel:aria-label",
-    "computedAriaExpanded:aria-expanded",
-    "ariaControls:aria-controls",
-    "tabindex",
-    "type",
-  ],
+  @notEmpty("args.icon")
+  btnIcon;
 
-  isDisabled: computed("disabled", "forceDisabled", function () {
-    return this.forceDisabled || this.disabled;
-  }),
+  @equal("args.display", "link")
+  btnLink;
 
-  forceDisabled: false,
+  @empty("computedLabel")
+  noText;
 
-  btnIcon: notEmpty("icon"),
+  constructor() {
+    super(...arguments);
+    if (typeof this.args.action === "string") {
+      deprecated(...ACTION_AS_STRING_DEPRECATION_ARGS);
+    }
+  }
 
-  btnLink: equal("display", "link"),
+  get forceDisabled() {
+    return !!this.args.isLoading;
+  }
 
-  @discourseComputed("icon", "computedLabel")
-  btnType(icon, translatedLabel) {
-    if (icon) {
-      return translatedLabel ? "btn-icon-text" : "btn-icon";
-    } else if (translatedLabel) {
+  get isDisabled() {
+    return this.forceDisabled || this.args.disabled;
+  }
+
+  get btnType() {
+    if (this.args.icon) {
+      return this.computedLabel ? "btn-icon-text" : "btn-icon";
+    } else if (this.computedLabel) {
       return "btn-text";
     }
-  },
+  }
 
-  noText: empty("computedLabel"),
-
-  @discourseComputed("title", "translatedTitle")
-  computedTitle(title, translatedTitle) {
-    if (title) {
-      return I18n.t(title);
+  get computedTitle() {
+    if (this.args.title) {
+      return I18n.t(this.args.title);
     }
-    return translatedTitle;
-  },
+    return this.args.translatedTitle;
+  }
 
-  @discourseComputed("label", "translatedLabel")
-  computedLabel(label, translatedLabel) {
-    if (label) {
-      return I18n.t(label);
+  get computedLabel() {
+    if (this.args.label) {
+      return I18n.t(this.args.label);
     }
-    return translatedLabel;
-  },
+    return this.args.translatedLabel;
+  }
 
-  @discourseComputed("ariaLabel", "translatedAriaLabel")
-  computedAriaLabel(ariaLabel, translatedAriaLabel) {
-    if (ariaLabel) {
-      return I18n.t(ariaLabel);
+  get computedAriaLabel() {
+    if (this.args.ariaLabel) {
+      return I18n.t(this.args.ariaLabel);
     }
-    if (translatedAriaLabel) {
-      return translatedAriaLabel;
+    if (this.args.translatedAriaLabel) {
+      return this.args.translatedAriaLabel;
     }
-  },
+  }
 
-  @discourseComputed("ariaExpanded")
-  computedAriaExpanded(ariaExpanded) {
-    if (ariaExpanded === true) {
+  get computedAriaExpanded() {
+    if (this.args.ariaExpanded === true) {
       return "true";
     }
-    if (ariaExpanded === false) {
+    if (this.args.ariaExpanded === false) {
       return "false";
     }
-  },
+  }
 
+  @action
   keyDown(e) {
-    if (this.onKeyDown) {
+    if (this.args.onKeyDown) {
       e.stopPropagation();
-      this.onKeyDown(e);
+      this.args.onKeyDown(e);
     } else if (e.key === "Enter") {
       this._triggerAction(e);
-      return false;
     }
-  },
+  }
 
+  @action
   click(event) {
     return this._triggerAction(event);
-  },
+  }
 
+  @action
   mouseDown(event) {
-    if (this.preventFocus) {
+    if (this.args.preventFocus) {
       event.preventDefault();
     }
-  },
+  }
 
   _triggerAction(event) {
-    let { action, route, href } = this;
+    const { action: actionVal, route, href } = this.args;
 
-    if (action || route || href?.length) {
-      if (action) {
-        if (typeof action === "string") {
-          // Note: This is deprecated in new Embers and needs to be removed in the future.
-          // There is already a warning in the console.
-          this.sendAction("action", this.actionParam);
-        } else if (typeof action === "object" && action.value) {
-          if (this.forwardEvent) {
-            action.value(this.actionParam, event);
+    if (actionVal || route || href?.length) {
+      if (actionVal) {
+        const { actionParam, forwardEvent } = this.args;
+
+        if (typeof actionVal === "string") {
+          deprecated(...ACTION_AS_STRING_DEPRECATION_ARGS);
+          if (this.parentView?.send) {
+            this.parentView.send(actionVal, actionParam);
           } else {
-            action.value(this.actionParam);
+            throw new Error(
+              "DButton could not find a target for the action. Use a closure action instead"
+            );
           }
-        } else if (typeof this.action === "function") {
-          if (this.forwardEvent) {
-            action(this.actionParam, event);
+        } else if (typeof actionVal === "object" && actionVal.value) {
+          if (forwardEvent) {
+            actionVal.value(actionParam, event);
           } else {
-            action(this.actionParam);
+            actionVal.value(actionParam);
+          }
+        } else if (typeof actionVal === "function") {
+          if (forwardEvent) {
+            actionVal(actionParam, event);
+          } else {
+            actionVal(actionParam);
           }
         }
-      }
-
-      if (route) {
+      } else if (route) {
         this.router.transitionTo(route);
-      }
-
-      if (href?.length) {
+      } else if (href?.length) {
         DiscourseURL.routeTo(href);
       }
 
@@ -164,5 +140,5 @@ export default Component.extend({
 
       return false;
     }
-  },
-});
+  }
+}

app/assets/javascripts/discourse/app/components/glimmer-component-with-deprecated-parent-view.js

@@ -0,0 +1,32 @@
+import Component from "@glimmer/component";
+import {
+  CustomComponentManager,
+  setInternalComponentManager,
+} from "@glimmer/manager";
+import EmberGlimmerComponentManager from "@glimmer/component/-private/ember-component-manager";
+
+class GlimmerComponentWithParentViewManager extends CustomComponentManager {
+  create(owner, componentClass, args, environment, dynamicScope) {
+    const result = super.create(...arguments);
+
+    result.component.parentView = dynamicScope.view;
+    dynamicScope.view = result.component;
+
+    return result;
+  }
+}
+
+/**
+ * This component has a lightly-extended version of Ember's default Glimmer component manager.
+ * It gives Glimmer components the ability to reference their parent view which can be useful
+ * when building backwards-compatible versions of components. Any use of the parentView property
+ * of the component should be considered deprecated.
+ */
+export default class GlimmerComponentWithDeprecatedParentView extends Component {}
+
+setInternalComponentManager(
+  new GlimmerComponentWithParentViewManager(
+    (owner) => new EmberGlimmerComponentManager(owner)
+  ),
+  GlimmerComponentWithDeprecatedParentView
+);

app/assets/javascripts/discourse/app/components/login-reply-button.js

@@ -1,7 +0,0 @@
-import Button from "discourse/components/d-button";
-
-export default Button.extend({
-  label: "topic.reply.title",
-  icon: "reply",
-  action: "showLogin",
-});

app/assets/javascripts/discourse/app/templates/components/d-button.hbs

@@ -1,16 +1,36 @@
-{{#if this.isLoading}}
-  {{~d-icon "spinner" class="loading-icon"~}}
-{{else}}
-  {{#if this.icon}}
-    {{~d-icon this.icon~}}
+{{! template-lint-disable no-down-event-binding }}
+<button
+  {{! For legacy compatibility. Prefer passing class as attributes. }}
+  class="{{@class}} {{if @isLoading "is-loading"}} {{if this.btnLink "btn-link" "btn"}} {{if this.noText "no-text"}} {{this.btnType}}"
+  {{! For legacy compatibility. Prefer passing these as html attributes. }}
+  id={{@id}}
+  form={{@form}}
+  aria-controls={{@ariaControls}}
+  aria-expanded={{this.computedAriaExpanded}}
+  tabindex={{@tabindex}}
+  type={{or @type "button"}}
+  ...attributes
+  disabled={{this.isDisabled}}
+  title={{this.computedTitle}}
+  aria-label={{this.computedAriaLabel}}
+  {{on "keydown" this.keyDown}}
+  {{on "click" this.click}}
+  {{on "mousedown" this.mouseDown}}
+>
+  {{#if @isLoading}}
+    {{~d-icon "spinner" class="loading-icon"~}}
+  {{else}}
+    {{#if @icon}}
+      {{~d-icon @icon~}}
+    {{/if}}
   {{/if}}
-{{/if}}
 
-{{~#if this.computedLabel~}}
-  <span class="d-button-label">{{html-safe this.computedLabel}}{{#if this.ellipsis}}&hellip;{{/if}}</span>
-{{~else if (not (has-block))~}}
-  &#8203;
-  {{! Zero-width space character, so icon-only button height = regular button height }}
-{{~/if~}}
+  {{~#if this.computedLabel~}}
+    <span class="d-button-label">{{html-safe this.computedLabel}}{{#if this.ellipsis}}&hellip;{{/if}}</span>
+  {{~else if (not (has-block))~}}
+    &#8203;
+    {{! Zero-width space character, so icon-only button height = regular button height }}
+  {{~/if~}}
 
-{{yield}}
+  {{yield}}
+</button>

app/assets/javascripts/discourse/tests/integration/components/d-button-test.js

@@ -1,9 +1,10 @@
 import { module, test } from "qunit";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
-import { render, triggerKeyEvent } from "@ember/test-helpers";
+import { click, render, triggerKeyEvent } from "@ember/test-helpers";
 import { exists, query } from "discourse/tests/helpers/qunit-helpers";
 import I18n from "I18n";
 import { hbs } from "ember-cli-htmlbars";
+import ClassicComponent from "@ember/component";
 
 module("Integration | Component | d-button", function (hooks) {
   setupRenderingTest(hooks);
@@ -241,4 +242,41 @@ module("Integration | Component | d-button", function (hooks) {
     await triggerKeyEvent(".btn", "keydown", "Enter");
     assert.strictEqual(this.foo, "bar");
   });
+
+  test("@action function is triggered on click", async function (assert) {
+    this.set("foo", null);
+    this.set("action", () => {
+      this.set("foo", "bar");
+    });
+
+    await render(hbs`<DButton @action={{this.action}} />`);
+
+    await click(".btn");
+
+    assert.strictEqual(this.foo, "bar");
+  });
+
+  test("@action can sendAction when passed a string", async function (assert) {
+    this.set("foo", null);
+    this.set("legacyActionTriggered", () => this.set("foo", "bar"));
+
+    this.classicComponent = ClassicComponent.extend({
+      actions: {
+        myLegacyAction() {
+          this.legacyActionTriggered();
+        },
+      },
+    });
+
+    await render(
+      hbs`<this.classicComponent @legacyActionTriggered={{this.legacyActionTriggered}}>
+          <DButton @action="myLegacyAction" />
+        </this.classicComponent>
+        `
+    );
+
+    await click(".btn");
+
+    assert.strictEqual(this.foo, "bar");
+  });
 });