DEV: Convert modal wrapper from named outlet to component (#21970)

This removes the modal container named-outlet/controller/template and replaces it with a component. Named outlets will be removed in Ember 4.x, so this change is part of that upgrade project.

Smaller changes include:
- update some of the computed values to be getters rather than calculated during `show()`.
- update tests which were previously depending on the modal class persisting after the modal was closed

Much of the logic in the service will be deprecated once we introduce component-based modals.

This work is split out from https://github.com/discourse/discourse/pull/21304

Previously merged in 80b77b2e and then reverted due to issues with the PM invite modal. This PR fixes the issue, and introduces a test which would have caught the issue.

app/assets/javascripts/discourse/app/components/d-modal-body.js

@@ -3,7 +3,6 @@ import { disableImplicitInjections } from "discourse/lib/implicit-injections";
 import { action } from "@ember/object";
 import { tracked } from "@glimmer/tracking";
 import { inject as service } from "@ember/service";
-import { getOwner } from "@ember/application";
 
 function pick(object, keys) {
   const result = {};
@@ -18,6 +17,7 @@ function pick(object, keys) {
 @disableImplicitInjections
 export default class DModalBody extends Component {
   @service appEvents;
+  @service modal;
 
   @tracked fixed = false;
 
@@ -29,7 +29,7 @@ export default class DModalBody extends Component {
     if (fixedParent) {
       this.fixed = true;
       $(fixedParent).modal("show");
-      getOwner(this).lookup("controller:modal").hidden = false;
+      this.modal.hidden = false;
     }
 
     this.appEvents.trigger(

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

@@ -6,11 +6,11 @@ import { disableImplicitInjections } from "discourse/lib/implicit-injections";
 import { inject as service } from "@ember/service";
 import { action } from "@ember/object";
 import { tracked } from "@glimmer/tracking";
-import { getOwner } from "@ember/application";
 
 @disableImplicitInjections
 export default class DModal extends Component {
   @service appEvents;
+  @service modal;
 
   @tracked wrapperElement;
   @tracked modalBodyData = {};
@@ -147,7 +147,7 @@ export default class DModal extends Component {
     }
 
     if (data.fixed) {
-      getOwner(this).lookup("controller:modal").hidden = false;
+      this.modal.hidden = false;
     }
 
     this.modalBodyData = data;

app/assets/javascripts/discourse/app/components/modal-container.hbs

@@ -0,0 +1,17 @@
+<DModal
+  @modalClass={{concat-class
+    this.modal.modalClass
+    (if this.modal.opts.panels "has-tabs")
+  }}
+  @title={{this.modal.title}}
+  @titleAriaElementId={{this.modal.opts.titleAriaElementId}}
+  @panels={{this.modal.opts.panels}}
+  @selectedPanel={{this.modal.selectedPanel}}
+  @onSelectPanel={{this.modal.onSelectPanel}}
+  @hidden={{this.modal.hidden}}
+  @class="hidden"
+  @errors={{this.modal.errors}}
+  @closeModal={{this.closeModal}}
+>
+  {{outlet "modalBody"}}
+</DModal>

app/assets/javascripts/discourse/app/components/modal-container.js

@@ -0,0 +1,12 @@
+import Component from "@glimmer/component";
+import { inject as service } from "@ember/service";
+import { action } from "@ember/object";
+
+export default class ModalContainer extends Component {
+  @service modal;
+
+  @action
+  closeModal(initiatedBy) {
+    this.modal.close(initiatedBy);
+  }
+}

app/assets/javascripts/discourse/app/controllers/modal.js

@@ -1,6 +0,0 @@
-import Controller from "@ember/controller";
-import { tracked } from "@glimmer/tracking";
-
-export default class ModalController extends Controller {
-  @tracked hidden = true;
-}

app/assets/javascripts/discourse/app/mixins/modal-functionality.js

@@ -16,8 +16,7 @@ export default Mixin.create({
 
   actions: {
     closeModal() {
-      this.modal.send("closeModal");
-      this.set("panels", []);
+      this.modal.close();
     },
   },
 });

app/assets/javascripts/discourse/app/routes/application.js

@@ -212,11 +212,6 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
     },
   },
 
-  renderTemplate() {
-    this.render("application");
-    this.render("modal", { into: "application", outlet: "modal" });
-  },
-
   handleShowLogin() {
     if (this.siteSettings.enable_discourse_connect) {
       const returnPath = encodeURIComponent(window.location.pathname);

app/assets/javascripts/discourse/app/services/modal.js

@@ -3,28 +3,66 @@ import { getOwner } from "@ember/application";
 import I18n from "I18n";
 import { dasherize } from "@ember/string";
 import { disableImplicitInjections } from "discourse/lib/implicit-injections";
+import { tracked } from "@glimmer/tracking";
 
 @disableImplicitInjections
 export default class ModalService extends Service {
   @service appEvents;
 
+  @tracked name;
+  @tracked opts = {};
+  @tracked selectedPanel;
+  @tracked hidden = true;
+
+  @tracked titleOverride;
+  @tracked modalClassOverride;
+  @tracked onSelectPanel;
+
+  get title() {
+    if (this.titleOverride) {
+      return this.titleOverride;
+    } else if (this.opts.titleTranslated) {
+      return this.opts.titleTranslated;
+    } else if (this.opts.title) {
+      return I18n.t(this.opts.title);
+    } else {
+      return null;
+    }
+  }
+
+  set title(value) {
+    this.titleOverride = value;
+  }
+
+  get modalClass() {
+    if (!this.#isRendered) {
+      return null;
+    }
+
+    return (
+      this.modalClassOverride ||
+      this.opts.modalClass ||
+      `${dasherize(this.name.replace(/^modals\//, "")).toLowerCase()}-modal`
+    );
+  }
+
+  set modalClass(value) {
+    this.modalClassOverride = value;
+  }
+
   show(name, opts = {}) {
     const container = getOwner(this);
     const route = container.lookup("route:application");
-    const modalController = route.controllerFor("modal");
 
-    modalController.set(
-      "modalClass",
-      opts.modalClass || `${dasherize(name).toLowerCase()}-modal`
-    );
+    this.opts = opts;
 
     const controllerName = opts.admin ? `modals/${name}` : name;
-    modalController.set("name", controllerName);
+    this.name = controllerName;
 
     let controller = container.lookup("controller:" + controllerName);
     const templateName = opts.templateName || dasherize(name);
 
-    const renderArgs = { into: "modal", outlet: "modalBody" };
+    const renderArgs = { into: "application", outlet: "modalBody" };
     if (controller) {
       renderArgs.controller = controllerName;
     } else {
@@ -40,40 +78,15 @@ export default class ModalService extends Service {
     const modalName = `modal/${templateName}`;
     const fullName = opts.admin ? `admin/templates/${modalName}` : modalName;
     route.render(fullName, renderArgs);
-    if (opts.title) {
-      modalController.set("title", I18n.t(opts.title));
-    } else if (opts.titleTranslated) {
-      modalController.set("title", opts.titleTranslated);
-    } else {
-      modalController.set("title", null);
-    }
-
-    if (opts.titleAriaElementId) {
-      modalController.set("titleAriaElementId", opts.titleAriaElementId);
-    }
 
     if (opts.panels) {
-      modalController.setProperties({
-        panels: opts.panels,
-        selectedPanel: opts.panels[0],
-      });
-
       if (controller.actions.onSelectPanel) {
-        modalController.set(
-          "onSelectPanel",
-          controller.actions.onSelectPanel.bind(controller)
-        );
+        this.onSelectPanel = controller.actions.onSelectPanel.bind(controller);
       }
-
-      modalController.set(
-        "modalClass",
-        `${modalController.get("modalClass")} has-tabs`
-      );
-    } else {
-      modalController.setProperties({ panels: [], selectedPanel: null });
+      this.selectedPanel = opts.panels[0];
     }
 
-    controller.set("modal", modalController);
+    controller.set("modal", this);
     const model = opts.model;
     if (model) {
       controller.set("model", model);
@@ -87,44 +100,44 @@ export default class ModalService extends Service {
   }
 
   close(initiatedBy) {
-    const route = getOwner(this).lookup("route:application");
-    let modalController = route.controllerFor("modal");
-    const controllerName = modalController.get("name");
+    const controllerName = this.name;
+    const controller = controllerName
+      ? getOwner(this).lookup(`controller:${controllerName}`)
+      : null;
 
-    if (controllerName) {
-      const controller = getOwner(this).lookup(`controller:${controllerName}`);
-      if (controller && controller.beforeClose) {
-        if (false === controller.beforeClose()) {
-          return;
-        }
-      }
+    if (controller?.beforeClose?.() === false) {
+      return;
     }
 
     getOwner(this)
       .lookup("route:application")
-      .render("hide-modal", { into: "modal", outlet: "modalBody" });
+      .render("hide-modal", { into: "application", outlet: "modalBody" });
     $(".d-modal.fixed-modal").modal("hide");
 
-    if (controllerName) {
-      const controller = getOwner(this).lookup(`controller:${controllerName}`);
+    if (controller) {
+      this.appEvents.trigger("modal:closed", {
+        name: controllerName,
+        controller,
+      });
 
-      if (controller) {
-        this.appEvents.trigger("modal:closed", {
-          name: controllerName,
-          controller,
+      if (controller.onClose) {
+        controller.onClose({
+          initiatedByCloseButton: initiatedBy === "initiatedByCloseButton",
+          initiatedByClickOut: initiatedBy === "initiatedByClickOut",
+          initiatedByESC: initiatedBy === "initiatedByESC",
         });
-
-        if (controller.onClose) {
-          controller.onClose({
-            initiatedByCloseButton: initiatedBy === "initiatedByCloseButton",
-            initiatedByClickOut: initiatedBy === "initiatedByClickOut",
-            initiatedByESC: initiatedBy === "initiatedByESC",
-          });
-        }
       }
-      modalController.set("name", null);
     }
-    modalController.hidden = true;
+    this.hidden = true;
+
+    this.name =
+      this.selectedPanel =
+      this.modalClassOverride =
+      this.titleOverride =
+      this.onSelectPanel =
+        null;
+
+    this.opts = {};
   }
 
   hide() {
@@ -134,4 +147,8 @@ export default class ModalService extends Service {
   reopen() {
     $(".d-modal.fixed-modal").modal("show");
   }
+
+  get #isRendered() {
+    return !!this.name;
+  }
 }

app/assets/javascripts/discourse/app/templates/application.hbs

@@ -85,7 +85,7 @@
     @outletArgs={{hash showFooter=this.showFooter}}
   />
 
-  {{outlet "modal"}}
+  <ModalContainer />
   <DialogHolder />
   <TopicEntrance />
   <ComposerContainer />

app/assets/javascripts/discourse/app/templates/modal.hbs

@@ -1,14 +0,0 @@
-<DModal
-  @modalClass={{this.modalClass}}
-  @title={{this.title}}
-  @titleAriaElementId={{this.titleAriaElementId}}
-  @subtitle={{this.subtitle}}
-  @panels={{this.panels}}
-  @selectedPanel={{this.selectedPanel}}
-  @onSelectPanel={{this.onSelectPanel}}
-  @hidden={{this.hidden}}
-  @errors={{this.errors}}
-  @closeModal={{route-action "closeModal"}}
->
-  {{outlet "modalBody"}}
-</DModal>

app/assets/javascripts/discourse/app/widgets/private-message-map.js

@@ -185,7 +185,7 @@ export default createWidget("private-message-map", {
         this.attach("button", {
           action: "showInvite",
           icon: "plus",
-          className: "btn btn-default no-text btn-icon",
+          className: "btn btn-default no-text btn-icon add-participant-btn",
         })
       );
     }

app/assets/javascripts/discourse/tests/acceptance/do-not-disturb-test.js

@@ -38,7 +38,7 @@ acceptance("Do not disturb", function (needs) {
 
     await click(tiles[0]);
 
-    assert.ok(query(".do-not-disturb-modal.hidden"), "modal is hidden");
+    assert.ok(query(".d-modal.hidden"), "modal is hidden");
 
     assert.ok(
       exists(".header-dropdown-toggle .do-not-disturb-background .d-icon-moon"),
@@ -69,7 +69,7 @@ acceptance("Do not disturb", function (needs) {
     );
 
     assert.ok(
-      query(".do-not-disturb-modal.hidden"),
+      query(".d-modal.hidden"),
       "DND modal is hidden after making a choice"
     );
 

app/assets/javascripts/discourse/tests/acceptance/modal-test.js

@@ -38,15 +38,15 @@ acceptance("Modal", function (needs) {
     await click(".login-button");
     assert.strictEqual(count(".d-modal:visible"), 1, "modal should appear");
 
-    const controller = getOwner(this).lookup("controller:modal");
-    assert.strictEqual(controller.name, "login");
+    const service = getOwner(this).lookup("service:modal");
+    assert.strictEqual(service.name, "login");
 
     await click(".modal-outer-container");
     assert.ok(
       !exists(".d-modal:visible"),
       "modal should disappear when you click outside"
     );
-    assert.strictEqual(controller.name, null);
+    assert.strictEqual(service.name, null);
 
     await click(".login-button");
     assert.strictEqual(count(".d-modal:visible"), 1, "modal should reappear");

app/assets/javascripts/discourse/tests/acceptance/personal-message-test.js

@@ -72,3 +72,17 @@ acceptance("Personal Message (regular user)", function (needs) {
     assert.true(DiscourseURL.redirectTo.calledWith("/"));
   });
 });
+
+acceptance("Personal Message - invite", function (needs) {
+  needs.user();
+
+  test("suggested messages", async function (assert) {
+    await visit("/t/pm-for-testing/12");
+    await click(".add-remove-participant-btn");
+    await click(".private-message-map .controls .add-participant-btn");
+
+    assert
+      .dom(".d-modal.share-and-invite .invite-user-control")
+      .exists("invite modal is displayed");
+  });
+});