fix(desktop): improve in-app browser preview menus (#761)

Tested: cd desktop && bun run test -- src/components/common/OpenWithMenu.test.tsx src/components/workspace/WorkspaceFileOpenWith.test.tsx src/components/browser/BrowserSurface.test.tsx src/lib/handlePreviewLink.test.ts src/lib/openWithContextForHref.test.ts

Tested: bun run check:desktop

Confidence: high

Scope-risk: narrow
This commit is contained in:
程序员阿江(Relakkes) 2026-06-12 16:22:40 +08:00
parent 8c99b0f892
commit 34fe0761d2
10 changed files with 244 additions and 16 deletions

View File

@ -1,5 +1,5 @@
import '@testing-library/jest-dom' import '@testing-library/jest-dom'
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
beforeAll(() => { beforeAll(() => {
@ -22,6 +22,7 @@ import { useOverlayStore } from '../../stores/overlayStore'
afterEach(() => { afterEach(() => {
cleanup() cleanup()
vi.restoreAllMocks()
Object.values(bridge).forEach((f) => f.mockReset()) Object.values(bridge).forEach((f) => f.mockReset())
useBrowserPanelStore.setState(useBrowserPanelStore.getInitialState(), true) useBrowserPanelStore.setState(useBrowserPanelStore.getInitialState(), true)
// browserPanelStore.open() now also opens the unified workbench; keep it isolated. // browserPanelStore.open() now also opens the unified workbench; keep it isolated.
@ -36,6 +37,28 @@ describe('BrowserSurface', () => {
expect(bridge.open).toHaveBeenCalledWith('http://localhost:5173/', expect.objectContaining({ width: expect.any(Number) })) expect(bridge.open).toHaveBeenCalledWith('http://localhost:5173/', expect.objectContaining({ width: expect.any(Number) }))
}) })
it('waits for local preview URLs before opening the native preview', async () => {
const url = 'http://127.0.0.1:59028/preview-fs/s1/66estmutl_files/index.html'
const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, { status: 200 }))
useBrowserPanelStore.getState().open('s1', url)
render(<BrowserSurface sessionId="s1" />)
expect(within(screen.getByTestId('preview-host')).getByLabelText('加载中')).toBeInTheDocument()
expect(bridge.open).not.toHaveBeenCalled()
await waitFor(() => {
expect(fetchSpy).toHaveBeenCalledWith(url, expect.objectContaining({
method: 'HEAD',
cache: 'no-store',
}))
})
await waitFor(() => {
expect(bridge.open).toHaveBeenCalledWith(url, expect.objectContaining({ width: expect.any(Number) }))
})
fetchSpy.mockRestore()
})
it('renders an empty address bar without opening a preview for a blank session', () => { it('renders an empty address bar without opening a preview for a blank session', () => {
useBrowserPanelStore.getState().ensureBlank('s1') useBrowserPanelStore.getState().ensureBlank('s1')
render(<BrowserSurface sessionId="s1" />) render(<BrowserSurface sessionId="s1" />)
@ -131,7 +154,9 @@ describe('BrowserSurface', () => {
useBrowserPanelStore.getState().open('s1', 'http://localhost:5173/') useBrowserPanelStore.getState().open('s1', 'http://localhost:5173/')
render(<BrowserSurface sessionId="s1" />) render(<BrowserSurface sessionId="s1" />)
expect(useBrowserPanelStore.getState().bySession['s1']!.loading).toBe(true) expect(useBrowserPanelStore.getState().bySession['s1']!.loading).toBe(true)
vi.advanceTimersByTime(15000) act(() => {
vi.advanceTimersByTime(15000)
})
expect(useBrowserPanelStore.getState().bySession['s1']!.loading).toBe(false) expect(useBrowserPanelStore.getState().bySession['s1']!.loading).toBe(false)
} finally { } finally {
vi.useRealTimers() vi.useRealTimers()

View File

@ -1,14 +1,46 @@
import { useEffect, useLayoutEffect, useRef } from 'react' import { useEffect, useLayoutEffect, useRef } from 'react'
import { Camera, MousePointer2 } from 'lucide-react' import { Camera, Loader2, MousePointer2 } from 'lucide-react'
import { BrowserAddressBar } from './BrowserAddressBar' import { BrowserAddressBar } from './BrowserAddressBar'
import { computeWebviewBounds } from './computeWebviewBounds' import { computeWebviewBounds } from './computeWebviewBounds'
import { isLoopbackHostname } from '../../lib/desktopRuntime'
import { previewBridge } from '../../lib/previewBridge' import { previewBridge } from '../../lib/previewBridge'
import { subscribePreviewEvents } from '../../lib/previewEvents' import { subscribePreviewEvents } from '../../lib/previewEvents'
import { useBrowserPanelStore } from '../../stores/browserPanelStore' import { useBrowserPanelStore } from '../../stores/browserPanelStore'
import { useOverlayStore } from '../../stores/overlayStore' import { useOverlayStore } from '../../stores/overlayStore'
const LOCAL_PREVIEW_PATH_PREFIXES = ['/preview-fs/', '/local-file/']
const LOCAL_PREVIEW_READY_TIMEOUT_MS = 2500
function shouldWaitForLocalPreview(url: string): boolean {
try {
const parsed = new URL(url)
return isLoopbackHostname(parsed.hostname) &&
LOCAL_PREVIEW_PATH_PREFIXES.some((prefix) => parsed.pathname.startsWith(prefix))
} catch {
return false
}
}
async function waitForLocalPreview(url: string): Promise<void> {
const controller = new AbortController()
const timeout = window.setTimeout(() => controller.abort(), LOCAL_PREVIEW_READY_TIMEOUT_MS)
try {
await fetch(url, {
method: 'HEAD',
cache: 'no-store',
signal: controller.signal,
})
} catch {
// Best-effort warmup only. The native webview still navigates so users can
// see the server's own error page or use Reload if the first probe raced.
} finally {
window.clearTimeout(timeout)
}
}
export function BrowserSurface({ sessionId }: { sessionId: string }) { export function BrowserSurface({ sessionId }: { sessionId: string }) {
const hostRef = useRef<HTMLDivElement>(null) const hostRef = useRef<HTMLDivElement>(null)
const loadSeqRef = useRef(0)
const session = useBrowserPanelStore((s) => s.bySession[sessionId]) const session = useBrowserPanelStore((s) => s.bySession[sessionId])
const store = useBrowserPanelStore.getState() const store = useBrowserPanelStore.getState()
const overlayCount = useOverlayStore((s) => s.count) const overlayCount = useOverlayStore((s) => s.count)
@ -19,12 +51,36 @@ export function BrowserSurface({ sessionId }: { sessionId: string }) {
previewBridge.setBounds(computeWebviewBounds(el.getBoundingClientRect())) previewBridge.setBounds(computeWebviewBounds(el.getBoundingClientRect()))
} }
const loadNativePreview = (
url: string,
action: () => Promise<void>,
) => {
const seq = loadSeqRef.current + 1
loadSeqRef.current = seq
void (async () => {
if (shouldWaitForLocalPreview(url)) {
await waitForLocalPreview(url)
}
if (loadSeqRef.current !== seq) return
await action()
})().catch(() => {
if (loadSeqRef.current === seq) {
useBrowserPanelStore.getState().setLoading(sessionId, false)
}
})
}
useLayoutEffect(() => { useLayoutEffect(() => {
const el = hostRef.current const el = hostRef.current
if (el && session?.url) { if (el && session?.url) {
previewBridge.open(session.url, computeWebviewBounds(el.getBoundingClientRect())) const bounds = computeWebviewBounds(el.getBoundingClientRect())
const url = session.url
loadNativePreview(url, () => previewBridge.open(url, bounds))
}
return () => {
loadSeqRef.current += 1
previewBridge.close()
} }
return () => { previewBridge.close() }
// The visibility-sync effect below owns setVisible() — including the // The visibility-sync effect below owns setVisible() — including the
// initial reveal — so it always factors in overlayCount. // initial reveal — so it always factors in overlayCount.
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
@ -75,14 +131,15 @@ export function BrowserSurface({ sessionId }: { sessionId: string }) {
const current = useBrowserPanelStore.getState().bySession[sessionId] const current = useBrowserPanelStore.getState().bySession[sessionId]
store.navigate(sessionId, url) store.navigate(sessionId, url)
if (current?.url) { if (current?.url) {
previewBridge.navigate(url) loadNativePreview(url, () => previewBridge.navigate(url))
return return
} }
const el = hostRef.current const el = hostRef.current
if (el) { if (el) {
previewBridge.open(url, computeWebviewBounds(el.getBoundingClientRect())) const bounds = computeWebviewBounds(el.getBoundingClientRect())
loadNativePreview(url, () => previewBridge.open(url, bounds))
} else { } else {
previewBridge.navigate(url) loadNativePreview(url, () => previewBridge.navigate(url))
} }
} }
@ -135,16 +192,32 @@ export function BrowserSurface({ sessionId }: { sessionId: string }) {
canGoForward={session.canGoForward} canGoForward={session.canGoForward}
loading={session.loading} loading={session.loading}
onNavigate={openOrNavigate} onNavigate={openOrNavigate}
onBack={() => { store.goBack(sessionId); store.setLoading(sessionId, true); previewBridge.navigate(useBrowserPanelStore.getState().bySession[sessionId]!.url) }} onBack={() => {
onForward={() => { store.goForward(sessionId); store.setLoading(sessionId, true); previewBridge.navigate(useBrowserPanelStore.getState().bySession[sessionId]!.url) }} store.goBack(sessionId)
store.setLoading(sessionId, true)
const url = useBrowserPanelStore.getState().bySession[sessionId]!.url
loadNativePreview(url, () => previewBridge.navigate(url))
}}
onForward={() => {
store.goForward(sessionId)
store.setLoading(sessionId, true)
const url = useBrowserPanelStore.getState().bySession[sessionId]!.url
loadNativePreview(url, () => previewBridge.navigate(url))
}}
onReload={() => { onReload={() => {
if (!session.url) return if (!session.url) return
store.setLoading(sessionId, true) store.setLoading(sessionId, true)
previewBridge.navigate(session.url) loadNativePreview(session.url, () => previewBridge.navigate(session.url))
}} }}
rightActions={previewActions} rightActions={previewActions}
/> />
<div ref={hostRef} className="flex-1" data-testid="preview-host" /> <div ref={hostRef} className="relative flex-1 overflow-hidden" data-testid="preview-host">
{session.loading && (
<div className="pointer-events-none absolute inset-0 flex items-center justify-center bg-[var(--color-surface)] text-[var(--color-text-tertiary)]">
<Loader2 size={18} className="animate-spin" aria-label="加载中" />
</div>
)}
</div>
</div> </div>
) )
} }

View File

@ -69,6 +69,13 @@ describe('OpenWithMenu', () => {
expect(onClose).toHaveBeenCalledTimes(1) expect(onClose).toHaveBeenCalledTimes(1)
}) })
it('scrolling the viewport calls onClose', () => {
const onClose = vi.fn()
render(<OpenWithMenu items={makeItems()} anchor={anchor} onClose={onClose} />)
fireEvent.scroll(window)
expect(onClose).toHaveBeenCalledTimes(1)
})
describe('triggerEl exclusion (re-click-trigger toggle support)', () => { describe('triggerEl exclusion (re-click-trigger toggle support)', () => {
it('does NOT call onClose when mousedown lands inside triggerEl', () => { it('does NOT call onClose when mousedown lands inside triggerEl', () => {
// Set up a trigger element + a child within it in the document. // Set up a trigger element + a child within it in the document.

View File

@ -61,9 +61,17 @@ export function OpenWithMenu({ items, anchor, onClose, triggerEl }: Props) {
onClose() onClose()
} }
const onKey = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose() } const onKey = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose() }
const onViewportMove = () => onClose()
document.addEventListener('mousedown', onDown) document.addEventListener('mousedown', onDown)
document.addEventListener('keydown', onKey) document.addEventListener('keydown', onKey)
return () => { document.removeEventListener('mousedown', onDown); document.removeEventListener('keydown', onKey) } window.addEventListener('scroll', onViewportMove, true)
window.addEventListener('resize', onViewportMove)
return () => {
document.removeEventListener('mousedown', onDown)
document.removeEventListener('keydown', onKey)
window.removeEventListener('scroll', onViewportMove, true)
window.removeEventListener('resize', onViewportMove)
}
}, [onClose, triggerEl]) }, [onClose, triggerEl])
return createPortal( return createPortal(

View File

@ -7,6 +7,8 @@ import { browserHost } from '../../lib/desktopHost/browserHost'
const openTarget = vi.hoisted(() => vi.fn()) const openTarget = vi.hoisted(() => vi.fn())
const shellOpen = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)) const shellOpen = vi.hoisted(() => vi.fn().mockResolvedValue(undefined))
const hostOpenPath = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)) const hostOpenPath = vi.hoisted(() => vi.fn().mockResolvedValue(undefined))
const browserOpen = vi.hoisted(() => vi.fn())
const openPreview = vi.hoisted(() => vi.fn().mockResolvedValue(undefined))
vi.mock('../../stores/openTargetStore', () => ({ vi.mock('../../stores/openTargetStore', () => ({
useOpenTargetStore: (sel: (s: unknown) => unknown) => useOpenTargetStore: (sel: (s: unknown) => unknown) =>
@ -27,6 +29,18 @@ vi.mock('../../i18n', () => ({
v?.target ? `${k}:${v.target}` : k, v?.target ? `${k}:${v.target}` : k,
})) }))
vi.mock('../../stores/browserPanelStore', () => ({
useBrowserPanelStore: {
getState: () => ({ open: browserOpen }),
},
}))
vi.mock('../../stores/workspacePanelStore', () => ({
useWorkspacePanelStore: {
getState: () => ({ openPreview }),
},
}))
import { WorkspaceFileOpenWith } from './WorkspaceFileOpenWith' import { WorkspaceFileOpenWith } from './WorkspaceFileOpenWith'
describe('WorkspaceFileOpenWith', () => { describe('WorkspaceFileOpenWith', () => {
@ -82,4 +96,60 @@ describe('WorkspaceFileOpenWith', () => {
expect(shellOpen).not.toHaveBeenCalled() expect(shellOpen).not.toHaveBeenCalled()
expect(hostOpenPath).not.toHaveBeenCalled() expect(hostOpenPath).not.toHaveBeenCalled()
}) })
it('offers workspace preview and in-app browser for generated html files with session context', () => {
const { getAllByRole } = render(
<WorkspaceFileOpenWith
absolutePath="/w/66estmutl_files/index.html"
sessionId="s1"
workspacePath="66estmutl_files/index.html"
/>,
)
const menuItems = getAllByRole('menuitem')
const labels = menuItems.map((el) => el.textContent)
expect(labels.some((l) => l?.includes('openWith.workspacePreview'))).toBe(true)
expect(labels.some((l) => l?.includes('openWith.inAppBrowser'))).toBe(true)
expect(labels.some((l) => l?.includes('VS Code'))).toBe(true)
expect(labels.some((l) => l?.includes('Finder'))).toBe(true)
})
it('opens the in-app browser preview URL from workspace html files', () => {
const { getAllByRole } = render(
<WorkspaceFileOpenWith
absolutePath="/w/66estmutl_files/index.html"
sessionId="s1"
workspacePath="66estmutl_files/index.html"
/>,
)
const menuItems = getAllByRole('menuitem')
const inAppItem = menuItems.find((el) => el.textContent?.includes('openWith.inAppBrowser'))
if (!inAppItem) throw new Error('In-app browser menu item not found')
fireEvent.click(inAppItem)
expect(browserOpen).toHaveBeenCalledWith(
's1',
'http://127.0.0.1:3456/preview-fs/s1/66estmutl_files/index.html',
)
})
it('opens the workspace preview for workspace files with session context', () => {
const { getAllByRole } = render(
<WorkspaceFileOpenWith
absolutePath="/w/report.md"
sessionId="s1"
workspacePath="report.md"
/>,
)
const menuItems = getAllByRole('menuitem')
const previewItem = menuItems.find((el) => el.textContent?.includes('openWith.workspacePreview'))
if (!previewItem) throw new Error('Workspace preview menu item not found')
fireEvent.click(previewItem)
expect(openPreview).toHaveBeenCalledWith('s1', 'report.md', 'file')
})
}) })

View File

@ -4,6 +4,10 @@ import { useTranslation, type TranslationKey } from '../../i18n'
import { useOpenTargetStore } from '../../stores/openTargetStore' import { useOpenTargetStore } from '../../stores/openTargetStore'
import { buildOpenWithItems, type OpenWithItem, type OpenWithDeps } from '../../lib/openWithItems' import { buildOpenWithItems, type OpenWithItem, type OpenWithDeps } from '../../lib/openWithItems'
import { getDesktopHost } from '../../lib/desktopHost' import { getDesktopHost } from '../../lib/desktopHost'
import { getServerBaseUrl } from '../../lib/desktopRuntime'
import { openWithContextForWorkspaceFile } from '../../lib/openWithContextForHref'
import { useBrowserPanelStore } from '../../stores/browserPanelStore'
import { useWorkspacePanelStore } from '../../stores/workspacePanelStore'
import { TargetIcon } from '../common/TargetIcon' import { TargetIcon } from '../common/TargetIcon'
function openExternal(path: string) { function openExternal(path: string) {
@ -12,9 +16,13 @@ function openExternal(path: string) {
export function WorkspaceFileOpenWith({ export function WorkspaceFileOpenWith({
absolutePath, absolutePath,
sessionId,
workspacePath,
onAfterSelect, onAfterSelect,
}: { }: {
absolutePath: string absolutePath: string
sessionId?: string
workspacePath?: string
onAfterSelect?: () => void onAfterSelect?: () => void
}) { }) {
const t = useTranslation() const t = useTranslation()
@ -30,8 +38,14 @@ export function WorkspaceFileOpenWith({
// but OpenWithDeps.t expects (key: string, vars?: Record<string, string>) => string. // but OpenWithDeps.t expects (key: string, vars?: Record<string, string>) => string.
// All keys buildOpenWithItems calls are valid TranslationKeys, so the cast is safe. // All keys buildOpenWithItems calls are valid TranslationKeys, so the cast is safe.
const deps: OpenWithDeps = { const deps: OpenWithDeps = {
openInAppBrowser: () => {}, openInAppBrowser: (url) => {
openWorkspacePreview: () => {}, if (!sessionId) return
useBrowserPanelStore.getState().open(sessionId, url)
},
openWorkspacePreview: (relPath) => {
if (!sessionId) return
void useWorkspacePanelStore.getState().openPreview(sessionId, relPath, 'file')
},
openSystem: (p) => openExternal(p), openSystem: (p) => openExternal(p),
openTarget: (id, p) => { openTarget: (id, p) => {
void openTarget(id, p) void openTarget(id, p)
@ -39,7 +53,12 @@ export function WorkspaceFileOpenWith({
t: (key, vars) => t(key as TranslationKey, vars), t: (key, vars) => t(key as TranslationKey, vars),
} }
const items: OpenWithItem[] = buildOpenWithItems( const items: OpenWithItem[] = buildOpenWithItems(
{ kind: 'file', absolutePath, previewable: false }, sessionId && workspacePath
? openWithContextForWorkspaceFile(workspacePath, absolutePath, {
sessionId,
serverBaseUrl: getServerBaseUrl(),
})
: { kind: 'file', absolutePath, previewable: false },
targets, targets,
deps, deps,
) )

View File

@ -1516,6 +1516,8 @@ export function WorkspacePanel({ sessionId, embedded = false }: WorkspacePanelPr
</button> </button>
<WorkspaceFileOpenWith <WorkspaceFileOpenWith
absolutePath={resolveWorkspaceAttachmentPath(status?.workDir, fileContextMenu.path)} absolutePath={resolveWorkspaceAttachmentPath(status?.workDir, fileContextMenu.path)}
sessionId={sessionId}
workspacePath={fileContextMenu.isDirectory ? undefined : fileContextMenu.path}
onAfterSelect={() => setFileContextMenu(null)} onAfterSelect={() => setFileContextMenu(null)}
/> />
</div> </div>

View File

@ -64,6 +64,17 @@ describe('handlePreviewLink', () => {
expect(deps.openFilePreview).not.toHaveBeenCalled() expect(deps.openFilePreview).not.toHaveBeenCalled()
}) })
it('routes generated *_files index.html to openBrowser with the preview-fs url', () => {
const deps = makeDeps()
const handled = handlePreviewLink('66estmutl_files/index.html', deps)
expect(handled).toBe(true)
expect(deps.openBrowser).toHaveBeenCalledWith(
's1',
'http://127.0.0.1:8787/preview-fs/s1/66estmutl_files/index.html',
)
expect(deps.openFilePreview).not.toHaveBeenCalled()
})
it('routes a frontend project index.html to workspace preview instead of static browser preview', () => { it('routes a frontend project index.html to workspace preview instead of static browser preview', () => {
const deps = makeDeps() const deps = makeDeps()
const handled = handlePreviewLink('todo-app/index.html', deps) const handled = handlePreviewLink('todo-app/index.html', deps)

View File

@ -1,5 +1,6 @@
const HTML_EXT = /\.(html?|xhtml)$/i const HTML_EXT = /\.(html?|xhtml)$/i
const INDEX_HTML_RE = /(^|\/)index\.html?$/i const INDEX_HTML_RE = /(^|\/)index\.html?$/i
const GENERATED_STATIC_FILES_DIR_RE = /(^|\/)[^/]+_files\/index\.html?$/i
const STATIC_OUTPUT_DIRS = new Set([ const STATIC_OUTPUT_DIRS = new Set([
'build', 'build',
@ -28,6 +29,7 @@ export function shouldOfferStaticHtmlPreview(filePath: string): boolean {
const normalized = normalizePathForPolicy(filePath) const normalized = normalizePathForPolicy(filePath)
if (!HTML_EXT.test(normalized)) return false if (!HTML_EXT.test(normalized)) return false
if (!INDEX_HTML_RE.test(normalized)) return true if (!INDEX_HTML_RE.test(normalized)) return true
if (GENERATED_STATIC_FILES_DIR_RE.test(normalized)) return true
return normalized return normalized
.split('/') .split('/')

View File

@ -68,6 +68,17 @@ describe('openWithContextForWorkspaceFile', () => {
}) })
}) })
it('generated *_files index.html rel path → has inAppBrowserUrl equal to previewFsUrl', () => {
const result = openWithContextForWorkspaceFile('66estmutl_files/index.html', '/w/proj/66estmutl_files/index.html', { sessionId: SESSION, serverBaseUrl: BASE })
expect(result).toEqual({
kind: 'file',
absolutePath: '/w/proj/66estmutl_files/index.html',
relPath: '66estmutl_files/index.html',
previewable: true,
inAppBrowserUrl: previewFsUrl(BASE, SESSION, '66estmutl_files/index.html'),
})
})
it('built dist index.html rel path → has inAppBrowserUrl equal to previewFsUrl', () => { it('built dist index.html rel path → has inAppBrowserUrl equal to previewFsUrl', () => {
const result = openWithContextForWorkspaceFile('todo-app/dist/index.html', '/w/proj/todo-app/dist/index.html', { sessionId: SESSION, serverBaseUrl: BASE }) const result = openWithContextForWorkspaceFile('todo-app/dist/index.html', '/w/proj/todo-app/dist/index.html', { sessionId: SESSION, serverBaseUrl: BASE })
expect(result).toEqual({ expect(result).toEqual({