mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix(desktop): simplify changed-file open-with menus
Changed-file rows are easier to scan when previewable files are grouped before source-only files, and the file open-with menu should only expose useful preview, editor, and reveal actions. Output target cards also no longer need a separate copy control now that open/open-with are the primary actions. Constraint: Keep the existing open-with API and menu components shared across changed files, workspace context menus, and output target cards. Rejected: Keep system-default open for files | it duplicated less predictable platform behavior and added a low-value final menu item. Confidence: high Scope-risk: narrow Directive: File open-with menus should stay ordered by immediate preview/open usefulness before editor and reveal actions. Tested: cd desktop && bun run test src/lib/openWithItems.test.ts src/components/workspace/WorkspaceFileOpenWith.test.tsx src/components/chat/AssistantOutputTargetCard.test.tsx src/components/chat/CurrentTurnChangeCard.test.tsx Tested: cd desktop && bun run lint Tested: cd desktop && bun run build Tested: git diff --check
This commit is contained in:
parent
14c37d5fc1
commit
7ca702a253
@ -93,12 +93,9 @@ describe('AssistantOutputTargetCard', () => {
|
||||
expect(openBrowser).toHaveBeenCalledWith('s1', 'http://localhost:5173/')
|
||||
})
|
||||
|
||||
it('copies the normalized path when Copy is clicked', () => {
|
||||
const writeText = vi.fn().mockResolvedValue(undefined)
|
||||
Object.assign(navigator, { clipboard: { writeText } })
|
||||
it('does not render a copy button for output target cards', () => {
|
||||
render(<AssistantOutputTargetCard target={markdownTarget} sessionId="s1" />)
|
||||
fireEvent.click(screen.getByLabelText('assistantOutputs.copy'))
|
||||
expect(writeText).toHaveBeenCalledWith('docs/readme.md')
|
||||
expect(screen.queryByLabelText('assistantOutputs.copy')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('opens the open-with menu with URL items for a localhost target', async () => {
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
import { useCallback, useState } from 'react'
|
||||
import type { MouseEvent as ReactMouseEvent } from 'react'
|
||||
import { ChevronDown, Copy, ExternalLink, Globe } from 'lucide-react'
|
||||
import { ChevronDown, ExternalLink, Globe } from 'lucide-react'
|
||||
import type { AssistantOutputTarget } from '../../lib/assistantOutputTargets'
|
||||
import { useTranslation, type TranslationKey } from '../../i18n'
|
||||
import { OpenWithMenu } from '../common/OpenWithMenu'
|
||||
@ -11,7 +11,6 @@ import { getServerBaseUrl } from '../../lib/desktopRuntime'
|
||||
import { useOpenTargetStore } from '../../stores/openTargetStore'
|
||||
import { useBrowserPanelStore } from '../../stores/browserPanelStore'
|
||||
import { useWorkspacePanelStore } from '../../stores/workspacePanelStore'
|
||||
import { copyTextToClipboard } from './clipboard'
|
||||
|
||||
type Props = {
|
||||
target: AssistantOutputTarget
|
||||
@ -53,11 +52,6 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
|
||||
})
|
||||
}, [sessionId, target.href])
|
||||
|
||||
const handleCopy = useCallback((event: ReactMouseEvent<HTMLButtonElement>) => {
|
||||
event.stopPropagation()
|
||||
void copyTextToClipboard(target.normalizedPath ?? target.href)
|
||||
}, [target.href, target.normalizedPath])
|
||||
|
||||
const handleOpenWith = useCallback((event: ReactMouseEvent<HTMLButtonElement>) => {
|
||||
event.stopPropagation()
|
||||
// Toggle: a second click on the same trigger closes the menu. OpenWithMenu's
|
||||
@ -125,15 +119,6 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
|
||||
>
|
||||
<ExternalLink size={14} strokeWidth={2.2} aria-hidden="true" />
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleCopy}
|
||||
aria-label={t('assistantOutputs.copy')}
|
||||
title={t('assistantOutputs.copy')}
|
||||
className="inline-flex h-8 w-8 items-center justify-center rounded-full border border-[var(--color-border)]/70 bg-[var(--color-surface)] text-[var(--color-text-secondary)] transition-colors hover:border-[var(--color-brand)]/35 hover:text-[var(--color-text-primary)] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--color-brand)]/35"
|
||||
>
|
||||
<Copy size={14} strokeWidth={2.2} aria-hidden="true" />
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
aria-label={t('openWith.title')}
|
||||
|
||||
@ -143,6 +143,25 @@ describe('CurrentTurnChangeCard – rich file row (icon / name / type)', () => {
|
||||
expect(screen.getByText('index.ts')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('sorts previewable changed files before source-only files', () => {
|
||||
renderCard([
|
||||
'/w/proj/package.json',
|
||||
'/w/proj/preview.md',
|
||||
'/w/proj/src/main.ts',
|
||||
'/w/proj/index.html',
|
||||
'/w/proj/style.css',
|
||||
])
|
||||
|
||||
const rows = screen.getAllByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })
|
||||
expect(rows.map((row) => row.textContent)).toEqual([
|
||||
expect.stringContaining('preview.md'),
|
||||
expect.stringContaining('index.html'),
|
||||
expect.stringContaining('package.json'),
|
||||
expect.stringContaining('main.ts'),
|
||||
expect.stringContaining('style.css'),
|
||||
])
|
||||
})
|
||||
|
||||
it('renders the extension badge for a markdown file', () => {
|
||||
renderCard(['/w/proj/README.md'])
|
||||
// The type subtitle contains the ext in uppercase: "· MD"
|
||||
|
||||
@ -42,10 +42,12 @@ export function CurrentTurnChangeCard({
|
||||
const [showAllFiles, setShowAllFiles] = useState(false)
|
||||
|
||||
const files = useMemo<ChangedFileEntry[]>(
|
||||
() => checkpoint.code.filesChanged.map((filePath) => ({
|
||||
apiPath: filePath,
|
||||
displayPath: relativizeWorkspacePath(filePath, workDir),
|
||||
})),
|
||||
() => checkpoint.code.filesChanged
|
||||
.map((filePath) => ({
|
||||
apiPath: filePath,
|
||||
displayPath: relativizeWorkspacePath(filePath, workDir),
|
||||
}))
|
||||
.sort((a, b) => Number(isPreviewableChangedFile(b.displayPath)) - Number(isPreviewableChangedFile(a.displayPath))),
|
||||
[checkpoint.code.filesChanged, workDir],
|
||||
)
|
||||
|
||||
|
||||
@ -32,20 +32,18 @@ describe('WorkspaceFileOpenWith', () => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('renders an item for the IDE target, the file_manager target, and a system-default item', () => {
|
||||
it('renders only IDE and file-manager items', () => {
|
||||
const { getAllByRole } = render(
|
||||
<WorkspaceFileOpenWith absolutePath="/w/report.md" />,
|
||||
)
|
||||
|
||||
const menuItems = getAllByRole('menuitem')
|
||||
// IDE + file_manager + system = 3 items
|
||||
expect(menuItems).toHaveLength(3)
|
||||
expect(menuItems).toHaveLength(2)
|
||||
|
||||
const labels = menuItems.map((el) => el.textContent)
|
||||
expect(labels.some((l) => l?.includes('VS Code'))).toBe(true)
|
||||
expect(labels.some((l) => l?.includes('Finder'))).toBe(true)
|
||||
// system default item uses the 'openWith.systemDefault' key (returned as-is by mock)
|
||||
expect(labels.some((l) => l?.includes('openWith.systemDefault'))).toBe(true)
|
||||
expect(labels.some((l) => l?.includes('openWith.systemDefault'))).toBe(false)
|
||||
})
|
||||
|
||||
it('clicking the IDE item calls openTarget and onAfterSelect', () => {
|
||||
@ -64,26 +62,8 @@ describe('WorkspaceFileOpenWith', () => {
|
||||
expect(onAfter).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('clicking the system-default item calls shellOpen and onAfterSelect', async () => {
|
||||
const onAfter = vi.fn()
|
||||
const { getAllByRole } = render(
|
||||
<WorkspaceFileOpenWith absolutePath="/w/report.md" onAfterSelect={onAfter} />,
|
||||
)
|
||||
|
||||
const menuItems = getAllByRole('menuitem')
|
||||
const systemItem = menuItems.find((el) => el.textContent?.includes('openWith.systemDefault'))
|
||||
if (!systemItem) throw new Error('System default menu item not found')
|
||||
|
||||
fireEvent.click(systemItem)
|
||||
|
||||
// onAfterSelect is synchronous (called before the dynamic import chain)
|
||||
expect(onAfter).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Flush the microtask queue for the dynamic import + .then chain
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await Promise.resolve()
|
||||
}
|
||||
|
||||
expect(shellOpen).toHaveBeenCalledWith('/w/report.md')
|
||||
it('does not call shell open from the file open-with menu', () => {
|
||||
render(<WorkspaceFileOpenWith absolutePath="/w/report.md" />)
|
||||
expect(shellOpen).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@ -43,6 +43,8 @@ export function WorkspaceFileOpenWith({
|
||||
deps,
|
||||
)
|
||||
|
||||
if (items.length === 0) return null
|
||||
|
||||
return (
|
||||
<>
|
||||
<div className="my-1 border-t border-[var(--color-border)]" role="separator" />
|
||||
|
||||
@ -96,7 +96,6 @@ export const en = {
|
||||
'openWith.title': 'Open with',
|
||||
'openWith.inAppBrowser': 'In-app browser',
|
||||
'openWith.systemBrowser': 'System browser',
|
||||
'openWith.systemDefault': 'Open with system default',
|
||||
'openWith.workspacePreview': 'Workspace preview',
|
||||
'openWith.openInTarget': 'Open in {target}',
|
||||
'openWith.revealInTarget': 'Reveal in {target}',
|
||||
@ -113,7 +112,6 @@ export const en = {
|
||||
'assistantOutputs.kind.localhost': 'Localhost',
|
||||
'assistantOutputs.moreOutputs': '+{count} more',
|
||||
'assistantOutputs.open': 'Open',
|
||||
'assistantOutputs.copy': 'Copy path',
|
||||
|
||||
// ─── Workspace Panel ───────────────────────────────
|
||||
'workspace.changedFiles': 'Changed files',
|
||||
|
||||
@ -98,7 +98,6 @@ export const zh: Record<TranslationKey, string> = {
|
||||
'openWith.title': '打开方式',
|
||||
'openWith.inAppBrowser': '应用内浏览器',
|
||||
'openWith.systemBrowser': '系统浏览器',
|
||||
'openWith.systemDefault': '系统默认打开',
|
||||
'openWith.workspacePreview': '工作台预览',
|
||||
'openWith.openInTarget': '用 {target} 打开',
|
||||
'openWith.revealInTarget': '在 {target} 中显示',
|
||||
@ -115,7 +114,6 @@ export const zh: Record<TranslationKey, string> = {
|
||||
'assistantOutputs.kind.localhost': '本地服务',
|
||||
'assistantOutputs.moreOutputs': '+{count} 个输出',
|
||||
'assistantOutputs.open': '打开',
|
||||
'assistantOutputs.copy': '复制路径',
|
||||
|
||||
// ─── Workspace Panel ───────────────────────────────
|
||||
'workspace.changedFiles': '已更改文件',
|
||||
|
||||
@ -114,11 +114,11 @@ describe('buildOpenWithItems – url context', () => {
|
||||
})
|
||||
|
||||
describe('buildOpenWithItems – file context with targets', () => {
|
||||
it('returns [preview, ide:code, fm:finder, system] ids', () => {
|
||||
it('returns preview/open targets without a system-default fallback', () => {
|
||||
const deps = makeDeps()
|
||||
const ctx: OpenWithContext = { kind: 'file', absolutePath: '/w/a.md', relPath: 'a.md', previewable: true }
|
||||
const items = buildOpenWithItems(ctx, [ideTarget, fmTarget], deps)
|
||||
expect(items.map((i) => i.id)).toEqual(['preview', 'ide:code', 'fm:finder', 'system'])
|
||||
expect(items.map((i) => i.id)).toEqual(['preview', 'ide:code', 'fm:finder'])
|
||||
})
|
||||
|
||||
it('preview calls openWorkspacePreview with relPath', () => {
|
||||
@ -139,13 +139,12 @@ describe('buildOpenWithItems – file context with targets', () => {
|
||||
expect(deps.openTarget).toHaveBeenCalledWith('code', '/w/a.md')
|
||||
})
|
||||
|
||||
it('system calls openSystem with absolutePath', () => {
|
||||
it('does not include a system-default item for files', () => {
|
||||
const deps = makeDeps()
|
||||
const ctx: OpenWithContext = { kind: 'file', absolutePath: '/w/a.md', relPath: 'a.md', previewable: true }
|
||||
const items = buildOpenWithItems(ctx, [ideTarget, fmTarget], deps)
|
||||
const sys = items.find((i) => i.id === 'system')!
|
||||
sys.onSelect()
|
||||
expect(deps.openSystem).toHaveBeenCalledWith('/w/a.md')
|
||||
expect(items.some((i) => i.id === 'system')).toBe(false)
|
||||
expect(deps.openSystem).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('ide item carries the target object', () => {
|
||||
@ -158,7 +157,7 @@ describe('buildOpenWithItems – file context with targets', () => {
|
||||
})
|
||||
|
||||
describe('buildOpenWithItems – file context with inAppBrowserUrl (no previewable)', () => {
|
||||
it('returns [in-app, system] ids for no targets + inAppBrowserUrl', () => {
|
||||
it('returns only the browser preview item for no targets + inAppBrowserUrl', () => {
|
||||
const deps = makeDeps()
|
||||
const ctx: OpenWithContext = {
|
||||
kind: 'file',
|
||||
@ -166,7 +165,7 @@ describe('buildOpenWithItems – file context with inAppBrowserUrl (no previewab
|
||||
inAppBrowserUrl: 'http://127.0.0.1:4321/preview-fs/s1/page.html',
|
||||
}
|
||||
const items = buildOpenWithItems(ctx, [], deps)
|
||||
expect(items.map((i) => i.id)).toEqual(['in-app', 'system'])
|
||||
expect(items.map((i) => i.id)).toEqual(['in-app'])
|
||||
})
|
||||
|
||||
it('in-app calls openInAppBrowser with inAppBrowserUrl', () => {
|
||||
|
||||
@ -76,6 +76,5 @@ export function buildOpenWithItems(ctx: OpenWithContext, targets: OpenTarget[],
|
||||
for (const target of targets.filter((x) => x.kind === 'file_manager')) {
|
||||
items.push({ id: `fm:${target.id}`, label: deps.t('openWith.revealInTarget', { target: target.label }), icon: 'file-manager', target, onSelect: () => deps.openTarget(target.id, ctx.absolutePath) })
|
||||
}
|
||||
items.push({ id: 'system', label: deps.t('openWith.systemDefault'), icon: 'system', onSelect: () => deps.openSystem(ctx.absolutePath) })
|
||||
return items
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user