diff --git a/CHANGELOG.md b/CHANGELOG.md index c64b28cb9..323cf3b7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 here. The rule is now the other way round: Haiku is the 200k exception and everything else is 1M, which is what a model released after your build reads without a new version. +- **A pull request with hundreds of changed files no longer locks the app.** + Every file in a diff panel starts expanded, and an expanded file builds its own + CodeMirror view — so a 191-file pull request built 191 editors in one go, each + followed by a language chunk re-configuring its document. Chromium offered to + kill the page, and the screen stayed unusable well after it recovered. A file's + editor is now built only once its card comes near the viewport; until then the + card holds a placeholder of the file's height, so the scrollbar still measures + the whole diff. An editor that has been built stays built, keeping its + selection and highlighting when you scroll back. That 191-file diff now settles + in 465 ms with 8 editors, against 13.7 s with 191. The dock's Review tab shares + the same cards and the same fix. Files changed also stopped refetching the diff + every time the window regained focus — a new commit still refreshes it. +- **The pull request screen no longer waits behind a bare "Loading…".** Both the + lookup and the diff are `gh` round-trips of about a second, and on a screen + that size a single muted line read as a failure. Each now holds the shape of + what is coming — title, actions, status line and tabs for the pull request; + file tree, toolbar and file cards for Files changed. ## [0.18.0] - 2026-07-25 diff --git a/frontend/src/components/diff/FileDiff.tsx b/frontend/src/components/diff/FileDiff.tsx index af8adb931..f463bdced 100644 --- a/frontend/src/components/diff/FileDiff.tsx +++ b/frontend/src/components/diff/FileDiff.tsx @@ -20,6 +20,15 @@ import { useDiffEditor } from "./useDiffEditor" // giant lockfile doesn't swamp the panel (expanding is one click away). const LARGE_FILE_LINES = 500 +// Height of one editor line, the knob sizing a file's placeholder before its +// editor exists. Measured against the diff theme's 12px font; a wrapped long +// line makes any single number a lower bound, so this only has to be close. +const LINE_HEIGHT_PX = 17 + +// How far outside the viewport a file's editor is built, so scrolling meets a +// rendered diff rather than a gap. +const MOUNT_MARGIN = "600px 0px" + interface FileDiffProps { file: DiffFile onInject: (text: string) => void @@ -114,7 +123,7 @@ export function FileDiff({ file, onInject, onDiscard, bulk, viewed, onViewed }: (file.binary ? (

