mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix(desktop): stabilize chat selection references (#351)
The chat reference action was still tied too closely to message-local mouseup timing, and the floating fixed-position control stayed inside virtualized message DOM. In Electron Chromium that made multi-line ranges race selection settlement and let transformed ancestors offset the button away from the viewport position we calculated. This reads settled document selections after pointerup/selectionchange, portals the action to document.body, and prefers right-side placement for multi-line selections. Constraint: Electron Chromium selectionchange can settle after message-local mouse events Rejected: Keep the popover inside the message node | transformed/virtualized ancestors offset fixed positioning in real browser layout Confidence: high Scope-risk: narrow Directive: Keep chat selection actions portaled; do not reparent under virtualized message rows without browser-coordinate verification Tested: cd desktop && bun run test -- src/components/chat/MessageList.test.tsx Tested: bun run check:desktop Tested: Playwright Chromium smoke for multi-line selection, single-line selection, and outside-click dismissal on local desktop harness
This commit is contained in:
parent
0ebb0582b0
commit
54f3c00414
@ -63,6 +63,9 @@ async function waitForSelectionMenuUpdate() {
|
||||
await new Promise<void>((resolve) => {
|
||||
requestAnimationFrame(() => resolve())
|
||||
})
|
||||
await new Promise<void>((resolve) => {
|
||||
requestAnimationFrame(() => resolve())
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
@ -119,10 +122,93 @@ async function selectMessageText(
|
||||
prepareMessageTextSelection(element, text, rect)
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.pointerDown(element, {
|
||||
button: 0,
|
||||
clientX: rect.left ?? 160,
|
||||
clientY: rect.top ?? 80,
|
||||
pointerId: 1,
|
||||
pointerType: 'mouse',
|
||||
})
|
||||
fireEvent.pointerUp(element, {
|
||||
button: 0,
|
||||
clientX: rect.right ?? 280,
|
||||
clientY: rect.bottom ?? 98,
|
||||
pointerId: 1,
|
||||
pointerType: 'mouse',
|
||||
})
|
||||
fireEvent.mouseUp(element, { clientX: 260, clientY: 104 })
|
||||
await Promise.resolve()
|
||||
})
|
||||
await waitForSelectionMenuUpdate()
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('button', { name: 'Add to chat' })).toBeTruthy()
|
||||
})
|
||||
}
|
||||
|
||||
async function selectAcrossMessageText(
|
||||
startElement: Element,
|
||||
startText: string,
|
||||
endElement: Element,
|
||||
endText: string,
|
||||
rect: Partial<DOMRect> = {},
|
||||
) {
|
||||
const startNode = findTextNodeContaining(startElement, startText)
|
||||
const endNode = findTextNodeContaining(endElement, endText)
|
||||
const startOffset = startNode.textContent?.indexOf(startText) ?? -1
|
||||
const endOffset = (endNode.textContent?.indexOf(endText) ?? -1) + endText.length
|
||||
const range = document.createRange()
|
||||
range.setStart(startNode, startOffset)
|
||||
range.setEnd(endNode, endOffset)
|
||||
Object.assign(range, {
|
||||
getBoundingClientRect: () => ({
|
||||
left: rect.left ?? 160,
|
||||
top: rect.top ?? 80,
|
||||
right: rect.right ?? 520,
|
||||
bottom: rect.bottom ?? 150,
|
||||
width: rect.width ?? 360,
|
||||
height: rect.height ?? 70,
|
||||
x: rect.x ?? rect.left ?? 160,
|
||||
y: rect.y ?? rect.top ?? 80,
|
||||
toJSON: () => ({}),
|
||||
}),
|
||||
})
|
||||
|
||||
const selectableRoot = startElement.closest('[data-message-shell]')?.parentElement?.parentElement
|
||||
Object.assign(selectableRoot ?? startElement, {
|
||||
getBoundingClientRect: () => ({
|
||||
left: 120,
|
||||
top: 48,
|
||||
right: 720,
|
||||
bottom: 320,
|
||||
width: 600,
|
||||
height: 272,
|
||||
x: 120,
|
||||
y: 48,
|
||||
toJSON: () => ({}),
|
||||
}),
|
||||
})
|
||||
|
||||
window.getSelection()?.removeAllRanges()
|
||||
window.getSelection()?.addRange(range)
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.pointerDown(startElement, {
|
||||
button: 0,
|
||||
clientX: rect.left ?? 160,
|
||||
clientY: rect.top ?? 80,
|
||||
pointerId: 1,
|
||||
pointerType: 'mouse',
|
||||
})
|
||||
fireEvent.pointerUp(endElement, {
|
||||
button: 0,
|
||||
clientX: rect.right ?? 520,
|
||||
clientY: rect.bottom ?? 150,
|
||||
pointerId: 1,
|
||||
pointerType: 'mouse',
|
||||
})
|
||||
await Promise.resolve()
|
||||
})
|
||||
await waitForSelectionMenuUpdate()
|
||||
}
|
||||
|
||||
describe('MessageList nested tool calls', () => {
|
||||
@ -1757,6 +1843,125 @@ describe('MessageList nested tool calls', () => {
|
||||
expect(floatingAddButton.style.top).toBe('12px')
|
||||
})
|
||||
|
||||
it('adds multi-line assistant reply selections across markdown blocks to the composer context', async () => {
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
[ACTIVE_TAB]: makeSessionState({
|
||||
messages: [{
|
||||
id: 'assistant-1',
|
||||
type: 'assistant_text',
|
||||
content: [
|
||||
'First line can start the selection.',
|
||||
'',
|
||||
'Second paragraph should still belong to the same chat message.',
|
||||
'',
|
||||
'- Third block can finish the selection.',
|
||||
].join('\n'),
|
||||
timestamp: 1,
|
||||
}],
|
||||
}),
|
||||
},
|
||||
})
|
||||
|
||||
render(<MessageList />)
|
||||
|
||||
const firstParagraph = screen.getByText('First line can start the selection.')
|
||||
const listItem = screen.getByText('Third block can finish the selection.')
|
||||
await selectAcrossMessageText(
|
||||
firstParagraph,
|
||||
'First line',
|
||||
listItem,
|
||||
'finish the selection',
|
||||
{ left: 160, top: 80, right: 520, bottom: 160, width: 360, height: 80 },
|
||||
)
|
||||
const floatingAddButton = screen.getByRole('button', { name: 'Add to chat' })
|
||||
|
||||
expect(floatingAddButton.style.left).toBe('530px')
|
||||
expect(floatingAddButton.style.top).toBe('98px')
|
||||
|
||||
fireEvent.click(floatingAddButton)
|
||||
|
||||
expect(useWorkspaceChatContextStore.getState().referencesBySession[ACTIVE_TAB]).toMatchObject([
|
||||
{
|
||||
kind: 'chat-selection',
|
||||
messageId: 'assistant-1',
|
||||
sourceRole: 'assistant',
|
||||
},
|
||||
])
|
||||
expect(useWorkspaceChatContextStore.getState().referencesBySession[ACTIVE_TAB]?.[0]?.quote).toContain('First line')
|
||||
expect(useWorkspaceChatContextStore.getState().referencesBySession[ACTIVE_TAB]?.[0]?.quote).toContain('finish the selection')
|
||||
})
|
||||
|
||||
it('shows the selected-message action after browser selectionchange for multi-line replies', async () => {
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
[ACTIVE_TAB]: makeSessionState({
|
||||
messages: [{
|
||||
id: 'assistant-1',
|
||||
type: 'assistant_text',
|
||||
content: [
|
||||
'Browser selection can settle after pointerup.',
|
||||
'',
|
||||
'The document selectionchange event should be enough to show the action.',
|
||||
].join('\n'),
|
||||
timestamp: 1,
|
||||
}],
|
||||
}),
|
||||
},
|
||||
})
|
||||
|
||||
render(<MessageList />)
|
||||
|
||||
const firstParagraph = screen.getByText('Browser selection can settle after pointerup.')
|
||||
const secondParagraph = screen.getByText('The document selectionchange event should be enough to show the action.')
|
||||
const startNode = findTextNodeContaining(firstParagraph, 'Browser selection')
|
||||
const endNode = findTextNodeContaining(secondParagraph, 'show the action')
|
||||
const range = document.createRange()
|
||||
range.setStart(startNode, startNode.textContent?.indexOf('Browser selection') ?? 0)
|
||||
range.setEnd(
|
||||
endNode,
|
||||
(endNode.textContent?.indexOf('show the action') ?? 0) + 'show the action'.length,
|
||||
)
|
||||
Object.assign(range, {
|
||||
getBoundingClientRect: () => ({
|
||||
left: 150,
|
||||
top: 76,
|
||||
right: 500,
|
||||
bottom: 140,
|
||||
width: 350,
|
||||
height: 64,
|
||||
x: 150,
|
||||
y: 76,
|
||||
toJSON: () => ({}),
|
||||
}),
|
||||
})
|
||||
|
||||
const selectableRoot = firstParagraph.closest('[data-chat-selectable-message]')
|
||||
Object.assign(selectableRoot ?? firstParagraph, {
|
||||
getBoundingClientRect: () => ({
|
||||
left: 120,
|
||||
top: 48,
|
||||
right: 720,
|
||||
bottom: 280,
|
||||
width: 600,
|
||||
height: 232,
|
||||
x: 120,
|
||||
y: 48,
|
||||
toJSON: () => ({}),
|
||||
}),
|
||||
})
|
||||
|
||||
window.getSelection()?.removeAllRanges()
|
||||
window.getSelection()?.addRange(range)
|
||||
|
||||
await act(async () => {
|
||||
document.dispatchEvent(new Event('selectionchange'))
|
||||
})
|
||||
await waitForSelectionMenuUpdate()
|
||||
|
||||
expect(screen.getByRole('button', { name: 'Add to chat' })).toBeTruthy()
|
||||
})
|
||||
|
||||
it('adds selected assistant reply text to the composer context', async () => {
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
@ -1775,7 +1980,11 @@ describe('MessageList nested tool calls', () => {
|
||||
|
||||
const assistantText = screen.getByText(/First inspect the file tree/)
|
||||
await selectMessageText(assistantText, 'quote the selected lines')
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Add to chat' }))
|
||||
const floatingAddButton = screen.getByRole('button', { name: 'Add to chat' })
|
||||
|
||||
expect(floatingAddButton.closest('[data-chat-selectable-message]')).toBeNull()
|
||||
|
||||
fireEvent.click(floatingAddButton)
|
||||
|
||||
expect(useWorkspaceChatContextStore.getState().referencesBySession[ACTIVE_TAB]).toMatchObject([
|
||||
{
|
||||
|
||||
@ -1,4 +1,5 @@
|
||||
import { useRef, useEffect, useMemo, memo, useState, useCallback, useDeferredValue, useLayoutEffect, type ReactNode } from 'react'
|
||||
import { createPortal } from 'react-dom'
|
||||
import { ArrowDown, BookMarked, Bot, CheckCircle2, ChevronDown, ChevronRight, CircleStop, FileStack, LoaderCircle, MessageCircle, Settings, Target, XCircle } from 'lucide-react'
|
||||
import { ApiError } from '../../api/client'
|
||||
import { sessionsApi, type SessionTurnCheckpoint } from '../../api/sessions'
|
||||
@ -141,7 +142,7 @@ function ChatSelectionMenu({
|
||||
const t = useTranslation()
|
||||
if (!selection) return null
|
||||
|
||||
return (
|
||||
return createPortal(
|
||||
<button
|
||||
ref={popoverRef}
|
||||
type="button"
|
||||
@ -152,7 +153,8 @@ function ChatSelectionMenu({
|
||||
>
|
||||
<MessageCircle size={21} strokeWidth={2.15} className="shrink-0 text-[var(--color-text-primary)]" aria-hidden="true" />
|
||||
<span>{t('chat.addSelectionToChat')}</span>
|
||||
</button>
|
||||
</button>,
|
||||
document.body,
|
||||
)
|
||||
}
|
||||
|
||||
@ -400,7 +402,7 @@ function SelectableChatMessage({
|
||||
}) {
|
||||
const rootRef = useRef<HTMLDivElement>(null)
|
||||
const selectionMenuRef = useRef<HTMLButtonElement>(null)
|
||||
const selectionStartedInsideRef = useRef(false)
|
||||
const lastSelectionPointerRef = useRef<SelectionPointer | null>(null)
|
||||
const selectionUpdateFrameRef = useRef<number | null>(null)
|
||||
const addReference = useWorkspaceChatContextStore((state) => state.addReference)
|
||||
const [selectionMenu, setSelectionMenu] = useState<ChatSelectionState | null>(null)
|
||||
@ -411,21 +413,31 @@ function SelectableChatMessage({
|
||||
|
||||
useEffect(() => {
|
||||
setSelectionMenu(null)
|
||||
selectionStartedInsideRef.current = false
|
||||
lastSelectionPointerRef.current = null
|
||||
}, [content, messageId])
|
||||
|
||||
const dismissSelectionMenu = useCallback(() => {
|
||||
setSelectionMenu(null)
|
||||
}, [])
|
||||
|
||||
const queueSelectionMenuUpdate = useCallback((pointer: SelectionPointer) => {
|
||||
const queueSelectionMenuUpdate = useCallback((pointer?: SelectionPointer) => {
|
||||
if (pointer) lastSelectionPointerRef.current = pointer
|
||||
|
||||
if (selectionUpdateFrameRef.current !== null) {
|
||||
window.cancelAnimationFrame(selectionUpdateFrameRef.current)
|
||||
}
|
||||
|
||||
selectionUpdateFrameRef.current = window.requestAnimationFrame(() => {
|
||||
selectionUpdateFrameRef.current = null
|
||||
setSelectionMenu(getChatSelectionFromContainer(rootRef.current, pointer))
|
||||
selectionUpdateFrameRef.current = window.requestAnimationFrame(() => {
|
||||
selectionUpdateFrameRef.current = null
|
||||
const root = rootRef.current
|
||||
const rootRect = root?.getBoundingClientRect()
|
||||
const fallbackPointer = lastSelectionPointerRef.current ?? {
|
||||
clientX: (rootRect?.left ?? 0) + 24,
|
||||
clientY: (rootRect?.top ?? 0) + 24,
|
||||
}
|
||||
setSelectionMenu(getChatSelectionFromContainer(root, fallbackPointer))
|
||||
})
|
||||
})
|
||||
}, [])
|
||||
|
||||
@ -438,21 +450,37 @@ function SelectableChatMessage({
|
||||
}, [])
|
||||
|
||||
useEffect(() => {
|
||||
const handlePointerDown = (event: PointerEvent) => {
|
||||
lastSelectionPointerRef.current = getSelectionPointer(event)
|
||||
}
|
||||
|
||||
const handlePointerUp = (event: PointerEvent) => {
|
||||
if (!selectionStartedInsideRef.current) return
|
||||
selectionStartedInsideRef.current = false
|
||||
queueSelectionMenuUpdate(getSelectionPointer(event))
|
||||
}
|
||||
|
||||
const handlePointerCancel = () => {
|
||||
selectionStartedInsideRef.current = false
|
||||
const handleMouseUp = (event: MouseEvent) => {
|
||||
queueSelectionMenuUpdate(getSelectionPointer(event))
|
||||
}
|
||||
|
||||
const handleSelectionChange = () => {
|
||||
queueSelectionMenuUpdate()
|
||||
}
|
||||
|
||||
const handleKeyUp = () => {
|
||||
queueSelectionMenuUpdate()
|
||||
}
|
||||
|
||||
document.addEventListener('pointerdown', handlePointerDown, true)
|
||||
document.addEventListener('pointerup', handlePointerUp, true)
|
||||
document.addEventListener('pointercancel', handlePointerCancel, true)
|
||||
document.addEventListener('mouseup', handleMouseUp, true)
|
||||
document.addEventListener('selectionchange', handleSelectionChange)
|
||||
document.addEventListener('keyup', handleKeyUp, true)
|
||||
return () => {
|
||||
document.removeEventListener('pointerdown', handlePointerDown, true)
|
||||
document.removeEventListener('pointerup', handlePointerUp, true)
|
||||
document.removeEventListener('pointercancel', handlePointerCancel, true)
|
||||
document.removeEventListener('mouseup', handleMouseUp, true)
|
||||
document.removeEventListener('selectionchange', handleSelectionChange)
|
||||
document.removeEventListener('keyup', handleKeyUp, true)
|
||||
}
|
||||
}, [queueSelectionMenuUpdate])
|
||||
|
||||
@ -479,12 +507,12 @@ function SelectableChatMessage({
|
||||
return (
|
||||
<div
|
||||
ref={rootRef}
|
||||
data-chat-selectable-message={role}
|
||||
onPointerDown={(event) => {
|
||||
if (event.pointerType === 'mouse' && event.button !== 0) return
|
||||
selectionStartedInsideRef.current = true
|
||||
lastSelectionPointerRef.current = getSelectionPointer(event)
|
||||
}}
|
||||
onMouseUp={(event) => {
|
||||
selectionStartedInsideRef.current = false
|
||||
queueSelectionMenuUpdate(getSelectionPointer(event))
|
||||
}}
|
||||
onKeyDown={(event) => {
|
||||
|
||||
@ -15,6 +15,11 @@ type SelectionRect = {
|
||||
height: number
|
||||
}
|
||||
|
||||
type SelectionGeometry = {
|
||||
rect: SelectionRect | DOMRect
|
||||
isMultiLine: boolean
|
||||
}
|
||||
|
||||
function clampValue(value: number, min: number, max: number) {
|
||||
return Math.max(min, Math.min(value, max))
|
||||
}
|
||||
@ -23,29 +28,42 @@ function isUsableRect(rect: SelectionRect | DOMRect) {
|
||||
return rect.width > 0 || rect.height > 0
|
||||
}
|
||||
|
||||
function getRangeSelectionRect(range: Range): SelectionRect | null {
|
||||
const boundingRect = typeof range.getBoundingClientRect === 'function'
|
||||
? range.getBoundingClientRect()
|
||||
: null
|
||||
if (boundingRect && isUsableRect(boundingRect)) return boundingRect
|
||||
|
||||
function getRangeSelectionGeometry(range: Range): SelectionGeometry | null {
|
||||
const clientRects = typeof range.getClientRects === 'function'
|
||||
? Array.from(range.getClientRects()).filter(isUsableRect)
|
||||
: []
|
||||
if (clientRects.length === 0) return null
|
||||
const boundingRect = typeof range.getBoundingClientRect === 'function'
|
||||
? range.getBoundingClientRect()
|
||||
: null
|
||||
|
||||
const left = Math.min(...clientRects.map((rect) => rect.left))
|
||||
const top = Math.min(...clientRects.map((rect) => rect.top))
|
||||
const right = Math.max(...clientRects.map((rect) => rect.right))
|
||||
const bottom = Math.max(...clientRects.map((rect) => rect.bottom))
|
||||
return {
|
||||
left,
|
||||
top,
|
||||
right,
|
||||
bottom,
|
||||
width: right - left,
|
||||
height: bottom - top,
|
||||
if (clientRects.length > 0) {
|
||||
const left = Math.min(...clientRects.map((rect) => rect.left))
|
||||
const top = Math.min(...clientRects.map((rect) => rect.top))
|
||||
const right = Math.max(...clientRects.map((rect) => rect.right))
|
||||
const bottom = Math.max(...clientRects.map((rect) => rect.bottom))
|
||||
const unionRect = {
|
||||
left,
|
||||
top,
|
||||
right,
|
||||
bottom,
|
||||
width: right - left,
|
||||
height: bottom - top,
|
||||
}
|
||||
const maxLineHeight = Math.max(...clientRects.map((rect) => rect.height))
|
||||
return {
|
||||
rect: boundingRect && isUsableRect(boundingRect) ? boundingRect : unionRect,
|
||||
isMultiLine: clientRects.length > 1 || unionRect.height > maxLineHeight * 1.6,
|
||||
}
|
||||
}
|
||||
|
||||
if (boundingRect && isUsableRect(boundingRect)) {
|
||||
return {
|
||||
rect: boundingRect,
|
||||
isMultiLine: boundingRect.height > 32,
|
||||
}
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
export function clearWindowSelection() {
|
||||
@ -67,7 +85,7 @@ export function getSelectionPopoverPosition(
|
||||
fallbackPointer?: { clientX: number; clientY: number }
|
||||
},
|
||||
) {
|
||||
const rect = getRangeSelectionRect(range)
|
||||
const geometry = getRangeSelectionGeometry(range)
|
||||
const rootRect = root.getBoundingClientRect()
|
||||
const pointerInsideRoot = fallbackPointer
|
||||
&& fallbackPointer.clientX >= rootRect.left
|
||||
@ -76,7 +94,7 @@ export function getSelectionPopoverPosition(
|
||||
&& fallbackPointer.clientY <= rootRect.bottom
|
||||
const fallbackX = pointerInsideRoot ? fallbackPointer.clientX : rootRect.left + 24
|
||||
const fallbackY = pointerInsideRoot ? fallbackPointer.clientY : rootRect.top + 24
|
||||
const selectionRect = rect ?? {
|
||||
const selectionRect = geometry?.rect ?? {
|
||||
left: fallbackX,
|
||||
top: fallbackY,
|
||||
right: fallbackX,
|
||||
@ -100,12 +118,15 @@ export function getSelectionPopoverPosition(
|
||||
x: centerX - menuWidth / 2,
|
||||
y: selectionRect.top - menuHeight - offset,
|
||||
}
|
||||
if (above.y >= VIEWPORT_MARGIN) return clampPosition(above)
|
||||
|
||||
const right = {
|
||||
x: selectionRect.right + offset,
|
||||
y: centerY - menuHeight / 2,
|
||||
}
|
||||
if (geometry?.isMultiLine && right.x + menuWidth <= viewportWidth - VIEWPORT_MARGIN) return clampPosition(right)
|
||||
|
||||
if (above.y >= VIEWPORT_MARGIN) return clampPosition(above)
|
||||
|
||||
if (right.x + menuWidth <= viewportWidth - VIEWPORT_MARGIN) return clampPosition(right)
|
||||
|
||||
const below = {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user