From 0f704b95a4e7966f37903ff83b32f4869bbbb0b2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 31 May 2026 07:21:22 -0700 Subject: [PATCH] fix: target current browser page for grab mode --- .../browser-pane/useGrabMode.test.ts | 87 +++++++++++++++++++ .../components/browser-pane/useGrabMode.ts | 3 + 2 files changed, 90 insertions(+) create mode 100644 src/renderer/src/components/browser-pane/useGrabMode.test.ts diff --git a/src/renderer/src/components/browser-pane/useGrabMode.test.ts b/src/renderer/src/components/browser-pane/useGrabMode.test.ts new file mode 100644 index 00000000000..048a3dc916a --- /dev/null +++ b/src/renderer/src/components/browser-pane/useGrabMode.test.ts @@ -0,0 +1,87 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +function createReactHookHarness() { + const refs: { current: unknown }[] = [] + const states: unknown[] = [] + const effects: { effect: () => void | (() => void); deps: readonly unknown[] | undefined }[] = [] + let refIndex = 0 + let stateIndex = 0 + + return { + beginRender: () => { + refIndex = 0 + stateIndex = 0 + effects.length = 0 + }, + effects, + react: { + useCallback: unknown>(callback: T): T => callback, + useEffect: (effect: () => void | (() => void), deps?: readonly unknown[]) => { + effects.push({ effect, deps }) + }, + useRef: (initialValue: T): { current: T } => { + const index = refIndex + refIndex += 1 + refs[index] ??= { current: initialValue } + return refs[index] as { current: T } + }, + useState: (initialValue: T): [T, (value: T) => void] => { + const index = stateIndex + stateIndex += 1 + states[index] ??= initialValue + return [ + states[index] as T, + (value: T) => { + states[index] = value + } + ] + } + } + } +} + +describe('useGrabMode', () => { + afterEach(() => { + vi.doUnmock('react') + vi.doUnmock('@/hooks/useMountedRef') + vi.resetModules() + vi.unstubAllGlobals() + }) + + it('uses the latest browser page when toggled before the page-change effect runs', async () => { + const harness = createReactHookHarness() + const setGrabMode = vi.fn(async () => ({ ok: true })) + vi.doMock('react', () => harness.react) + vi.doMock('@/hooks/useMountedRef', () => ({ + useMountedRef: () => ({ current: true }) + })) + vi.stubGlobal('window', { + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + api: { + browser: { + setGrabMode, + awaitGrabSelection: vi.fn(() => new Promise(() => {})), + cancelGrab: vi.fn() + } + } + }) + const { useGrabMode } = await import('./useGrabMode') + const render = (browserPageId: string) => { + harness.beginRender() + // oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly. + return useGrabMode(browserPageId) + } + + render('page-1') + harness.effects[0]?.effect() + const grab = render('page-2') + grab.toggle() + await Promise.resolve() + + expect(setGrabMode).toHaveBeenCalledWith({ + browserPageId: 'page-2', + enabled: true + }) + }) +}) diff --git a/src/renderer/src/components/browser-pane/useGrabMode.ts b/src/renderer/src/components/browser-pane/useGrabMode.ts index 26a25493562..ccf9851ffb5 100644 --- a/src/renderer/src/components/browser-pane/useGrabMode.ts +++ b/src/renderer/src/components/browser-pane/useGrabMode.ts @@ -46,6 +46,9 @@ export function useGrabMode(browserPageId: string): GrabModeHook { const activeOpIdRef = useRef(null) const grabTabIdRef = useRef(null) const browserTabIdRef = useRef(browserPageId) + // Why: toolbar/key handlers from the latest render can fire before passive + // effects run after a page switch, so keep the target page current in render. + browserTabIdRef.current = browserPageId const mountedRef = useMountedRef() // Why: when the browser page changes while grab is active, cancel the