From 079de160ca7e00464cc0938c64bb7cd16d47347b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 30 May 2026 17:39:21 -0700 Subject: [PATCH] Move contextual copy toast cleanup to editor dispose (#3356) * Move contextual copy toast cleanup to editor dispose * Fix contextual copy dispose cleanup Co-authored-by: Orca --------- Co-authored-by: Jinwoo-H Co-authored-by: Orca --- .../editor/setup-contextual-copy.test.ts | 108 ++++++++++++++++++ .../editor/setup-contextual-copy.ts | 7 ++ .../editor/useContextualCopySetup.tsx | 11 +- 3 files changed, 116 insertions(+), 10 deletions(-) create mode 100644 src/renderer/src/components/editor/setup-contextual-copy.test.ts diff --git a/src/renderer/src/components/editor/setup-contextual-copy.test.ts b/src/renderer/src/components/editor/setup-contextual-copy.test.ts new file mode 100644 index 00000000000..ce3da798b73 --- /dev/null +++ b/src/renderer/src/components/editor/setup-contextual-copy.test.ts @@ -0,0 +1,108 @@ +import type { editor } from 'monaco-editor' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { setupContextualCopy } from './setup-contextual-copy' + +vi.mock('@/hooks/useShortcutLabel', () => ({ + formatShortcutLabel: () => 'Copy Context' +})) + +vi.mock('@/lib/monaco-setup', () => ({ + monaco: { + editor: { + ContentWidgetPositionPreference: { + ABOVE: 1, + BELOW: 2 + } + } + } +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ keybindings: {} }) + } +})) + +vi.mock('@/lib/primary-selection', () => ({ + PRIMARY_SELECTION_MAX_LENGTH: 10_000, + isPrimarySelectionEnabled: () => false, + setPrimarySelectionText: () => {} +})) + +describe('setupContextualCopy', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('clears editor-scoped contextual copy cleanup on dispose', () => { + const clearTimeout = vi.fn() + const clearInterval = vi.fn() + vi.stubGlobal('window', { + clearInterval, + clearTimeout, + setInterval: vi.fn(() => 1), + setTimeout: vi.fn(() => 2) + }) + vi.stubGlobal('document', { + createElement: () => ({ + className: '', + offsetHeight: 28, + style: { display: '' }, + textContent: '' + }) + }) + + const editorDomNode = { + addEventListener: vi.fn(), + removeEventListener: vi.fn() + } + const selectionDispose = vi.fn() + const scrollDispose = vi.fn() + const focusDispose = vi.fn() + const blurDispose = vi.fn() + let disposeEditor = (): void => {} + const editorInstance = { + addContentWidget: vi.fn(), + getContainerDomNode: () => editorDomNode, + getModel: () => null, + getSelection: () => null, + hasTextFocus: () => false, + layoutContentWidget: vi.fn(), + onDidBlurEditorText: () => ({ dispose: blurDispose }), + onDidChangeCursorSelection: () => ({ dispose: selectionDispose }), + onDidDispose: (listener: () => void) => { + disposeEditor = listener + return { dispose: vi.fn() } + }, + onDidFocusEditorText: () => ({ dispose: focusDispose }), + onDidScrollChange: () => ({ dispose: scrollDispose }), + removeContentWidget: vi.fn() + } as unknown as editor.IStandaloneCodeEditor + const copyToastTimeoutRef = { current: 42 } + const setCopyToast = vi.fn() + + setupContextualCopy({ + editorInstance, + filePath: 'src/example.ts', + setCopyToast, + propsRef: { + current: { + language: 'typescript', + relativePath: 'src/example.ts' + } + }, + copyToastTimeoutRef + }) + + disposeEditor() + + expect(selectionDispose).toHaveBeenCalledTimes(1) + expect(scrollDispose).toHaveBeenCalledTimes(1) + expect(focusDispose).toHaveBeenCalledTimes(1) + expect(blurDispose).toHaveBeenCalledTimes(1) + expect(clearTimeout).toHaveBeenCalledWith(42) + expect(copyToastTimeoutRef.current).toBeNull() + expect(setCopyToast).toHaveBeenCalledWith(null) + expect(editorDomNode.removeEventListener).toHaveBeenCalledTimes(3) + }) +}) diff --git a/src/renderer/src/components/editor/setup-contextual-copy.ts b/src/renderer/src/components/editor/setup-contextual-copy.ts index 71eadd719ca..6f94b7a5e88 100644 --- a/src/renderer/src/components/editor/setup-contextual-copy.ts +++ b/src/renderer/src/components/editor/setup-contextual-copy.ts @@ -300,6 +300,13 @@ export function setupContextualCopy({ scrollListener.dispose() focusListener.dispose() blurListener.dispose() + // Why: the confirmation toast timeout belongs to the Monaco editor that + // scheduled it, so editor disposal is the earliest reliable cleanup point. + if (copyToastTimeoutRef.current !== null) { + window.clearTimeout(copyToastTimeoutRef.current) + copyToastTimeoutRef.current = null + setCopyToast(null) + } if (primarySelectionTimer !== null) { window.clearTimeout(primarySelectionTimer) primarySelectionTimer = null diff --git a/src/renderer/src/components/editor/useContextualCopySetup.tsx b/src/renderer/src/components/editor/useContextualCopySetup.tsx index c65e0783f3c..5c3f31dbbfc 100644 --- a/src/renderer/src/components/editor/useContextualCopySetup.tsx +++ b/src/renderer/src/components/editor/useContextualCopySetup.tsx @@ -1,4 +1,4 @@ -import React, { useRef, useState, useEffect, useCallback } from 'react' +import React, { useRef, useState, useCallback } from 'react' import type { editor } from 'monaco-editor' import { setupContextualCopy } from './setup-contextual-copy' @@ -6,15 +6,6 @@ export function useContextualCopySetup() { const [copyToast, setCopyToast] = useState<{ left: number; top: number } | null>(null) const copyToastTimeoutRef = useRef(null) - useEffect(() => { - const toastRef = copyToastTimeoutRef - return () => { - if (toastRef.current !== null) { - window.clearTimeout(toastRef.current) - } - } - }, []) - const setupCopy = useCallback( ( editorInstance: editor.IStandaloneCodeEditor,