From 2b5bf56475c5a8afd2ccd40df4e63e58d4d88c87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Mon, 18 May 2026 14:40:56 +0800 Subject: [PATCH] Make selection actions track selected text instead of the cursor The add-to-chat popover was keyed off the mouse-up location, so broad selections could cover the selected text and stale popovers could remain visible. Shared placement and dismissal logic now anchors the action to the selected range, prefers the space above the selection like Codex App, and clears stale selections on outside clicks. Constraint: Desktop chat and workspace previews share the same selection affordance and must keep their existing add-reference behavior. Rejected: Keep cursor-based placement | it can hide selected text and makes multi-line selections unpredictable. Confidence: high Scope-risk: moderate Directive: Keep future selection actions range-anchored and outside-click dismissible across chat and workspace surfaces. Tested: bun run check:desktop Tested: Browser verification on http://127.0.0.1:5174 showed the popover above selection, no overlap, and outside click clearing it. Not-tested: Native packaged Tauri window. Related: https://github.com/NanmiCoder/cc-haha/issues/484 --- .../src/components/chat/MessageList.test.tsx | 78 +++++++++++++++- desktop/src/components/chat/MessageList.tsx | 65 ++++++------- .../workspace/WorkspacePanel.test.tsx | 67 +++++++++++-- .../components/workspace/WorkspacePanel.tsx | 81 ++++++++-------- .../src/hooks/useSelectionPopoverDismiss.ts | 93 +++++++++++++++++++ desktop/src/i18n/locales/zh.ts | 4 +- 6 files changed, 304 insertions(+), 84 deletions(-) create mode 100644 desktop/src/hooks/useSelectionPopoverDismiss.ts diff --git a/desktop/src/components/chat/MessageList.test.tsx b/desktop/src/components/chat/MessageList.test.tsx index 3db66ebd..2a33d50b 100644 --- a/desktop/src/components/chat/MessageList.test.tsx +++ b/desktop/src/components/chat/MessageList.test.tsx @@ -905,8 +905,8 @@ describe('MessageList nested tool calls', () => { await selectMessageText(userText, 'workspace selection behavior') const floatingAddButton = screen.getByRole('button', { name: 'Add to chat' }) - expect(floatingAddButton.style.left).toBe('260px') - expect(floatingAddButton.style.top).toBe('112px') + expect(floatingAddButton.style.left).toBe('141px') + expect(floatingAddButton.style.top).toBe('26px') fireEvent.click(floatingAddButton) @@ -955,6 +955,80 @@ describe('MessageList nested tool calls', () => { ]) }) + it('dismisses the selected-message action when clicking outside the popover', async () => { + useChatStore.setState({ + sessions: { + [ACTIVE_TAB]: makeSessionState({ + messages: [{ + id: 'assistant-1', + type: 'assistant_text', + content: 'Clicking outside should clear this selected reply.', + timestamp: 1, + }], + }), + }, + }) + + render() + + const assistantText = screen.getByText(/Clicking outside should clear/) + await selectMessageText(assistantText, 'selected reply') + expect(screen.getByRole('button', { name: 'Add to chat' })).toBeTruthy() + + await act(async () => { + fireEvent.pointerDown(document.body) + await Promise.resolve() + }) + + expect(screen.queryByRole('button', { name: 'Add to chat' })).toBeNull() + expect(window.getSelection()?.toString()).toBe('') + }) + + it('keeps only the latest selected-message action when selecting across messages', async () => { + useChatStore.setState({ + sessions: { + [ACTIVE_TAB]: makeSessionState({ + messages: [ + { + id: 'assistant-1', + type: 'assistant_text', + content: 'First assistant reply can be selected.', + timestamp: 1, + }, + { + id: 'assistant-2', + type: 'assistant_text', + content: 'Second assistant reply should replace it.', + timestamp: 2, + }, + ], + }), + }, + }) + + render() + + const firstText = screen.getByText(/First assistant reply/) + const secondText = screen.getByText(/Second assistant reply/) + await selectMessageText(firstText, 'First assistant reply') + expect(screen.getAllByRole('button', { name: 'Add to chat' })).toHaveLength(1) + + await act(async () => { + fireEvent.pointerDown(secondText) + await Promise.resolve() + }) + await selectMessageText(secondText, 'Second assistant reply') + + expect(screen.getAllByRole('button', { name: 'Add to chat' })).toHaveLength(1) + fireEvent.click(screen.getByRole('button', { name: 'Add to chat' })) + expect(useWorkspaceChatContextStore.getState().referencesBySession[ACTIVE_TAB]).toMatchObject([ + { + messageId: 'assistant-2', + quote: 'Second assistant reply', + }, + ]) + }) + it('does not force-scroll to the bottom while the user is reading history', async () => { const scrollIntoView = vi.fn() Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', { diff --git a/desktop/src/components/chat/MessageList.tsx b/desktop/src/components/chat/MessageList.tsx index 20b7e1b1..41108e9c 100644 --- a/desktop/src/components/chat/MessageList.tsx +++ b/desktop/src/components/chat/MessageList.tsx @@ -1,5 +1,5 @@ import { useRef, useEffect, useMemo, memo, useState, useCallback, useLayoutEffect, type ReactNode } from 'react' -import { ArrowDown, BookMarked, Bot, CheckCircle2, ChevronDown, ChevronRight, CircleStop, LoaderCircle, Settings, Target, XCircle } from 'lucide-react' +import { ArrowDown, BookMarked, Bot, CheckCircle2, ChevronDown, ChevronRight, CircleStop, LoaderCircle, MessageCircle, Settings, Target, XCircle } from 'lucide-react' import { ApiError } from '../../api/client' import { sessionsApi, type SessionTurnCheckpoint } from '../../api/sessions' import { useChatStore } from '../../stores/chatStore' @@ -22,6 +22,7 @@ import { InlineTaskSummary } from './InlineTaskSummary' import { CurrentTurnChangeCard } from './CurrentTurnChangeCard' import type { AgentTaskNotification, UIMessage } from '../../types/chat' import { ConfirmDialog } from '../shared/ConfirmDialog' +import { clearWindowSelection, getSelectionPopoverPosition, useSelectionPopoverDismiss } from '../../hooks/useSelectionPopoverDismiss' type ToolCall = Extract type ToolResult = Extract @@ -62,11 +63,9 @@ type ChatSelectionState = { y: number } -const CHAT_SELECTION_MENU_OFFSET = 8 - -function clampValue(value: number, min: number, max: number) { - return Math.max(min, Math.min(value, max)) -} +const CHAT_SELECTION_MENU_OFFSET = 10 +const CHAT_SELECTION_MENU_WIDTH = 158 +const CHAT_SELECTION_MENU_HEIGHT = 44 function getElementForNode(node: Node | null): Element | null { if (!node) return null @@ -74,32 +73,12 @@ function getElementForNode(node: Node | null): Element | null { } function getChatSelectionPosition(range: Range, root: HTMLElement, pointer: { clientX: number; clientY: number }) { - const rect = typeof range.getBoundingClientRect === 'function' - ? range.getBoundingClientRect() - : null - const rootRect = root.getBoundingClientRect() - const pointerInsideRoot = - pointer.clientX >= rootRect.left && - pointer.clientX <= rootRect.right && - pointer.clientY >= rootRect.top && - pointer.clientY <= rootRect.bottom - const fallbackX = rect && rect.width > 0 - ? rect.left + rect.width / 2 - : rect?.left ?? rootRect.left + 24 - const fallbackY = rect - ? rect.bottom + CHAT_SELECTION_MENU_OFFSET - : rootRect.top + 24 - const unclampedX = pointerInsideRoot ? pointer.clientX : fallbackX - const unclampedY = pointerInsideRoot ? pointer.clientY + CHAT_SELECTION_MENU_OFFSET : fallbackY - const minX = Math.max(12, rootRect.left + 8) - const maxX = Math.max(minX, Math.min(window.innerWidth - 160, rootRect.right - 136)) - const minY = Math.max(12, rootRect.top + 8) - const maxY = Math.max(minY, Math.min(window.innerHeight - 48, rootRect.bottom - 40)) - - return { - x: clampValue(unclampedX, minX, maxX), - y: clampValue(unclampedY, minY, maxY), - } + return getSelectionPopoverPosition(range, root, { + menuWidth: CHAT_SELECTION_MENU_WIDTH, + menuHeight: CHAT_SELECTION_MENU_HEIGHT, + offset: CHAT_SELECTION_MENU_OFFSET, + fallbackPointer: pointer, + }) } function getChatSelectionFromContainer( @@ -129,22 +108,25 @@ function getChatSelectionFromContainer( function ChatSelectionMenu({ selection, onAdd, + popoverRef, }: { selection: ChatSelectionState | null onAdd: () => void + popoverRef: { current: HTMLButtonElement | null } }) { const t = useTranslation() if (!selection) return null return ( ) @@ -299,6 +281,7 @@ function SelectableChatMessage({ children: ReactNode }) { const rootRef = useRef(null) + const selectionMenuRef = useRef(null) const addReference = useWorkspaceChatContextStore((state) => state.addReference) const [selectionMenu, setSelectionMenu] = useState(null) const t = useTranslation() @@ -310,6 +293,16 @@ function SelectableChatMessage({ setSelectionMenu(null) }, [content, messageId]) + const dismissSelectionMenu = useCallback(() => { + setSelectionMenu(null) + }, []) + + useSelectionPopoverDismiss({ + active: Boolean(selectionMenu), + popoverRef: selectionMenuRef, + onDismiss: dismissSelectionMenu, + }) + const addCurrentSelectionToChat = useCallback(() => { if (!sessionId || !selectionMenu) return addReference(sessionId, { @@ -321,7 +314,7 @@ function SelectableChatMessage({ messageId, }) setSelectionMenu(null) - window.getSelection()?.removeAllRanges() + clearWindowSelection() }, [addReference, messageId, role, selectionMenu, sessionId, sourceName]) return ( @@ -335,7 +328,7 @@ function SelectableChatMessage({ }} > {children} - + ) } diff --git a/desktop/src/components/workspace/WorkspacePanel.test.tsx b/desktop/src/components/workspace/WorkspacePanel.test.tsx index d3a652ec..f5bfaf0c 100644 --- a/desktop/src/components/workspace/WorkspacePanel.test.tsx +++ b/desktop/src/components/workspace/WorkspacePanel.test.tsx @@ -144,13 +144,13 @@ async function selectWorkspaceCodeText( Object.assign(range, { getBoundingClientRect: () => ({ left: 120, - top: 40, + top: 100, right: 240, - bottom: 58, + bottom: 118, width: 120, height: 18, x: 120, - y: 40, + y: 100, toJSON: () => ({}), }), }) @@ -160,7 +160,7 @@ async function selectWorkspaceCodeText( selection?.addRange(range) await act(async () => { - fireEvent.mouseUp(code, { clientX: 180, clientY: 72 }) + fireEvent.mouseUp(code, { clientX: 180, clientY: 122 }) await Promise.resolve() }) await flushReactWork() @@ -1444,8 +1444,8 @@ describe('WorkspacePanel', () => { const addButtons = view.getAllByRole('button', { name: 'Add to chat' }) const floatingAddButton = addButtons[addButtons.length - 1]! - expect(floatingAddButton.style.left).toBe('180px') - expect(floatingAddButton.style.top).toBe('80px') + expect(floatingAddButton.style.left).toBe('101px') + expect(floatingAddButton.style.top).toBe('46px') fireEvent.keyDown(view.getByTestId('workspace-code').parentElement?.parentElement ?? view.getByTestId('workspace-code'), { key: 'Escape', @@ -1454,6 +1454,61 @@ describe('WorkspacePanel', () => { expect(view.queryAllByRole('button', { name: 'Add to chat' })).toHaveLength(1) }) + it('dismisses the selected-code action when clicking outside the popover', async () => { + await setWorkspaceState((state) => ({ + ...state, + panelBySession: { + ...state.panelBySession, + 'session-selection-dismiss': { + isOpen: true, + activeView: 'all', + }, + }, + statusBySession: { + ...state.statusBySession, + 'session-selection-dismiss': { + state: 'ok', + workDir: '/repo', + repoName: 'repo', + branch: 'main', + isGitRepo: true, + changedFiles: [], + }, + }, + previewTabsBySession: { + ...state.previewTabsBySession, + 'session-selection-dismiss': [{ + id: 'file:src/App.ts', + path: 'src/App.ts', + kind: 'file', + title: 'App.ts', + language: 'text', + content: 'const title = "Todo"\nexport default title', + state: 'ok', + size: 42, + }], + }, + activePreviewTabIdBySession: { + ...state.activePreviewTabIdBySession, + 'session-selection-dismiss': 'file:src/App.ts', + }, + })) + + const view = await renderPanel('session-selection-dismiss') + + await selectWorkspaceCodeText(view, 1, 'const title = "Todo"', 1, 'const title = "Todo"') + expect(view.getAllByRole('button', { name: 'Add to chat' })).toHaveLength(2) + + await act(async () => { + fireEvent.pointerDown(document.body) + await Promise.resolve() + }) + await flushReactWork() + + expect(view.queryAllByRole('button', { name: 'Add to chat' })).toHaveLength(1) + expect(window.getSelection()?.toString()).toBe('') + }) + it('adds selected markdown text from a preview to the chat context', async () => { await setWorkspaceState((state) => ({ ...state, diff --git a/desktop/src/components/workspace/WorkspacePanel.tsx b/desktop/src/components/workspace/WorkspacePanel.tsx index 1adfa216..3bd6c2b6 100644 --- a/desktop/src/components/workspace/WorkspacePanel.tsx +++ b/desktop/src/components/workspace/WorkspacePanel.tsx @@ -1,4 +1,5 @@ -import { useEffect, useMemo, useRef, useState, type MouseEvent } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState, type MouseEvent } from 'react' +import { MessageCircle } from 'lucide-react' import { Highlight } from 'prism-react-renderer' import type { WorkspaceChangedFile, @@ -18,6 +19,7 @@ import { useChatStore } from '../../stores/chatStore' import { useWorkspaceChatContextStore } from '../../stores/workspaceChatContextStore' import { useUIStore } from '../../stores/uiStore' import { copyTextToClipboard } from '../chat/clipboard' +import { clearWindowSelection, getSelectionPopoverPosition, useSelectionPopoverDismiss } from '../../hooks/useSelectionPopoverDismiss' import { MarkdownRenderer } from '../markdown/MarkdownRenderer' import { getFileExtension, @@ -84,7 +86,9 @@ const FILE_STATUS_META: Record = {} const EMPTY_PREVIEW_TABS: WorkspacePreviewTab[] = [] const EMPTY_EXPANDED_PATHS: string[] = [] -const SELECTION_MENU_OFFSET = 8 +const SELECTION_MENU_OFFSET = 10 +const SELECTION_MENU_WIDTH = 158 +const SELECTION_MENU_HEIGHT = 44 const FILE_BADGE_META: Record = { ts: { label: 'TS', className: 'bg-[var(--color-secondary)]/14 text-[var(--color-secondary)]' }, tsx: { label: 'TSX', className: 'bg-[var(--color-secondary)]/14 text-[var(--color-secondary)]' }, @@ -239,37 +243,13 @@ function getLineNumberFromNode(node: Node | null, root: HTMLElement) { return Number.isFinite(line) ? line : undefined } -function clampValue(value: number, min: number, max: number) { - return Math.max(min, Math.min(value, max)) -} - function getSelectionPosition(range: Range, root: HTMLElement, pointer?: SelectionPointer) { - const rect = typeof range.getBoundingClientRect === 'function' - ? range.getBoundingClientRect() - : null - const rootRect = root.getBoundingClientRect() - const pointerInsideRoot = pointer - && pointer.clientX >= rootRect.left - && pointer.clientX <= rootRect.right - && pointer.clientY >= rootRect.top - && pointer.clientY <= rootRect.bottom - const fallbackX = rect && rect.width > 0 - ? rect.left + rect.width / 2 - : rect?.left ?? rootRect.left + 24 - const fallbackY = rect - ? rect.bottom + SELECTION_MENU_OFFSET - : rootRect.top + 24 - const unclampedX = pointerInsideRoot ? pointer.clientX : fallbackX - const unclampedY = pointerInsideRoot ? pointer.clientY + SELECTION_MENU_OFFSET : fallbackY - const minX = Math.max(12, rootRect.left + 8) - const maxX = Math.max(minX, Math.min(window.innerWidth - 160, rootRect.right - 136)) - const minY = Math.max(12, rootRect.top + 8) - const maxY = Math.max(minY, Math.min(window.innerHeight - 48, rootRect.bottom - 40)) - - return { - x: clampValue(unclampedX, minX, maxX), - y: clampValue(unclampedY, minY, maxY), - } + return getSelectionPopoverPosition(range, root, { + menuWidth: SELECTION_MENU_WIDTH, + menuHeight: SELECTION_MENU_HEIGHT, + offset: SELECTION_MENU_OFFSET, + fallbackPointer: pointer, + }) } function getTextSelectionFromContainer( @@ -321,22 +301,25 @@ function getLineRangeForText(value: string, text: string) { function FloatingSelectionMenu({ selection, onAdd, + popoverRef, }: { selection: FloatingSelectionMenuState | null onAdd: () => void + popoverRef: { current: HTMLButtonElement | null } }) { const t = useTranslation() if (!selection) return null return ( ) @@ -452,6 +435,7 @@ function CodeSurface({ }) { const t = useTranslation() const surfaceRef = useRef(null) + const selectionMenuRef = useRef(null) const [commentLine, setCommentLine] = useState(null) const [commentDraft, setCommentDraft] = useState('') const [showAllLines, setShowAllLines] = useState(false) @@ -469,6 +453,16 @@ function CodeSurface({ setSelectionMenu(null) }, [language, value]) + const dismissSelectionMenu = useCallback(() => { + setSelectionMenu(null) + }, []) + + useSelectionPopoverDismiss({ + active: Boolean(selectionMenu), + popoverRef: selectionMenuRef, + onDismiss: dismissSelectionMenu, + }) + const submitLineComment = () => { if (!commentLine || !commentDraft.trim()) return onAddLineComment(commentLine, commentDraft.trim(), activeQuote) @@ -497,7 +491,7 @@ function CodeSurface({ endLine: selectionMenu.endLine, }) setSelectionMenu(null) - window.getSelection()?.removeAllRanges() + clearWindowSelection() } const renderLineCommentEditor = (lineNumber: number) => { @@ -650,7 +644,7 @@ function CodeSurface({ )} - + ) } @@ -663,12 +657,23 @@ function MarkdownSurface({ onAddSelection: (selection: WorkspaceTextSelection) => void }) { const surfaceRef = useRef(null) + const selectionMenuRef = useRef(null) const [selectionMenu, setSelectionMenu] = useState(null) useEffect(() => { setSelectionMenu(null) }, [value]) + const dismissSelectionMenu = useCallback(() => { + setSelectionMenu(null) + }, []) + + useSelectionPopoverDismiss({ + active: Boolean(selectionMenu), + popoverRef: selectionMenuRef, + onDismiss: dismissSelectionMenu, + }) + const handleSelectionMouseUp = (event: MouseEvent) => { setSelectionMenu(getTextSelectionFromContainer( surfaceRef.current, @@ -685,7 +690,7 @@ function MarkdownSurface({ endLine: selectionMenu.endLine, }) setSelectionMenu(null) - window.getSelection()?.removeAllRanges() + clearWindowSelection() } return ( @@ -704,7 +709,7 @@ function MarkdownSurface({ className="workspace-markdown-preview prose-p:text-[14px] prose-p:leading-7 prose-h1:text-[24px] prose-h2:text-[18px] prose-h3:text-[15px] prose-code:text-[12px] prose-pre:my-4" /> - + ) } diff --git a/desktop/src/hooks/useSelectionPopoverDismiss.ts b/desktop/src/hooks/useSelectionPopoverDismiss.ts new file mode 100644 index 00000000..a7c17acf --- /dev/null +++ b/desktop/src/hooks/useSelectionPopoverDismiss.ts @@ -0,0 +1,93 @@ +import { useEffect } from 'react' + +type ElementRef = { + current: HTMLElement | null +} + +const VIEWPORT_MARGIN = 12 + +function clampValue(value: number, min: number, max: number) { + return Math.max(min, Math.min(value, max)) +} + +export function clearWindowSelection() { + window.getSelection()?.removeAllRanges() +} + +export function getSelectionPopoverPosition( + range: Range, + root: HTMLElement, + { + menuWidth, + menuHeight, + offset, + fallbackPointer, + }: { + menuWidth: number + menuHeight: number + offset: number + fallbackPointer?: { clientX: number; clientY: number } + }, +) { + const rect = typeof range.getBoundingClientRect === 'function' + ? range.getBoundingClientRect() + : null + const rootRect = root.getBoundingClientRect() + const hasUsableRangeRect = Boolean(rect && (rect.width > 0 || rect.height > 0)) + const pointerInsideRoot = fallbackPointer + && fallbackPointer.clientX >= rootRect.left + && fallbackPointer.clientX <= rootRect.right + && fallbackPointer.clientY >= rootRect.top + && fallbackPointer.clientY <= rootRect.bottom + const fallbackLeft = pointerInsideRoot ? fallbackPointer.clientX - menuWidth / 2 : rootRect.left + 24 + const fallbackTop = pointerInsideRoot ? fallbackPointer.clientY : rootRect.top + 24 + const selectionLeft = hasUsableRangeRect ? rect!.left : fallbackLeft + const selectionRight = hasUsableRangeRect ? rect!.right : selectionLeft + menuWidth + const selectionTop = hasUsableRangeRect ? rect!.top : fallbackTop + const selectionBottom = hasUsableRangeRect ? rect!.bottom : selectionTop + menuHeight + const viewportWidth = window.innerWidth || document.documentElement.clientWidth || rootRect.right + VIEWPORT_MARGIN + const viewportHeight = window.innerHeight || document.documentElement.clientHeight || rootRect.bottom + VIEWPORT_MARGIN + const minX = VIEWPORT_MARGIN + const maxX = Math.max(minX, viewportWidth - menuWidth - VIEWPORT_MARGIN) + const minY = VIEWPORT_MARGIN + const maxY = Math.max(minY, viewportHeight - menuHeight - VIEWPORT_MARGIN) + const aboveY = selectionTop - menuHeight - offset + const belowY = selectionBottom + offset + const y = aboveY >= VIEWPORT_MARGIN || belowY + menuHeight > viewportHeight - VIEWPORT_MARGIN + ? aboveY + : belowY + const centerX = selectionLeft + (selectionRight - selectionLeft) / 2 + + return { + x: clampValue(centerX - menuWidth / 2, minX, maxX), + y: clampValue(y, minY, maxY), + } +} + +export function useSelectionPopoverDismiss({ + active, + popoverRef, + onDismiss, +}: { + active: boolean + popoverRef: ElementRef + onDismiss: () => void +}) { + useEffect(() => { + if (!active) return + + const handlePointerDown = (event: PointerEvent) => { + const popover = popoverRef.current + const target = event.target + if (popover && target instanceof Node && popover.contains(target)) { + return + } + + onDismiss() + clearWindowSelection() + } + + document.addEventListener('pointerdown', handlePointerDown, true) + return () => document.removeEventListener('pointerdown', handlePointerDown, true) + }, [active, onDismiss, popoverRef]) +} diff --git a/desktop/src/i18n/locales/zh.ts b/desktop/src/i18n/locales/zh.ts index 750cfd9f..03ded6da 100644 --- a/desktop/src/i18n/locales/zh.ts +++ b/desktop/src/i18n/locales/zh.ts @@ -122,7 +122,7 @@ export const zh: Record = { 'workspace.showAllLoadedLines': '显示全部已加载行', 'workspace.collapsePreview': '收起预览', 'workspace.addToChat': '添加到聊天', - 'workspace.addSelectionToChat': '添加至对话', + 'workspace.addSelectionToChat': '添加到对话', 'workspace.copyPath': '复制路径', 'workspace.pathCopied': '路径已复制。', 'workspace.localComment': '本地评论', @@ -907,7 +907,7 @@ export const zh: Record = { 'chat.dropFilesHint': '会以文件路径附加到当前输入。', 'chat.workspaceReferencesOnly': '已添加 {count} 个工作区引用', 'chat.contextReferencesOnly': '已添加 {count} 个引用', - 'chat.addSelectionToChat': '添加至对话', + 'chat.addSelectionToChat': '添加到对话', 'chat.userMessageReference': '用户消息', 'chat.assistantMessageReference': 'AI 回复', 'chat.slashCommands': '斜杠命令',