From 213d9dbe413fc96418fc29352229fa9fd1039de1 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 7 Jun 2023 11:33:03 +0900 Subject: [PATCH] Revert "DEV: Convert modal wrapper from named outlet to component (#21932)" (#21964) This reverts commit 80b77b2e65e9ea4897ee9d000c2bd9f6297e87d7. Some modal functionality has been broken like inviting an existing user to a PM --- .../discourse/app/components/d-modal-body.js | 4 +- .../discourse/app/components/d-modal.js | 4 +- .../app/components/modal-container.hbs | 17 -- .../app/components/modal-container.js | 12 -- .../discourse/app/controllers/modal.js | 6 + .../app/mixins/modal-functionality.js | 3 +- .../discourse/app/routes/application.js | 5 + .../discourse/app/services/modal.js | 146 ++++++++---------- .../discourse/app/templates/application.hbs | 2 +- .../discourse/app/templates/modal.hbs | 14 ++ .../tests/acceptance/do-not-disturb-test.js | 4 +- .../discourse/tests/acceptance/modal-test.js | 6 +- 12 files changed, 103 insertions(+), 120 deletions(-) delete mode 100644 app/assets/javascripts/discourse/app/components/modal-container.hbs delete mode 100644 app/assets/javascripts/discourse/app/components/modal-container.js create mode 100644 app/assets/javascripts/discourse/app/controllers/modal.js create mode 100644 app/assets/javascripts/discourse/app/templates/modal.hbs diff --git a/app/assets/javascripts/discourse/app/components/d-modal-body.js b/app/assets/javascripts/discourse/app/components/d-modal-body.js index 6a3ffe4115b..bb6f9c857f4 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal-body.js +++ b/app/assets/javascripts/discourse/app/components/d-modal-body.js @@ -3,6 +3,7 @@ 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 = {}; @@ -17,7 +18,6 @@ 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"); - this.modal.hidden = false; + getOwner(this).lookup("controller:modal").hidden = false; } this.appEvents.trigger( diff --git a/app/assets/javascripts/discourse/app/components/d-modal.js b/app/assets/javascripts/discourse/app/components/d-modal.js index 7890e49a07d..c7ace74afe2 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal.js +++ b/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) { - this.modal.hidden = false; + getOwner(this).lookup("controller:modal").hidden = false; } this.modalBodyData = data; diff --git a/app/assets/javascripts/discourse/app/components/modal-container.hbs b/app/assets/javascripts/discourse/app/components/modal-container.hbs deleted file mode 100644 index f822de045ca..00000000000 --- a/app/assets/javascripts/discourse/app/components/modal-container.hbs +++ /dev/null @@ -1,17 +0,0 @@ - - {{outlet "modalBody"}} - \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/modal-container.js b/app/assets/javascripts/discourse/app/components/modal-container.js deleted file mode 100644 index a12b79783c8..00000000000 --- a/app/assets/javascripts/discourse/app/components/modal-container.js +++ /dev/null @@ -1,12 +0,0 @@ -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); - } -} diff --git a/app/assets/javascripts/discourse/app/controllers/modal.js b/app/assets/javascripts/discourse/app/controllers/modal.js new file mode 100644 index 00000000000..18cc7d77425 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/modal.js @@ -0,0 +1,6 @@ +import Controller from "@ember/controller"; +import { tracked } from "@glimmer/tracking"; + +export default class ModalController extends Controller { + @tracked hidden = true; +} diff --git a/app/assets/javascripts/discourse/app/mixins/modal-functionality.js b/app/assets/javascripts/discourse/app/mixins/modal-functionality.js index ee302c08fc1..6287199a755 100644 --- a/app/assets/javascripts/discourse/app/mixins/modal-functionality.js +++ b/app/assets/javascripts/discourse/app/mixins/modal-functionality.js @@ -16,7 +16,8 @@ export default Mixin.create({ actions: { closeModal() { - this.modal.close(); + this.modal.send("closeModal"); + this.set("panels", []); }, }, }); diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js index d5403812d83..e162cbaf230 100644 --- a/app/assets/javascripts/discourse/app/routes/application.js +++ b/app/assets/javascripts/discourse/app/routes/application.js @@ -212,6 +212,11 @@ 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); diff --git a/app/assets/javascripts/discourse/app/services/modal.js b/app/assets/javascripts/discourse/app/services/modal.js index b6290aa74c5..4cf00307f5e 100644 --- a/app/assets/javascripts/discourse/app/services/modal.js +++ b/app/assets/javascripts/discourse/app/services/modal.js @@ -3,66 +3,28 @@ 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"); - this.opts = opts; + modalController.set( + "modalClass", + opts.modalClass || `${dasherize(name).toLowerCase()}-modal` + ); const controllerName = opts.admin ? `modals/${name}` : name; - this.name = controllerName; + modalController.set("name", controllerName); let controller = container.lookup("controller:" + controllerName); const templateName = opts.templateName || dasherize(name); - const renderArgs = { into: "application", outlet: "modalBody" }; + const renderArgs = { into: "modal", outlet: "modalBody" }; if (controller) { renderArgs.controller = controllerName; } else { @@ -78,12 +40,40 @@ export default class ModalService extends Service { const modalName = `modal/${templateName}`; const fullName = opts.admin ? `admin/templates/${modalName}` : modalName; route.render(fullName, renderArgs); - - if (controller.actions.onSelectPanel) { - this.onSelectPanel = controller.actions.onSelectPanel.bind(controller); + if (opts.title) { + modalController.set("title", I18n.t(opts.title)); + } else if (opts.titleTranslated) { + modalController.set("title", opts.titleTranslated); + } else { + modalController.set("title", null); } - controller.set("modal", this); + 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) + ); + } + + modalController.set( + "modalClass", + `${modalController.get("modalClass")} has-tabs` + ); + } else { + modalController.setProperties({ panels: [], selectedPanel: null }); + } + + controller.set("modal", modalController); const model = opts.model; if (model) { controller.set("model", model); @@ -97,44 +87,44 @@ export default class ModalService extends Service { } close(initiatedBy) { - const controllerName = this.name; - const controller = controllerName - ? getOwner(this).lookup(`controller:${controllerName}`) - : null; + const route = getOwner(this).lookup("route:application"); + let modalController = route.controllerFor("modal"); + const controllerName = modalController.get("name"); - if (controller?.beforeClose?.() === false) { - return; + if (controllerName) { + const controller = getOwner(this).lookup(`controller:${controllerName}`); + if (controller && controller.beforeClose) { + if (false === controller.beforeClose()) { + return; + } + } } getOwner(this) .lookup("route:application") - .render("hide-modal", { into: "application", outlet: "modalBody" }); + .render("hide-modal", { into: "modal", outlet: "modalBody" }); $(".d-modal.fixed-modal").modal("hide"); - if (controller) { - this.appEvents.trigger("modal:closed", { - name: controllerName, - controller, - }); + if (controllerName) { + const controller = getOwner(this).lookup(`controller:${controllerName}`); - if (controller.onClose) { - controller.onClose({ - initiatedByCloseButton: initiatedBy === "initiatedByCloseButton", - initiatedByClickOut: initiatedBy === "initiatedByClickOut", - initiatedByESC: initiatedBy === "initiatedByESC", + if (controller) { + this.appEvents.trigger("modal:closed", { + name: controllerName, + controller, }); + + if (controller.onClose) { + controller.onClose({ + initiatedByCloseButton: initiatedBy === "initiatedByCloseButton", + initiatedByClickOut: initiatedBy === "initiatedByClickOut", + initiatedByESC: initiatedBy === "initiatedByESC", + }); + } } + modalController.set("name", null); } - this.hidden = true; - - this.name = - this.selectedPanel = - this.modalClassOverride = - this.titleOverride = - this.onSelectPanel = - null; - - this.opts = {}; + modalController.hidden = true; } hide() { @@ -144,8 +134,4 @@ export default class ModalService extends Service { reopen() { $(".d-modal.fixed-modal").modal("show"); } - - get #isRendered() { - return !!this.name; - } } diff --git a/app/assets/javascripts/discourse/app/templates/application.hbs b/app/assets/javascripts/discourse/app/templates/application.hbs index 7c505749539..6371d165e9b 100644 --- a/app/assets/javascripts/discourse/app/templates/application.hbs +++ b/app/assets/javascripts/discourse/app/templates/application.hbs @@ -85,7 +85,7 @@ @outletArgs={{hash showFooter=this.showFooter}} /> - + {{outlet "modal"}} diff --git a/app/assets/javascripts/discourse/app/templates/modal.hbs b/app/assets/javascripts/discourse/app/templates/modal.hbs new file mode 100644 index 00000000000..782924904f7 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/modal.hbs @@ -0,0 +1,14 @@ + + {{outlet "modalBody"}} + \ No newline at end of file diff --git a/app/assets/javascripts/discourse/tests/acceptance/do-not-disturb-test.js b/app/assets/javascripts/discourse/tests/acceptance/do-not-disturb-test.js index 29da79004cc..94f066cdeff 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/do-not-disturb-test.js +++ b/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(".d-modal.hidden"), "modal is hidden"); + assert.ok(query(".do-not-disturb-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(".d-modal.hidden"), + query(".do-not-disturb-modal.hidden"), "DND modal is hidden after making a choice" ); diff --git a/app/assets/javascripts/discourse/tests/acceptance/modal-test.js b/app/assets/javascripts/discourse/tests/acceptance/modal-test.js index b27e6ab25d2..91fc61547b1 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/modal-test.js +++ b/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 service = getOwner(this).lookup("service:modal"); - assert.strictEqual(service.name, "login"); + const controller = getOwner(this).lookup("controller:modal"); + assert.strictEqual(controller.name, "login"); await click(".modal-outer-container"); assert.ok( !exists(".d-modal:visible"), "modal should disappear when you click outside" ); - assert.strictEqual(service.name, null); + assert.strictEqual(controller.name, null); await click(".login-button"); assert.strictEqual(count(".d-modal:visible"), 1, "modal should reappear");