fix(desktop): open-with trigger toggles its menu (re-click closes; ignore trigger in outside-close)

This commit is contained in:
程序员阿江(Relakkes) 2026-05-31 00:09:32 +08:00
parent f17b0828a0
commit 144ee5a779
5 changed files with 113 additions and 13 deletions

View File

@ -107,4 +107,17 @@ describe('AssistantOutputTargetCard', () => {
expect(await screen.findByText('openWith.inAppBrowser')).toBeInTheDocument()
expect(screen.getByText('openWith.systemBrowser')).toBeInTheDocument()
})
it('re-clicking the same open-with trigger TOGGLES the menu closed', async () => {
render(<AssistantOutputTargetCard target={localhostTarget} sessionId="s1" />)
const trigger = screen.getByLabelText('openWith.title')
// 1st click → opens
fireEvent.click(trigger)
expect(await screen.findByText('openWith.inAppBrowser')).toBeInTheDocument()
// 2nd click on the SAME trigger → closes (toggle)
fireEvent.click(trigger)
expect(screen.queryByText('openWith.inAppBrowser')).not.toBeInTheDocument()
})
})

View File

@ -21,7 +21,7 @@ type Props = {
export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props) {
const t = useTranslation()
const [openWith, setOpenWith] = useState<{ items: OpenWithItem[]; anchor: DOMRect } | null>(null)
const [openWith, setOpenWith] = useState<{ items: OpenWithItem[]; anchor: DOMRect; triggerEl: HTMLElement } | null>(null)
const isLocalhost = target.kind === 'localhost-url'
const typeInfo = describeFileType(target.normalizedPath ?? target.href)
@ -60,7 +60,15 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
const handleOpenWith = useCallback((event: ReactMouseEvent<HTMLButtonElement>) => {
event.stopPropagation()
const rect = event.currentTarget.getBoundingClientRect()
// Toggle: a second click on the same trigger closes the menu. OpenWithMenu's
// outside-mousedown ignores the trigger, so the trigger's own click is the
// only path that can close it on re-click.
if (openWith) {
setOpenWith(null)
return
}
const triggerEl = event.currentTarget
const rect = triggerEl.getBoundingClientRect()
void (async () => {
await useOpenTargetStore.getState().ensureTargets()
const targets = useOpenTargetStore.getState().targets
@ -77,9 +85,9 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
openTarget: (id, abs) => { void useOpenTargetStore.getState().openTarget(id, abs) },
t: (k, v) => t(k as TranslationKey, v),
})
setOpenWith({ items, anchor: rect })
setOpenWith({ items, anchor: rect, triggerEl })
})()
}, [sessionId, t, target.href, workDir])
}, [openWith, sessionId, t, target.href, workDir])
return (
<section className="flex items-start gap-3 rounded-[var(--radius-lg)] border border-[var(--color-border)]/70 bg-[var(--color-surface-container-low)] px-3 py-2.5 shadow-sm">
@ -137,7 +145,7 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
</button>
</div>
{openWith && <OpenWithMenu items={openWith.items} anchor={openWith.anchor} onClose={() => setOpenWith(null)} />}
{openWith && <OpenWithMenu items={openWith.items} anchor={openWith.anchor} triggerEl={openWith.triggerEl} onClose={() => setOpenWith(null)} />}
</section>
)
}

View File

@ -38,7 +38,7 @@ export function CurrentTurnChangeCard({
onUndo,
}: CurrentTurnChangeCardProps) {
const t = useTranslation()
const [openWith, setOpenWith] = useState<{ items: OpenWithItem[]; anchor: DOMRect } | null>(null)
const [openWith, setOpenWith] = useState<{ items: OpenWithItem[]; anchor: DOMRect; triggerEl: HTMLElement } | null>(null)
const [showAllFiles, setShowAllFiles] = useState(false)
const files = useMemo<ChangedFileEntry[]>(
@ -63,7 +63,15 @@ export function CurrentTurnChangeCard({
const handleOpenWith = useCallback((event: ReactMouseEvent<HTMLButtonElement>, fileEntry: ChangedFileEntry) => {
event.stopPropagation()
const rect = event.currentTarget.getBoundingClientRect()
// Toggle: if the menu is already open, a second click on the trigger closes it
// (the OpenWithMenu's outside-mousedown handler excludes the trigger, so its
// own click is the only thing that can close it on re-click).
if (openWith) {
setOpenWith(null)
return
}
const triggerEl = event.currentTarget
const rect = triggerEl.getBoundingClientRect()
void (async () => {
await useOpenTargetStore.getState().ensureTargets()
const targets = useOpenTargetStore.getState().targets
@ -78,9 +86,9 @@ export function CurrentTurnChangeCard({
openTarget: (id, abs) => { void useOpenTargetStore.getState().openTarget(id, abs) },
t: (k, v) => t(k as TranslationKey, v),
})
setOpenWith({ items, anchor: rect })
setOpenWith({ items, anchor: rect, triggerEl })
})()
}, [sessionId, t])
}, [openWith, sessionId, t])
const cardLabel = isLatest
? t('chat.turnChangesLatestCardLabel')
@ -193,7 +201,7 @@ export function CurrentTurnChangeCard({
</div>
)}
{openWith && <OpenWithMenu items={openWith.items} anchor={openWith.anchor} onClose={() => setOpenWith(null)} />}
{openWith && <OpenWithMenu items={openWith.items} anchor={openWith.anchor} triggerEl={openWith.triggerEl} onClose={() => setOpenWith(null)} />}
</section>
)
}

