From c61fc848ba3031cc7c839c39ce8d4280629a0c9d Mon Sep 17 00:00:00 2001 From: Brendonovich <14191578+Brendonovich@users.noreply.github.com> Date: Thu, 22 Oct 2026 05:29:14 +0000 Subject: [PATCH] fix(app): scroll file links to their line in wrapped files The stored scroll for a file link was an estimate from a fixed 24px line height, so wrapped lines left the target far below the view. Once the file renders, measure the selected row and scroll to it. Clicking a link into an already open file also remounted the code view at the top: the reread replaced the loaded value the view was built from. Read the text through contents() so the view updates in place. --- packages/app/e2e/regression/review.spec.ts | 40 ++++++++++++ packages/gui-extensions/src/file/context.ts | 4 +- packages/gui-extensions/src/file/renderer.tsx | 5 ++ packages/gui-extensions/src/file/view.tsx | 63 +++++++++++++++---- 4 files changed, 100 insertions(+), 12 deletions(-) diff --git a/packages/app/e2e/regression/review.spec.ts b/packages/app/e2e/regression/review.spec.ts index 9e6dcd244575..a3ef3325b9a5 100644 --- a/packages/app/e2e/regression/review.spec.ts +++ b/packages/app/e2e/regression/review.spec.ts @@ -597,6 +597,46 @@ test("rereads a file each time an artifact link opens it", async ({ page }) => { await expect(missingTab).toHaveCount(0) }) +test("a file link scrolls its line into view in a new or open file tab", async ({ page }) => { + // Every other line wraps, so a line's offset cannot be computed from the line height. + const source = Array.from( + { length: 900 }, + (_, index) => `export const value${index + 1} = "${"wrapped ".repeat(index % 2 === 0 ? 24 : 1)}"`, + ).join("\n") + + await openSession(page, { + name: "LineLink", + fileList: () => [], + findFiles: () => [fileNode("C:/OpenCode/LineLink", "src/long.ts")], + fileContent: () => source, + pageMessages: () => ({ + items: [ + { id: "msg_prompt", type: "user", text: "Where is it?", time: { created: 1 } }, + { + id: "msg_reply", + type: "assistant", + agent: "build", + model: { id: "test", providerID: "opencode" }, + content: [{ type: "text", text: "See `src/long.ts:658-659`, then `src/long.ts:120`." }], + time: { created: 2, completed: 3 }, + }, + ], + }), + }) + + const panel = page.locator("#review-panel") + + for (const link of [ + { name: "src/long.ts:658-659", line: 658 }, + { name: "src/long.ts:120", line: 120 }, + ]) { + await page.getByText(link.name, { exact: true }).click() + const row = panel.locator(`[data-line="${link.line}"]`) + await expect(row).toHaveAttribute("data-selected-line", /.+/) + await expect(row).toBeInViewport() + } +}) + /** What the image case's frame probe records on the window. */ type ImageProbe = { problems: string[]; frames: number; running: boolean } diff --git a/packages/gui-extensions/src/file/context.ts b/packages/gui-extensions/src/file/context.ts index 6e8d11bc4c49..f7f79de76aeb 100644 --- a/packages/gui-extensions/src/file/context.ts +++ b/packages/gui-extensions/src/file/context.ts @@ -42,10 +42,12 @@ export interface FileShared { get(session: string, path: string): LineRange | null | undefined set(session: string, files: Record): void } - /** Reveals a selected line range on a mounted file view when a link action targets it. */ + /** Reveals a selected line range on a file view when a link action targets it. */ readonly reveal: { register(session: string, path: string, run: () => void): () => void run(session: string, path: string): void + /** Whether a reveal ran since the last take; the view scrolls to the selection once its row renders. */ + take(session: string, path: string): boolean } /** The tab is the session's selected side tab. */ active(session: MountedSession, id: string): boolean diff --git a/packages/gui-extensions/src/file/renderer.tsx b/packages/gui-extensions/src/file/renderer.tsx index dd9fc96d3ce8..0cf7a8f3de7f 100644 --- a/packages/gui-extensions/src/file/renderer.tsx +++ b/packages/gui-extensions/src/file/renderer.tsx @@ -71,6 +71,7 @@ const setup: Setup = (ctx) => { const [filters, setFilters] = createStore>({}) const revealListeners = new Map void>>() + const pendingReveals = new Set() let clickController: AbortController | undefined // Tab objects per session screen, which stays while it routes another session, reused so neither strip updates nor a @@ -216,8 +217,12 @@ const setup: Setup = (ctx) => { } }, run(session, path) { + pendingReveals.add(`${session}\n${path}`) revealListeners.get(`${session}\n${path}`)?.forEach((fn) => fn()) }, + take(session, path) { + return pendingReveals.delete(`${session}\n${path}`) + }, }, active, open, diff --git a/packages/gui-extensions/src/file/view.tsx b/packages/gui-extensions/src/file/view.tsx index 9f49fac6e75d..f81b33ee38d5 100644 --- a/packages/gui-extensions/src/file/view.tsx +++ b/packages/gui-extensions/src/file/view.tsx @@ -64,7 +64,14 @@ type NoteState = { openedComment: string | null; commenting: LineRange | null; s type FindState = { find: FileSearchHandle | null } -function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos: ScrollPos) => void }) { +// Four unwrapped lines above a revealed line: the context the estimate in `renderer.tsx` leaves. +const REVEAL_MARGIN = 4 * 24 + +function createScrollSync(input: { + get: () => ScrollPos | undefined + set: (pos: ScrollPos) => void + restored: () => void +}) { const state: ScrollSyncState = {} const [code, setCode] = createSignal([]) @@ -133,7 +140,7 @@ function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos: const pos = input.get() - if (!pos) return + if (!pos) return input.restored() sync() @@ -145,9 +152,21 @@ function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos: if (el.scrollTop !== pos.y) el.scrollTop = pos.y - if (code().length > 0) return + if (code().length === 0 && el.scrollLeft !== pos.x) el.scrollLeft = pos.x + + input.restored() + } + + // Scrolls a rendered line to the top of the view, with a few lines of context above it. + const scrollToLine = (line: number) => { + const el = state.scroll + const row = el?.querySelector("diffs-container")?.shadowRoot?.querySelector(`[data-line="${line}"]`) + + if (!el || !(row instanceof HTMLElement)) return false + + el.scrollTop += row.getBoundingClientRect().top - el.getBoundingClientRect().top - REVEAL_MARGIN - if (el.scrollLeft !== pos.x) el.scrollLeft = pos.x + return true } const queueRestore = () => { @@ -185,6 +204,7 @@ function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos: return { handleScroll, queueRestore, + scrollToLine, setViewport, } } @@ -239,9 +259,27 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio return shared.handoff.get(props.session.key, p) ?? null }) + // A link's stored scroll is an estimate from unwrapped line heights; wrapped lines and annotations push the real row + // further down. The link's reveal is taken when the stored scroll restores, so a view the link mounts sees it too, and + // stays pending until the row is measured or the rendered file has no row for it. + const reveal = { pending: false, rendered: false } + const scrollSync = createScrollSync({ get: () => layout.scroll.get(props.session, key()), set: (pos) => layout.scroll.set(props.session, key(), pos), + restored: () => { + const p = path() + + if (p && shared.reveal.take(props.session.key, p)) reveal.pending = true + + if (!reveal.pending) return + + const range = selectedLines() + + if ((range && scrollSync.scrollToLine(Math.min(range.start, range.end))) || reveal.rendered) { + reveal.pending = false + } + }, }) const selectionPreview = (source: string, selection: FileSelection) => { @@ -448,14 +486,14 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio }, ) - const renderFile = (source: string) => ( + const renderFile = () => (
{ + reveal.rendered = true scrollSync.queueRestore() }} annotations={commentsUi.annotations()} @@ -492,10 +531,12 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio
) - // The code view scrolls inside ScrollView so line state and scroll position persist per tab. - const codeView = (source: string) => ( + // The code view scrolls inside ScrollView so line state and scroll position persist per tab. It reads the text through + // `contents()`, not the loaded value, so a reread (every link click forces one) updates it in place instead of + // remounting it scrolled to the top. + const codeView = () => ( - {renderFile(source)} + {renderFile()} ) @@ -504,13 +545,13 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio {(value) => ( - + )}