mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 16:02:56 +00:00
Move PR comment action into the comments header (#4458)
- Replace the collapsed composer row with a compact plus action in the comments header, opening the composer only when requested. - Hide clean branch compare summaries and remove the inactive view-mode toggle from compare toolbar actions.
This commit is contained in:
@@ -1,78 +1,13 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { ListTree } from 'lucide-react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import {
|
||||
buildResolveConflictsPrompt,
|
||||
CompareSummary,
|
||||
CompareSummaryToolbarButton,
|
||||
getNextSourceControlViewMode,
|
||||
normalizeSourceControlViewMode,
|
||||
pickDefaultSourceControlAgent,
|
||||
readCommitDraftForWorktree,
|
||||
refreshSourceControlAfterRemoteAction,
|
||||
requestSourceControlViewModePreferenceWrite,
|
||||
shouldRenderCommitArea,
|
||||
type SourceControlViewModePreferenceWriteState,
|
||||
writeCommitDraftForWorktree
|
||||
} from './SourceControl'
|
||||
import type { GitBranchCompareSummary } from '../../../../shared/types'
|
||||
|
||||
type ReactElementLike = {
|
||||
type: unknown
|
||||
props: Record<string, unknown>
|
||||
}
|
||||
|
||||
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
|
||||
if (node == null || typeof node === 'string' || typeof node === 'number') {
|
||||
return
|
||||
}
|
||||
if (Array.isArray(node)) {
|
||||
node.forEach((entry) => visit(entry, cb))
|
||||
return
|
||||
}
|
||||
const element = node as ReactElementLike
|
||||
cb(element)
|
||||
if (element.props?.children) {
|
||||
visit(element.props.children, cb)
|
||||
}
|
||||
}
|
||||
|
||||
function findInnerButton(node: unknown): ReactElementLike {
|
||||
let found: ReactElementLike | null = null
|
||||
visit(node, (entry) => {
|
||||
if (entry.type === Button) {
|
||||
found = entry
|
||||
}
|
||||
})
|
||||
if (!found) {
|
||||
throw new Error('inner Button not found')
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
function findCompareSummaryToolbarButton(node: unknown, label: string): ReactElementLike {
|
||||
let found: ReactElementLike | null = null
|
||||
visit(node, (entry) => {
|
||||
if (entry.type === CompareSummaryToolbarButton && entry.props.label === label) {
|
||||
found = entry
|
||||
}
|
||||
})
|
||||
if (!found) {
|
||||
throw new Error(`toolbar button not found: ${label}`)
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
const readySummary: GitBranchCompareSummary = {
|
||||
baseRef: 'origin/main',
|
||||
baseOid: 'base',
|
||||
compareRef: 'feature',
|
||||
headOid: 'head',
|
||||
mergeBase: 'base',
|
||||
changedFiles: 2,
|
||||
commitsAhead: 1,
|
||||
status: 'ready'
|
||||
}
|
||||
|
||||
describe('SourceControl commit drafts by worktree', () => {
|
||||
it('returns an empty draft when the selected worktree has no message', () => {
|
||||
@@ -207,134 +142,4 @@ describe('SourceControl view mode preference', () => {
|
||||
expect(normalizeSourceControlViewMode('list')).toBe('list')
|
||||
expect(normalizeSourceControlViewMode('tree')).toBe('tree')
|
||||
})
|
||||
|
||||
it('toggles between list and tree', () => {
|
||||
expect(getNextSourceControlViewMode('list')).toBe('tree')
|
||||
expect(getNextSourceControlViewMode('tree')).toBe('list')
|
||||
})
|
||||
|
||||
it('does not persist the fallback list mode before settings hydrate', () => {
|
||||
const writeState: SourceControlViewModePreferenceWriteState = {
|
||||
writeChain: Promise.resolve(),
|
||||
writeSeq: 0
|
||||
}
|
||||
const setOptimisticMode = vi.fn()
|
||||
const updateSettings = vi.fn()
|
||||
|
||||
const result = requestSourceControlViewModePreferenceWrite({
|
||||
hydrated: false,
|
||||
currentMode: 'list',
|
||||
writeState,
|
||||
setOptimisticMode,
|
||||
updateSettings
|
||||
})
|
||||
|
||||
expect(result).toBeNull()
|
||||
expect(setOptimisticMode).not.toHaveBeenCalled()
|
||||
expect(updateSettings).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('queues rapid toggle writes so the last intent clears optimistic state', async () => {
|
||||
const writeState: SourceControlViewModePreferenceWriteState = {
|
||||
writeChain: Promise.resolve(),
|
||||
writeSeq: 0
|
||||
}
|
||||
const optimisticModes: ('list' | 'tree' | null)[] = []
|
||||
const firstWrite: { resolve: (() => void) | null } = { resolve: null }
|
||||
const updateSettings = vi.fn(
|
||||
({ sourceControlViewMode }: { sourceControlViewMode: 'list' | 'tree' }) => {
|
||||
if (sourceControlViewMode === 'tree') {
|
||||
return new Promise<void>((resolve) => {
|
||||
firstWrite.resolve = resolve
|
||||
})
|
||||
}
|
||||
return Promise.resolve()
|
||||
}
|
||||
)
|
||||
|
||||
expect(
|
||||
requestSourceControlViewModePreferenceWrite({
|
||||
hydrated: true,
|
||||
currentMode: 'list',
|
||||
writeState,
|
||||
setOptimisticMode: (mode) => optimisticModes.push(mode),
|
||||
updateSettings
|
||||
})
|
||||
).toBe('tree')
|
||||
await Promise.resolve()
|
||||
|
||||
expect(
|
||||
requestSourceControlViewModePreferenceWrite({
|
||||
hydrated: true,
|
||||
currentMode: 'tree',
|
||||
writeState,
|
||||
setOptimisticMode: (mode) => optimisticModes.push(mode),
|
||||
updateSettings
|
||||
})
|
||||
).toBe('list')
|
||||
await Promise.resolve()
|
||||
|
||||
expect(updateSettings).toHaveBeenCalledTimes(1)
|
||||
expect(updateSettings).toHaveBeenLastCalledWith({ sourceControlViewMode: 'tree' })
|
||||
|
||||
expect(firstWrite.resolve).not.toBeNull()
|
||||
firstWrite.resolve?.()
|
||||
await writeState.writeChain
|
||||
await Promise.resolve()
|
||||
|
||||
expect(updateSettings).toHaveBeenCalledTimes(2)
|
||||
expect(updateSettings).toHaveBeenLastCalledWith({ sourceControlViewMode: 'list' })
|
||||
expect(optimisticModes).toEqual(['tree', 'list', null])
|
||||
})
|
||||
|
||||
it('wires the compare toolbar toggle label and click handler from the rendered mode', () => {
|
||||
const onToggleViewMode = vi.fn()
|
||||
const node = CompareSummary({
|
||||
summary: readySummary,
|
||||
viewMode: 'tree',
|
||||
onChangeBaseRef: vi.fn(),
|
||||
onToggleViewMode,
|
||||
onRetry: vi.fn()
|
||||
})
|
||||
|
||||
const toggle = findCompareSummaryToolbarButton(node, 'Show changes as list')
|
||||
expect(toggle.props.disabled).toBeUndefined()
|
||||
|
||||
const onClick = toggle.props.onClick
|
||||
expect(typeof onClick).toBe('function')
|
||||
if (typeof onClick === 'function') {
|
||||
onClick()
|
||||
}
|
||||
expect(onToggleViewMode).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('renders the hydrated-disabled toolbar toggle as inert', () => {
|
||||
const onToggleViewMode = vi.fn()
|
||||
const node = CompareSummary({
|
||||
summary: readySummary,
|
||||
viewMode: 'list',
|
||||
onChangeBaseRef: vi.fn(),
|
||||
onToggleViewMode,
|
||||
viewModeToggleDisabled: true,
|
||||
onRetry: vi.fn()
|
||||
})
|
||||
const toggle = findCompareSummaryToolbarButton(node, 'Show changes as tree')
|
||||
expect(toggle.props.disabled).toBe(true)
|
||||
|
||||
const button = findInnerButton(
|
||||
CompareSummaryToolbarButton({
|
||||
icon: ListTree,
|
||||
label: 'Show changes as tree',
|
||||
onClick: onToggleViewMode,
|
||||
disabled: true
|
||||
})
|
||||
)
|
||||
expect(button.props['aria-disabled']).toBe(true)
|
||||
const onClick = button.props.onClick
|
||||
expect(typeof onClick).toBe('function')
|
||||
if (typeof onClick === 'function') {
|
||||
onClick()
|
||||
}
|
||||
expect(onToggleViewMode).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,145 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
CompareSummary,
|
||||
CompareSummaryToolbarButton,
|
||||
shouldShowCompareSummary
|
||||
} from './SourceControl'
|
||||
import type { GitBranchCompareSummary } from '../../../../shared/types'
|
||||
|
||||
type ReactElementLike = {
|
||||
type: unknown
|
||||
props: Record<string, unknown>
|
||||
}
|
||||
|
||||
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
|
||||
if (node == null || typeof node === 'string' || typeof node === 'number') {
|
||||
return
|
||||
}
|
||||
if (Array.isArray(node)) {
|
||||
node.forEach((entry) => visit(entry, cb))
|
||||
return
|
||||
}
|
||||
const element = node as ReactElementLike
|
||||
cb(element)
|
||||
if (element.props?.children) {
|
||||
visit(element.props.children, cb)
|
||||
}
|
||||
}
|
||||
|
||||
function collectText(node: unknown): string {
|
||||
if (node == null) {
|
||||
return ''
|
||||
}
|
||||
if (typeof node === 'string' || typeof node === 'number') {
|
||||
return String(node)
|
||||
}
|
||||
if (Array.isArray(node)) {
|
||||
return node.map(collectText).join('')
|
||||
}
|
||||
const element = node as ReactElementLike
|
||||
return collectText(element.props?.children)
|
||||
}
|
||||
|
||||
function findCompareSummaryToolbarButton(node: unknown, label: string): ReactElementLike {
|
||||
let found: ReactElementLike | null = null
|
||||
visit(node, (entry) => {
|
||||
if (entry.type === CompareSummaryToolbarButton && entry.props.label === label) {
|
||||
found = entry
|
||||
}
|
||||
})
|
||||
if (!found) {
|
||||
throw new Error(`toolbar button not found: ${label}`)
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
function collectCompareSummaryToolbarLabels(node: unknown): string[] {
|
||||
const labels: string[] = []
|
||||
visit(node, (entry) => {
|
||||
if (entry.type === CompareSummaryToolbarButton && typeof entry.props.label === 'string') {
|
||||
labels.push(entry.props.label)
|
||||
}
|
||||
})
|
||||
return labels
|
||||
}
|
||||
|
||||
const readySummary: GitBranchCompareSummary = {
|
||||
baseRef: 'origin/main',
|
||||
baseOid: 'base',
|
||||
compareRef: 'feature',
|
||||
headOid: 'head',
|
||||
mergeBase: 'base',
|
||||
changedFiles: 2,
|
||||
commitsAhead: 1,
|
||||
status: 'ready'
|
||||
}
|
||||
|
||||
describe('SourceControl compare summary', () => {
|
||||
it('wires toolbar actions without rendering the dead view-mode toggle', () => {
|
||||
const onChangeBaseRef = vi.fn()
|
||||
const onRetry = vi.fn()
|
||||
const node = CompareSummary({
|
||||
summary: readySummary,
|
||||
onChangeBaseRef,
|
||||
onRetry
|
||||
})
|
||||
|
||||
expect(collectCompareSummaryToolbarLabels(node)).toEqual([
|
||||
'Change base ref',
|
||||
'Refresh branch compare'
|
||||
])
|
||||
|
||||
const changeBaseRef = findCompareSummaryToolbarButton(node, 'Change base ref').props.onClick
|
||||
if (typeof changeBaseRef === 'function') {
|
||||
changeBaseRef()
|
||||
}
|
||||
expect(onChangeBaseRef).toHaveBeenCalledTimes(1)
|
||||
|
||||
const refresh = findCompareSummaryToolbarButton(node, 'Refresh branch compare').props.onClick
|
||||
if (typeof refresh === 'function') {
|
||||
refresh()
|
||||
}
|
||||
expect(onRetry).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('omits the whole compare row when the branch has no commits ahead', () => {
|
||||
const cleanSummary = { ...readySummary, commitsAhead: 0 }
|
||||
const node = CompareSummary({
|
||||
summary: cleanSummary,
|
||||
onChangeBaseRef: vi.fn(),
|
||||
onRetry: vi.fn()
|
||||
})
|
||||
|
||||
expect(shouldShowCompareSummary(cleanSummary)).toBe(false)
|
||||
expect(node).toBeNull()
|
||||
const text = collectText(node)
|
||||
expect(text).not.toContain('0 commits ahead')
|
||||
expect(text).not.toContain('origin/main')
|
||||
})
|
||||
|
||||
it('keeps non-zero summary copy compact', () => {
|
||||
const node = CompareSummary({
|
||||
summary: readySummary,
|
||||
onChangeBaseRef: vi.fn(),
|
||||
onRetry: vi.fn()
|
||||
})
|
||||
|
||||
const text = collectText(node)
|
||||
expect(text).toContain('1 ahead')
|
||||
expect(text).not.toContain('1 commit ahead of origin/main')
|
||||
})
|
||||
|
||||
it('omits the view-mode toggle from unavailable compare rows', () => {
|
||||
const node = CompareSummary({
|
||||
summary: {
|
||||
...readySummary,
|
||||
status: 'error',
|
||||
errorMessage: 'Unable to compare'
|
||||
},
|
||||
onChangeBaseRef: vi.fn(),
|
||||
onRetry: vi.fn()
|
||||
})
|
||||
|
||||
expect(collectCompareSummaryToolbarLabels(node)).toEqual(['Change base ref', 'Retry'])
|
||||
})
|
||||
})
|
||||
@@ -20,8 +20,6 @@ import {
|
||||
FolderOpen,
|
||||
GitMerge,
|
||||
GitPullRequestArrow,
|
||||
List,
|
||||
ListTree,
|
||||
MessageSquare,
|
||||
Trash,
|
||||
Trash2,
|
||||
@@ -160,7 +158,6 @@ import type {
|
||||
GitConflictKind,
|
||||
GitConflictOperation,
|
||||
GitStatusEntry,
|
||||
GlobalSettings,
|
||||
SourceControlViewMode,
|
||||
TuiAgent
|
||||
} from '../../../../shared/types'
|
||||
@@ -371,57 +368,6 @@ export function normalizeSourceControlViewMode(value: unknown): SourceControlVie
|
||||
return value === 'tree' || value === 'list' ? value : 'list'
|
||||
}
|
||||
|
||||
export function getNextSourceControlViewMode(mode: SourceControlViewMode): SourceControlViewMode {
|
||||
return mode === 'tree' ? 'list' : 'tree'
|
||||
}
|
||||
|
||||
export type SourceControlViewModePreferenceWriteState = {
|
||||
writeChain: Promise<void>
|
||||
writeSeq: number
|
||||
}
|
||||
|
||||
export function requestSourceControlViewModePreferenceWrite({
|
||||
hydrated,
|
||||
currentMode,
|
||||
writeState,
|
||||
setOptimisticMode,
|
||||
updateSettings
|
||||
}: {
|
||||
hydrated: boolean
|
||||
currentMode: SourceControlViewMode
|
||||
writeState: SourceControlViewModePreferenceWriteState
|
||||
setOptimisticMode: (mode: SourceControlViewMode | null) => void
|
||||
updateSettings: (
|
||||
updates: Pick<GlobalSettings, 'sourceControlViewMode'>
|
||||
) => Promise<GlobalSettings | void>
|
||||
}): SourceControlViewMode | null {
|
||||
if (!hydrated) {
|
||||
return null
|
||||
}
|
||||
const next = getNextSourceControlViewMode(currentMode)
|
||||
const writeSeq = writeState.writeSeq + 1
|
||||
writeState.writeSeq = writeSeq
|
||||
setOptimisticMode(next)
|
||||
|
||||
// Why: settings writes cross IPC. Queue them so rapid toolbar clicks keep
|
||||
// the user's final intent as the persisted value even if earlier writes
|
||||
// would otherwise resolve after later clicks.
|
||||
const write = writeState.writeChain
|
||||
.catch(() => undefined)
|
||||
.then(() => updateSettings({ sourceControlViewMode: next }))
|
||||
.then(() => undefined)
|
||||
writeState.writeChain = write
|
||||
void write
|
||||
.finally(() => {
|
||||
if (writeState.writeSeq === writeSeq) {
|
||||
setOptimisticMode(null)
|
||||
}
|
||||
})
|
||||
.catch(() => undefined)
|
||||
|
||||
return next
|
||||
}
|
||||
|
||||
type GitStatusSourceControlTreeNode = SourceControlTreeNode<
|
||||
GitStatusEntry,
|
||||
(typeof SECTION_ORDER)[number]
|
||||
@@ -1122,7 +1068,6 @@ function SourceControlInner(): React.JSX.Element {
|
||||
const isRemoteOperationActive = useAppStore((s) => s.isRemoteOperationActive)
|
||||
const inFlightRemoteOpKind = useAppStore((s) => s.inFlightRemoteOpKind)
|
||||
const settings = useAppStore((s) => s.settings)
|
||||
const updateSettings = useAppStore((s) => s.updateSettings)
|
||||
const openSettingsTarget = useAppStore((s) => s.openSettingsTarget)
|
||||
const openSettingsPage = useAppStore((s) => s.openSettingsPage)
|
||||
const hostedReviewCache = useAppStore((s) => s.hostedReviewCache)
|
||||
@@ -1279,26 +1224,10 @@ function SourceControlInner(): React.JSX.Element {
|
||||
const [collapsedSections, setCollapsedSections] = useState<Set<string>>(
|
||||
createDefaultCollapsedSections
|
||||
)
|
||||
const [optimisticSourceControlViewMode, setOptimisticSourceControlViewMode] =
|
||||
useState<SourceControlViewMode | null>(null)
|
||||
const sourceControlViewModeWriteStateRef = useRef<SourceControlViewModePreferenceWriteState>({
|
||||
writeChain: Promise.resolve(),
|
||||
writeSeq: 0
|
||||
})
|
||||
const persistedSourceControlViewMode = normalizeSourceControlViewMode(
|
||||
settings?.sourceControlViewMode
|
||||
)
|
||||
const sourceControlViewMode = optimisticSourceControlViewMode ?? persistedSourceControlViewMode
|
||||
const isSourceControlViewModeHydrated = settings !== null
|
||||
const handleToggleSourceControlViewMode = useCallback(() => {
|
||||
requestSourceControlViewModePreferenceWrite({
|
||||
hydrated: isSourceControlViewModeHydrated,
|
||||
currentMode: sourceControlViewMode,
|
||||
writeState: sourceControlViewModeWriteStateRef.current,
|
||||
setOptimisticMode: setOptimisticSourceControlViewMode,
|
||||
updateSettings
|
||||
})
|
||||
}, [isSourceControlViewModeHydrated, sourceControlViewMode, updateSettings])
|
||||
const sourceControlViewMode = persistedSourceControlViewMode
|
||||
const [collapsedTreeDirs, setCollapsedTreeDirs] = useState<Set<string>>(new Set())
|
||||
const [baseRefDialogOpen, setBaseRefDialogOpen] = useState(false)
|
||||
const [pendingDiscard, setPendingDiscard] = useState<PendingDiscardConfirmation | null>(null)
|
||||
@@ -4169,14 +4098,11 @@ function SourceControlInner(): React.JSX.Element {
|
||||
)}
|
||||
</div>
|
||||
|
||||
{scope === 'all' && (
|
||||
{scope === 'all' && shouldShowCompareSummary(branchSummary) && (
|
||||
<div className="border-b border-border px-3 py-2">
|
||||
<CompareSummary
|
||||
summary={branchSummary}
|
||||
viewMode={sourceControlViewMode}
|
||||
onChangeBaseRef={() => setBaseRefDialogOpen(true)}
|
||||
onToggleViewMode={handleToggleSourceControlViewMode}
|
||||
viewModeToggleDisabled={!isSourceControlViewModeHydrated}
|
||||
onRetry={() => void refreshBranchCompare()}
|
||||
/>
|
||||
</div>
|
||||
@@ -5946,21 +5872,25 @@ export function CommitArea({
|
||||
)
|
||||
}
|
||||
|
||||
export function shouldShowCompareSummary(summary: GitBranchCompareSummary | null): boolean {
|
||||
if (!summary || summary.status === 'loading') {
|
||||
return true
|
||||
}
|
||||
if (summary.status !== 'ready') {
|
||||
return true
|
||||
}
|
||||
return typeof summary.commitsAhead === 'number' && summary.commitsAhead > 0
|
||||
}
|
||||
|
||||
export function CompareSummary({
|
||||
summary,
|
||||
viewMode,
|
||||
onChangeBaseRef,
|
||||
onToggleViewMode,
|
||||
viewModeToggleDisabled,
|
||||
onRetry
|
||||
}: {
|
||||
summary: GitBranchCompareSummary | null
|
||||
viewMode: SourceControlViewMode
|
||||
onChangeBaseRef: () => void
|
||||
onToggleViewMode: () => void
|
||||
viewModeToggleDisabled?: boolean
|
||||
onRetry: () => void
|
||||
}): React.JSX.Element {
|
||||
}): React.JSX.Element | null {
|
||||
if (!summary || summary.status === 'loading') {
|
||||
return (
|
||||
<div className="flex items-center gap-2 text-xs text-muted-foreground">
|
||||
@@ -5982,37 +5912,34 @@ export function CompareSummary({
|
||||
label="Change base ref"
|
||||
onClick={onChangeBaseRef}
|
||||
/>
|
||||
<CompareSummaryToolbarButton
|
||||
icon={viewMode === 'tree' ? List : ListTree}
|
||||
label={viewMode === 'tree' ? 'Show changes as list' : 'Show changes as tree'}
|
||||
onClick={onToggleViewMode}
|
||||
disabled={viewModeToggleDisabled}
|
||||
/>
|
||||
<CompareSummaryToolbarButton icon={RefreshCw} label="Retry" onClick={onRetry} />
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
const commitsAhead = summary.commitsAhead
|
||||
const showCommitsAhead = typeof commitsAhead === 'number' && commitsAhead > 0
|
||||
const commitsAheadTitle = showCommitsAhead
|
||||
? `${commitsAhead} ${commitsAhead === 1 ? 'commit' : 'commits'} ahead of ${summary.baseRef}`
|
||||
: undefined
|
||||
|
||||
if (!showCommitsAhead) {
|
||||
return null
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="flex items-center gap-2 text-xs text-muted-foreground">
|
||||
{summary.commitsAhead !== undefined && (
|
||||
<span title={`Comparing against ${summary.baseRef}`}>
|
||||
{summary.commitsAhead} commits ahead of {summary.baseRef}
|
||||
</span>
|
||||
)}
|
||||
<span className="flex min-w-0 items-center gap-1" title={commitsAheadTitle}>
|
||||
<ArrowUp className="size-3" />
|
||||
<span>{commitsAhead} ahead</span>
|
||||
</span>
|
||||
<div className="ml-auto flex shrink-0 items-center gap-2">
|
||||
<CompareSummaryToolbarButton
|
||||
icon={Settings2}
|
||||
label="Change base ref"
|
||||
onClick={onChangeBaseRef}
|
||||
/>
|
||||
<CompareSummaryToolbarButton
|
||||
icon={viewMode === 'tree' ? List : ListTree}
|
||||
label={viewMode === 'tree' ? 'Show changes as list' : 'Show changes as tree'}
|
||||
onClick={onToggleViewMode}
|
||||
disabled={viewModeToggleDisabled}
|
||||
/>
|
||||
<CompareSummaryToolbarButton
|
||||
icon={RefreshCw}
|
||||
label="Refresh branch compare"
|
||||
@@ -6026,13 +5953,11 @@ export function CompareSummary({
|
||||
export function CompareSummaryToolbarButton({
|
||||
icon: Icon,
|
||||
label,
|
||||
onClick,
|
||||
disabled = false
|
||||
onClick
|
||||
}: {
|
||||
icon: LucideIcon
|
||||
label: string
|
||||
onClick: () => void
|
||||
disabled?: boolean
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Tooltip>
|
||||
@@ -6041,17 +5966,9 @@ export function CompareSummaryToolbarButton({
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className={cn(
|
||||
'text-muted-foreground hover:text-foreground',
|
||||
disabled && 'cursor-not-allowed opacity-50'
|
||||
)}
|
||||
className="text-muted-foreground hover:text-foreground"
|
||||
aria-label={label}
|
||||
aria-disabled={disabled}
|
||||
onClick={() => {
|
||||
if (!disabled) {
|
||||
onClick()
|
||||
}
|
||||
}}
|
||||
onClick={onClick}
|
||||
>
|
||||
<Icon className="size-3.5" />
|
||||
</Button>
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import React from 'react'
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { TooltipProvider } from '@/components/ui/tooltip'
|
||||
import type { PRCheckDetail, PRComment, PRInfo } from '../../../../shared/types'
|
||||
import {
|
||||
CheckJobLogTail,
|
||||
@@ -11,6 +12,10 @@ import {
|
||||
PRTriageStrip
|
||||
} from './checks-panel-content'
|
||||
|
||||
function renderWithTooltips(element: React.ReactElement): string {
|
||||
return renderToStaticMarkup(React.createElement(TooltipProvider, null, element))
|
||||
}
|
||||
|
||||
function makePR(overrides: Partial<PRInfo> = {}): PRInfo {
|
||||
return {
|
||||
number: 42,
|
||||
@@ -102,7 +107,7 @@ describe('MergeConflictNotice', () => {
|
||||
})
|
||||
|
||||
describe('PRCommentsList', () => {
|
||||
it('places the collapsed add-comment action after existing comments', () => {
|
||||
it('places the collapsed add-comment action in the comments header', () => {
|
||||
const comments: PRComment[] = [
|
||||
{
|
||||
id: 1,
|
||||
@@ -114,7 +119,7 @@ describe('PRCommentsList', () => {
|
||||
}
|
||||
]
|
||||
|
||||
const markup = renderToStaticMarkup(
|
||||
const markup = renderWithTooltips(
|
||||
React.createElement(PRCommentsList, {
|
||||
comments,
|
||||
commentsLoading: false,
|
||||
@@ -122,14 +127,16 @@ describe('PRCommentsList', () => {
|
||||
})
|
||||
)
|
||||
|
||||
expect(markup.indexOf('Existing review context')).toBeLessThan(
|
||||
markup.indexOf('Add a comment...')
|
||||
expect(markup.indexOf('aria-label="Add comment"')).toBeLessThan(
|
||||
markup.indexOf('Existing review context')
|
||||
)
|
||||
expect(markup).toContain('lucide-plus')
|
||||
expect(markup).not.toContain('Add a comment...')
|
||||
expect(markup).not.toContain('Add a PR comment')
|
||||
})
|
||||
|
||||
it('uses the collapsed composer as the empty comments state', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
it('uses the header plus action as the empty comments state', () => {
|
||||
const markup = renderWithTooltips(
|
||||
React.createElement(PRCommentsList, {
|
||||
comments: [],
|
||||
commentsLoading: false,
|
||||
@@ -137,7 +144,9 @@ describe('PRCommentsList', () => {
|
||||
})
|
||||
)
|
||||
|
||||
expect(markup).toContain('Start conversation...')
|
||||
expect(markup).toContain('aria-label="Start conversation"')
|
||||
expect(markup).toContain('lucide-plus')
|
||||
expect(markup).not.toContain('Start conversation...')
|
||||
expect(markup).not.toContain('No comments yet')
|
||||
expect(markup).not.toContain('Add a comment')
|
||||
expect((markup.match(/lucide-message-square/g) ?? []).length).toBe(1)
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
Copy,
|
||||
Check,
|
||||
MessageSquare,
|
||||
Plus,
|
||||
ChevronDown,
|
||||
ChevronRight,
|
||||
Sparkles,
|
||||
@@ -22,6 +23,7 @@ import {
|
||||
} from 'lucide-react'
|
||||
import { ExternalLink } from 'lucide-react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import {
|
||||
Accordion,
|
||||
AccordionContent,
|
||||
@@ -1564,34 +1566,20 @@ export function PRCommentsList({
|
||||
setIsAddingComment(false)
|
||||
}, [])
|
||||
|
||||
const renderAddCommentSurface = (empty: boolean): React.JSX.Element => (
|
||||
const renderAddCommentComposer = (empty: boolean): React.JSX.Element => (
|
||||
<div
|
||||
ref={addCommentSurfaceRef}
|
||||
className={cn(empty ? 'px-3 py-2' : 'border-t border-border px-3 py-2')}
|
||||
>
|
||||
{isAddingComment ? (
|
||||
<RightPanelCommentComposer
|
||||
placeholder={empty ? 'Start conversation...' : 'Add a PR comment'}
|
||||
submitLabel="Comment"
|
||||
autoFocus
|
||||
disabled={commentsDisabled}
|
||||
disabledReason={commentsDisabledReason}
|
||||
onCancel={cancelAddComment}
|
||||
onSubmit={onAddComment ?? (async () => ({ ok: false, error: 'Commenting unavailable.' }))}
|
||||
/>
|
||||
) : (
|
||||
// Why: the empty comments state should be a single composer affordance;
|
||||
// duplicating "no comments" copy or the header icon makes the panel noisy.
|
||||
<button
|
||||
type="button"
|
||||
disabled={commentsDisabled}
|
||||
title={commentsDisabled ? commentsDisabledReason : undefined}
|
||||
className="flex h-10 w-full min-w-0 items-center rounded-md border border-input bg-background px-3 text-left text-[12px] text-muted-foreground shadow-xs transition-colors hover:border-ring/50 hover:bg-accent/30 hover:text-foreground disabled:cursor-not-allowed disabled:opacity-60"
|
||||
onClick={startAddComment}
|
||||
>
|
||||
<span className="truncate">{empty ? 'Start conversation...' : 'Add a comment...'}</span>
|
||||
</button>
|
||||
)}
|
||||
<RightPanelCommentComposer
|
||||
placeholder={empty ? 'Start conversation...' : 'Add a PR comment'}
|
||||
submitLabel="Send"
|
||||
autoFocus
|
||||
disabled={commentsDisabled}
|
||||
disabledReason={commentsDisabledReason}
|
||||
onCancel={cancelAddComment}
|
||||
onSubmit={onAddComment ?? (async () => ({ ok: false, error: 'Commenting unavailable.' }))}
|
||||
/>
|
||||
</div>
|
||||
)
|
||||
|
||||
@@ -1599,12 +1587,37 @@ export function PRCommentsList({
|
||||
<div className="border-t border-border">
|
||||
{/* Header */}
|
||||
<div className="border-b border-border px-3 py-2">
|
||||
<div className="flex items-center gap-2">
|
||||
<div className="flex min-w-0 items-center gap-2">
|
||||
<MessageSquare className="size-3.5 text-muted-foreground" />
|
||||
<span className="text-[11px] font-medium text-foreground">Comments</span>
|
||||
{comments.length > 0 && (
|
||||
<span className="text-[10px] text-muted-foreground">{comments.length}</span>
|
||||
)}
|
||||
{onAddComment && !isAddingComment && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
aria-label={comments.length === 0 ? 'Start conversation' : 'Add comment'}
|
||||
disabled={commentsDisabled}
|
||||
title={commentsDisabled ? commentsDisabledReason : undefined}
|
||||
className="-mr-1 ml-auto text-muted-foreground hover:text-foreground"
|
||||
onClick={startAddComment}
|
||||
>
|
||||
<Plus className="size-3" />
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" sideOffset={4}>
|
||||
{commentsDisabled && commentsDisabledReason
|
||||
? commentsDisabledReason
|
||||
: comments.length === 0
|
||||
? 'Start conversation'
|
||||
: 'Add comment'}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)}
|
||||
</div>
|
||||
{comments.length > 0 && (
|
||||
<div className="mt-2 grid grid-cols-3 rounded-md border border-border bg-background p-0.5">
|
||||
@@ -1640,12 +1653,14 @@ export function PRCommentsList({
|
||||
<div className="flex items-center justify-center py-6">
|
||||
<LoaderCircle className="size-4 animate-spin text-muted-foreground" />
|
||||
</div>
|
||||
) : comments.length === 0 && onAddComment ? (
|
||||
renderAddCommentSurface(true)
|
||||
) : comments.length === 0 && isAddingComment && onAddComment ? (
|
||||
renderAddCommentComposer(true)
|
||||
) : comments.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-5 text-[11px] text-muted-foreground">
|
||||
No comments
|
||||
</div>
|
||||
!onAddComment && (
|
||||
<div className="flex items-center justify-center py-5 text-[11px] text-muted-foreground">
|
||||
No comments
|
||||
</div>
|
||||
)
|
||||
) : visibleComments.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-5 text-[11px] text-muted-foreground">
|
||||
{getPRCommentAudienceEmptyLabel(commentFilter)}
|
||||
@@ -1684,7 +1699,7 @@ export function PRCommentsList({
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
{onAddComment && comments.length > 0 && renderAddCommentSurface(false)}
|
||||
{onAddComment && comments.length > 0 && isAddingComment && renderAddCommentComposer(false)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import React, { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { Bold, Code2, Italic, List, LoaderCircle, Quote, Send } from 'lucide-react'
|
||||
import { Bold, Code2, Italic, List, Quote } from 'lucide-react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import { ShortcutKeyCombo } from '@/components/ShortcutKeyCombo'
|
||||
@@ -232,15 +232,11 @@ export function RightPanelCommentComposer({
|
||||
<Button
|
||||
type="button"
|
||||
size="xs"
|
||||
aria-label={submitLabel}
|
||||
disabled={disabled || submitting || body.trim().length === 0}
|
||||
onClick={() => void submit()}
|
||||
>
|
||||
{submitting ? (
|
||||
<LoaderCircle className="size-3 animate-spin" />
|
||||
) : (
|
||||
<Send className="size-3" />
|
||||
)}
|
||||
{submitLabel}
|
||||
{submitting ? 'Sending...' : submitLabel}
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" sideOffset={4}>
|
||||
|
||||
Reference in New Issue
Block a user