mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 16:02:15 +00:00
fix(diff-view): defer loading large untracked files and refactor fallbac
Split on-demand load decision logic to distinguish tracked vs untracked files — large untracked files now properly defer loading while untracked images remain automatic. Extract fallback height computation into a dedicated function to centralize the logic for render-limited and in-flight-loading states, reducing code duplication and clarifying when to use bounded fallback heights.
This commit is contained in:
@@ -16,7 +16,7 @@ describe('LargeDiffLoadPrompt', () => {
|
||||
</div>
|
||||
)
|
||||
|
||||
expect(screen.getByText('Large diffs are not rendered by default.')).toBeDefined()
|
||||
screen.getByText('Large diffs are not rendered by default.')
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Load diff' }))
|
||||
|
||||
expect(onLoad).toHaveBeenCalledOnce()
|
||||
|
||||
@@ -23,8 +23,18 @@ describe('combined diff on-demand loading', () => {
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('automatically loads diffs when line counts are unavailable', () => {
|
||||
expect(shouldLoadCombinedDiffOnDemand({ added: undefined, removed: undefined })).toBe(false)
|
||||
it('automatically loads tracked diffs when line counts are unavailable', () => {
|
||||
expect(
|
||||
shouldLoadCombinedDiffOnDemand({ added: undefined, removed: undefined, area: 'unstaged' })
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('defers untracked files whose line counts were skipped as too large', () => {
|
||||
expect(shouldLoadCombinedDiffOnDemand({ area: 'untracked', path: 'data/dump.json' })).toBe(true)
|
||||
})
|
||||
|
||||
it('automatically loads untracked images that report no line counts', () => {
|
||||
expect(shouldLoadCombinedDiffOnDemand({ area: 'untracked', path: 'docs/Shot.PNG' })).toBe(false)
|
||||
})
|
||||
|
||||
it('defers untracked diffs when only additions are reported', () => {
|
||||
|
||||
@@ -1,16 +1,30 @@
|
||||
import type { GitStatusEntry } from '../../../../shared/git-status-types'
|
||||
import { IMAGE_FILE_EXTENSIONS } from '../../../../shared/image-file-extensions'
|
||||
|
||||
export const MAX_AUTOMATIC_DIFF_CHANGED_LINES = 10_000
|
||||
|
||||
function isImagePath(path: string | undefined): boolean {
|
||||
const lowerPath = path?.toLowerCase()
|
||||
return lowerPath !== undefined && IMAGE_FILE_EXTENSIONS.some((ext) => lowerPath.endsWith(ext))
|
||||
}
|
||||
|
||||
export function shouldLoadCombinedDiffOnDemand({
|
||||
added,
|
||||
removed
|
||||
removed,
|
||||
area,
|
||||
path
|
||||
}: {
|
||||
added?: number
|
||||
removed?: number
|
||||
area?: GitStatusEntry['area']
|
||||
path?: string
|
||||
}): boolean {
|
||||
// Untracked text files report additions only; both fields are absent when
|
||||
// line counts are unavailable (for example, binary or oversized files).
|
||||
if (added === undefined && removed === undefined) {
|
||||
return false
|
||||
// Untracked files lose their counts once they exceed the status-scan size cap
|
||||
// (MAX_UNTRACKED_LINE_COUNT_BYTES), so an uncounted untracked file is either
|
||||
// oversized text or binary — both too costly to auto-load. Images stay automatic
|
||||
// because their preview is the point of the row.
|
||||
return area === 'untracked' && !isImagePath(path)
|
||||
}
|
||||
return (added ?? 0) + (removed ?? 0) > MAX_AUTOMATIC_DIFF_CHANGED_LINES
|
||||
}
|
||||
|
||||
+3
-1
@@ -138,7 +138,9 @@ export function useCombinedDiffViewRestore({
|
||||
entries.map((entry) => {
|
||||
const loadOnDemand = shouldLoadCombinedDiffOnDemand({
|
||||
added: 'added' in entry ? entry.added : undefined,
|
||||
removed: 'removed' in entry ? entry.removed : undefined
|
||||
removed: 'removed' in entry ? entry.removed : undefined,
|
||||
area: 'area' in entry ? entry.area : undefined,
|
||||
path: entry.path
|
||||
})
|
||||
return {
|
||||
key: getCombinedDiffFileTreeSectionKey(treeMode, entry),
|
||||
|
||||
+3
-3
@@ -5,7 +5,8 @@ import type { ProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks'
|
||||
import type { DiffSection } from '../../diff-section-types'
|
||||
import {
|
||||
getDiffSectionEstimatedHeight,
|
||||
isIntrinsicHeightImageDiff
|
||||
isIntrinsicHeightImageDiff,
|
||||
usesLargeDiffFallbackHeight
|
||||
} from '../../diff-section-layout'
|
||||
|
||||
const COMBINED_DIFF_OVERSCAN = 5
|
||||
@@ -48,8 +49,7 @@ export function useCombinedDiffVirtualizer({
|
||||
? undefined
|
||||
: (section.added ?? 0) + (section.removed ?? 0),
|
||||
useIntrinsicImageHeight: isIntrinsicHeightImageDiff(section.diffResult),
|
||||
isLargeDiffLimited:
|
||||
section.largeDiffRenderLimit?.limited === true || section.loadOnDemand === true,
|
||||
isLargeDiffLimited: usesLargeDiffFallbackHeight(section),
|
||||
lineCounts: section.largeDiffRenderLimit?.lineCounts ?? undefined
|
||||
})
|
||||
},
|
||||
|
||||
@@ -3,7 +3,8 @@ import {
|
||||
getDiffSectionBodyHeight,
|
||||
getLargeDiffFallbackBodyHeight,
|
||||
getDiffSectionEstimatedHeight,
|
||||
isIntrinsicHeightImageDiff
|
||||
isIntrinsicHeightImageDiff,
|
||||
usesLargeDiffFallbackHeight
|
||||
} from './diff-section-layout'
|
||||
import type { GitDiffResult } from '../../../../shared/git-diff-compare-types'
|
||||
|
||||
@@ -227,6 +228,24 @@ describe('diff section layout', () => {
|
||||
).toBe(188)
|
||||
})
|
||||
|
||||
it('keeps the deferred fallback height while a loaded-on-demand diff fetches', () => {
|
||||
const section = {
|
||||
path: 'big.txt',
|
||||
added: 50_000,
|
||||
removed: 0,
|
||||
largeDiffRenderLimit: null
|
||||
}
|
||||
expect(usesLargeDiffFallbackHeight({ ...section, loading: false, loadOnDemand: true })).toBe(
|
||||
true
|
||||
)
|
||||
expect(usesLargeDiffFallbackHeight({ ...section, loading: true, loadOnDemand: false })).toBe(
|
||||
true
|
||||
)
|
||||
expect(
|
||||
usesLargeDiffFallbackHeight({ ...section, added: 3, loading: true, loadOnDemand: false })
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('estimates collapsed virtualized sections as header-only rows', () => {
|
||||
expect(
|
||||
getDiffSectionEstimatedHeight({
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import type { GitDiffResult } from '../../../../shared/git-diff-compare-types'
|
||||
import { shouldLoadCombinedDiffOnDemand } from './combined-diff-on-demand-load'
|
||||
import type { DiffSection } from './diff-section-types'
|
||||
import { countLinesLikeSplit, type DiffLineCounts } from './large-diff-render-limit'
|
||||
|
||||
const DIFF_LINE_HEIGHT = 19
|
||||
@@ -28,6 +30,25 @@ export function getLargeDiffFallbackBodyHeight(): number {
|
||||
return LARGE_DIFF_FALLBACK_BODY_HEIGHT
|
||||
}
|
||||
|
||||
/**
|
||||
* Rows that size from the bounded fallback instead of their (absent) content:
|
||||
* render-limited rows, rows still waiting behind the load prompt, and — while
|
||||
* the fetch is in flight — rows the user just loaded, so starting a load does
|
||||
* not shrink the section to the empty-content minimum and shift the list.
|
||||
*/
|
||||
export function usesLargeDiffFallbackHeight(
|
||||
section: Pick<
|
||||
DiffSection,
|
||||
'added' | 'area' | 'largeDiffRenderLimit' | 'loading' | 'loadOnDemand' | 'path' | 'removed'
|
||||
>
|
||||
): boolean {
|
||||
return (
|
||||
section.largeDiffRenderLimit?.limited === true ||
|
||||
section.loadOnDemand === true ||
|
||||
(section.loading && shouldLoadCombinedDiffOnDemand(section))
|
||||
)
|
||||
}
|
||||
|
||||
export function getDiffSectionBodyHeight({
|
||||
measuredContentHeight,
|
||||
originalContent,
|
||||
|
||||
@@ -3,7 +3,8 @@ import { computeLineStats } from './diff-line-stats'
|
||||
import {
|
||||
getDiffSectionBodyHeight,
|
||||
getLargeDiffFallbackBodyHeight,
|
||||
isIntrinsicHeightImageDiff
|
||||
isIntrinsicHeightImageDiff,
|
||||
usesLargeDiffFallbackHeight
|
||||
} from './diff-section-layout'
|
||||
import type { DiffSection } from './diff-section-types'
|
||||
|
||||
@@ -20,7 +21,7 @@ export function useDiffSectionLayoutMetrics({
|
||||
isLargeDiffLimited: boolean
|
||||
} {
|
||||
const renderLimit = section.largeDiffRenderLimit
|
||||
const isLargeDiffLimited = renderLimit?.limited === true || section.loadOnDemand === true
|
||||
const isLargeDiffLimited = usesLargeDiffFallbackHeight(section)
|
||||
const lineStats = useMemo(
|
||||
() =>
|
||||
section.loading || section.error || isLargeDiffLimited
|
||||
|
||||
Reference in New Issue
Block a user