Skip to content

Commit bd3cd75

Browse files
fehmerMiodec
andauthored
refactor: add currentTarget back to regular listeners, add childTarget to onChild listeners (@fehmer) (monkeytypegame#7273)
rework `onChild` to behave mostly like jQuery `.on` with selector. - we remove `currentTarget` from the `onChild` event handler because native events and jQuery events have different values for it - the jQuery `currentTarget` is available with `childTarget` in our events --------- Co-authored-by: Miodec <[email protected]>
1 parent 86ed9c2 commit bd3cd75

File tree

10 files changed

+386
-92
lines changed

10 files changed

+386
-92
lines changed

frontend/__tests__/setup-jsdom.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import $ from "jquery";
2+
3+
//@ts-expect-error add to global
4+
global["$"] = $;
5+
//@ts-expect-error add to global
6+
global["jQuery"] = $;

frontend/__tests__/setup-tests.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import { vi } from "vitest";
22
import $ from "jquery";
33
import { ElementWithUtils } from "../src/ts/utils/dom";
44

5-
//@ts-expect-error add to globl
5+
//@ts-expect-error add to global
66
global["$"] = $;
7-
//@ts-expect-error add to globl
7+
//@ts-expect-error add to global
88
global["jQuery"] = $;
99

1010
vi.mock("../src/ts/constants/env-config", () => ({

frontend/__tests__/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
"noEmit": true
77
},
88
"files": ["vitest.d.ts"],
9-
"include": ["./**/*.spec.ts", "./setup-tests.ts"]
9+
"include": ["./**/*.spec.ts", "./**/*.jsdom-spec.ts", "./setup-tests.ts"]
1010
}
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { screen } from "@testing-library/dom";
3+
import { userEvent } from "@testing-library/user-event";
4+
import { qs } from "../../src/ts/utils/dom";
5+
6+
describe("dom", () => {
7+
describe("ElementWithUtils", () => {
8+
describe("onChild", () => {
9+
const handler = vi.fn();
10+
11+
function registerOnChild(event: string, selector: string): void {
12+
const parent = qs("#parent");
13+
parent?.onChild(event, selector, (e) =>
14+
handler({
15+
target: e.target,
16+
childTarget: e.childTarget,
17+
//@ts-expect-error will be added later, check TODO on the ChildEvent
18+
currentTarget: e.currentTarget,
19+
}),
20+
);
21+
}
22+
23+
beforeEach(() => {
24+
handler.mockReset();
25+
26+
document.body.innerHTML = "";
27+
const root = document.createElement("div");
28+
29+
root.innerHTML = `
30+
<div id="parent" data-testid="parent">
31+
<section id="decoy">
32+
<div id="mid1" data-testid="mid1" class="middle">
33+
<div id="inner1" class="inner">test</div>
34+
<div id="inner2" data-testid="inner2" class="inner">
35+
test
36+
<button id="button" data-testid="button">click</button>
37+
</div>
38+
</div>
39+
<div id="mid2" class="middle">
40+
<div id="inner3" class="inner">test</div>
41+
<div id="inner4" class="inner">test</div>
42+
</div>
43+
</section>
44+
</div>
45+
`;
46+
document.body.appendChild(root);
47+
});
48+
49+
it("should not fire when parent element is clicked", async () => {
50+
//GIVEN
51+
registerOnChild("click", "div");
52+
53+
//WHEN
54+
await userEvent.click(screen.getByTestId("parent"));
55+
56+
//THEN
57+
expect(handler).not.toHaveBeenCalled();
58+
});
59+
60+
it("should fire when selector is clicked", async () => {
61+
//GIVEN
62+
registerOnChild("click", "div");
63+
64+
//WHEN
65+
const clickTarget = screen.getByTestId("mid1");
66+
await userEvent.click(clickTarget);
67+
68+
//THEN
69+
expect(handler).toHaveBeenCalledWith(
70+
expect.objectContaining({
71+
target: clickTarget,
72+
childTarget: clickTarget,
73+
currentTarget: screen.getByTestId("parent"),
74+
}),
75+
);
76+
});
77+
78+
it("should fire when child of selector is clicked", async () => {
79+
//GIVEN
80+
registerOnChild("click", "div.middle");
81+
82+
//WHEN
83+
const selectorTarget = screen.getByTestId("mid1");
84+
const clickTarget = screen.getByTestId("button");
85+
await userEvent.click(clickTarget);
86+
87+
//THEN
88+
expect(handler).toHaveBeenCalledWith(
89+
expect.objectContaining({
90+
target: clickTarget,
91+
childTarget: selectorTarget,
92+
currentTarget: screen.getByTestId("parent"),
93+
}),
94+
);
95+
});
96+
97+
it("should fire on each element matching the selector from the child up to the parent", async () => {
98+
//GIVEN
99+
registerOnChild("click", "div.middle, div.inner");
100+
101+
//WHEN
102+
let clickTarget = screen.getByTestId("button");
103+
await userEvent.click(clickTarget);
104+
105+
//THEN
106+
107+
//This is the same behavior as jQuery `.on` with selector.
108+
//The handler will be called two times,
109+
//It does NOT call on the <section> or the parent element itself
110+
expect(handler).toHaveBeenCalledTimes(2);
111+
112+
//First call is for childTarget inner2 (grand child of parent)
113+
expect(handler).toHaveBeenNthCalledWith(
114+
1,
115+
expect.objectContaining({
116+
target: clickTarget,
117+
childTarget: screen.getByTestId("inner2"),
118+
currentTarget: screen.getByTestId("parent"),
119+
}),
120+
);
121+
122+
//Second call is for childTarget mid1 (child of parent)
123+
expect(handler).toHaveBeenNthCalledWith(
124+
2,
125+
expect.objectContaining({
126+
target: clickTarget,
127+
childTarget: screen.getByTestId("mid1"),
128+
currentTarget: screen.getByTestId("parent"),
129+
}),
130+
);
131+
132+
//WHEN click on mid1 handler is only called one time
133+
handler.mockReset();
134+
clickTarget = screen.getByTestId("mid1");
135+
await userEvent.click(clickTarget);
136+
137+
//THEN
138+
expect(handler).toHaveBeenCalledTimes(1);
139+
140+
expect(handler).toHaveBeenCalledWith(
141+
expect.objectContaining({
142+
target: clickTarget,
143+
childTarget: clickTarget,
144+
currentTarget: screen.getByTestId("parent"),
145+
}),
146+
);
147+
});
148+
});
149+
});
150+
});

frontend/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@
6161
"@fortawesome/fontawesome-free": "5.15.4",
6262
"@monkeytype/oxlint-config": "workspace:*",
6363
"@monkeytype/typescript-config": "workspace:*",
64+
"@testing-library/dom": "10.4.1",
65+
"@testing-library/user-event": "14.6.1",
6466
"@types/canvas-confetti": "1.4.3",
6567
"@types/chartjs-plugin-trendline": "1.0.1",
6668
"@types/damerau-levenshtein": "1.0.0",

frontend/src/ts/elements/input-validation.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from "@monkeytype/schemas/configs";
99
import Config, { setConfig } from "../config";
1010
import * as Notifications from "../elements/notifications";
11-
import { DomUtilsEvent, ElementWithUtils } from "../utils/dom";
11+
import { ElementWithUtils } from "../utils/dom";
1212

1313
export type ValidationResult = {
1414
status: "checking" | "success" | "failed" | "warning";
@@ -60,7 +60,7 @@ export function createInputEventHandler<T>(
6060
callback: (result: ValidationResult) => void,
6161
validation: Validation<T>,
6262
inputValueConvert?: (val: string) => T,
63-
): (e: DomUtilsEvent) => Promise<void> {
63+
): (e: Event) => Promise<void> {
6464
let callIsValid =
6565
validation.isValid !== undefined
6666
? debounceIfNeeded(

frontend/src/ts/utils/dom.ts

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,18 @@ type ElementWithValue =
105105
| HTMLTextAreaElement
106106
| HTMLSelectElement;
107107

108-
export type DomUtilsEvent<T extends Event = Event> = Omit<T, "currentTarget">;
108+
//TODO: after the migration from jQuery to dom-utils we might want to add currentTarget back to the event object, if we have a use-case for it.
109+
// For now we remove it because currentTarget is not the same element when using dom-utils intead of jQuery to get compile errors.
110+
export type OnChildEvent<T extends Event = Event> = Omit<T, "currentTarget"> & {
111+
/**
112+
* target element matching the selector. This emulates the behavior of `currentTarget` in jQuery events registered with `.on(events, selector, handler)`
113+
*/
114+
childTarget: EventTarget | null;
115+
};
109116

110-
type DomUtilsEventListenerOrEventListenerObject =
111-
| { (evt: DomUtilsEvent): void }
112-
| { handleEvent(object: DomUtilsEvent): void };
117+
type OnChildEventListenerOrEventListenerObject =
118+
| { (evt: OnChildEvent): void }
119+
| { handleEvent(object: OnChildEvent): void };
113120

114121
export class ElementWithUtils<T extends HTMLElement = HTMLElement> {
115122
/**
@@ -244,56 +251,80 @@ export class ElementWithUtils<T extends HTMLElement = HTMLElement> {
244251
*/
245252
on<K extends keyof HTMLElementEventMap>(
246253
event: K,
247-
handler: (this: T, ev: DomUtilsEvent<HTMLElementEventMap[K]>) => void,
254+
handler: (this: T, ev: HTMLElementEventMap[K]) => void,
248255
): this;
249-
on(event: string, handler: DomUtilsEventListenerOrEventListenerObject): this;
256+
on(event: string, handler: EventListenerOrEventListenerObject): this;
250257
on(
251258
event: keyof HTMLElementEventMap | string,
252259
handler:
253-
| DomUtilsEventListenerOrEventListenerObject
254-
| ((this: T, ev: DomUtilsEvent) => void),
260+
| EventListenerOrEventListenerObject
261+
| ((this: T, ev: Event) => void),
255262
): this {
256263
// this type was some AI magic but if it works it works
257264
this.native.addEventListener(
258265
event,
259-
handler as DomUtilsEventListenerOrEventListenerObject,
266+
handler as EventListenerOrEventListenerObject,
260267
);
261268
return this;
262269
}
263270

264271
/**
265-
* Attach an event listener to child elements matching the query.
272+
* Attach an event listener to child elements matching the selector.
266273
* Useful for dynamically added elements.
274+
*
275+
* The handler is not called when the event occurs directly on the bound element, but only for descendants (inner elements)
276+
* that match the selector. Bubbles the event from the event target up to the element where the handler is attached
277+
* (i.e., innermost to outermost element) and runs the handler for any elements along that path matching the selector.
267278
*/
268279
onChild<K extends keyof HTMLElementEventMap>(
269280
event: K,
270-
query: string,
281+
/**
282+
* A selector string to filter the descendants of the selected elements that will call the handler.
283+
*/
284+
selector: string,
271285
handler: (
272286
this: HTMLElement,
273-
ev: DomUtilsEvent<HTMLElementEventMap[K]>,
287+
ev: OnChildEvent<HTMLElementEventMap[K]>,
274288
) => void,
275289
): this;
276290
onChild(
277291
event: string,
278-
query: string,
279-
handler: DomUtilsEventListenerOrEventListenerObject,
292+
/**
293+
* A selector string to filter the descendants of the selected elements that will call the handler.
294+
*/
295+
selector: string,
296+
handler: OnChildEventListenerOrEventListenerObject,
280297
): this;
281298
onChild(
282299
event: keyof HTMLElementEventMap | string,
283-
query: string,
300+
/**
301+
* A selector string to filter the descendants of the selected elements that will call the handler.
302+
*/
303+
selector: string,
284304
handler:
285-
| DomUtilsEventListenerOrEventListenerObject
286-
| ((this: HTMLElement, ev: DomUtilsEvent) => void),
305+
| OnChildEventListenerOrEventListenerObject
306+
| ((this: HTMLElement, ev: OnChildEvent) => void),
287307
): this {
288-
// this type was some AI magic but if it works it works
289308
this.native.addEventListener(event, (e) => {
290309
const target = e.target as HTMLElement;
291-
if (target !== null && target.matches(query)) {
310+
if (target === null) return; //ignore event
311+
312+
let childTarget = target.closest(selector);
313+
//bubble up until no match found or the parent element is reached
314+
while (childTarget !== null && childTarget !== this.native) {
292315
if (typeof handler === "function") {
293-
handler.call(target, e);
316+
handler.call(
317+
childTarget as HTMLElement,
318+
Object.assign(e, { childTarget }),
319+
);
294320
} else {
295-
handler.handleEvent(e);
321+
handler.handleEvent(Object.assign(e, { childTarget }));
296322
}
323+
324+
childTarget =
325+
childTarget.parentElement !== null
326+
? childTarget.parentElement.closest(selector)
327+
: null;
297328
}
298329
});
299330
return this;
@@ -680,21 +711,20 @@ export class ElementsWithUtils<
680711
*/
681712
on<K extends keyof HTMLElementEventMap>(
682713
event: K,
683-
handler: (this: T, ev: DomUtilsEvent<HTMLElementEventMap[K]>) => void,
714+
handler: (this: T, ev: HTMLElementEventMap[K]) => void,
684715
): this;
685-
on(event: string, handler: DomUtilsEventListenerOrEventListenerObject): this;
716+
on(event: string, handler: EventListenerOrEventListenerObject): this;
686717
on(
687718
event: keyof HTMLElementEventMap | string,
688719
handler:
689-
| DomUtilsEventListenerOrEventListenerObject
690-
| ((this: T, ev: DomUtilsEvent) => void),
720+
| EventListenerOrEventListenerObject
721+
| ((this: T, ev: Event) => void),
691722
): this {
692723
for (const item of this) {
693724
item.on(event, handler);
694725
}
695726
return this;
696727
}
697-
698728
/**
699729
* Set attribute value on all elements in the array
700730
*/

0 commit comments

Comments
 (0)