From d258f3f8bc5a478521ea6c6e0b895097bed542bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=87=BF=E9=A9=B9-=E4=BC=AF=E4=B9=90?= Date: Thu, 27 Aug 2026 12:53:12 +0800 Subject: [PATCH 1/3] fix(editor): dismiss the selection popup when its surface leaves the viewport (#425) The portaled position:fixed add-to-conversation button stays pinned to its viewport anchor while the sidebar keeps every tab mounted (tab switches only set display:none; collapsing the panel translates it off-screen), so the popup could outlive its editor surface. Extract the popup into useSelectionPopup and close it on any mousedown outside the button, Escape, document hidden, window blur, and - the tab-switch/collapse path - when an IntersectionObserver reports the editor surface off-viewport. --- src/client/TextEditor.tsx | 84 ++++++--------- src/client/selection-popup.ts | 157 +++++++++++++++++++++++++++ tests/selection-popup.spec.tsx | 190 +++++++++++++++++++++++++++++++++ 3 files changed, 379 insertions(+), 52 deletions(-) create mode 100644 src/client/selection-popup.ts create mode 100644 tests/selection-popup.spec.tsx diff --git a/src/client/TextEditor.tsx b/src/client/TextEditor.tsx index 25114c9..c698d8d 100644 --- a/src/client/TextEditor.tsx +++ b/src/client/TextEditor.tsx @@ -28,6 +28,7 @@ import { cmSurfaceTheme, CmThemeCompartment } from './cm-themes.ts' import { isDarkScheme, subscribeColorScheme } from './theme.ts' import { SandboxStatusBar } from './SandboxStatusBar.tsx' import { appendToDraft } from './conversation-draft.ts' +import { useSelectionPopup } from './selection-popup.ts' import { buildSelectionInsert, linesOfSelection } from './selection-payload.ts' import { lazyChunkComponent } from './lazy-chunk.tsx' import { analyzeMarkdownHtml } from './markdown-html.ts' @@ -41,13 +42,6 @@ import css from './sidebar.module.css' /** Previewable files (rendered output vs source editing). */ type ViewMode = 'preview' | 'edit' -/** The floating "add to conversation" action: payload + viewport anchor. */ -interface SelectionPopup { - insert: string - left: number - top: number -} - /** * The sandbox tokens of the HTML preview iframe. NO allow-same-origin (the * preview must stay in an opaque origin — with the route's own origin it @@ -71,36 +65,23 @@ export function TextEditor(props: FileViewerProps) { const themeCompRef = useRef(null) /** The app's resolved color scheme; the editor re-themes in place on flips. */ const [dark, setDark] = useState(() => isDarkScheme()) - /** The floating "add to conversation" popup (viewport-anchored; null = hidden). */ - const [popup, setPopup] = useState(null) - /** Live mirror of the popup state for click-time reads (no re-render race). */ - const popupRef = useRef(null) /** The markdown preview container (selection-containment + line lookup). */ const mdRef = useRef(null) + const markdown = viewerId === 'markdown' + const html = viewerId === 'html' - const hidePopup = (): void => { - popupRef.current = null - setPopup(null) - } - - /** Anchor the popup above the selection center; clamp inside the viewport. */ - const showPopup = (insert: string, left: number, top: number): void => { - const next: SelectionPopup = { - insert, - left: Math.min(Math.max(left, 80), window.innerWidth - 80), - top, - } - popupRef.current = next - setPopup(next) - } - - /** The popup button's click: insert the stored payload into the draft. */ - const commitPopup = (): void => { - const current = popupRef.current - if (current === null) return - appendToDraft(ctx, scope.sessionId, current.insert) - hidePopup() - } + /** + * The floating "add to conversation" popup (viewport-anchored; null = + * hidden). The hook owns show/hide/commit plus the global dismissal + * listeners (outside mousedown, Escape, hidden tab/window, surface + * leaving the viewport) — see selection-popup.ts. + */ + const selectionPopup = useSelectionPopup({ + onCommit: (insert) => { appendToDraft(ctx, scope.sessionId, insert) }, + // The surface that must stay on screen: the markdown preview container + // in preview mode, the CodeMirror host otherwise. + getSurface: () => (markdown && mode === 'preview' ? mdRef.current : hostRef.current), + }) useEffect(() => subscribeColorScheme(() => { setDark(isDarkScheme()) }), []) @@ -110,7 +91,7 @@ export function TextEditor(props: FileViewerProps) { setDraft(null) setDirty(false) setSaveState('idle') - hidePopup() + selectionPopup.hide() }, [content]) // Create the CodeMirror editor once the content is loaded. The view owns @@ -159,33 +140,33 @@ export function TextEditor(props: FileViewerProps) { ...(viewerId === 'code' || viewerId === 'markdown' ? [ CodeMirrorView.updateListener.of((update) => { if (update.geometryChanged || update.viewportChanged) { - hidePopup() + selectionPopup.hide() return } if (!update.view.hasFocus) { - hidePopup() + selectionPopup.hide() return } if (!(update.selectionSet || update.docChanged || update.focusChanged)) return const sel = update.state.selection.main if (sel.empty) { - hidePopup() + selectionPopup.hide() return } const text = update.state.sliceDoc(sel.from, sel.to) if (text.trim() === '') { - hidePopup() + selectionPopup.hide() return } // Page coordinates (the document root may scroll); the popup is // position:fixed, so convert to viewport coordinates. const rect = update.view.coordsAtPos(sel.head) if (rect === null) { - hidePopup() + selectionPopup.hide() return } const doc = update.state.doc - showPopup( + selectionPopup.show( buildSelectionInsert(path, scope.cwd, { start: doc.lineAt(sel.from).number, end: doc.lineAt(sel.to).number, @@ -222,7 +203,7 @@ export function TextEditor(props: FileViewerProps) { // it becomes visible again (CodeMirror sizes itself on reveal). A mode // flip also invalidates any anchored selection popup. useEffect(() => { - hidePopup() + selectionPopup.hide() if (mode === 'edit') viewRef.current?.requestMeasure() }, [mode]) @@ -242,8 +223,6 @@ export function TextEditor(props: FileViewerProps) { }) } - const markdown = viewerId === 'markdown' - const html = viewerId === 'html' /** The markdown source the preview renders (draft wins over saved content). */ const mdText = draft ?? content ?? '' /** The preview source: `mdText` with local image destinations rewritten to @@ -295,22 +274,22 @@ export function TextEditor(props: FileViewerProps) { const handlePreviewMouseUp = (): void => { const sel = window.getSelection() if (sel === null || sel.isCollapsed || sel.anchorNode === null || sel.focusNode === null) { - hidePopup() + selectionPopup.hide() return } const host = mdRef.current if (host === null || !host.contains(sel.anchorNode) || !host.contains(sel.focusNode)) { - hidePopup() + selectionPopup.hide() return } const text = sel.toString() if (text.trim() === '') { - hidePopup() + selectionPopup.hide() return } const rect = sel.getRangeAt(0).getBoundingClientRect() const lines = linesOfSelection(mdText, text) - showPopup( + selectionPopup.show( buildSelectionInsert(path, scope.cwd, lines ?? undefined, text), rect.left + rect.width / 2, rect.top, @@ -401,7 +380,7 @@ export function TextEditor(props: FileViewerProps) { className={css.editorMd} ref={mdRef} onMouseUp={handlePreviewMouseUp} - onScroll={hidePopup} + onScroll={selectionPopup.hide} > {/* The fence copy-button labels must come from this plugin's own dictionary: the DSH MarkdownText/CodeBlock are cordis-free and @@ -449,15 +428,16 @@ export function TextEditor(props: FileViewerProps) { /> )} - {popup !== null && createPortal( + {selectionPopup.popup !== null && createPortal( , diff --git a/src/client/selection-popup.ts b/src/client/selection-popup.ts new file mode 100644 index 0000000..5386d9c --- /dev/null +++ b/src/client/selection-popup.ts @@ -0,0 +1,157 @@ +/** + * The floating "add selection to conversation" popup shared by the text + * viewers (markdown preview + the catch-all code viewer): a viewport-anchored + * button portaled to `document.body`, kept alive across the selection gesture + * and committed on click. + * + * Dismissal contract (fixes upstream issue #425): the popup must never + * outlive its editor surface. The sidebar keeps every tab MOUNTED — switching + * tabs only flips the pane cell to `display:none` and collapsing the panel + * translates it off-screen — while the portaled `position:fixed` button stays + * pinned to its viewport anchor, ignoring both. The caller already hides on + * surface scrolls, selection collapse and mode flips; this hook adds the + * global dismissal that covers everything else: + * + * - any `mousedown` outside the button (tab bar, composer, another pane, + * the collapse toggle, …) closes it; + * - `Escape` closes it; + * - the document going hidden (`visibilitychange`) or the window losing + * focus closes it; + * - an `IntersectionObserver` on the editor surface closes it as soon as the + * surface leaves the viewport — the tab-switch (`display:none`) and + * panel-collapse (translated off-screen) paths have no DOM events of their + * own, so the geometry signal is the only reliable one. + * + * The button's own `mousedown` is never treated as an outside click: the + * caller preventDefaults it to keep the selection/caret alive until the + * click commits (the hook's capture-phase listener runs first and must not + * hide for it). + */ +import { useEffect, useRef, useState, type RefObject } from 'react' + +/** The floating "add to conversation" action: payload + viewport anchor. */ +export interface SelectionPopup { + insert: string + left: number + top: number +} + +export interface SelectionPopupOptions { + /** Commit the payload into the composer draft (button click). */ + onCommit(insert: string): void + /** + * The DOM surface that must stay on screen for the popup to live: the + * markdown preview container in preview mode, the CodeMirror host + * otherwise. Called lazily (refs are null until the content loads). + */ + getSurface(): HTMLElement | null +} + +export interface SelectionPopupControls { + /** The current popup (null = hidden). */ + popup: SelectionPopup | null + /** Attach to the portaled button element. */ + buttonRef: RefObject + /** Anchor the popup above a selection (viewport-clamped). */ + show(insert: string, left: number, top: number): void + /** Hide the popup (idempotent). */ + hide(): void + /** The button's click: commit the stored payload, then hide. */ + commit(): void +} + +export function useSelectionPopup(options: SelectionPopupOptions): SelectionPopupControls { + // Latest-callback refs: the dismissal listeners live for the mount's + // lifetime, so they must not capture stale closures across renders. + const onCommitRef = useRef(options.onCommit) + const getSurfaceRef = useRef(options.getSurface) + onCommitRef.current = options.onCommit + getSurfaceRef.current = options.getSurface + + const [popup, setPopup] = useState(null) + /** Live mirror for click/event-time reads (no re-render race). */ + const popupRef = useRef(null) + /** The portaled button itself (for the outside-click guard). */ + const buttonRef = useRef(null) + /** The surface visibility observer (created lazily on open). */ + const observerRef = useRef(null) + + const show = (insert: string, left: number, top: number): void => { + const next: SelectionPopup = { + insert, + left: Math.min(Math.max(left, 80), window.innerWidth - 80), + top, + } + popupRef.current = next + setPopup(next) + } + + const hide = (): void => { + popupRef.current = null + setPopup(null) + } + + const commit = (): void => { + const current = popupRef.current + if (current === null) return + onCommitRef.current(current.insert) + hide() + } + + // Global dismissal listeners, registered once; every handler reads the + // live popup ref so it no-ops while no popup is open. + useEffect(() => { + const onMouseDown = (event: MouseEvent): void => { + if (popupRef.current === null) return + const button = buttonRef.current + if (button !== null && (button === event.target || button.contains(event.target as Node))) return + hide() + } + const onKeyDown = (event: KeyboardEvent): void => { + if (event.key === 'Escape' && popupRef.current !== null) hide() + } + const onVisibilityChange = (): void => { + if (document.hidden && popupRef.current !== null) hide() + } + const onWindowBlur = (): void => { + if (popupRef.current !== null) hide() + } + document.addEventListener('mousedown', onMouseDown, true) + document.addEventListener('keydown', onKeyDown, true) + document.addEventListener('visibilitychange', onVisibilityChange) + window.addEventListener('blur', onWindowBlur) + return () => { + document.removeEventListener('mousedown', onMouseDown, true) + document.removeEventListener('keydown', onKeyDown, true) + document.removeEventListener('visibilitychange', onVisibilityChange) + window.removeEventListener('blur', onWindowBlur) + observerRef.current?.disconnect() + observerRef.current = null + } + }, []) + + // Re-target the visibility observer whenever a popup opens: the surface + // refs may have been null at mount (content loads async, mode flips swap + // the preview container for the CodeMirror host), so the surface is only + // trustworthy at open time. + useEffect(() => { + if (popup === null) return + observerRef.current?.disconnect() + observerRef.current = null + if (typeof IntersectionObserver === 'undefined') return + const surface = getSurfaceRef.current() + if (surface === null) return + const observer = new IntersectionObserver((entries) => { + for (const entry of entries) { + if (!entry.isIntersecting) hide() + } + }, { threshold: 0 }) + observerRef.current = observer + observer.observe(surface) + // eslint-disable-next-line react-hooks/exhaustive-deps -- the boolean + // flip is the only thing that must re-run this; an anchor move while + // open keeps the same surface. + }, [popup !== null]) + + return { popup, buttonRef, show, hide, commit } +} \ No newline at end of file diff --git a/tests/selection-popup.spec.tsx b/tests/selection-popup.spec.tsx new file mode 100644 index 0000000..c366c2e --- /dev/null +++ b/tests/selection-popup.spec.tsx @@ -0,0 +1,190 @@ +/** + * useSelectionPopup spec — the "add to conversation" popup's global dismissal + * contract (upstream issue #425 ①). The portaled fixed button must hide on an + * outside mousedown, on Escape, when the document goes hidden, when the + * window loses focus, and as soon as the editor surface leaves the viewport + * (the geometry signal behind tab switches and panel collapse) — yet it must + * survive a mousedown ON the button so the click can still commit the + * selection. + */ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { createElement, useRef } from 'react' +import { createPortal } from 'react-dom' +import { createRoot } from 'react-dom/client' +import { act } from 'react-dom/test-utils' +import { useSelectionPopup } from '../src/client/selection-popup.ts' + +// The act() environment flag (React 18.2 reads it before flushing effects). +;(globalThis as Record).IS_REACT_ACT_ENVIRONMENT = true + +/** jsdom ships no IntersectionObserver; a controllable stand-in. */ +class FakeIntersectionObserver implements IntersectionObserver { + static last: FakeIntersectionObserver | null = null + readonly root: Element | Document | null = null + readonly rootMargin = '0px' + readonly thresholds: readonly number[] = [0] + readonly targets = new Set() + constructor(readonly callback: IntersectionObserverCallback) { + FakeIntersectionObserver.last = this + } + observe(target: Element): void { this.targets.add(target) } + unobserve(target: Element): void { this.targets.delete(target) } + disconnect(): void { this.targets.clear() } + takeRecords(): IntersectionObserverEntry[] { return [] } + /** Test hook: report that every observed target is (or is not) intersecting. */ + fire(intersecting: boolean): void { + this.callback([{ isIntersecting: intersecting } as IntersectionObserverEntry], this) + } +} + +interface HarnessCtl { + show(insert: string): void + button(): HTMLButtonElement | null +} + +let ctl: HarnessCtl + +/** The popup consumer's rendering shape: the hook + a portaled button. */ +function Harness(props: { onCommit: (insert: string) => void }) { + const surfaceRef = useRef(null) + const surface = useSelectionPopup({ + onCommit: props.onCommit, + getSurface: () => surfaceRef.current, + }) + ctl = { + show: (insert: string) => act(() => { surface.show(insert, 100, 100) }), + button: () => document.querySelector('[data-popup]'), + } + return createElement('div', null, + createElement('div', { ref: surfaceRef, 'data-surface': true }), + surface.popup === null ? null : createPortal( + createElement('button', { + type: 'button', + ref: surface.buttonRef, + 'data-popup': true, + onClick: surface.commit, + }, surface.popup.insert), + document.body, + ), + ) +} + +let container: HTMLDivElement +let root: ReturnType +let commitCalls: string[] + +function mount(): void { + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + commitCalls = [] + act(() => { + root.render(createElement(Harness, { onCommit: (insert: string) => { commitCalls.push(insert) } })) + }) +} + +function unmount(): void { + act(() => { root.unmount() }) + container.remove() +} + +const RealIntersectionObserver = globalThis.IntersectionObserver + +beforeEach(() => { + ;(globalThis as { IntersectionObserver: unknown }).IntersectionObserver = FakeIntersectionObserver + FakeIntersectionObserver.last = null +}) + +afterEach(() => { + ;(globalThis as { IntersectionObserver: unknown }).IntersectionObserver = RealIntersectionObserver + document.body.innerHTML = '' +}) + +describe('useSelectionPopup', () => { + it('shows the popup button with the payload', () => { + mount() + ctl.show('PAYLOAD') + const button = ctl.button() + expect(button).not.toBeNull() + expect(button?.textContent).toBe('PAYLOAD') + unmount() + }) + + it('commits the payload on click, then hides', () => { + mount() + ctl.show('PAYLOAD') + const button = ctl.button()! + act(() => { button.dispatchEvent(new MouseEvent('click', { bubbles: true })) }) + expect(commitCalls).toEqual(['PAYLOAD']) + expect(ctl.button()).toBeNull() + unmount() + }) + + it('hides on a mousedown outside the button', () => { + mount() + ctl.show('PAYLOAD') + const elsewhere = document.createElement('div') + document.body.append(elsewhere) + act(() => { elsewhere.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })) }) + expect(ctl.button()).toBeNull() + unmount() + }) + + it('survives a mousedown ON the button so the click can commit', () => { + mount() + ctl.show('PAYLOAD') + const button = ctl.button()! + act(() => { button.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })) }) + expect(ctl.button()).not.toBeNull() + unmount() + }) + + it('hides on Escape', () => { + mount() + ctl.show('PAYLOAD') + act(() => { document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })) }) + expect(ctl.button()).toBeNull() + unmount() + }) + + it('hides when the document goes hidden (browser tab switched away)', () => { + mount() + ctl.show('PAYLOAD') + const prototypeDescriptor = Object.getOwnPropertyDescriptor(Document.prototype, 'hidden') + Object.defineProperty(document, 'hidden', { value: true, configurable: true }) + try { + act(() => { document.dispatchEvent(new Event('visibilitychange')) }) + expect(ctl.button()).toBeNull() + } finally { + if (prototypeDescriptor !== undefined) Object.defineProperty(document, 'hidden', prototypeDescriptor) + else delete (document as { hidden?: boolean }).hidden + } + unmount() + }) + + it('hides when the window loses focus', () => { + mount() + ctl.show('PAYLOAD') + act(() => { window.dispatchEvent(new Event('blur')) }) + expect(ctl.button()).toBeNull() + unmount() + }) + + it('watches the editor surface: off-viewport (tab switch / collapse) hides, on-viewport keeps', () => { + mount() + ctl.show('PAYLOAD') + const observer = FakeIntersectionObserver.last + expect(observer).not.toBeNull() + expect(observer!.targets.size).toBe(1) + // The observed target is the hook's surface element. + expect([...observer!.targets][0]?.getAttribute('data-surface')).toBe('true') + // Surfaced on screen (or display:none removed): the popup stays. + act(() => { observer!.fire(true) }) + expect(ctl.button()).not.toBeNull() + // Hidden / translated off-screen: the popup must not survive it. + act(() => { observer!.fire(false) }) + expect(ctl.button()).toBeNull() + unmount() + }) +}) \ No newline at end of file From a3379ef17d8a0665d3cb36a6872ac03706da029b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=87=BF=E9=A9=B9-=E4=BC=AF=E4=B9=90?= Date: Thu, 27 Aug 2026 12:53:47 +0800 Subject: [PATCH 2/3] fix(conversation): insert the selection popup text at the composer caret (#425) appendToDraft always spliced at the end of the draft. The conversation service exposes no caret API, so probe the live composer textarea selection from the DOM (value-sync guarded) and splice at that position with whitespace-aware joins; an unknown or stale caret falls back to appending. The explorer @-reference shares this path, so it inserts at the caret too. --- src/client/conversation-draft.ts | 84 +++++++++++++++++++-- tests/conversation-draft.spec.ts | 122 +++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 6 deletions(-) create mode 100644 tests/conversation-draft.spec.ts diff --git a/src/client/conversation-draft.ts b/src/client/conversation-draft.ts index 9047054..f6c8fef 100644 --- a/src/client/conversation-draft.ts +++ b/src/client/conversation-draft.ts @@ -1,16 +1,88 @@ /** - * Append text to the current session's composer draft through the + * Insert text into the current session's composer draft through the * conversation service — the shared path behind the explorer's @-reference * button and the viewer selection popup. The service is resolved lazily * through `ctx.get` (the inject-free read the app's own plugins use); a * missing service or scope degrades to a logged no-op, never a crash. + * + * Insert position (fixes upstream issue #425): the draft store only exposes + * the whole string (`getSnapshot().draft` + `setDraft(text)`) — there is no + * caret API on the conversation service. The composer's `