Skip to content

Commit 8181fbd

Browse files
thetthet
authored
Merge pull request #1065 from Patternslib/fix-validation
Fix validation
2 parents 591e162 + fbc20a8 commit 8181fbd

File tree

14 files changed

+314
-84
lines changed

14 files changed

+314
-84
lines changed

‎src/core/registry.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,10 @@ const registry = {
128128
},
129129

130130
orderPatterns(patterns) {
131-
// XXX: Bit of a hack. We need the validation pattern to be
132-
// parsed and initiated before the inject pattern. So we make
133-
// sure here, that it appears first. Not sure what would be
134-
// the best solution. Perhaps some kind of way to register
135-
// patterns "before" or "after" other patterns.
136-
if (patterns.includes("validation") && patterns.includes("inject")) {
131+
// Always add pat-validation as first pattern, so that it can prevent
132+
// other patterns from reacting to submit events if form validation
133+
// fails.
134+
if (patterns.includes("validation")) {
137135
patterns.splice(patterns.indexOf("validation"), 1);
138136
patterns.unshift("validation");
139137
}

‎src/pat/close-panel/close-panel.js‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,40 @@
11
import Base from "../../core/base";
2-
import dom from "../../core/dom";
2+
import events from "../../core/events";
3+
import utils from "../../core/utils";
34

45
export default Base.extend({
56
name: "close-panel",
67
trigger: ".close-panel",
78

89
init() {
9-
this.el.addEventListener("click", (e) => {
10-
// Find the first element which has a close-panel or is a dialog.
11-
// This should the panel-root itself.
12-
const panel = this.el.closest(".has-close-panel, dialog");
10+
// Close panel support for dialog panels
11+
// Other modals are handled in pat-modal.
12+
const dialog_panel = this.el.closest("dialog");
13+
if (dialog_panel) {
14+
events.add_event_listener(
15+
dialog_panel,
16+
"close-panel",
17+
"close-panel--dialog",
18+
() => {
19+
dialog_panel.close();
20+
}
21+
);
22+
}
1323

14-
if (!panel) {
15-
// Nothing to do. Exiting.
16-
return;
17-
} else if (panel.tagName === "DIALOG") {
18-
// Close the dialog.
19-
panel.close();
20-
} else if (panel.classList.contains("has-close-panel")) {
21-
// Get the close panel method.
22-
const close_method = dom.get_data(panel, "close_panel");
24+
this.el.addEventListener("click", async (e) => {
25+
await utils.timeout(0); // Wait for other patterns, like pat-validation.
2326

24-
// Now execute the method and close the panel.
25-
close_method && close_method(e);
27+
if (
28+
e.target.matches("[type=submit], button:not([type=button])") &&
29+
this.el.closest("form")?.checkValidity() === false
30+
) {
31+
// Prevent closing an invalid form when submitting.
32+
return;
2633
}
34+
35+
this.el.dispatchEvent(
36+
new Event("close-panel", { bubbles: true, cancelable: true })
37+
);
2738
});
2839
},
2940
});

‎src/pat/close-panel/close-panel.test.js‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import utils from "../../core/utils";
2+
import pat_modal from "../modal/modal";
23
import registry from "../../core/registry";
34

45
import "./close-panel";
5-
import "../modal/modal";
66
import "../tooltip/tooltip";
77

88
describe("pat close-panel", function () {
@@ -28,20 +28,21 @@ describe("pat close-panel", function () {
2828
document.querySelector(".pat-tooltip").click();
2929
await utils.timeout(1); // wait a tick async to settle.
3030
expect(document.querySelectorAll(".tooltip-container").length).toBe(1);
31-
expect(document.querySelectorAll(".tooltip-container.has-close-panel").length).toBe(1); // prettier-ignore
3231
expect(document.querySelectorAll(".tooltip-container .pat-tooltip--close-button.close-panel").length).toBe(1); // prettier-ignore
3332

3433
document.querySelector(".pat-tooltip--close-button").click();
35-
await utils.timeout(1); // wait a tick async to settle.
34+
await utils.timeout(1); // close-button is async - wait for it.
35+
await utils.timeout(1); // hide is async - wait for it.
3636
expect(document.querySelectorAll(".tooltip-container").length).toBe(0);
3737
expect(document.querySelectorAll(".tooltip-container .pat-tooltip--close-button").length).toBe(0); // prettier-ignore
3838

3939
document.querySelector("#close-modal").click();
40-
await utils.timeout(1); // wait a tick async to settle.
40+
await utils.timeout(1); // close-button is async - wait for it.
41+
await utils.timeout(1); // destroy is async - wait for it.
4142
expect(document.querySelectorAll(".pat-modal").length).toBe(0);
4243
});
4344

44-
it("Closes a dialog's panel.", function () {
45+
it("Closes a dialog's panel.", async function () {
4546
document.body.innerHTML = `
4647
<dialog open>
4748
<button class="close-panel">close</button>
@@ -55,7 +56,37 @@ describe("pat close-panel", function () {
5556
expect(dialog.open).toBe(true);
5657

5758
document.querySelector(".close-panel").click();
59+
await utils.timeout(1); // close-button is async - wait for it.
5860

5961
expect(dialog.open).toBe(false);
6062
});
63+
64+
it("Prevents closing a panel with an invalid form when submitting but allow to cancel and close.", async function () {
65+
const spy_destroy_modal = jest.spyOn(pat_modal.prototype, "destroy");
66+
67+
document.body.innerHTML = `
68+
<div class="pat-modal">
69+
<form action="." class="pat-validation">
70+
<input name="ok" required />
71+
<button class="close-panel submit">submit</button>
72+
<button class="close-panel cancel" type="button">cancel</button>
73+
</form>
74+
</div>
75+
`;
76+
const el = document.querySelector("form");
77+
78+
registry.scan(document.body);
79+
await utils.timeout(1); // wait a tick for async to settle.
80+
81+
el.querySelector("button.submit").click();
82+
await utils.timeout(1); // wait a tick for async to settle.
83+
84+
expect(spy_destroy_modal).not.toHaveBeenCalled();
85+
86+
// A non-submit close-panel button does not check for validity.
87+
el.querySelector("button.cancel").click();
88+
await utils.timeout(1); // wait a tick for async to settle.
89+
90+
expect(spy_destroy_modal).toHaveBeenCalled();
91+
});
6192
});

‎src/pat/inject/inject.js‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import "../../core/jquery-ext"; // for :scrollable for autoLoading-visible
22
import $ from "jquery";
33
import ajax from "../ajax/ajax";
44
import dom from "../../core/dom";
5+
import events from "../../core/events";
56
import logging from "../../core/logging";
67
import Parser from "../../core/parser";
78
import registry from "../../core/registry";
@@ -578,7 +579,7 @@ const inject = {
578579
},
579580

580581
async _onInjectSuccess($el, cfgs, ev) {
581-
let data = ev && ev.jqxhr && ev.jqxhr.responseText;
582+
let data = ev?.jqxhr?.responseText;
582583
if (!data) {
583584
log.warn("No response content, aborting", ev);
584585
return;
@@ -751,6 +752,34 @@ const inject = {
751752
$el.removeData("pat-inject-triggered")
752753
);
753754

755+
// Prevent closing the panel while injection is in progress.
756+
let do_close_panel = false;
757+
events.add_event_listener(
758+
$el[0],
759+
"close-panel",
760+
"pat-inject--close-panel",
761+
(e) => {
762+
e.stopPropagation();
763+
e.stopImmediatePropagation();
764+
do_close_panel = true;
765+
}
766+
);
767+
$el.on("pat-ajax-success.pat-inject", async () => {
768+
// Wait for the next tick to ensure that the close-panel listener
769+
// is called before this one, even for non-async local injects.
770+
await utils.timeout(1);
771+
// Only close the panel if a close-panel event was catched previously.
772+
if (do_close_panel) {
773+
do_close_panel = false;
774+
// Remove the close-panel event listener.
775+
events.remove_event_listener($el[0], "pat-inject--close-panel");
776+
// Re-trigger close-panel event if it was caught while injection was in progress.
777+
$el[0].dispatchEvent(
778+
new Event("close-panel", { bubbles: true, cancelable: true })
779+
);
780+
}
781+
});
782+
754783
if (cfgs[0].url.length) {
755784
ajax.request($el, {
756785
"url": cfgs[0].url,

‎src/pat/modal/modal.js‎

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import $ from "jquery";
22
import Base from "../../core/base";
33
import Parser from "../../core/parser";
4-
import dom from "../../core/dom";
54
import events from "../../core/events";
65
import inject from "../inject/inject";
76
import registry from "../../core/registry";
@@ -118,8 +117,9 @@ export default Base.extend({
118117
},
119118

120119
_init_handlers() {
121-
this.el.classList.add("has-close-panel");
122-
dom.set_data(this.el, "close_panel", this._close_handler.bind(this));
120+
events.add_event_listener(this.el, "close-panel", "pat-modal--close-panel", () =>
121+
this.destroy()
122+
);
123123

124124
$(document).on("keyup.pat-modal", this._onKeyUp.bind(this));
125125
if (this.options.closing.indexOf("outside") !== -1) {
@@ -158,16 +158,6 @@ export default Base.extend({
158158
}
159159
},
160160

161-
_close_handler(e) {
162-
if (e.target.matches("[type=submit], button:not([type=button])")) {
163-
// submit + close
164-
this.destroy_inject(e);
165-
} else {
166-
// close only
167-
this.destroy();
168-
}
169-
},
170-
171161
getTallestChild() {
172162
let $tallest_child;
173163
for (const child of $("*", this.$el)) {
@@ -206,27 +196,4 @@ export default Base.extend({
206196
$("body").removeClass("modal-active");
207197
$("body").removeClass("modal-panel");
208198
},
209-
210-
destroy_inject(e) {
211-
const button = e.target;
212-
const form = button.form;
213-
214-
if (form && form.classList.contains("pat-inject")) {
215-
// if the modal contains a form with pat-inject, wait for injection
216-
// to be finished and then destroy the modal.
217-
const destroy_handler = () => {
218-
this.destroy();
219-
events.remove_event_listener(form, "pat-modal--destroy-inject");
220-
};
221-
events.add_event_listener(
222-
form,
223-
"pat-inject-success",
224-
"pat-modal--destroy-inject",
225-
destroy_handler.bind(this)
226-
);
227-
} else {
228-
// if working without form injection, just destroy.
229-
this.destroy();
230-
}
231-
},
232199
});

‎src/pat/modal/modal.test.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ describe("pat-modal", function () {
198198
await utils.timeout(1); // wait a tick for async to settle.
199199

200200
document.querySelector("button.close-panel[type=submit]").click();
201+
await utils.timeout(1); // close-button is async - wait for it.
201202
await utils.timeout(1); // wait a tick for pat-inject to settle.
202203
await utils.timeout(1); // wait a tick for pat-modal destroy to settle.
203204

@@ -236,6 +237,7 @@ describe("pat-modal", function () {
236237
await utils.timeout(1); // wait a tick for async to settle.
237238

238239
document.querySelector(".form2 button.close-panel[type=submit]").click();
240+
await utils.timeout(1); // close-button is async - wait for it.
239241
await utils.timeout(1); // wait a tick for pat-inject to settle.
240242
await utils.timeout(1); // wait a tick for pat-modal destroy to settle.
241243

@@ -289,16 +291,15 @@ describe("pat-modal", function () {
289291
await utils.timeout(1); // wait a tick for async to settle.
290292

291293
const spy_destroy = jest.spyOn(instance, "destroy");
292-
const spy_destroy_inject = jest.spyOn(instance, "destroy_inject");
293294

294295
// ``destroy`` was already initialized with instantiating the pattern above.
295296
// Call init again for new instantiation.
296297
instance.init($(".pat-modal"));
297298

298299
document.querySelector("#close-modal").click();
300+
await utils.timeout(1); // close-button is async - wait for it.
299301
await utils.timeout(1); // wait a tick for pat-inject to settle.
300302

301-
expect(spy_destroy_inject).toHaveBeenCalledTimes(1);
302303
expect(spy_destroy).toHaveBeenCalledTimes(1);
303304

304305
// re-attach and re-initialize. Event handler should only be active once.
@@ -307,10 +308,10 @@ describe("pat-modal", function () {
307308
new pattern(document.querySelector(".pat-modal"));
308309
await utils.timeout(1); // wait a tick for async to settle.
309310
document.querySelector("#close-modal").click();
311+
await utils.timeout(1); // close-button is async - wait for it.
310312
await utils.timeout(1); // wait a tick for pat-inject to settle.
311313
// Previous mocks still active.
312314
// Handlers executed exactly once.
313-
expect(spy_destroy_inject).toHaveBeenCalledTimes(1);
314315
expect(spy_destroy).toHaveBeenCalledTimes(1);
315316
});
316317
});

‎src/pat/navigation/navigation.test.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,6 @@ describe("2 - Navigation pattern tests - mark after navigation injection", funct
212212
const w11 = nav.querySelector(".w11");
213213
const a11 = nav.querySelector(".a11");
214214

215-
console.log(document.body.innerHTML);
216-
217215
expect(w1.classList.contains("current")).toBeFalsy();
218216
expect(w1.classList.contains("navigation-in-path")).toBeTruthy();
219217
expect(a1.classList.contains("current")).toBeFalsy();

‎src/pat/notification/notification.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,12 @@ export default Base.extend({
8989
}
9090

9191
if (wrapper.querySelector(".close-panel")) {
92-
wrapper.classList.add("has-close-panel");
93-
dom.set_data(wrapper, "close_panel", this.onClick.bind(this));
92+
events.add_event_listener(
93+
wrapper,
94+
"close-panel",
95+
"pat-notification--close-panel",
96+
this.onClick.bind(this)
97+
);
9498
registry.scan(wrapper, ["close-panel"]);
9599
} else {
96100
events.add_event_listener(

‎src/pat/notification/notification.test.js‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import Pattern from "./notification";
2+
import utils from "../../core/utils";
23

34
describe("pat notification", function () {
45
afterEach(() => {
56
document.body.innerHTML = "";
67
});
78

8-
it("Gets initialized and will disappear as per default settings.", function () {
9+
it("Gets initialized and will disappear as per default settings.", async function () {
910
document.body.innerHTML = `
1011
<p class="pat-notification">
1112
okay
@@ -15,13 +16,18 @@ describe("pat notification", function () {
1516
const el = document.querySelector(".pat-notification");
1617
expect(el.parentNode).toBe(document.body);
1718

18-
new Pattern(el);
19+
const instance = new Pattern(el);
20+
const spy_remove = jest.spyOn(instance, "remove");
1921

2022
expect(el.parentNode).not.toBe(document.body);
2123
expect(el.parentNode.classList.contains("pat-notification-panel")).toBe(true);
22-
expect(el.parentNode.classList.contains("has-close-panel")).toBe(true);
2324

24-
// With jQuery animation, we cannot easily test closing the panel yet.
25-
// TODO: Fix when jQuery animation was removed.
25+
const btn_close = el.parentNode.querySelector(".close-panel");
26+
expect(btn_close).not.toBe(null);
27+
btn_close.click();
28+
29+
await utils.timeout(1); // close-button is async - wait for it.
30+
31+
expect(spy_remove).toHaveBeenCalled();
2632
});
2733
});

‎src/pat/tooltip/tooltip.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import $ from "jquery";
22
import Base from "../../core/base";
3-
import dom from "../../core/dom";
43
import logging from "../../core/logging";
54
import Parser from "../../core/parser";
65
import events from "../../core/events";
@@ -290,8 +289,12 @@ export default Base.extend({
290289
}
291290

292291
// Store reference to method for closing panels on the tooltip element instance.
293-
tippy_classes.push("has-close-panel");
294-
dom.set_data(this.tippy.popper, "close_panel", this.hide.bind(this));
292+
events.add_event_listener(
293+
this.tippy.popper,
294+
"close-panel",
295+
"pat-tooltip--close-panel",
296+
() => this.hide()
297+
);
295298

296299
this.tippy.popper.classList.add(...tippy_classes);
297300

0 commit comments

Comments
 (0)