Skip to content

Commit 57420e5

Browse files
committed
Estimate patch height before fully loaded to reduce layout shift and
improve linking consistency, add Go>Go to Selection menu action
1 parent 64a3157 commit 57420e5

File tree

3 files changed

+63
-34
lines changed

3 files changed

+63
-34
lines changed

web/src/lib/components/diff/ConciseDiffView.svelte

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
type SearchSegment,
1515
} from "$lib/components/diff/concise-diff-view.svelte";
1616
import Spinner from "$lib/components/Spinner.svelte";
17-
import { onMount } from "svelte";
1817
import { type MutableValue } from "$lib/util";
1918
import { box } from "svelte-toolbelt";
2019
import { boolAttr } from "runed";
@@ -124,6 +123,14 @@
124123
const endIdx = selection.end.idx;
125124
return Math.floor((startIdx + endIdx) / 2);
126125
});
126+
127+
let heightEstimateRem = $derived.by(() => {
128+
if (!parsedPatch) return 1.25;
129+
const rawLineCount = parsedPatch.hunks.reduce((sum, hunk) => sum + hunk.lines.length, 0);
130+
const headerAndSpacerLines = parsedPatch.hunks.length * 2;
131+
const totalLines = rawLineCount + headerAndSpacerLines;
132+
return totalLines * 1.25;
133+
});
127134
</script>
128135

129136
{#snippet lineContent(line: PatchLine, lineType: PatchLineTypeProps, innerLineType: InnerPatchLineTypeProps)}
@@ -156,18 +163,16 @@
156163
{#each lineSearchSegments as searchSegment, index (index)}
157164
{#if searchSegment.highlighted}<span
158165
{@attach (element) => {
159-
onMount(() => {
160-
if (jumpToSearchResult && searchSegment.id === activeSearchResult) {
161-
jumpToSearchResult = false;
162-
// See similar code & comment below around jumping to selections
163-
const scheduledJump = setTimeout(() => {
164-
element.scrollIntoView({ block: "center", inline: "center" });
165-
}, 100);
166-
return () => {
167-
clearTimeout(scheduledJump);
168-
};
169-
}
170-
});
166+
if (jumpToSearchResult && searchSegment.id === activeSearchResult) {
167+
jumpToSearchResult = false;
168+
// See similar code & comment below around jumping to selections
169+
const scheduledJump = setTimeout(() => {
170+
element.scrollIntoView({ block: "center", inline: "center" });
171+
}, 200);
172+
return () => {
173+
clearTimeout(scheduledJump);
174+
};
175+
}
171176
}}
172177
class={{
173178
"bg-[#d4a72c66]": searchSegment.id !== activeSearchResult,
@@ -209,30 +214,33 @@
209214
data-selection-start={boolAttr(view.isSelectionStart(hunkIndex, lineIndex))}
210215
data-selection-end={boolAttr(view.isSelectionEnd(hunkIndex, lineIndex))}
211216
{@attach (element) => {
212-
onMount(() => {
213-
if (jumpToSelection && selection && selection.hunk === hunkIndex && selectionMidpoint === lineIndex) {
214-
jumpToSelection = false;
215-
// Need to schedule because otherwise the vlist rendering surrounding elements may shift things
216-
// and cause the element to scroll to the wrong position
217-
// This is not 100% reliable but is good enough for now
218-
const scheduledJump = setTimeout(() => {
219-
element.scrollIntoView({ block: "center", inline: "center" });
220-
}, 200);
221-
return () => {
222-
if (scheduledJump) {
223-
clearTimeout(scheduledJump);
224-
}
225-
};
226-
}
227-
});
217+
if (jumpToSelection && selection && selection.hunk === hunkIndex && selectionMidpoint === lineIndex) {
218+
jumpToSelection = false;
219+
// Need to schedule because otherwise the vlist rendering surrounding elements may shift things
220+
// and cause the element to scroll to the wrong position
221+
// This is not 100% reliable but is good enough for now
222+
const scheduledJump = setTimeout(() => {
223+
element.scrollIntoView({ block: "center", inline: "center" });
224+
}, 200);
225+
return () => {
226+
if (scheduledJump) {
227+
clearTimeout(scheduledJump);
228+
}
229+
};
230+
}
228231
}}
229232
>
230233
{@render lineContentWrapper(line, hunkIndex, lineIndex, lineType, innerPatchLineTypeProps[line.innerPatchLineType])}
231234
</div>
232235
{/snippet}
233236

234237
{#await Promise.all([view.rootStyle, view.diffViewerPatch])}
235-
<div class="flex items-center justify-center bg-neutral-2 p-4"><Spinner /></div>
238+
<div class="relative bg-neutral-2" style="min-height: {heightEstimateRem}rem;">
239+
<!-- 2.25 rem for file header offset -->
240+
<div class="sticky top-[2.25rem] flex items-center justify-center p-4">
241+
<Spinner />
242+
</div>
243+
</div>
236244
{:then [rootStyle, diffViewerPatch]}
237245
<div
238246
id={uid}
@@ -271,7 +279,6 @@
271279
272280
display: grid;
273281
grid-template-columns: min-content min-content auto;
274-
contain: layout style paint;
275282
}
276283
.diff-content[data-wrap="true"] {
277284
word-break: break-all;

web/src/lib/components/menu-bar/MenuBar.svelte

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,5 +118,25 @@
118118
</Menubar.Content>
119119
</Menubar.Portal>
120120
</Menubar.Menu>
121+
<Menubar.Menu>
122+
<Menubar.Trigger class="btn-ghost px-2 py-1 text-sm data-[state=open]:btn-ghost-hover">Go</Menubar.Trigger>
123+
<Menubar.Portal>
124+
<Menubar.Content class="z-20 border bg-neutral text-sm" align="start">
125+
<Menubar.Item
126+
class="btn-ghost px-2 py-1 select-none"
127+
onSelect={() => {
128+
if (viewer.selection) {
129+
viewer.scrollToFile(viewer.selection.file.index, {
130+
focus: !viewer.selection.lines,
131+
});
132+
if (viewer.selection.lines) {
133+
viewer.jumpToSelection = true;
134+
}
135+
}
136+
}}>Go to Selection</Menubar.Item
137+
>
138+
</Menubar.Content>
139+
</Menubar.Portal>
140+
</Menubar.Menu>
121141
<SidebarToggle class="my-auto mr-2 ml-auto" />
122142
</Menubar.Root>

web/src/lib/diff-viewer.svelte.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,10 @@ export class MultiFileDiffViewerState {
552552

553553
this.fileDetails = tempDetails;
554554
this.fileStates = statesArray;
555+
this.stats = this.countStats();
556+
557+
await tick();
558+
await animationFramePromise();
555559

556560
if (this.urlSelection) {
557561
const urlSelection = this.urlSelection;
@@ -567,7 +571,7 @@ export class MultiFileDiffViewerState {
567571
keepFocus: true,
568572
});
569573
this.scrollToFile(file.index, {
570-
focus: urlSelection.lines === undefined,
574+
focus: !urlSelection.lines,
571575
});
572576
} else {
573577
await goto(`?${page.url.searchParams}`, {
@@ -576,8 +580,6 @@ export class MultiFileDiffViewerState {
576580
}
577581
}
578582

579-
this.stats = this.countStats();
580-
581583
return true;
582584
} catch (e) {
583585
this.clear(); // Clear any partially loaded state

0 commit comments

Comments
 (0)