collective
Browse / github.com/storytold/artcraft / patch
fix(video-editor): end property scrubbing when its field disappears
fix(video-editor): end property scrubbing when its field disappears ## Summary A property scrub registers document pointer listeners that currently survive the field being removed. Subsequent pointer movement can keep previewing edits through the removed field's callback. Escape releasing pointer lock or losing window focus also leaves the scrub alive. Give each scrub an owned, idempotent cleanup path. Remove its listeners when the field disappears or the interaction ends, release only its own pointer lock, and avoid committing through an unmounted field. Ordinary scrubbing still commits once. ## Verification - Actual mounted `NumberField`: frozen main fai
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- rudycelekli (imported from pull request #2098, open)
- Base
28a39a0de4a2fc2eccc237df2046c24245bd12ab- Size
- +167 −5 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
39f95a4faec60d0d8f1de518
How it went
No other operator has reported using it yet.
Diff
raw · frontend/libs/components/video-editor/src/lib/components/ui/number-field.spec.tsx frontend/libs/components/video-editor/src/lib/components/ui/number-field.tsx
new file mode 100644@@ -0,0 +1,141 @@+import { act } from "react";+import { createRoot, type Root } from "react-dom/client";+import { NumberField } from "./number-field";++const ORIGINAL_POINTER_LOCK_ELEMENT = Object.getOwnPropertyDescriptor(document, "pointerLockElement");+const ORIGINAL_EXIT_POINTER_LOCK = Object.getOwnPropertyDescriptor(document, "exitPointerLock");+const ORIGINAL_REQUEST_POINTER_LOCK = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "requestPointerLock");+const ORIGINAL_ACT_ENVIRONMENT = Object.getOwnPropertyDescriptor(globalThis, "IS_REACT_ACT_ENVIRONMENT");++const mounted: Array<{ root: Root; node: HTMLDivElement }> = [];+let pointerLockOwner: Element | null;+let exitPointerLock: ReturnType<typeof vi.fn>;++vi.mock("../../utils/ui", () => ({ cn: (...values: unknown[]) => values.filter(Boolean).join(" ") }));+vi.mock("./button", () => ({ Button: () => null }));++beforeEach(() => {+ pointerLockOwner = null;+ exitPointerLock = vi.fn(() => { pointerLockOwner = null; });+ Object.defineProperty(document, "pointerLockElement", { configurable: true, get: () => pointerLockOwner });+ Object.defineProperty(document, "exitPointerLock", { configurable: true, value: exitPointerLock });+ Object.defineProperty(HTMLElement.prototype, "requestPointerLock", { configurable: true, value: function () { pointerLockOwner = this; } });+ (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;+});++afterEach(() => {+ for (const fixture of mounted.splice(0)) {+ act(() => fixture.root.unmount());+ fixture.node.remove();+ }+ // Also release a leaked baseline session, so it cannot contaminate another test.+ document.dispatchEvent(new MouseEvent("pointerup"));+ restoreDescriptor(document, "pointerLockElement", ORIGINAL_POINTER_LOCK_ELEMENT);+ restoreDescriptor(document, "exitPointerLock", ORIGINAL_EXIT_POINTER_LOCK);+ restoreDescriptor(HTMLElement.prototype, "requestPointerLock", ORIGINAL_REQUEST_POINTER_LOCK);+ restoreDescriptor(globalThis, "IS_REACT_ACT_ENVIRONMENT", ORIGINAL_ACT_ENVIRONMENT);+ vi.restoreAllMocks();+});++it("stops property previews and releases its pointer lock when the field disappears", () => {+ const fixture = mountField();+ fixture.start();+ move(0);+ move(3);+ expect(fixture.values).toEqual([13]);+ act(() => fixture.root.unmount());+ const lockAfterUnmount = pointerLockOwner;+ move(7);+ expect(fixture.values).toEqual([13]);+ expect(lockAfterUnmount).toBeNull();+ expect(fixture.commits).not.toHaveBeenCalled();+});++it("ends the scrub once when Escape releases pointer lock", () => {+ const fixture = mountField();+ fixture.start();+ move(0);+ move(3);+ pointerLockOwner = null;+ act(() => document.dispatchEvent(new Event("pointerlockchange")));+ move(7);+ act(() => document.dispatchEvent(new MouseEvent("pointerup")));+ expect(fixture.values).toEqual([13]);+ expect(fixture.commits).toHaveBeenCalledTimes(1);+});++it("ends the scrub when the window loses focus before pointerup", () => {+ const fixture = mountField();+ fixture.start();+ move(0);+ move(3);+ act(() => window.dispatchEvent(new Event("blur")));+ move(7);+ expect(fixture.values).toEqual([13]);+ expect(fixture.commits).toHaveBeenCalledTimes(1);+ expect(pointerLockOwner).toBeNull();+});++it("does not release a different control's pointer lock when the field disappears", () => {+ const fixture = mountField();+ fixture.start();+ const otherOwner = document.createElement("button");+ pointerLockOwner = otherOwner;+ act(() => fixture.root.unmount());+ act(() => document.dispatchEvent(new MouseEvent("pointerup")));+ expect(exitPointerLock).not.toHaveBeenCalled();+ expect(pointerLockOwner).toBe(otherOwner);+});++it("keeps ordinary scrubbing and commits exactly once on pointerup", () => {+ const fixture = mountField();+ fixture.start();+ move(99); // Ignore the first pointer-lock warp delta, as in the live field.+ move(3);+ move(-1);+ act(() => document.dispatchEvent(new MouseEvent("pointerup")));+ move(100);+ expect(fixture.values).toEqual([13, 12]);+ expect(fixture.commits).toHaveBeenCalledTimes(1);+ expect(pointerLockOwner).toBeNull();+});++it("keeps disabled and non-primary pointer actions inert", () => {+ const fixture = mountField(true);+ fixture.start();+ move(0);+ move(3);+ expect(fixture.values).toEqual([]);+ expect(fixture.commits).not.toHaveBeenCalled();+ expect(pointerLockOwner).toBeNull();+ const enabled = mountField();+ enabled.start(1);+ move(0);+ move(3);+ expect(enabled.values).toEqual([]);+ expect(enabled.commits).not.toHaveBeenCalled();+});++function mountField(disabled = false) {+ const node = document.createElement("div");+ document.body.append(node);+ const root = createRoot(node);+ const values: number[] = [];+ const commits = vi.fn();+ act(() => root.render(<NumberField icon="X" value="10" disabled={disabled} onScrub={(value) => values.push(value)} onScrubEnd={commits} onChange={() => {}} />));+ mounted.push({ root, node });+ return { root, values, commits, start(button = 0) {+ act(() => node.querySelector("button")!.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, button })));+ } };+}++function move(movementX: number) {+ const event = new MouseEvent("pointermove", { bubbles: true });+ Object.defineProperty(event, "movementX", { value: movementX });+ act(() => document.dispatchEvent(event));+}++function restoreDescriptor(target: object, name: string, descriptor: PropertyDescriptor | undefined) {+ if (descriptor) Object.defineProperty(target, name, descriptor);+ else Reflect.deleteProperty(target, name);+}@@ -2,7 +2,7 @@ import { cn } from "../../utils/ui"; import { clamp } from "../../utils/math";-import { useRef, useState, useLayoutEffect, type ComponentProps } from "react";+import { useRef, useState, useEffect, useLayoutEffect, type ComponentProps } from "react"; import { useFocusLock } from "../../hooks/use-focus-lock"; import { Button } from "./button"; import { HugeiconsIcon } from "@hugeicons/react";@@ -139,10 +139,15 @@ function NumberField({ const ghostRef = useRef<HTMLSpanElement>(null); const startValueRef = useRef(0); const cumulativeDeltaRef = useRef(0);+ const cleanupScrubRef = useRef<((commit: boolean) => void) | null>(null); const [isInputFocused, setIsInputFocused] = useState(false); const [suffixLeft, setSuffixLeft] = useState(0); const ghostValue = Array.isArray(value) ? value.join(", ") : String(value ?? ""); + useEffect(() => () => {+ cleanupScrubRef.current?.(false);+ }, []);+ useLayoutEffect(() => { if (!suffix) { setSuffixLeft(0);@@ -166,11 +171,14 @@ function NumberField({ const handleIconPointerDown = (event: React.PointerEvent) => { if (!onScrub || disabled || event.button !== 0) return;+ cleanupScrubRef.current?.(false);+ const scrubTarget = iconRef.current;+ if (!scrubTarget) return; const parsed = parseFloat(String(value ?? "0")); startValueRef.current = Number.isNaN(parsed) ? 0 : parsed; cumulativeDeltaRef.current = 0; let hasReceivedFirstMove = false;- iconRef.current?.requestPointerLock();+ scrubTarget.requestPointerLock(); const handlePointerMove = (moveEvent: PointerEvent) => { // first movementX after pointer lock often contains a bogus warp delta@@ -192,15 +200,28 @@ function NumberField({ onScrub(newValue); }; - const handlePointerUp = () => {+ const cleanup = (commit: boolean) => {+ if (cleanupScrubRef.current !== cleanup) return;+ cleanupScrubRef.current = null; document.removeEventListener("pointermove", handlePointerMove); document.removeEventListener("pointerup", handlePointerUp);- document.exitPointerLock();- onScrubEnd?.();+ document.removeEventListener("pointerlockchange", handlePointerLockChange);+ window.removeEventListener("blur", handlePointerUp);+ if (document.pointerLockElement === scrubTarget) {+ document.exitPointerLock();+ }+ if (commit) onScrubEnd?.();+ };+ const handlePointerUp = () => cleanup(true);+ const handlePointerLockChange = () => {+ if (document.pointerLockElement !== scrubTarget) cleanup(true); }; + cleanupScrubRef.current = cleanup; document.addEventListener("pointermove", handlePointerMove); document.addEventListener("pointerup", handlePointerUp);+ document.addEventListener("pointerlockchange", handlePointerLockChange);+ window.addEventListener("blur", handlePointerUp); }; const canScrub = Boolean(icon && onScrub);