From f57fdee2f647b9cf525aaca83d19d4da5215dfaf Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 1 Aug 2019 15:40:51 -0400 Subject: [PATCH] FIX: Modal `onClose` was being called repeatedly This happened because the modal controller was not clearing the `name` attribute, which is used for looking up the controller to call `onClose` on. Every page navigation would call the method over and over, breaking state in odd ways. --- .../discourse/routes/application.js.es6 | 15 +++++++++++---- test/javascripts/acceptance/modal-test.js.es6 | 8 ++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6 index 4dbc79fded2..f9cdcab186d 100644 --- a/app/assets/javascripts/discourse/routes/application.js.es6 +++ b/app/assets/javascripts/discourse/routes/application.js.es6 @@ -156,10 +156,17 @@ const ApplicationRoute = Discourse.Route.extend(OpenComposer, { this.render("hide-modal", { into: "modal", outlet: "modalBody" }); const route = getOwner(this).lookup("route:application"); - const name = route.controllerFor("modal").get("name"); - const controller = getOwner(this).lookup(`controller:${name}`); - if (controller && controller.onClose) { - controller.onClose(); + let modalController = route.controllerFor("modal"); + const controllerName = modalController.get("name"); + + if (controllerName) { + const controller = getOwner(this).lookup( + `controller:${controllerName}` + ); + if (controller && controller.onClose) { + controller.onClose(); + } + modalController.set("name", null); } }, diff --git a/test/javascripts/acceptance/modal-test.js.es6 b/test/javascripts/acceptance/modal-test.js.es6 index 11754b8b52a..c15d23bd5c9 100644 --- a/test/javascripts/acceptance/modal-test.js.es6 +++ b/test/javascripts/acceptance/modal-test.js.es6 @@ -1,9 +1,9 @@ -import { acceptance } from "helpers/qunit-helpers"; +import { acceptance, controllerFor } from "helpers/qunit-helpers"; import showModal from "discourse/lib/show-modal"; acceptance("Modal"); -QUnit.test("modal", async assert => { +QUnit.test("modal", async function(assert) { await visit("/"); assert.ok( @@ -14,11 +14,15 @@ QUnit.test("modal", async assert => { await click(".login-button"); assert.ok(find(".d-modal:visible").length === 1, "modal should appear"); + let controller = controllerFor("modal"); + assert.equal(controller.name, "login"); + await click(".modal-outer-container"); assert.ok( find(".d-modal:visible").length === 0, "modal should disappear when you click outside" ); + assert.equal(controller.name, null); await click(".login-button"); assert.ok(find(".d-modal:visible").length === 1, "modal should reappear");