Binary file

) : ( - + ))} ) @@ -126,6 +135,40 @@ interface DiffBodyProps { onInject: (text: string) => void } +// A panel holds one CodeMirror view per expanded file, and a wide diff (a PR +// touching 191 files) built them all in a single commit — enough to lock the +// page. LazyDiffBody defers each one until its card comes near the viewport; +// once built, the editor stays, so scrolling back keeps its selection and +// highlighting. Collapsing the file still destroys it. +function LazyDiffBody({ doc, path, onInject }: DiffBodyProps) { + const placeholder = useRef(null) + const [mounted, setMounted] = useState(false) + + useEffect(() => { + const target = placeholder.current + if (!target) { + return + } + const observer = new IntersectionObserver( + ([entry]) => { + if (entry.isIntersecting) { + setMounted(true) + } + }, + { rootMargin: MOUNT_MARGIN }, + ) + observer.observe(target) + return () => observer.disconnect() + }, []) + + if (mounted) { + return + } + // The placeholder has to stand in for the file's height: zero-height cards + // would all sit in the viewport at once and nothing would be deferred. + return
+} + // DiffBody exists as its own component so collapsing the file unmounts it, // destroying the CodeMirror view instead of keeping it alive off-screen. // diff --git a/frontend/src/components/pulls/Pulls.tsx b/frontend/src/components/pulls/Pulls.tsx index 2b522a69b..704a9de4b 100644 --- a/frontend/src/components/pulls/Pulls.tsx +++ b/frontend/src/components/pulls/Pulls.tsx @@ -44,9 +44,13 @@ import { } from "@/components/ui/dropdown-menu" import { Input } from "@/components/ui/input" import { Label } from "@/components/ui/label" +import { Skeleton } from "@/components/ui/skeleton" import { PullsChecks } from "./PullsChecks" import { PullsFiles } from "./PullsFiles" +// Ragged widths, so the body placeholder reads as prose instead of a block. +const BODY_ROWS = ["w-full", "w-11/12", "w-4/5", "w-2/3", "w-5/6", "w-1/2"] + // Pulls is the per-project pull-request screen: it fills the main area on top of // the persistent terminals (like Settings), showing the active session's branch // PR — status, body and the full diff — with merge, create and open actions. It @@ -124,7 +128,7 @@ export function Pulls() { /> ) } else if (loading) { - body = Loading… + body = } else { body = } @@ -132,6 +136,42 @@ export function Pulls() { return
{body}
} +// Looking the pull request up is a gh round-trip, and the screen covers the +// whole app while it runs — a lone "Loading…" in the middle of that much empty +// space reads as a failure. The skeleton holds the header the answer will fill: +// title, actions, status line, tabs, then the body. +function PullSkeleton() { + return ( +
+
+
+ +
+ + + +
+
+
+ + + +
+
+ + + +
+
+
+ {BODY_ROWS.map((width) => ( + + ))} +
+
+ ) +} + // EditState carries a pending "edit commit message" merge: which method to run // and the message the dialog is editing. null when the dialog is closed. interface EditState { diff --git a/frontend/src/components/pulls/PullsFiles.tsx b/frontend/src/components/pulls/PullsFiles.tsx index 55cbef75a..fdbcd318b 100644 --- a/frontend/src/components/pulls/PullsFiles.tsx +++ b/frontend/src/components/pulls/PullsFiles.tsx @@ -6,6 +6,7 @@ import { CollapseAllAction, useDiffBulk } from "@/components/diff/diff-bulk" import { FileDiff } from "@/components/diff/FileDiff" import { DiffStat } from "@/components/DiffStat" import { FileTree } from "@/components/FileTree" +import { Skeleton } from "@/components/ui/skeleton" import { buildTree } from "@/lib/git/file-tree" import { fileFingerprint, @@ -21,6 +22,52 @@ import { usePullRequestDiff } from "@/lib/pulls/use-pull-request-diff" // so the choice survives leaving the screen. const TREE_HIDDEN_KEY = "lich.pulls.tree.hidden" +// Ragged widths, so the placeholders read as file names and code rather than a +// stack of identical bars. +const TREE_ROWS = ["w-28", "w-40", "w-24", "w-36", "w-32", "w-44", "w-24", "w-36"] +const CODE_ROWS = ["w-3/5", "w-4/5", "w-2/5", "w-11/12", "w-1/2", "w-3/4", "w-1/3", "w-2/3"] + +// The diff is a gh round-trip, so this tab is empty for about a second. A bare +// "Loading…" line reads as a broken tab on a screen that wide; the skeleton +// stands in for the shape that arrives — tree, toolbar, then file cards. +function FilesSkeleton({ tree }: { tree: boolean }) { + return ( +
+ {tree && ( +
+ {TREE_ROWS.map((width) => ( + + ))} +
+ )} +
+
+ + +
+
+ {[0, 1, 2].map((card) => ( +
+
+ + + + + +
+
+ {CODE_ROWS.map((width) => ( + + ))} +
+
+ ))} +
+
+
+ ) +} + interface PullsFilesProps { path: string /** The checkout's HEAD; a new commit refetches the diff. */ @@ -59,7 +106,7 @@ export function PullsFiles({ path, head, pullRequest, onInject }: PullsFilesProp return Couldn’t load the diff: {error} } if (files === null) { - return Loading… + return } if (files.length === 0) { return No file changes diff --git a/frontend/src/components/ui/skeleton.tsx b/frontend/src/components/ui/skeleton.tsx new file mode 100644 index 000000000..1520fd9af --- /dev/null +++ b/frontend/src/components/ui/skeleton.tsx @@ -0,0 +1,16 @@ +import { cn } from "@/lib/utils" + +function Skeleton({ className, ...props }: React.ComponentProps<"div">) { + return ( +
+ ) +} + +export { Skeleton } diff --git a/frontend/src/lib/pulls/use-pull-request-diff.ts b/frontend/src/lib/pulls/use-pull-request-diff.ts index 0dcf9521d..b2d4f3ac0 100644 --- a/frontend/src/lib/pulls/use-pull-request-diff.ts +++ b/frontend/src/lib/pulls/use-pull-request-diff.ts @@ -10,9 +10,10 @@ export interface PullRequestDiffState { // usePullRequestDiff fetches and parses the PR's unified diff (gh pr diff) for // the Files changed tab. Fetched on demand — the diff is a gh round-trip and can -// be large — and refetched when the checkout's HEAD moves or on window focus, -// like the PR detail. files is null while loading or on error; an empty array -// is a PR with no file changes. +// be large — and refetched when the checkout's HEAD moves. Unlike the PR detail +// next door it does not refresh on window focus: re-rendering a wide diff is far +// too expensive to spend on every alt-tab back into the app. files is null while +// loading or on error; an empty array is a PR with no file changes. export function usePullRequestDiff(path: string, head: string): PullRequestDiffState { const [files, setFiles] = useState(null) const [error, setError] = useState(null) @@ -40,10 +41,8 @@ export function usePullRequestDiff(path: string, head: string): PullRequestDiffS useEffect(() => { refresh() - window.addEventListener("focus", refresh) return () => { seq.current++ - window.removeEventListener("focus", refresh) } }, [refresh, head])