From 4df3ca2ed3d4a293ca1b55dba1615409e2ef7deb Mon Sep 17 00:00:00 2001 From: eti Date: Mon, 7 Sep 2026 12:38:27 +0200 Subject: [PATCH] web/repo: drive the diff scroller from the page scroll Signed-off-by: eti --- web/src/lib/components/repo/CommitView.svelte | 4 +- .../lib/components/repo/DiffCodeView.svelte | 58 +++- .../lib/components/repo/pulls/PullDiff.svelte | 319 ++++++++++-------- .../repo/pulls/PullDiffFiles.svelte | 10 +- .../[[range]]/PullViewPage.stories.svelte | 13 +- 5 files changed, 237 insertions(+), 167 deletions(-) diff --git a/web/src/lib/components/repo/CommitView.svelte b/web/src/lib/components/repo/CommitView.svelte index 5587fca08..ae16ba4df 100644 --- a/web/src/lib/components/repo/CommitView.svelte +++ b/web/src/lib/components/repo/CommitView.svelte @@ -106,7 +106,7 @@ const patch = $derived(browser && patchUrl ? getPatchStream(patchUrl) : undefined); -
+
@@ -120,6 +120,6 @@ {downloadUrls} {patchUrl} {patch} - stickyTop="viewport" + hasPageHeader={false} />
diff --git a/web/src/lib/components/repo/DiffCodeView.svelte b/web/src/lib/components/repo/DiffCodeView.svelte index dd20cfd82..4c7306a0f 100644 --- a/web/src/lib/components/repo/DiffCodeView.svelte +++ b/web/src/lib/components/repo/DiffCodeView.svelte @@ -36,6 +36,8 @@ onSetOpen?: (key: string, open: boolean) => void; onWant?: (key: string) => void; onFocus?: (key: string) => void; + dockTop?: () => number; + onScrollExtent?: (max: number) => void; } let { @@ -47,13 +49,16 @@ isOpen, onSetOpen, onWant, - onFocus + onFocus, + dockTop = () => 0, + onScrollExtent }: Props = $props(); const HEADER_HEIGHT = 37; const LOOKAHEAD_ITEMS = 8; let root: HTMLElement | undefined = $state(); + let contentHeight = $state(0); let view: CodeView | undefined = $state.raw(); let initError = $state(); @@ -232,10 +237,48 @@ } }; + const syncScroll = () => { + const element = root; + if (!element) return; + const container = element.firstElementChild; + if (container) { + const style = getComputedStyle(container); + contentHeight = + container.getBoundingClientRect().height + + parseFloat(style.marginTop) + + parseFloat(style.marginBottom); + } + const max = Math.max(0, element.scrollHeight - element.clientHeight); + onScrollExtent?.(max); + const position = Math.min(Math.max(0, window.scrollY - dockTop()), max); + if (Math.abs(element.scrollTop - position) < 0.5) return; + element.scrollTop = position; + }; + + $effect(() => { + const instance = view; + const element = root; + if (!instance || !element) return; + syncScroll(); + const observer = new ResizeObserver(syncScroll); + observer.observe(element); + if (element.firstElementChild) observer.observe(element.firstElementChild); + window.addEventListener("scroll", syncScroll, { passive: true }); + window.addEventListener("resize", syncScroll); + return () => { + observer.disconnect(); + window.removeEventListener("scroll", syncScroll); + window.removeEventListener("resize", syncScroll); + }; + }); + $effect(() => { const instance = view; if (!instance) return; - return instance.subscribeToScroll(() => requestContents()); + return instance.subscribeToScroll(() => { + requestContents(); + syncScroll(); + }); }); $effect(() => { @@ -248,6 +291,7 @@ if (item?.type !== "diff" || item.collapsed === !open) continue; instance.updateItem({ ...item, collapsed: !open, version: bumpVersion(file.key) }); } + syncScroll(); }); $effect(() => { @@ -266,6 +310,7 @@ if (current?.type === "diff" && current.fileDiff === parsed) continue; instance.updateItem(itemOf(file)); } + syncScroll(); }); let appliedStyle: DiffStyle | undefined; @@ -282,14 +327,14 @@ appliedFiles = files; instance.setItems(files.map(itemOf)); } + syncScroll(); }); export const scrollToFile = (key: string): number | undefined => { const instance = view; const top = instance?.getTopForItem(key); if (!instance || top == null) return undefined; - root?.scrollIntoView({ block: "nearest" }); - instance.scrollTo({ type: "item", id: key, align: "start", behavior: "instant" }); + window.scrollTo({ top: dockTop() + top, behavior: "instant" }); return top; }; @@ -299,10 +344,11 @@ {initError}
{:else} -
+
{/if} diff --git a/web/src/lib/components/repo/pulls/PullDiff.svelte b/web/src/lib/components/repo/pulls/PullDiff.svelte index ac2df880c..cf3190047 100644 --- a/web/src/lib/components/repo/pulls/PullDiff.svelte +++ b/web/src/lib/components/repo/pulls/PullDiff.svelte @@ -40,7 +40,7 @@ patchUrl?: string; /** a patch already streaming in parallel with the file list */ patch?: PatchStream; - stickyTop?: "page" | "viewport"; + hasPageHeader?: boolean; } let { @@ -53,10 +53,38 @@ downloadUrls, patchUrl, patch, - stickyTop = "page" + hasPageHeader = true }: Props = $props(); let treeOpen = $state(true); + let shell = $state(); + let dock = $state(0); + let diffScroll = $state(0); + const HEADER_HEIGHT = 60; + const GAP = 16; + const stickyTop = $derived(hasPageHeader ? HEADER_HEIGHT : 0); + const maxHeight = $derived( + hasPageHeader ? `calc(100svh - ${HEADER_HEIGHT + GAP}px)` : "100svh" + ); + const dockOffset = $derived(hasPageHeader ? HEADER_HEIGHT : GAP); + $effect(() => { + const element = shell; + if (!element) return; + const measure = () => { + dock = window.scrollY + element.getBoundingClientRect().top - dockOffset; + }; + measure(); + const observer = new ResizeObserver(measure); + observer.observe(element); + window.addEventListener("scroll", measure, { passive: true }); + window.addEventListener("resize", measure); + return () => { + observer.disconnect(); + window.removeEventListener("scroll", measure); + window.removeEventListener("resize", measure); + }; + }); + const dockTop = () => dock; // only pages without a url for it keep the style here, so the toolbar and the // address bar can never disagree let localStyle = $state(routeStyle); @@ -78,161 +106,152 @@ {#if loading && !diff} {:else} - - - {#if treeOpen} - - - {/if} - -
+ -
-
- {#if !treeOpen} + + {#if treeOpen} +
+ + + {/if} + +
+
- {error} +
+ {#if !treeOpen} +
+
+ + {#if downloadUrls} + + {#snippet trigger()} + + Download commit + {/snippet} + + + + + + {/if} + + {#each ["unified", "split"] as const as style (style)} + + {/each} + +
- {:else if diff && diff.files.length > 0} - - {#key diff} - (openStates[key] = open)} - patchUrl={patchUrl ?? downloadUrls?.diff} - /> - {/key} - {:else if diff} -
- No change between two revisions. -
- {:else} -
-
- {/if} -
- + {#key diff} + (diffScroll = max)} + bind:this={files} + {isOpen} + onSetOpen={(key, open) => (openStates[key] = open)} + patchUrl={patchUrl ?? downloadUrls?.diff} + /> + {/key} + {:else if diff} +
+ No change between two revisions. +
+ {:else} +
+
+ {/if} +
+ + +
{/if} diff --git a/web/src/lib/components/repo/pulls/PullDiffFiles.svelte b/web/src/lib/components/repo/pulls/PullDiffFiles.svelte index 2f03839ed..0aa52759e 100644 --- a/web/src/lib/components/repo/pulls/PullDiffFiles.svelte +++ b/web/src/lib/components/repo/pulls/PullDiffFiles.svelte @@ -22,6 +22,8 @@ patchUrl?: string; /** a patch already streaming in parallel with this diff's file list */ patch?: PatchStream; + dockTop?: () => number; + onScrollExtent?: (max: number) => void; } let { @@ -31,7 +33,9 @@ isOpen = () => true, onSetOpen, patchUrl, - patch + patch, + dockTop, + onScrollExtent }: Props = $props(); // parsed metadata only: raw text is dropped after parsing. a map, because @@ -149,7 +153,7 @@ a diff of this size --> -
+
onSetOpen?.(key, open)} + {dockTop} + {onScrollExtent} onWant={(key) => queue.want(key)} onFocus={(key) => queue.focus(key)} /> diff --git a/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/PullViewPage.stories.svelte b/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/PullViewPage.stories.svelte index 48b528eed..0b8dcd917 100644 --- a/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/PullViewPage.stories.svelte +++ b/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/PullViewPage.stories.svelte @@ -143,9 +143,11 @@ Depends on the hydrant filter change, so this is stacked on \`sv-fe\`.`; const diffRendered = async ({ canvas }: PlayContext) => { const card = await canvas.findByText("src/lib/notifications.svelte.ts"); - card.scrollIntoView(); await waitFor( - () => expect(card.closest("details")?.querySelector("diffs-container")).not.toBeNull(), + () => + expect( + card.closest("diffs-container")?.querySelector("[data-diff]") + ).not.toBeNull(), { timeout: 5000 } ); }; @@ -233,7 +235,7 @@ Depends on the hydrant filter change, so this is stacked on \`sv-fe\`.`; { + play={async ({ canvas }) => { await waitFor(() => expect(canvas.getByRole("button", { name: /merge \(rebase\)/i })).toBeDisabled() ); @@ -248,10 +250,7 @@ Depends on the hydrant filter change, so this is stacked on \`sv-fe\`.`; "#file-src%2Flib%2Fcomponents%2Fnotifications%2FNotificationList.svelte" ); await userEvent.click(conflict); - // the scroller, not the browser, honours the anchor: proof is that the - // second file is under the reader after the click - const scroller = canvasElement.querySelector(".codeview"); - await waitFor(() => expect(scroller?.scrollTop ?? 0).toBeGreaterThan(0), { + await waitFor(() => expect(window.scrollY).toBeGreaterThan(0), { timeout: 5000 }); }} -- 2.51.2