diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 36a2275bca4..d99c61237ea 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -85,6 +85,14 @@ import { import type { DiffSection } from '@/components/editor/diff-section-types' import type { CombinedDiffFileTreeEntry } from '@/components/editor/combined-diff-file-tree-model' import { CHECK_COLOR, CHECK_ICON } from '@/components/right-sidebar/checks-panel-content' +import { + createGitHubChecksTabState, + resolveGitHubChecksTabState, + toggleGitHubChecksTabExpandedKey, + updateGitHubChecksTabDetails, + updateGitHubChecksTabLocalChecks, + type CheckDetailsLoadState +} from '@/components/github-checks-tab-state' import { filterPRCommentsByAudience, getPRCommentAudienceCounts, @@ -151,7 +159,6 @@ import type { GitBranchChangeEntry, GitDiffResult, PRCheckDetail, - PRCheckRunDetails, PRComment, TuiAgent } from '../../../shared/types' @@ -3271,12 +3278,6 @@ function pickDefaultAgent( return AGENT_CATALOG.find((entry) => enabledAgents.includes(entry.id))?.id ?? null } -type CheckDetailsLoadState = { - loading: boolean - details: PRCheckRunDetails | null - error: string | null -} - function getCheckDetailsKey(check: PRCheckDetail): string { return String(check.checkRunId ?? check.workflowRunId ?? check.url ?? check.name) } @@ -3316,15 +3317,18 @@ function ChecksTab({ variant?: 'compact' | 'page' onChecksUpdated: (checks: PRCheckDetail[]) => void }): React.JSX.Element { - const [localChecks, setLocalChecks] = useState(null) const [refreshing, setRefreshing] = useState(false) const [rerunning, setRerunning] = useState(false) const [fixingChecks, setFixingChecks] = useState(false) - const [expandedCheckKey, setExpandedCheckKey] = useState(null) - const [detailsByCheckKey, setDetailsByCheckKey] = useState>( - {} - ) + const [checksState, setChecksState] = useState(() => createGitHubChecksTabState(checks)) const mountedRef = useMountedRef() + const resolvedChecksState = resolveGitHubChecksTabState(checksState, checks) + if (resolvedChecksState !== checksState) { + // Why: parent check refreshes replace the source list; clear local refresh + // and inline detail state before stale rows/details can paint. + setChecksState(resolvedChecksState) + } + const { localChecks, expandedCheckKey, detailsByCheckKey } = resolvedChecksState const list = useMemo(() => localChecks ?? checks ?? [], [checks, localChecks]) const prRepo = useMemo(() => parseOwnerRepoFromItemUrl(item.url), [item.url]) const sorted = [...list].sort( @@ -3353,12 +3357,6 @@ function ChecksTab({ : 'text-muted-foreground' const canFixBrokenChecks = Boolean((repoId ?? item.repoId) && failedChecks.length > 0) - useEffect(() => { - setLocalChecks(null) - setExpandedCheckKey(null) - setDetailsByCheckKey({}) - }, [checks]) - const handleRefresh = useCallback(async (): Promise => { if (!repoPath) { toast.error('Unable to refresh checks without a repository path.') @@ -3373,7 +3371,7 @@ function ChecksTab({ headSha, noCache: true })) as PRCheckDetail[] - setLocalChecks(nextChecks) + setChecksState((current) => updateGitHubChecksTabLocalChecks(current, nextChecks)) onChecksUpdated(nextChecks) return nextChecks } catch (err) { @@ -3493,7 +3491,7 @@ function ChecksTab({ const handleToggleCheckDetails = useCallback( (check: PRCheckDetail): void => { const key = getCheckDetailsKey(check) - setExpandedCheckKey((current) => (current === key ? null : key)) + setChecksState((current) => toggleGitHubChecksTabExpandedKey(current, key)) if ( !repoPath || detailsByCheckKey[key] || @@ -3501,10 +3499,9 @@ function ChecksTab({ ) { return } - setDetailsByCheckKey((current) => ({ - ...current, - [key]: { loading: true, details: null, error: null } - })) + setChecksState((current) => + updateGitHubChecksTabDetails(current, key, { loading: true, details: null, error: null }) + ) void window.api.gh .prCheckDetails({ repoPath, @@ -3519,27 +3516,25 @@ function ChecksTab({ if (!mountedRef.current) { return } - setDetailsByCheckKey((current) => ({ - ...current, - [key]: { + setChecksState((current) => + updateGitHubChecksTabDetails(current, key, { loading: false, details, error: details ? null : 'No inline details are available for this check.' - } - })) + }) + ) }) .catch((err) => { if (!mountedRef.current) { return } - setDetailsByCheckKey((current) => ({ - ...current, - [key]: { + setChecksState((current) => + updateGitHubChecksTabDetails(current, key, { loading: false, details: null, error: err instanceof Error ? err.message : 'Failed to load check details.' - } - })) + }) + ) }) }, [detailsByCheckKey, mountedRef, prRepo, repoId, repoPath] diff --git a/src/renderer/src/components/PullRequestPage.tsx b/src/renderer/src/components/PullRequestPage.tsx index 67a5a0e2a0f..d96bec1c613 100644 --- a/src/renderer/src/components/PullRequestPage.tsx +++ b/src/renderer/src/components/PullRequestPage.tsx @@ -84,6 +84,14 @@ import { import type { DiffSection } from '@/components/editor/diff-section-types' import type { CombinedDiffFileTreeEntry } from '@/components/editor/combined-diff-file-tree-model' import { CHECK_COLOR, CHECK_ICON } from '@/components/right-sidebar/checks-panel-content' +import { + createGitHubChecksTabState, + resolveGitHubChecksTabState, + toggleGitHubChecksTabExpandedKey, + updateGitHubChecksTabDetails, + updateGitHubChecksTabLocalChecks, + type CheckDetailsLoadState +} from '@/components/github-checks-tab-state' import { filterPRCommentsByAudience, getPRCommentAudienceCounts, @@ -147,7 +155,6 @@ import type { GitBranchChangeEntry, GitDiffResult, PRCheckDetail, - PRCheckRunDetails, PRComment, TuiAgent } from '../../../shared/types' @@ -3525,12 +3532,6 @@ function pickDefaultAgent( return AGENT_CATALOG.find((entry) => enabledAgents.includes(entry.id))?.id ?? null } -type CheckDetailsLoadState = { - loading: boolean - details: PRCheckRunDetails | null - error: string | null -} - function getCheckDetailsKey(check: PRCheckDetail): string { return String(check.checkRunId ?? check.workflowRunId ?? check.url ?? check.name) } @@ -3570,15 +3571,18 @@ function ChecksTab({ variant?: 'compact' | 'page' onChecksUpdated: (checks: PRCheckDetail[]) => void }): React.JSX.Element { - const [localChecks, setLocalChecks] = useState(null) const [refreshing, setRefreshing] = useState(false) const [rerunning, setRerunning] = useState(false) const [fixingChecks, setFixingChecks] = useState(false) - const [expandedCheckKey, setExpandedCheckKey] = useState(null) - const [detailsByCheckKey, setDetailsByCheckKey] = useState>( - {} - ) + const [checksState, setChecksState] = useState(() => createGitHubChecksTabState(checks)) const mountedRef = useMountedRef() + const resolvedChecksState = resolveGitHubChecksTabState(checksState, checks) + if (resolvedChecksState !== checksState) { + // Why: parent check refreshes replace the source list; clear local refresh + // and inline detail state before stale rows/details can paint. + setChecksState(resolvedChecksState) + } + const { localChecks, expandedCheckKey, detailsByCheckKey } = resolvedChecksState const list = useMemo(() => localChecks ?? checks ?? [], [checks, localChecks]) const prRepo = useMemo(() => parseOwnerRepoFromItemUrl(item.url), [item.url]) const sorted = [...list].sort( @@ -3607,12 +3611,6 @@ function ChecksTab({ : 'text-muted-foreground' const canFixBrokenChecks = Boolean((repoId ?? item.repoId) && failedChecks.length > 0) - useEffect(() => { - setLocalChecks(null) - setExpandedCheckKey(null) - setDetailsByCheckKey({}) - }, [checks]) - const handleRefresh = useCallback(async (): Promise => { if (!repoPath) { toast.error('Unable to refresh checks without a repository path.') @@ -3627,7 +3625,7 @@ function ChecksTab({ headSha, noCache: true })) as PRCheckDetail[] - setLocalChecks(nextChecks) + setChecksState((current) => updateGitHubChecksTabLocalChecks(current, nextChecks)) onChecksUpdated(nextChecks) return nextChecks } catch (err) { @@ -3747,7 +3745,7 @@ function ChecksTab({ const handleToggleCheckDetails = useCallback( (check: PRCheckDetail): void => { const key = getCheckDetailsKey(check) - setExpandedCheckKey((current) => (current === key ? null : key)) + setChecksState((current) => toggleGitHubChecksTabExpandedKey(current, key)) if ( !repoPath || detailsByCheckKey[key] || @@ -3755,10 +3753,9 @@ function ChecksTab({ ) { return } - setDetailsByCheckKey((current) => ({ - ...current, - [key]: { loading: true, details: null, error: null } - })) + setChecksState((current) => + updateGitHubChecksTabDetails(current, key, { loading: true, details: null, error: null }) + ) void window.api.gh .prCheckDetails({ repoPath, @@ -3773,27 +3770,25 @@ function ChecksTab({ if (!mountedRef.current) { return } - setDetailsByCheckKey((current) => ({ - ...current, - [key]: { + setChecksState((current) => + updateGitHubChecksTabDetails(current, key, { loading: false, details, error: details ? null : 'No inline details are available for this check.' - } - })) + }) + ) }) .catch((err) => { if (!mountedRef.current) { return } - setDetailsByCheckKey((current) => ({ - ...current, - [key]: { + setChecksState((current) => + updateGitHubChecksTabDetails(current, key, { loading: false, details: null, error: err instanceof Error ? err.message : 'Failed to load check details.' - } - })) + }) + ) }) }, [detailsByCheckKey, mountedRef, prRepo, repoId, repoPath] diff --git a/src/renderer/src/components/github-checks-tab-state.test.ts b/src/renderer/src/components/github-checks-tab-state.test.ts new file mode 100644 index 00000000000..e4e9a63bae9 --- /dev/null +++ b/src/renderer/src/components/github-checks-tab-state.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from 'vitest' +import type { PRCheckDetail } from '../../../shared/types' +import { + createGitHubChecksTabState, + resolveGitHubChecksTabState, + toggleGitHubChecksTabExpandedKey, + updateGitHubChecksTabDetails, + updateGitHubChecksTabLocalChecks +} from './github-checks-tab-state' + +const check = (name: string): PRCheckDetail => ({ + name, + status: 'completed', + conclusion: 'success', + url: null +}) + +describe('github checks tab state', () => { + it('preserves local check state while the source checks reference is unchanged', () => { + const sourceChecks = [check('unit')] + const state = updateGitHubChecksTabLocalChecks(createGitHubChecksTabState(sourceChecks), [ + check('refreshed') + ]) + + expect(resolveGitHubChecksTabState(state, sourceChecks)).toBe(state) + }) + + it('resets local checks and expanded details when source checks change', () => { + const oldSource = [check('old')] + const nextSource = [check('next')] + const stateWithDetails = updateGitHubChecksTabDetails( + toggleGitHubChecksTabExpandedKey( + updateGitHubChecksTabLocalChecks(createGitHubChecksTabState(oldSource), [check('local')]), + 'unit' + ), + 'unit', + { loading: true, details: null, error: null } + ) + + expect(resolveGitHubChecksTabState(stateWithDetails, nextSource)).toEqual({ + sourceChecks: nextSource, + localChecks: null, + expandedCheckKey: null, + detailsByCheckKey: {} + }) + }) + + it('toggles expanded check keys without discarding loaded details', () => { + const sourceChecks = [check('unit')] + const state = updateGitHubChecksTabDetails(createGitHubChecksTabState(sourceChecks), 'unit', { + loading: false, + details: null, + error: 'No details' + }) + + const expanded = toggleGitHubChecksTabExpandedKey(state, 'unit') + const collapsed = toggleGitHubChecksTabExpandedKey(expanded, 'unit') + + expect(expanded.expandedCheckKey).toBe('unit') + expect(collapsed.expandedCheckKey).toBeNull() + expect(collapsed.detailsByCheckKey).toBe(state.detailsByCheckKey) + }) +}) diff --git a/src/renderer/src/components/github-checks-tab-state.ts b/src/renderer/src/components/github-checks-tab-state.ts new file mode 100644 index 00000000000..2995af7d0b9 --- /dev/null +++ b/src/renderer/src/components/github-checks-tab-state.ts @@ -0,0 +1,66 @@ +import type { PRCheckDetail, PRCheckRunDetails } from '../../../shared/types' + +export type CheckDetailsLoadState = { + loading: boolean + details: PRCheckRunDetails | null + error: string | null +} + +export type GitHubChecksTabState = { + sourceChecks: GitHubChecksSource + localChecks: PRCheckDetail[] | null + expandedCheckKey: string | null + detailsByCheckKey: Record +} + +type GitHubChecksSource = readonly PRCheckDetail[] | null | undefined + +export function createGitHubChecksTabState(sourceChecks: GitHubChecksSource): GitHubChecksTabState { + return { + sourceChecks, + localChecks: null, + expandedCheckKey: null, + detailsByCheckKey: {} + } +} + +export function resolveGitHubChecksTabState( + state: GitHubChecksTabState, + sourceChecks: GitHubChecksSource +): GitHubChecksTabState { + return state.sourceChecks === sourceChecks ? state : createGitHubChecksTabState(sourceChecks) +} + +export function updateGitHubChecksTabLocalChecks( + state: GitHubChecksTabState, + localChecks: PRCheckDetail[] +): GitHubChecksTabState { + return { + ...state, + localChecks + } +} + +export function toggleGitHubChecksTabExpandedKey( + state: GitHubChecksTabState, + key: string +): GitHubChecksTabState { + return { + ...state, + expandedCheckKey: state.expandedCheckKey === key ? null : key + } +} + +export function updateGitHubChecksTabDetails( + state: GitHubChecksTabState, + key: string, + details: CheckDetailsLoadState +): GitHubChecksTabState { + return { + ...state, + detailsByCheckKey: { + ...state.detailsByCheckKey, + [key]: details + } + } +}