diff --git a/desktop/src/components/workspace/WorkspaceDiffSurface.test.tsx b/desktop/src/components/workspace/WorkspaceDiffSurface.test.tsx index ca2cda8a..98e710c7 100644 --- a/desktop/src/components/workspace/WorkspaceDiffSurface.test.tsx +++ b/desktop/src/components/workspace/WorkspaceDiffSurface.test.tsx @@ -49,6 +49,52 @@ describe('WorkspaceDiffSurface', () => { highlightRenderSpy.mockClear() }) + it('keeps one scroll surface while hiding redundant single-file patch chrome', () => { + render() + + const scrollSurface = screen.getByTestId('workspace-diff-scroll') + expect(scrollSurface.className).toContain('min-h-0') + expect(scrollSurface.className).toContain('overflow-auto') + expect(scrollSurface).toHaveStyle({ containerType: 'inline-size' }) + expect(screen.getByTestId('workspace-diff-content').className).toContain('w-max') + expect(screen.queryByTestId('workspace-diff-file-header')).not.toBeInTheDocument() + expect(screen.queryByText('--- a/src/a.ts')).not.toBeInTheDocument() + expect(screen.queryByText('+++ b/src/a.ts')).not.toBeInTheDocument() + expect(screen.getByText('@@ -10,2 +10,3 @@')).toBeInTheDocument() + expect(getCodeRow('const a = 1')).toBeInTheDocument() + }) + + it('does not spend the visible line limit on hidden single-file patch metadata', () => { + render() + + expect(screen.getByText('@@ -10,2 +10,3 @@')).toBeInTheDocument() + expect(getCodeRow('const a = 1')).toBeInTheDocument() + expect(document.querySelector('[data-row-text="const b = 2"]')).not.toBeInTheDocument() + }) + + it('uses the Codex-style compact number gutter without a dedicated comment column', () => { + render() + + const code = screen.getByTestId('workspace-code') + const row = getCodeRow('const b = 3').closest('[data-diff-row-id]') + const gutter = row?.querySelector('[data-diff-number-gutter]') + const commentButton = screen.getByRole('button', { name: 'Comment on src/a.ts new line 11' }) + + expect(code.style.getPropertyValue('--workspace-diff-gutter-width')).toBe('6ch') + expect(row).toHaveStyle({ + gridTemplateColumns: 'var(--workspace-diff-gutter-width) minmax(max-content, 1fr)', + }) + expect(gutter).toHaveTextContent('11') + expect(gutter).toContainElement(commentButton) + expect(gutter?.querySelector('[data-diff-gutter-utility-slot]')).toContainElement(commentButton) + expect(gutter?.className).toContain('bg-[var(--color-diff-added-bg)]') + expect(getCodeRow('const a = 1').closest('[data-diff-row-id]')?.querySelector('[data-diff-number-gutter]')?.className).toContain('bg-[var(--color-code-bg)]') + expect(commentButton.className).toContain('h-5') + expect(commentButton.className).toContain('w-5') + expect(row?.querySelectorAll('[data-diff-line-number]')).toHaveLength(1) + expect(row?.className).toContain('min-h-5') + }) + it('submits a forward range with its source coordinates and quote', () => { const onAddComment = vi.fn() render() @@ -104,12 +150,23 @@ describe('WorkspaceDiffSurface', () => { expect(lastRow).toHaveAttribute('aria-selected', 'true') expect(lastRow).toHaveAttribute('data-range-edge', 'end') expect(document.querySelectorAll('[data-diff-selection-rail]')).toHaveLength(3) + document.querySelectorAll('[data-diff-selection-rail]').forEach((rail) => { + expect(rail.closest('[data-diff-number-gutter]')).not.toBeNull() + }) expect(firstRow?.className).toContain('bg-[var(--color-info-container)]') expect(firstRow?.className).not.toContain('bg-[var(--color-diff-added-bg)]') expect(screen.getByTestId('workspace-code').className).toContain('text-[13px]') const editor = screen.getByRole('textbox', { name: 'Review comment' }) - expect(editor.closest('[data-diff-editor]')?.className).toContain('rounded-[10px]') - expect(editor.className).toContain('var(--color-info)') + const editorContainer = editor.closest('[data-diff-editor]') + expect(editorContainer?.className).toContain('max-w-3xl') + expect(editorContainer?.className).toContain('sticky') + expect(editorContainer).toHaveStyle({ left: 'var(--workspace-diff-gutter-width)' }) + expect(editorContainer).toHaveStyle({ width: 'min(48rem, calc(100cqi - var(--workspace-diff-gutter-width) - 0.75rem))' }) + expect(editorContainer?.className).not.toContain('ml-[116px]') + expect(editorContainer?.className).not.toContain('min-w-[420px]') + expect(editorContainer).toHaveTextContent('Local comment') + expect(editor.className).toContain('min-h-0') + expect(editor.className).not.toContain('shadow-[inset_0_0_0_1px') expect(screen.getByRole('button', { name: 'Comment on src/a.ts new line 10' })).not.toHaveAttribute('data-selection-focus') const focusedGutter = screen.getByRole('button', { name: 'Comment on src/a.ts new line 12' }) expect(focusedGutter).toHaveAttribute('data-selection-focus', 'true') @@ -257,14 +314,14 @@ describe('WorkspaceDiffSurface', () => { expect(lastVisibleButton).toHaveFocus() expect(visibleButtons.filter((button) => button.tabIndex === 0)).toHaveLength(1) - expect(screen.getByText('Showing first 5 of 11 loaded lines.')).toBeInTheDocument() + expect(screen.getByText('Showing first 5 of 9 loaded lines.')).toBeInTheDocument() expect(screen.getByRole('button', { name: 'Show all loaded lines' })).toBeInTheDocument() }) it('invalidates a hidden selection on collapse while preserving its draft and visible roving target', () => { render() fireEvent.click(screen.getByRole('button', { name: 'Show all loaded lines' })) - fireEvent.click(screen.getByRole('button', { name: 'Comment on src/a.ts new line 11' })) + fireEvent.click(screen.getByRole('button', { name: 'Comment on src/a.ts new line 12' })) fireEvent.change(screen.getByRole('textbox', { name: 'Review comment' }), { target: { value: 'Keep this collapsed draft' }, }) diff --git a/desktop/src/components/workspace/WorkspaceDiffSurface.tsx b/desktop/src/components/workspace/WorkspaceDiffSurface.tsx index ab353891..1de50ce9 100644 --- a/desktop/src/components/workspace/WorkspaceDiffSurface.tsx +++ b/desktop/src/components/workspace/WorkspaceDiffSurface.tsx @@ -5,6 +5,7 @@ import { useMemo, useRef, useState, + type CSSProperties, type KeyboardEvent, type MouseEvent, } from 'react' @@ -110,6 +111,7 @@ export interface WorkspaceDiffSurfaceProps { path: string className?: string lineLimit?: number + hideSingleFileHeader?: boolean onAddComment?: (selection: WorkspaceDiffCommentSelection, note: string) => void } @@ -136,6 +138,14 @@ function rowTone(row: WorkspaceDiffRow) { return 'hover:bg-[var(--color-surface-hover)]' } +function gutterTone(row: WorkspaceDiffRow, selected: boolean) { + if (selected) return 'bg-[var(--color-info-container)]' + if (row.kind === 'addition') return 'bg-[var(--color-diff-added-bg)]' + if (row.kind === 'deletion') return 'bg-[var(--color-diff-removed-bg)]' + if (row.kind === 'hunk') return 'bg-[var(--color-diff-highlight-bg)]' + return 'bg-[var(--color-code-bg)] group-hover:bg-[var(--color-surface-hover)]' +} + function prefixTone(row: WorkspaceDiffRow) { if (row.kind === 'addition') return 'text-[var(--color-diff-added-text)]' if (row.kind === 'deletion') return 'text-[var(--color-diff-removed-text)]' @@ -148,19 +158,40 @@ function codeTone(row: WorkspaceDiffRow) { return '' } +function isStructuralMetadata(row: WorkspaceDiffRow) { + if (row.kind !== 'metadata') return false + return row.text.startsWith('diff --') || row.text.startsWith('--- ') || row.text.startsWith('+++ ') +} + export function WorkspaceDiffSurface({ value, path, className = 'min-h-0 flex-1 overflow-auto bg-[var(--color-code-bg)]', lineLimit = WORKSPACE_PREVIEW_LINE_LIMIT, + hideSingleFileHeader = false, onAddComment, }: WorkspaceDiffSurfaceProps) { const t = useTranslation() const files = useMemo(() => parseWorkspaceDiff(value), [value]) const rows = useMemo(() => files.flatMap((file) => file.rows), [files]) + const lineNumberCharacters = useMemo( + () => rows.reduce((maximum, row) => Math.max( + maximum, + row.oldLine === null ? 0 : String(row.oldLine).length, + row.newLine === null ? 0 : String(row.newLine).length, + ), 3), + [rows], + ) + const codeStyle = { + '--workspace-diff-gutter-width': `${lineNumberCharacters + 3}ch`, + } as CSSProperties + const showFileHeaders = !hideSingleFileHeader || files.length > 1 const displayItemIds = useMemo( - () => files.flatMap((file) => [`${file.id}-header`, ...file.rows.map((row) => row.id)]), - [files], + () => files.flatMap((file) => [ + ...(showFileHeaders ? [`${file.id}-header`] : []), + ...file.rows.filter((row) => !isStructuralMetadata(row)).map((row) => row.id), + ]), + [files, showFileHeaders], ) const [review, setReview] = useState(emptyReviewState) const [status, setStatus] = useState(null) @@ -402,21 +433,26 @@ export function WorkspaceDiffSurface({ const renderEditor = () => review.selection && (
- {status && ( -
- {t(`workspace.diffReview.${status}`)} -
- )} -
- +
+ -
+
{t('workspace.localComment')}
+
{sideLabel(review.selection.side)} L{review.selection.lineStart}{review.selection.lineEnd === review.selection.lineStart ? '' : `-L${review.selection.lineEnd}`}
+ {status && ( +
+ {t(`workspace.diffReview.${status}`)} +
+ )}