mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 07:40:43 +00:00
cleanup: drop unused disposer return from installSafariPinchZoomBlocker (#3992)
Follow-up to #3901 (cc @evanpelle). ## Description: In the review on #3901, evanpelle pointed out that the disposer returned by `installSafariPinchZoomBlocker` is never called at the call site in `Main.ts`, and asked whether there's any reason to return it. There isn't — the listeners live for the document's lifetime and the browser releases them on teardown — so this PR drops the disposer. ### Changes - `installSafariPinchZoomBlocker` now returns `void`. Removed the `return () => { ... }` block and the `@returns` JSDoc line. Added a sentence explaining why no disposer is needed. - Tests: dropped the disposer-removal test, switched the behavior tests to use fresh detached `<div>` elements (no document state leak across tests), and verified the default-target = `document` case with `vi.spyOn(document, 'addEventListener').mockImplementation(() => {})` so no real listener actually attaches to the shared jsdom document. Net diff: -23 lines (30 insertions, 53 deletions). ### What I tested - `npm test` — 1245 + 65 tests pass, including the 4 surviving tests for this helper - `npm run build-prod` — succeeds (tsc + vite) - `npx eslint` — clean - `npx prettier --check` on the touched files — clean ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: @vansszh
This commit is contained in:
@@ -13,16 +13,18 @@
|
||||
* (Chrome, Firefox, every Android browser) treat the listeners as a no-op,
|
||||
* so it is safe to install them unconditionally.
|
||||
*
|
||||
* The listeners live for the document's lifetime; the browser releases them
|
||||
* when the page is torn down, so no disposer is needed.
|
||||
*
|
||||
* @param target - The EventTarget to attach the listeners to. Defaults to
|
||||
* `document`, which is the scope Safari uses to decide whether to zoom
|
||||
* the page.
|
||||
* @returns A function that removes the installed listeners.
|
||||
*
|
||||
* @see https://github.com/openfrontio/OpenFrontIO/issues/2330
|
||||
*/
|
||||
export function installSafariPinchZoomBlocker(
|
||||
target: EventTarget = document,
|
||||
): () => void {
|
||||
): void {
|
||||
const block = (e: Event) => {
|
||||
e.preventDefault();
|
||||
};
|
||||
@@ -31,10 +33,4 @@ export function installSafariPinchZoomBlocker(
|
||||
for (const type of events) {
|
||||
target.addEventListener(type, block);
|
||||
}
|
||||
|
||||
return () => {
|
||||
for (const type of events) {
|
||||
target.removeEventListener(type, block);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -15,69 +15,50 @@ function dispatchCancelableGestureEvent(
|
||||
}
|
||||
|
||||
describe("installSafariPinchZoomBlocker", () => {
|
||||
it("prevents the default action of each Safari gesture event on document", () => {
|
||||
const uninstall = installSafariPinchZoomBlocker();
|
||||
|
||||
try {
|
||||
for (const type of GESTURE_EVENTS) {
|
||||
const event = dispatchCancelableGestureEvent(document, type);
|
||||
expect(event.defaultPrevented).toBe(true);
|
||||
}
|
||||
} finally {
|
||||
uninstall();
|
||||
}
|
||||
});
|
||||
|
||||
it("attaches listeners to the provided target", () => {
|
||||
it("calls preventDefault on each Safari gesture event dispatched at the target", () => {
|
||||
const target = document.createElement("div");
|
||||
const uninstall = installSafariPinchZoomBlocker(target);
|
||||
|
||||
try {
|
||||
for (const type of GESTURE_EVENTS) {
|
||||
const event = dispatchCancelableGestureEvent(target, type);
|
||||
expect(event.defaultPrevented).toBe(true);
|
||||
}
|
||||
} finally {
|
||||
uninstall();
|
||||
}
|
||||
});
|
||||
|
||||
it("removes the listeners when the returned disposer is called", () => {
|
||||
const target = document.createElement("div");
|
||||
const uninstall = installSafariPinchZoomBlocker(target);
|
||||
uninstall();
|
||||
installSafariPinchZoomBlocker(target);
|
||||
|
||||
for (const type of GESTURE_EVENTS) {
|
||||
const event = dispatchCancelableGestureEvent(target, type);
|
||||
expect(event.defaultPrevented).toBe(false);
|
||||
expect(event.defaultPrevented).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("does not affect events on unrelated targets", () => {
|
||||
const scope = document.createElement("div");
|
||||
const other = document.createElement("div");
|
||||
const uninstall = installSafariPinchZoomBlocker(scope);
|
||||
it("defaults to attaching the listeners to document", () => {
|
||||
const addEventListenerSpy = vi
|
||||
.spyOn(document, "addEventListener")
|
||||
.mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
const event = dispatchCancelableGestureEvent(other, "gesturestart");
|
||||
expect(event.defaultPrevented).toBe(false);
|
||||
installSafariPinchZoomBlocker();
|
||||
|
||||
for (const type of GESTURE_EVENTS) {
|
||||
expect(addEventListenerSpy).toHaveBeenCalledWith(
|
||||
type,
|
||||
expect.any(Function),
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
uninstall();
|
||||
addEventListenerSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not affect events dispatched at unrelated targets", () => {
|
||||
const scope = document.createElement("div");
|
||||
const other = document.createElement("div");
|
||||
installSafariPinchZoomBlocker(scope);
|
||||
|
||||
const event = dispatchCancelableGestureEvent(other, "gesturestart");
|
||||
expect(event.defaultPrevented).toBe(false);
|
||||
});
|
||||
|
||||
it("leaves unrelated event types alone", () => {
|
||||
const uninstall = installSafariPinchZoomBlocker();
|
||||
const target = document.createElement("div");
|
||||
installSafariPinchZoomBlocker(target);
|
||||
|
||||
try {
|
||||
const event = new Event("touchstart", {
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
});
|
||||
document.dispatchEvent(event);
|
||||
expect(event.defaultPrevented).toBe(false);
|
||||
} finally {
|
||||
uninstall();
|
||||
}
|
||||
const event = new Event("touchstart", { bubbles: true, cancelable: true });
|
||||
target.dispatchEvent(event);
|
||||
expect(event.defaultPrevented).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user