mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
fix(web): keep line numbers clear when code wraps (#1260)
* fix(web): reserve code gutter padding * fix(web): expose diff wrap controls * test(ci): run terminal wrap regression
This commit is contained in:
@@ -0,0 +1,18 @@
|
||||
<!doctype html>
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="UTF-8" />
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1" />
|
||||
<title>HAPI terminal wrap fixture</title>
|
||||
<style>
|
||||
html { background: #fff }
|
||||
html[data-theme="dark"] { background: #1c1c1e; color-scheme: dark }
|
||||
body { margin: 0; padding: 16px; font-family: system-ui, sans-serif }
|
||||
#root { max-width: 720px; margin: 0 auto }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<div id="root"></div>
|
||||
<script type="module" src="./terminal-wrap-fixture.tsx"></script>
|
||||
</body>
|
||||
</html>
|
||||
@@ -0,0 +1,66 @@
|
||||
import React from 'react'
|
||||
import ReactDOM from 'react-dom/client'
|
||||
import '../src/index.css'
|
||||
import { CliOutputBlock } from '../src/components/CliOutputBlock'
|
||||
import { DiffView } from '../src/components/DiffView'
|
||||
import { ToolCard } from '../src/components/ToolCard/ToolCard'
|
||||
import { I18nProvider } from '../src/lib/i18n-context'
|
||||
import type { ApiClient } from '../src/api/client'
|
||||
import type { ToolCallBlock } from '../src/chat/types'
|
||||
|
||||
const terminalPayload = `<command-name>node scripts/render-report --source ./fixtures/mobile-terminal-wrap-fidelity-with-a-deliberately-long-path-and-unbroken-identifier.json --destination ./artifacts/mobile-preview.md</command-name><command-args>--format markdown
|
||||
--include "한글 mixed-language summary"
|
||||
--filter "status:active AND owner:platform"
|
||||
--verbose</command-args><local-command-stdout>| 항목 | 상태 | 설명 |
|
||||
| --- | --- | --- |
|
||||
| mobile-wrap | 성공 | 한글과 English text are both preserved |
|
||||
|
||||
long stdout text wraps naturally at the mobile code surface without changing source whitespace
|
||||
${Array.from({ length: 100 }, (_, index) => `row-${String(index + 1).padStart(3, '0')} | value`).join('\n')}</local-command-stdout>`
|
||||
|
||||
const codexDiffBlock: ToolCallBlock = {
|
||||
kind: 'tool-call', id: 'fixture-codex-diff', localId: null, createdAt: 1_000,
|
||||
tool: {
|
||||
id: 'fixture-codex-diff', name: 'CodexDiff', state: 'completed',
|
||||
input: { unified_diff: 'diff --git a/example.ts b/example.ts\n--- a/example.ts\n+++ b/example.ts\n@@ -1 +1 @@\n-before\n+after with a deliberately long value that must wrap in the CodexDiff ToolCard' },
|
||||
createdAt: 1_000, startedAt: 1_000, completedAt: 1_100, execStartedAt: null, execCompletedAt: null, description: null,
|
||||
}, children: [],
|
||||
}
|
||||
|
||||
function TerminalWrapFixture() {
|
||||
return (
|
||||
<div className="flex flex-col gap-4" data-testid="terminal-wrap-fixture">
|
||||
<CliOutputBlock text={terminalPayload} />
|
||||
<div data-testid="diff-preview">
|
||||
<DiffView
|
||||
oldString="const status = 'before'\n"
|
||||
newString="const status = 'after with a deliberately long value that must use the shared global wrap preference'\n"
|
||||
filePath="src/mobile-terminal.ts"
|
||||
/>
|
||||
</div>
|
||||
<div data-testid="diff-inline">
|
||||
<DiffView
|
||||
oldString="const status = 'before'\n"
|
||||
newString="const status = 'after with a deliberately long value that must wrap in the standalone inline surface'\n"
|
||||
filePath="src/standalone-inline.ts"
|
||||
variant="inline"
|
||||
size="comfortable"
|
||||
/>
|
||||
</div>
|
||||
<div data-testid="toolcard-codex-diff">
|
||||
<ToolCard api={{} as ApiClient} sessionId="fixture-session" metadata={null} terminalToolDisplayMode="detailed" disabled={false} onDone={() => {}} block={codexDiffBlock} />
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
const rootEl = document.getElementById('root')
|
||||
if (rootEl) {
|
||||
ReactDOM.createRoot(rootEl).render(
|
||||
<React.StrictMode>
|
||||
<I18nProvider>
|
||||
<TerminalWrapFixture />
|
||||
</I18nProvider>
|
||||
</React.StrictMode>
|
||||
)
|
||||
}
|
||||
@@ -104,6 +104,29 @@ describe('CodeBlock', () => {
|
||||
expect(screen.getByRole('button', { pressed: true })).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it.each([1, 12, 123])('reserves the gutter padding outside the %i-digit number track', (lineCount) => {
|
||||
const { container } = render(
|
||||
<I18nProvider>
|
||||
<CodeBlock code={Array.from({ length: lineCount }, (_, index) => `line ${index + 1}`).join('\n')} language="text" />
|
||||
</I18nProvider>
|
||||
)
|
||||
|
||||
const grid = container.querySelector('[data-hapi-code-grid="true"]') as HTMLElement
|
||||
expect(grid.style.gridTemplateColumns).toBe('calc(3ch + 1.5rem) max-content')
|
||||
})
|
||||
|
||||
it('uses the natural pre-wrap layout without hiding or shifting leading whitespace', () => {
|
||||
window.localStorage.setItem('hapi-code-wrap', '1')
|
||||
const source = ' \t --format a-deliberately-long-terminal-argument'
|
||||
const { container } = render(<I18nProvider><CodeBlock code={source} language="shellscript" /></I18nProvider>)
|
||||
const codeCell = container.querySelector('[data-code-cell]') as HTMLElement
|
||||
|
||||
expect(codeCell.style.paddingLeft).toBe('')
|
||||
expect(codeCell.style.tabSize).toBe('')
|
||||
expect(codeCell.querySelector('[data-code-leading-indent]')).toBeNull()
|
||||
expect(codeCell.textContent).toBe(source)
|
||||
})
|
||||
|
||||
it('renders the plain-text fallback as per-line rows when highlighting is unavailable', () => {
|
||||
const { container } = render(
|
||||
<I18nProvider>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { CSSProperties, ReactNode } from 'react'
|
||||
import { type CSSProperties, type ReactNode } from 'react'
|
||||
import { useCopyToClipboard } from '@/hooks/useCopyToClipboard'
|
||||
import { useCodeWrap } from '@/hooks/useCodeWrap'
|
||||
import { useShikiHighlightedLines, splitCodeLines } from '@/lib/shiki'
|
||||
@@ -9,6 +9,7 @@ const DEFAULT_COLLAPSE_LINE_THRESHOLD = 18
|
||||
const DEFAULT_COLLAPSE_CHAR_THRESHOLD = 1800
|
||||
const DEFAULT_COLLAPSED_HEIGHT = 260
|
||||
const DEFAULT_SCROLL_HEIGHT = 420
|
||||
const GUTTER_HORIZONTAL_PADDING_REM = 1.5
|
||||
|
||||
function shouldCollapseCode(code: string, lineThreshold: number, charThreshold: number): boolean {
|
||||
if (code.length > charThreshold) return true
|
||||
@@ -75,7 +76,7 @@ export function CodeBlock(props: {
|
||||
// (minmax(0,1fr)) so long lines wrap instead of overflowing; unwrapped it
|
||||
// grows to its content (max-content) inside the horizontal-scroll body.
|
||||
const codeGridStyle = {
|
||||
gridTemplateColumns: `${lineNumberWidth}ch ${codeWrap ? 'minmax(0, 1fr)' : 'max-content'}`
|
||||
gridTemplateColumns: `calc(${lineNumberWidth}ch + ${GUTTER_HORIZONTAL_PADDING_REM}rem) ${codeWrap ? 'minmax(0, 1fr)' : 'max-content'}`
|
||||
} satisfies CSSProperties
|
||||
const codeCellStyle = codeWrap
|
||||
? { whiteSpace: 'pre-wrap' as const, wordBreak: 'break-word' as const }
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { cleanup, render, screen } from '@testing-library/react'
|
||||
import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
|
||||
import { I18nProvider } from '@/lib/i18n-context'
|
||||
import { DiffView } from '@/components/DiffView'
|
||||
|
||||
@@ -73,7 +73,7 @@ describe('DiffView', () => {
|
||||
expect(row?.children[1]).toHaveClass('text-left')
|
||||
})
|
||||
|
||||
it('comfortable rows default to whitespace-pre (wrap off, no toggle button on DiffView)', () => {
|
||||
it('comfortable rows default to whitespace-pre and expose a shared wrap toggle', () => {
|
||||
const { container } = render(
|
||||
<I18nProvider>
|
||||
<DiffView
|
||||
@@ -86,10 +86,43 @@ describe('DiffView', () => {
|
||||
)
|
||||
|
||||
expect(container.querySelector('.whitespace-pre:not(.whitespace-pre-wrap)')).not.toBeNull()
|
||||
// DiffView consumes the global wrap value but exposes no toggle
|
||||
// button; assert by aria-pressed absence rather than a localized title.
|
||||
expect(screen.queryByRole('button', { pressed: false })).toBeNull()
|
||||
expect(screen.queryByRole('button', { pressed: true })).toBeNull()
|
||||
const wrapToggle = screen.getByRole('button', { pressed: false })
|
||||
fireEvent.click(wrapToggle)
|
||||
expect(screen.getByRole('button', { pressed: true })).toBeInTheDocument()
|
||||
expect(container.querySelector('.whitespace-pre-wrap')).not.toBeNull()
|
||||
})
|
||||
|
||||
it('keeps the visible preview header trigger and wrap action as sibling buttons', () => {
|
||||
const { container } = render(
|
||||
<I18nProvider>
|
||||
<DiffView oldString="before\n" newString="after\n" filePath="example.ts" />
|
||||
</I18nProvider>
|
||||
)
|
||||
|
||||
expect(container.querySelectorAll('[data-hapi-code-wrap-toggle="true"]')).toHaveLength(1)
|
||||
expect(container.querySelectorAll('button button')).toHaveLength(0)
|
||||
expect(container.querySelectorAll('button[aria-haspopup="dialog"]')).toHaveLength(1)
|
||||
expect(screen.getByRole('button', { name: 'Open diff for example.ts' })).toContainElement(screen.getByText('View'))
|
||||
const wrapToggle = container.querySelector('[data-hapi-code-wrap-toggle="true"]')!
|
||||
expect(wrapToggle).toHaveAttribute('data-hapi-share-export-exclude', 'true')
|
||||
expect(wrapToggle).toHaveAttribute('data-hapi-wrap-enable-label')
|
||||
expect(wrapToggle).toHaveAttribute('data-hapi-wrap-disable-label')
|
||||
})
|
||||
|
||||
it('renders the same toggle in the opened preview dialog and restores focus to its header trigger', async () => {
|
||||
render(
|
||||
<I18nProvider>
|
||||
<DiffView oldString="before\n" newString="after\n" filePath="example.ts" />
|
||||
</I18nProvider>
|
||||
)
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open diff for example.ts' }))
|
||||
const dialog = screen.getByRole('dialog')
|
||||
expect(within(dialog).getByRole('button', { pressed: false })).toBeInTheDocument()
|
||||
fireEvent.click(within(dialog).getByRole('button', { name: 'Close' }))
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('button', { name: 'Open diff for example.ts' })).toHaveFocus()
|
||||
})
|
||||
})
|
||||
|
||||
it('compact rows also follow the global wrap preference (previously hard-coded to wrap)', () => {
|
||||
|
||||
+110
-79
@@ -4,6 +4,7 @@ import { useMemo } from 'react'
|
||||
import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle, DialogTrigger } from '@/components/ui/dialog'
|
||||
import { usePointerFocusRing } from '@/hooks/usePointerFocusRing'
|
||||
import { useCodeWrap } from '@/hooks/useCodeWrap'
|
||||
import { WrapIcon } from '@/components/icons'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { useTranslation } from '@/lib/use-translation'
|
||||
|
||||
@@ -42,6 +43,33 @@ function DiffStatBadge(props: { tone: 'added' | 'removed'; value: number }) {
|
||||
)
|
||||
}
|
||||
|
||||
function DiffWrapToggle() {
|
||||
const { t } = useTranslation()
|
||||
const { codeWrap, setCodeWrap } = useCodeWrap()
|
||||
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
data-hapi-code-wrap-toggle="true"
|
||||
data-hapi-wrap-enable-label={t('code.wrap.enable')}
|
||||
data-hapi-wrap-disable-label={t('code.wrap.disable')}
|
||||
data-hapi-share-export-exclude="true"
|
||||
onClick={(event) => {
|
||||
event.stopPropagation()
|
||||
setCodeWrap(!codeWrap)
|
||||
}}
|
||||
className={cn(
|
||||
'rounded-md p-1 transition-colors hover:bg-[var(--app-code-copy-hover-bg)] hover:text-[var(--app-fg)]',
|
||||
codeWrap ? 'text-[var(--app-fg)]' : 'text-[var(--app-code-header-fg)]'
|
||||
)}
|
||||
title={t(codeWrap ? 'code.wrap.disable' : 'code.wrap.enable')}
|
||||
aria-pressed={codeWrap}
|
||||
>
|
||||
<WrapIcon className="h-3.5 w-3.5" />
|
||||
</button>
|
||||
)
|
||||
}
|
||||
|
||||
export function DiffView(props: {
|
||||
oldString: string
|
||||
newString: string
|
||||
@@ -88,20 +116,20 @@ export function DiffView(props: {
|
||||
|
||||
return (
|
||||
<Dialog>
|
||||
<DialogTrigger asChild>
|
||||
<button
|
||||
type="button"
|
||||
aria-label={props.filePath ? `Open diff for ${props.filePath}` : 'Open diff preview'}
|
||||
className={cn(
|
||||
'w-full text-left focus:outline-none focus-visible:ring-2 focus-visible:ring-[var(--app-link)]',
|
||||
suppressFocusRing && 'focus-visible:ring-0'
|
||||
)}
|
||||
onPointerDown={onTriggerPointerDown}
|
||||
onKeyDown={onTriggerKeyDown}
|
||||
onBlur={onTriggerBlur}
|
||||
>
|
||||
<div className="overflow-hidden rounded-2xl bg-[var(--app-code-bg)] transition-colors">
|
||||
<div className="flex items-center justify-between gap-3 bg-[var(--app-code-header-bg)] px-3 py-2">
|
||||
<div className="overflow-hidden rounded-2xl bg-[var(--app-code-bg)] transition-colors">
|
||||
<div className="flex items-center gap-3 bg-[var(--app-code-header-bg)] px-3 py-2">
|
||||
<DialogTrigger asChild>
|
||||
<button
|
||||
type="button"
|
||||
aria-label={props.filePath ? `Open diff for ${props.filePath}` : 'Open diff preview'}
|
||||
className={cn(
|
||||
'flex min-w-0 flex-1 items-center justify-between gap-3 text-left focus:outline-none focus-visible:ring-2 focus-visible:ring-[var(--app-link)]',
|
||||
suppressFocusRing && 'focus-visible:ring-0'
|
||||
)}
|
||||
onPointerDown={onTriggerPointerDown}
|
||||
onKeyDown={onTriggerKeyDown}
|
||||
onBlur={onTriggerBlur}
|
||||
>
|
||||
<div className="min-w-0">
|
||||
<div className="truncate font-mono text-[11px] uppercase tracking-[0.08em] text-[var(--app-code-header-fg)]">
|
||||
{props.filePath ?? t('diff.title')}
|
||||
@@ -115,20 +143,21 @@ export function DiffView(props: {
|
||||
<DiffStatBadge tone="removed" value={stats.deletions} />
|
||||
<span className="text-xs font-medium text-[var(--app-link)]">{t('diff.view')}</span>
|
||||
</div>
|
||||
</div>
|
||||
<div className="max-h-40 overflow-hidden">
|
||||
<DiffInlineView
|
||||
oldString={props.oldString}
|
||||
newString={props.newString}
|
||||
additions={stats.additions}
|
||||
deletions={stats.deletions}
|
||||
showHeader={false}
|
||||
size={props.size}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</button>
|
||||
</DialogTrigger>
|
||||
</button>
|
||||
</DialogTrigger>
|
||||
<DiffWrapToggle />
|
||||
</div>
|
||||
<div className="max-h-40 overflow-hidden">
|
||||
<DiffInlineView
|
||||
oldString={props.oldString}
|
||||
newString={props.newString}
|
||||
additions={stats.additions}
|
||||
deletions={stats.deletions}
|
||||
showHeader={false}
|
||||
size={props.size}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<DialogContent className="max-w-5xl">
|
||||
<DialogHeader>
|
||||
<DialogTitle className="break-all">{title}</DialogTitle>
|
||||
@@ -175,67 +204,69 @@ function DiffInlineView(props: {
|
||||
let oldLineNumber = 1
|
||||
let newLineNumber = 1
|
||||
|
||||
const body = (
|
||||
<div
|
||||
className={cn(
|
||||
codeWrap ? '' : 'overflow-x-auto',
|
||||
props.scrollY ? 'overflow-y-auto' : 'overflow-y-hidden'
|
||||
)}
|
||||
style={props.scrollY ? { maxHeight: props.maxHeight ?? 420 } : undefined}
|
||||
>
|
||||
<div className={cn(
|
||||
'font-mono',
|
||||
isComfortable ? 'text-sm leading-6' : 'text-xs',
|
||||
codeWrap ? 'w-full' : 'w-max min-w-full'
|
||||
)}>
|
||||
{diff.map((part, i) => {
|
||||
const lines = splitDiffLines(part.value)
|
||||
|
||||
return (
|
||||
<div key={i}>
|
||||
{lines.map((line, j) => {
|
||||
const prefix = part.added ? '+' : part.removed ? '-' : ' '
|
||||
const leftNumber = part.added ? '' : String(oldLineNumber++)
|
||||
const rightNumber = part.removed ? '' : String(newLineNumber++)
|
||||
const rowClass = cn(
|
||||
'grid min-w-full gap-3',
|
||||
isComfortable ? 'px-4' : 'px-3',
|
||||
isComfortable ? 'py-0' : 'py-1.5',
|
||||
part.added && 'bg-[var(--app-diff-added-bg)] text-[var(--app-diff-added-text)]',
|
||||
part.removed && 'bg-[var(--app-diff-removed-bg)] text-[var(--app-diff-removed-text)]'
|
||||
)
|
||||
|
||||
return (
|
||||
<div key={j} className={rowClass} style={rowStyle}>
|
||||
<div className={cn('text-left text-[var(--app-hint)]/80', isComfortable ? 'text-xs leading-6' : 'text-[10px]')}>{leftNumber}</div>
|
||||
<div className={cn('text-left text-[var(--app-hint)]/80', isComfortable ? 'text-xs leading-6' : 'text-[10px]')}>{rightNumber}</div>
|
||||
<div className={cn(
|
||||
codeWrap ? 'min-w-0 whitespace-pre-wrap break-words' : 'whitespace-pre'
|
||||
)}>
|
||||
<span className="mr-2 inline-block w-3 text-[var(--app-hint)]/90">{prefix}</span>
|
||||
<span>{line}</span>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
|
||||
return (
|
||||
<div className={cn('overflow-hidden bg-[var(--app-code-bg)]', props.showHeader ? 'rounded-2xl' : 'rounded-none')}>
|
||||
{props.showHeader ? (
|
||||
<div className="flex items-center justify-between gap-3 bg-[var(--app-code-header-bg)] px-3 py-2">
|
||||
<div className="min-w-0 truncate font-mono text-[11px] uppercase tracking-[0.08em] text-[var(--app-code-header-fg)]">
|
||||
{props.filePath ?? 'Diff'}
|
||||
</div>
|
||||
<div className="min-w-0 flex-1 truncate font-mono text-[11px] uppercase tracking-[0.08em] text-[var(--app-code-header-fg)]">{props.filePath ?? 'Diff'}</div>
|
||||
<div className="flex shrink-0 items-center gap-1.5">
|
||||
<DiffStatBadge tone="added" value={props.additions} />
|
||||
<DiffStatBadge tone="removed" value={props.deletions} />
|
||||
<DiffWrapToggle />
|
||||
</div>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
<div
|
||||
className={cn(
|
||||
codeWrap ? '' : 'overflow-x-auto',
|
||||
props.scrollY ? 'overflow-y-auto' : 'overflow-y-hidden'
|
||||
)}
|
||||
style={props.scrollY ? { maxHeight: props.maxHeight ?? 420 } : undefined}
|
||||
>
|
||||
<div className={cn(
|
||||
'font-mono',
|
||||
isComfortable ? 'text-sm leading-6' : 'text-xs',
|
||||
codeWrap ? 'w-full' : 'w-max min-w-full'
|
||||
)}>
|
||||
{diff.map((part, i) => {
|
||||
const lines = splitDiffLines(part.value)
|
||||
|
||||
return (
|
||||
<div key={i}>
|
||||
{lines.map((line, j) => {
|
||||
const prefix = part.added ? '+' : part.removed ? '-' : ' '
|
||||
const leftNumber = part.added ? '' : String(oldLineNumber++)
|
||||
const rightNumber = part.removed ? '' : String(newLineNumber++)
|
||||
const rowClass = cn(
|
||||
'grid min-w-full gap-3',
|
||||
isComfortable ? 'px-4' : 'px-3',
|
||||
isComfortable ? 'py-0' : 'py-1.5',
|
||||
part.added && 'bg-[var(--app-diff-added-bg)] text-[var(--app-diff-added-text)]',
|
||||
part.removed && 'bg-[var(--app-diff-removed-bg)] text-[var(--app-diff-removed-text)]'
|
||||
)
|
||||
|
||||
return (
|
||||
<div key={j} className={rowClass} style={rowStyle}>
|
||||
<div className={cn('text-left text-[var(--app-hint)]/80', isComfortable ? 'text-xs leading-6' : 'text-[10px]')}>{leftNumber}</div>
|
||||
<div className={cn('text-left text-[var(--app-hint)]/80', isComfortable ? 'text-xs leading-6' : 'text-[10px]')}>{rightNumber}</div>
|
||||
<div className={cn(
|
||||
codeWrap ? 'min-w-0 whitespace-pre-wrap break-words' : 'whitespace-pre'
|
||||
)}>
|
||||
<span className="mr-2 inline-block w-3 text-[var(--app-hint)]/90">{prefix}</span>
|
||||
<span>{line}</span>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
{body}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -61,6 +61,10 @@ describe('ToolCard terminal display mode helpers', () => {
|
||||
expect(shouldShowInlineToolCardBody('Task', false, 'detailed')).toBe(false)
|
||||
expect(shouldShowInlineToolCardBody('Agent', false, 'detailed')).toBe(false)
|
||||
expect(shouldShowInlineToolCardBody('Read', true, 'detailed')).toBe(false)
|
||||
expect(shouldShowInlineToolCardBody('Edit', true, 'detailed')).toBe(false)
|
||||
expect(shouldShowInlineToolCardBody('MultiEdit', true, 'detailed')).toBe(false)
|
||||
expect(shouldShowInlineToolCardBody('Write', true, 'detailed')).toBe(false)
|
||||
expect(shouldShowInlineToolCardBody('CodexDiff', true, 'detailed')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { ChatBlock, ChatToolCall, ToolCallBlock } from '@/chat/types'
|
||||
import type { ApiClient } from '@/api/client'
|
||||
import type { SessionMetadataSummary } from '@/types/api'
|
||||
import { memo, useEffect, useMemo, useState, type KeyboardEvent, type MouseEvent, type ReactNode } from 'react'
|
||||
import { memo, useEffect, useMemo, useRef, useState, type KeyboardEvent, type MouseEvent, type ReactNode } from 'react'
|
||||
import { getClaudeModelLabel, isObject, safeStringify } from '@hapi/protocol'
|
||||
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'
|
||||
import { CodeBlock } from '@/components/CodeBlock'
|
||||
@@ -430,6 +430,7 @@ function ToolCardInner(props: ToolCardProps) {
|
||||
const useCompactTerminalCard = shouldUseCompactTerminalToolCard(toolName, props.terminalToolDisplayMode)
|
||||
const showInline = shouldShowInlineToolCardBody(toolName, presentation.minimal, props.terminalToolDisplayMode)
|
||||
const CompactToolView = showInline ? getToolViewComponent(toolName) : null
|
||||
const compactViewOwnsInteractions = toolName === 'CodexDiff'
|
||||
const ResultToolView = getToolResultViewComponent(toolName)
|
||||
const permission = props.block.tool.permission
|
||||
const isAskUserQuestion = isAskUserQuestionToolName(toolName)
|
||||
@@ -447,15 +448,18 @@ function ToolCardInner(props: ToolCardProps) {
|
||||
: (subtitle ? 'mt-1' : 'mt-0')
|
||||
const stateColor = toolStatusColorClass(props.block.tool.state)
|
||||
const { suppressFocusRing, onTriggerPointerDown, onTriggerKeyDown, onTriggerBlur } = usePointerFocusRing()
|
||||
const inlineDetailInvokerRef = useRef<HTMLElement | null>(null)
|
||||
const openDetails = () => setDetailsOpen(true)
|
||||
const openDetailsFromInlinePreview = (event: MouseEvent<HTMLElement>) => {
|
||||
if (isNestedInteractiveElement(event)) return
|
||||
inlineDetailInvokerRef.current = event.currentTarget
|
||||
openDetails()
|
||||
}
|
||||
const openDetailsFromInlinePreviewKeyDown = (event: KeyboardEvent<HTMLElement>) => {
|
||||
if (isNestedInteractiveElement(event)) return
|
||||
if (event.key === 'Enter' || event.key === ' ') {
|
||||
event.preventDefault()
|
||||
inlineDetailInvokerRef.current = event.currentTarget
|
||||
openDetails()
|
||||
}
|
||||
}
|
||||
@@ -526,7 +530,18 @@ function ToolCardInner(props: ToolCardProps) {
|
||||
{header}
|
||||
</button>
|
||||
</DialogTrigger>
|
||||
<DialogContent className="max-w-2xl" closeButtonClassName="top-2" aria-describedby={undefined}>
|
||||
<DialogContent
|
||||
className="max-w-2xl"
|
||||
closeButtonClassName="top-2"
|
||||
aria-describedby={undefined}
|
||||
onCloseAutoFocus={(event) => {
|
||||
const invoker = inlineDetailInvokerRef.current
|
||||
if (!invoker?.isConnected) return
|
||||
event.preventDefault()
|
||||
invoker.focus()
|
||||
inlineDetailInvokerRef.current = null
|
||||
}}
|
||||
>
|
||||
<DialogHeader className="text-left">
|
||||
<DialogTitle>{toolTitle}</DialogTitle>
|
||||
</DialogHeader>
|
||||
@@ -545,7 +560,11 @@ function ToolCardInner(props: ToolCardProps) {
|
||||
|
||||
{showInline ? (
|
||||
CompactToolView ? (
|
||||
<div
|
||||
compactViewOwnsInteractions ? (
|
||||
<div className={cn(inlineBodySpacing, 'rounded-xl')}>
|
||||
<CompactToolView block={props.block} metadata={props.metadata} surface="inline" />
|
||||
</div>
|
||||
) : <div
|
||||
className={cn(
|
||||
inlineBodySpacing,
|
||||
'cursor-pointer rounded-xl focus:outline-none focus-visible:ring-2 focus-visible:ring-[var(--app-link)]'
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
import { fireEvent, render, screen } from '@testing-library/react'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { ApiClient } from '@/api/client'
|
||||
import type { ToolCallBlock } from '@/chat/types'
|
||||
import { ToolCard } from '@/components/ToolCard/ToolCard'
|
||||
import { I18nProvider } from '@/lib/i18n-context'
|
||||
|
||||
const block: ToolCallBlock = {
|
||||
kind: 'tool-call', id: 'codex-diff-1', localId: null, createdAt: 1_000,
|
||||
tool: {
|
||||
id: 'codex-diff-1', name: 'CodexDiff', state: 'completed',
|
||||
input: { unified_diff: 'diff --git a/example.ts b/example.ts\n--- a/example.ts\n+++ b/example.ts\n@@ -1 +1 @@\n-before\n+after' },
|
||||
createdAt: 1_000, startedAt: 1_000, completedAt: 1_100, execStartedAt: null, execCompletedAt: null, description: null,
|
||||
}, children: [],
|
||||
}
|
||||
|
||||
describe('ToolCard CodexDiff inline interactions', () => {
|
||||
it('lets a non-minimal CodexDiff own its preview and wrap controls', () => {
|
||||
const { container } = render(
|
||||
<I18nProvider><ToolCard api={{} as ApiClient} sessionId="session-1" metadata={null} terminalToolDisplayMode="detailed" disabled={false} onDone={() => {}} block={block} /></I18nProvider>
|
||||
)
|
||||
|
||||
expect(container.querySelectorAll('[role="button"] button')).toHaveLength(0)
|
||||
const wrapToggle = container.querySelector('[data-hapi-code-wrap-toggle="true"]')!
|
||||
fireEvent.click(wrapToggle)
|
||||
expect(screen.queryByRole('dialog')).toBeNull()
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open diff preview' }))
|
||||
expect(screen.getByRole('dialog')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,32 @@
|
||||
import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { ApiClient } from '@/api/client'
|
||||
import type { ToolCallBlock } from '@/chat/types'
|
||||
import { ToolCard } from '@/components/ToolCard/ToolCard'
|
||||
import { I18nProvider } from '@/lib/i18n-context'
|
||||
|
||||
const block: ToolCallBlock = {
|
||||
kind: 'tool-call', id: 'bash-1', localId: null, createdAt: 1_000,
|
||||
tool: {
|
||||
id: 'bash-1', name: 'Bash', state: 'completed', input: { command: 'echo preview' },
|
||||
createdAt: 1_000, startedAt: 1_000, completedAt: 1_100, execStartedAt: null, execCompletedAt: null, description: null,
|
||||
}, children: [],
|
||||
}
|
||||
|
||||
describe('ToolCard inline detail focus', () => {
|
||||
it('restores focus to both the header trigger and an inline preview invoker', async () => {
|
||||
const { container } = render(
|
||||
<I18nProvider><ToolCard api={{} as ApiClient} sessionId="session-1" metadata={null} terminalToolDisplayMode="detailed" disabled={false} onDone={() => {}} block={block} /></I18nProvider>
|
||||
)
|
||||
|
||||
const headerTrigger = container.querySelector('button')!
|
||||
fireEvent.click(headerTrigger)
|
||||
fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Close' }))
|
||||
await waitFor(() => expect(headerTrigger).toHaveFocus())
|
||||
|
||||
const inlinePreview = container.querySelector('[role="button"]') as HTMLElement
|
||||
fireEvent.click(inlinePreview)
|
||||
fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Close' }))
|
||||
await waitFor(() => expect(inlinePreview).toHaveFocus())
|
||||
})
|
||||
})
|
||||
@@ -28,6 +28,7 @@ export function MultiEditView(props: ToolViewProps) {
|
||||
key={idx}
|
||||
oldString={edit.old_string}
|
||||
newString={edit.new_string}
|
||||
variant="inline"
|
||||
/>
|
||||
))}
|
||||
{edits.length > MAX_COMPACT_EDITS ? (
|
||||
|
||||
Reference in New Issue
Block a user