View File

@ -69,6 +69,65 @@ describe('OpenWithMenu', () => {
expect(onClose).toHaveBeenCalledTimes(1)
})
describe('triggerEl exclusion (re-click-trigger toggle support)', () => {
it('does NOT call onClose when mousedown lands inside triggerEl', () => {
// Set up a trigger element + a child within it in the document.
const trigger = document.createElement('button')
const triggerChild = document.createElement('span')
trigger.appendChild(triggerChild)
document.body.appendChild(trigger)
const onClose = vi.fn()
render(<OpenWithMenu items={makeItems()} anchor={anchor} onClose={onClose} triggerEl={trigger} />)
// mousedown on the trigger itself
fireEvent.mouseDown(trigger)
expect(onClose).not.toHaveBeenCalled()
// mousedown on a descendant of the trigger
fireEvent.mouseDown(triggerChild)
expect(onClose).not.toHaveBeenCalled()
document.body.removeChild(trigger)
})
it('STILL calls onClose for true outside clicks (not menu, not trigger)', () => {
const trigger = document.createElement('button')
document.body.appendChild(trigger)
const outside = document.createElement('div')
document.body.appendChild(outside)
const onClose = vi.fn()
render(<OpenWithMenu items={makeItems()} anchor={anchor} onClose={onClose} triggerEl={trigger} />)
fireEvent.mouseDown(outside)
expect(onClose).toHaveBeenCalledTimes(1)
document.body.removeChild(trigger)
document.body.removeChild(outside)
})
it('Escape still closes when triggerEl is provided', () => {
const trigger = document.createElement('button')
document.body.appendChild(trigger)
const onClose = vi.fn()
render(<OpenWithMenu items={makeItems()} anchor={anchor} onClose={onClose} triggerEl={trigger} />)
fireEvent.keyDown(document, { key: 'Escape' })
expect(onClose).toHaveBeenCalledTimes(1)
document.body.removeChild(trigger)
})
it('item click still closes the menu when triggerEl is provided', () => {
const trigger = document.createElement('button')
document.body.appendChild(trigger)
const onClose = vi.fn()
const onSelect1 = vi.fn()
render(<OpenWithMenu items={makeItems(onSelect1)} anchor={anchor} onClose={onClose} triggerEl={trigger} />)
fireEvent.click(screen.getByText('In-app browser'))
expect(onSelect1).toHaveBeenCalledTimes(1)
expect(onClose).toHaveBeenCalledTimes(1)
document.body.removeChild(trigger)
})
})
it('flips above the anchor when it would overflow the viewport bottom', () => {
// The trigger often sits right above the composer; the menu must not render off-screen below it.
Object.defineProperty(window, 'innerHeight', { value: 300, configurable: true })

View File

@ -8,6 +8,10 @@ type Props = {
items: OpenWithItem[]
anchor: { top: number; bottom: number; left: number; right: number }
onClose: () => void
// Optional trigger element to exclude from outside-close detection. When the
// user clicks the same trigger that opened this menu, the trigger's own
// click handler is responsible for toggling — don't double-close here.
triggerEl?: HTMLElement | null
}
function ItemIcon({ item }: { item: OpenWithItem }) {
@ -19,7 +23,7 @@ function ItemIcon({ item }: { item: OpenWithItem }) {
const MARGIN = 8
export function OpenWithMenu({ items, anchor, onClose }: Props) {
export function OpenWithMenu({ items, anchor, onClose, triggerEl }: Props) {
const ref = useRef<HTMLDivElement>(null)
// Initial guess; corrected (before paint) by the layout effect once we can measure the menu.
const [pos, setPos] = useState<{ top: number; left: number }>(() => ({
@ -47,12 +51,20 @@ export function OpenWithMenu({ items, anchor, onClose }: Props) {
}, [anchor])
useEffect(() => {
const onDown = (e: MouseEvent) => { if (!ref.current?.contains(e.target as Node)) onClose() }
const onDown = (e: MouseEvent) => {
const target = e.target as Node | null
if (!target) return
// Ignore clicks inside the menu itself, AND clicks inside the trigger
// element that opened it — the trigger's own onClick handler will toggle.
if (ref.current?.contains(target)) return
if (triggerEl && triggerEl.contains(target)) return
onClose()
}
const onKey = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose() }
document.addEventListener('mousedown', onDown)
document.addEventListener('keydown', onKey)
return () => { document.removeEventListener('mousedown', onDown); document.removeEventListener('keydown', onKey) }
}, [onClose])
}, [onClose, triggerEl])
return createPortal(
<div