From 305b532c224839cfb0af34ba8b7e4fbe031e0eae Mon Sep 17 00:00:00 2001 From: Johannes Raggam Date: Wed, 24 Nov 2021 17:46:24 +0100 Subject: [PATCH 1/2] fix(pat model): Fix close-panel with multiple inject forms. Support close-panel with multiple forms.pat-inject in a modal, for example together with pat-stacks. Previously only the first form used to attach the event handler which listens for the injection success event for closing the modal. In these cases the modal wasn't closed properly. --- src/pat/modal/modal.js | 12 +++++++----- src/pat/modal/modal.test.js | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/pat/modal/modal.js b/src/pat/modal/modal.js index fc82bb17e..7146c0ad4 100644 --- a/src/pat/modal/modal.js +++ b/src/pat/modal/modal.js @@ -101,9 +101,11 @@ export default Base.extend({ if (document.activeElement) { document.activeElement.focus(); } + this._init_handlers(); this.resize(); this.setPosition(); + $("body").addClass("modal-active"); this.el.dispatchEvent( new Event("pat-modal-ready", { bubbles: true, cancelable: true }) @@ -225,9 +227,10 @@ export default Base.extend({ $("body").removeClass("modal-panel"); }, - destroy_inject() { - const form = this.el.querySelector("form.pat-inject"); - if (form) { + destroy_inject(e) { + const button = e.target; + const form = button.form; + if (form && form.classList.contains("pat-inject")) { // if the modal contains a for mwith pat-inject, wait for injection // to be finished and then destroy the modal. const destroy_handler = () => { @@ -241,8 +244,7 @@ export default Base.extend({ destroy_handler.bind(this) ); } else { - // if working without injection, destroy after waiting a tick to let - // eventually registered on-submit handlers kick in first. + // if working without form injection, just destroy. this.destroy(); } }, diff --git a/src/pat/modal/modal.test.js b/src/pat/modal/modal.test.js index 481032975..3a6bca600 100644 --- a/src/pat/modal/modal.test.js +++ b/src/pat/modal/modal.test.js @@ -206,6 +206,44 @@ describe("pat-modal", function () { expect(document.querySelector("#target2").textContent).toBe("there"); }); + it("Submit form, do injection and close overlay with multiple forms.", async function () { + await import("../inject/inject"); + const registry = (await import("../../core/registry")).default; + + jest.spyOn($, "ajax").mockImplementation(() => deferred); + answer( + `
hello.
there
` + ); + + document.body.innerHTML = ` +
+
+ +
+
+ +
+
+
+
+ `; + registry.scan(document.body); + await utils.timeout(1); // wait a tick for async to settle. + + document.querySelector(".form2 button.close-panel[type=submit]").click(); + await utils.timeout(1); // wait a tick for pat-inject to settle. + await utils.timeout(1); // wait a tick for pat-modal destroy to settle. + + expect(document.querySelector(".pat-modal")).toBe(null); + expect(document.querySelector("#target").textContent).toBe("hello."); + }); + it("Ensure destroy callback isn't called multiple times.", async function () { document.body.innerHTML = `
From 701ee950aeaa61e7b7301f83418d2442f15e8086 Mon Sep 17 00:00:00 2001 From: Johannes Raggam Date: Thu, 25 Nov 2021 00:18:19 +0100 Subject: [PATCH 2/2] feat(pat modal): Reinitialize handlers after the modal's DOM has changed. --- src/pat/modal/modal.js | 9 +++++++-- src/pat/modal/modal.test.js | 29 +++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/pat/modal/modal.js b/src/pat/modal/modal.js index 7146c0ad4..821f58e5e 100644 --- a/src/pat/modal/modal.js +++ b/src/pat/modal/modal.js @@ -67,7 +67,7 @@ export default Base.extend({ inject.init(this.$el, opts); }, - _init_div1() { + async _init_div1() { const $header = $("
"); if (this.options.closing.indexOf("close-button") !== -1) { $( @@ -102,7 +102,6 @@ export default Base.extend({ document.activeElement.focus(); } - this._init_handlers(); this.resize(); this.setPosition(); @@ -110,6 +109,12 @@ export default Base.extend({ this.el.dispatchEvent( new Event("pat-modal-ready", { bubbles: true, cancelable: true }) ); + + // Wait a bit to let any pattern initializations settle before initializing handlers. + await utils.timeout(2); + this._init_handlers(); + const modal_observer = new MutationObserver(this._init_handlers.bind(this)); + modal_observer.observe(this.el, { childList: true, subtree: true }); }, _init_handlers() { diff --git a/src/pat/modal/modal.test.js b/src/pat/modal/modal.test.js index 3a6bca600..ab8bf5ac4 100644 --- a/src/pat/modal/modal.test.js +++ b/src/pat/modal/modal.test.js @@ -196,6 +196,7 @@ describe("pat-modal", function () { `; registry.scan(document.body); await utils.timeout(1); // wait a tick for async to settle. + await utils.timeout(2); // wait a tick for async to settle. document.querySelector("button.close-panel[type=submit]").click(); await utils.timeout(1); // wait a tick for pat-inject to settle. @@ -235,6 +236,7 @@ describe("pat-modal", function () { `; registry.scan(document.body); await utils.timeout(1); // wait a tick for async to settle. + await utils.timeout(2); // wait a tick for async to settle. document.querySelector(".form2 button.close-panel[type=submit]").click(); await utils.timeout(1); // wait a tick for pat-inject to settle. @@ -244,6 +246,26 @@ describe("pat-modal", function () { expect(document.querySelector("#target").textContent).toBe("hello."); }); + it("Initialize modal also when modal contents change.", async function () { + document.body.innerHTML = ` +
+
+ `; + const instance = new pattern(document.querySelector(".pat-modal")); + const spy_init_handlers = jest.spyOn(instance, "_init_handlers"); + expect(spy_init_handlers).toHaveBeenCalledTimes(0); + + // first call is invoked after some ticks to allow any other + // patterns to modify the dom before the handlers are initialized. + await utils.timeout(2); // wait for init to happen. + expect(spy_init_handlers).toHaveBeenCalledTimes(1); + + // Provoke a DOM subtree change and the MutationObserver to kick in + document.querySelector(".pat-modal").innerHTML = "
"; + await utils.timeout(1); // wait a tick for async to settle. + expect(spy_init_handlers).toHaveBeenCalledTimes(2); + }); + it("Ensure destroy callback isn't called multiple times.", async function () { document.body.innerHTML = `
@@ -253,12 +275,13 @@ describe("pat-modal", function () { const instance = new pattern(document.querySelector(".pat-modal")); await utils.timeout(1); // wait a tick for async to settle. + await utils.timeout(2); // wait a tick for async to settle. const spy_destroy = jest.spyOn(instance, "destroy"); // ``destroy`` was already initialized with instantiating the pattern above. // Call init again for new instantiation. - instance.init($(".pat-modal")); + await instance.init($(".pat-modal")); document.querySelector("#close-modal").click(); await utils.timeout(1); // wait a tick for pat-modal destroy to settle. @@ -285,13 +308,14 @@ describe("pat-modal", function () { pattern_inject.init($(".pat-inject")); const instance = new pattern(document.querySelector(".pat-modal")); await utils.timeout(1); // wait a tick for async to settle. + await utils.timeout(2); // wait a tick for async to settle. const spy_destroy = jest.spyOn(instance, "destroy"); const spy_destroy_inject = jest.spyOn(instance, "destroy_inject"); // ``destroy`` was already initialized with instantiating the pattern above. // Call init again for new instantiation. - instance.init($(".pat-modal")); + await instance.init($(".pat-modal")); document.querySelector("#close-modal").click(); await utils.timeout(1); // wait a tick for pat-inject to settle. @@ -304,6 +328,7 @@ describe("pat-modal", function () { pattern_inject.init($(".pat-inject")); new pattern(document.querySelector(".pat-modal")); await utils.timeout(1); // wait a tick for async to settle. + await utils.timeout(2); // wait a tick for async to settle. document.querySelector("#close-modal").click(); await utils.timeout(1); // wait a tick for pat-inject to settle. // Previous mocks still active.