chore(app): simplify review pane (#17066)

This commit is contained in:
Adam
2026-03-11 12:24:51 -05:00
committed by GitHub
parent 9c585bb58b
commit bcc0d19867
7 changed files with 319 additions and 809 deletions

View File

@@ -0,0 +1,186 @@
import { waitSessionIdle, withSession } from "../actions"
import { test, expect } from "../fixtures"
import { createSdk } from "../utils"
const count = 14
function body(mark: string) {
return [
`title ${mark}`,
`mark ${mark}`,
...Array.from({ length: 32 }, (_, i) => `line ${String(i + 1).padStart(2, "0")} ${mark}`),
]
}
function files(tag: string) {
return Array.from({ length: count }, (_, i) => {
const id = String(i).padStart(2, "0")
return {
file: `review-scroll-${id}.txt`,
mark: `${tag}-${id}`,
}
})
}
function seed(list: ReturnType<typeof files>) {
const out = ["*** Begin Patch"]
for (const item of list) {
out.push(`*** Add File: ${item.file}`)
for (const line of body(item.mark)) out.push(`+${line}`)
}
out.push("*** End Patch")
return out.join("\n")
}
function edit(file: string, prev: string, next: string) {
return ["*** Begin Patch", `*** Update File: ${file}`, "@@", `-mark ${prev}`, `+mark ${next}`, "*** End Patch"].join(
"\n",
)
}
async function patch(sdk: ReturnType<typeof createSdk>, sessionID: string, patchText: string) {
await sdk.session.promptAsync({
sessionID,
agent: "build",
system: [
"You are seeding deterministic e2e UI state.",
"Your only valid response is one apply_patch tool call.",
`Use this JSON input: ${JSON.stringify({ patchText })}`,
"Do not call any other tools.",
"Do not output plain text.",
].join("\n"),
parts: [{ type: "text", text: "Apply the provided patch exactly once." }],
})
await waitSessionIdle(sdk, sessionID, 120_000)
}
async function show(page: Parameters<typeof test>[0]["page"]) {
const btn = page.getByRole("button", { name: "Toggle review" }).first()
await expect(btn).toBeVisible()
if ((await btn.getAttribute("aria-expanded")) !== "true") await btn.click()
await expect(btn).toHaveAttribute("aria-expanded", "true")
}
async function expand(page: Parameters<typeof test>[0]["page"]) {
const close = page.getByRole("button", { name: /^Collapse all$/i }).first()
const open = await close
.isVisible()
.then((value) => value)
.catch(() => false)
const btn = page.getByRole("button", { name: /^Expand all$/i }).first()
if (open) {
await close.click()
await expect(btn).toBeVisible()
}
await expect(btn).toBeVisible()
await btn.click()
await expect(close).toBeVisible()
}
async function waitMark(page: Parameters<typeof test>[0]["page"], file: string, mark: string) {
await page.waitForFunction(
({ file, mark }) => {
const head = Array.from(document.querySelectorAll("h3")).find(
(node) => node instanceof HTMLElement && node.textContent?.includes(file),
)
if (!(head instanceof HTMLElement)) return false
return Array.from(head.parentElement?.querySelectorAll("diffs-container") ?? []).some((host) => {
if (!(host instanceof HTMLElement)) return false
const root = host.shadowRoot
return root?.textContent?.includes(`mark ${mark}`) ?? false
})
},
{ file, mark },
{ timeout: 60_000 },
)
}
test("review keeps scroll position after a live diff update", async ({ page, withProject }) => {
test.setTimeout(180_000)
const tag = `review-${Date.now()}`
const list = files(tag)
const hit = list[list.length - 2]!
const next = `${tag}-live`
await page.setViewportSize({ width: 1600, height: 1000 })
await withProject(async (project) => {
const sdk = createSdk(project.directory)
await withSession(sdk, `e2e review ${tag}`, async (session) => {
await patch(sdk, session.id, seed(list))
await expect
.poll(
async () => {
const info = await sdk.session.get({ sessionID: session.id }).then((res) => res.data)
return info?.summary?.files ?? 0
},
{ timeout: 60_000 },
)
.toBe(list.length)
await expect
.poll(
async () => {
const diff = await sdk.session.diff({ sessionID: session.id }).then((res) => res.data ?? [])
return diff.length
},
{ timeout: 60_000 },
)
.toBe(list.length)
await project.gotoSession(session.id)
await show(page)
const tab = page.getByRole("tab", { name: /Review/i }).first()
await expect(tab).toBeVisible()
await tab.click()
const view = page.locator('[data-slot="session-review-scroll"] .scroll-view__viewport').first()
await expect(view).toBeVisible()
const heads = page.getByRole("heading", { level: 3 }).filter({ hasText: /^review-scroll-/ })
await expect(heads).toHaveCount(list.length, {
timeout: 60_000,
})
await expand(page)
await waitMark(page, hit.file, hit.mark)
const row = page
.getByRole("heading", { level: 3, name: new RegExp(hit.file.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")) })
.first()
await expect(row).toBeVisible()
await row.evaluate((el) => el.scrollIntoView({ block: "center" }))
await expect.poll(() => view.evaluate((el) => el.scrollTop)).toBeGreaterThan(200)
const prev = await view.evaluate((el) => el.scrollTop)
await patch(sdk, session.id, edit(hit.file, hit.mark, next))
await expect
.poll(
async () => {
const diff = await sdk.session.diff({ sessionID: session.id }).then((res) => res.data ?? [])
const item = diff.find((item) => item.file === hit.file)
return typeof item?.after === "string" ? item.after : ""
},
{ timeout: 60_000 },
)
.toContain(`mark ${next}`)
await waitMark(page, hit.file, next)
await expect
.poll(async () => Math.abs((await view.evaluate((el) => el.scrollTop)) - prev), { timeout: 60_000 })
.toBeLessThanOrEqual(16)
})
})
})

View File

@@ -862,6 +862,36 @@ export default function Page() {
</div>
)
const reviewEmpty = (input: { loadingClass: string; emptyClass: string }) => {
if (store.changes === "turn") return emptyTurn()
if (hasReview() && !diffsReady()) {
return <div class={input.loadingClass}>{language.t("session.review.loadingChanges")}</div>
}
if (reviewEmptyKey() === "session.review.noVcs") {
return (
<div class={input.emptyClass}>
<div class="flex flex-col gap-3">
<div class="text-14-medium text-text-strong">Create a Git repository</div>
<div class="text-14-regular text-text-base max-w-md" style={{ "line-height": "var(--line-height-normal)" }}>
Track, review, and undo changes in this project
</div>
</div>
<Button size="large" disabled={ui.git} onClick={initGit}>
{ui.git ? "Creating Git repository..." : "Create Git repository"}
</Button>
</div>
)
}
return (
<div class={input.emptyClass}>
<div class="text-14-regular text-text-weak max-w-56">{language.t(reviewEmptyKey())}</div>
</div>
)
}
const reviewContent = (input: {
diffStyle: DiffStyle
onDiffStyleChange?: (style: DiffStyle) => void
@@ -870,98 +900,25 @@ export default function Page() {
emptyClass: string
}) => (
<Show when={!store.deferRender}>
<Switch>
<Match when={store.changes === "turn" && !!params.id}>
<SessionReviewTab
title={changesTitle()}
empty={emptyTurn()}
diffs={reviewDiffs}
view={view}
diffStyle={input.diffStyle}
onDiffStyleChange={input.onDiffStyleChange}
onScrollRef={(el) => setTree("reviewScroll", el)}
focusedFile={tree.activeDiff}
onLineComment={(comment) => addCommentToContext({ ...comment, origin: "review" })}
onLineCommentUpdate={updateCommentInContext}
onLineCommentDelete={removeCommentFromContext}
lineCommentActions={reviewCommentActions()}
comments={comments.all()}
focusedComment={comments.focus()}
onFocusedCommentChange={comments.setFocus}
onViewFile={openReviewFile}
classes={input.classes}
/>
</Match>
<Match when={hasReview()}>
<Show
when={diffsReady()}
fallback={<div class={input.loadingClass}>{language.t("session.review.loadingChanges")}</div>}
>
<SessionReviewTab
title={changesTitle()}
diffs={reviewDiffs}
view={view}
diffStyle={input.diffStyle}
onDiffStyleChange={input.onDiffStyleChange}
onScrollRef={(el) => setTree("reviewScroll", el)}
focusedFile={tree.activeDiff}
onLineComment={(comment) => addCommentToContext({ ...comment, origin: "review" })}
onLineCommentUpdate={updateCommentInContext}
onLineCommentDelete={removeCommentFromContext}
lineCommentActions={reviewCommentActions()}
comments={comments.all()}
focusedComment={comments.focus()}
onFocusedCommentChange={comments.setFocus}
onViewFile={openReviewFile}
classes={input.classes}
/>
</Show>
</Match>
<Match when={true}>
<SessionReviewTab
title={changesTitle()}
empty={
store.changes === "turn" ? (
emptyTurn()
) : reviewEmptyKey() === "session.review.noVcs" ? (
<div class={input.emptyClass}>
<div class="flex flex-col gap-3">
<div class="text-14-medium text-text-strong">Create a Git repository</div>
<div
class="text-14-regular text-text-base max-w-md"
style={{ "line-height": "var(--line-height-normal)" }}
>
Track, review, and undo changes in this project
</div>
</div>
<Button size="large" disabled={ui.git} onClick={initGit}>
{ui.git ? "Creating Git repository..." : "Create Git repository"}
</Button>
</div>
) : (
<div class={input.emptyClass}>
<div class="text-14-regular text-text-weak max-w-56">{language.t(reviewEmptyKey())}</div>
</div>
)
}
diffs={reviewDiffs}
view={view}
diffStyle={input.diffStyle}
onDiffStyleChange={input.onDiffStyleChange}
onScrollRef={(el) => setTree("reviewScroll", el)}
focusedFile={tree.activeDiff}
onLineComment={(comment) => addCommentToContext({ ...comment, origin: "review" })}
onLineCommentUpdate={updateCommentInContext}
onLineCommentDelete={removeCommentFromContext}
lineCommentActions={reviewCommentActions()}
comments={comments.all()}
focusedComment={comments.focus()}
onFocusedCommentChange={comments.setFocus}
onViewFile={openReviewFile}
classes={input.classes}
/>
</Match>
</Switch>
<SessionReviewTab
title={changesTitle()}
empty={reviewEmpty(input)}
diffs={reviewDiffs}
view={view}
diffStyle={input.diffStyle}
onDiffStyleChange={input.onDiffStyleChange}
onScrollRef={(el) => setTree("reviewScroll", el)}
focusedFile={tree.activeDiff}
onLineComment={(comment) => addCommentToContext({ ...comment, origin: "review" })}
onLineCommentUpdate={updateCommentInContext}
onLineCommentDelete={removeCommentFromContext}
lineCommentActions={reviewCommentActions()}
comments={comments.all()}
focusedComment={comments.focus()}
onFocusedCommentChange={comments.setFocus}
onViewFile={openReviewFile}
classes={input.classes}
/>
</Show>
)

View File

@@ -1,10 +1,8 @@
import { sampledChecksum } from "@opencode-ai/util/encode"
import {
DEFAULT_VIRTUAL_FILE_METRICS,
type ExpansionDirections,
type DiffLineAnnotation,
type FileContents,
type FileDiffMetadata,
File as PierreFile,
type FileDiffOptions,
FileDiff,
@@ -22,7 +20,7 @@ import { ComponentProps, createEffect, createMemo, createSignal, onCleanup, onMo
import { createDefaultOptions, styleVariables } from "../pierre"
import { markCommentedDiffLines, markCommentedFileLines } from "../pierre/commented-lines"
import { fixDiffSelection, findDiffSide, type DiffSelectionSide } from "../pierre/diff-selection"
import { createFileFind, type FileFindReveal } from "../pierre/file-find"
import { createFileFind } from "../pierre/file-find"
import {
applyViewerScheme,
clearReadyWatcher,
@@ -65,21 +63,11 @@ type SharedProps<T> = {
search?: FileSearchControl
}
export type FileSearchReveal = FileFindReveal
export type FileSearchHandle = {
focus: () => void
setQuery: (value: string) => void
clear: () => void
reveal: (hit: FileSearchReveal) => boolean
expand: (hit: FileSearchReveal) => boolean
refresh: () => void
}
export type FileSearchControl = {
shortcuts?: "global" | "disabled"
showBar?: boolean
disableVirtualization?: boolean
register: (handle: FileSearchHandle | null) => void
}
@@ -121,40 +109,6 @@ const sharedKeys = [
const textKeys = ["file", ...sharedKeys] as const
const diffKeys = ["before", "after", ...sharedKeys] as const
function expansionForHit(diff: FileDiffMetadata, hit: FileSearchReveal) {
if (diff.isPartial || diff.hunks.length === 0) return
const side =
hit.side === "deletions"
? {
start: (hunk: FileDiffMetadata["hunks"][number]) => hunk.deletionStart,
count: (hunk: FileDiffMetadata["hunks"][number]) => hunk.deletionCount,
}
: {
start: (hunk: FileDiffMetadata["hunks"][number]) => hunk.additionStart,
count: (hunk: FileDiffMetadata["hunks"][number]) => hunk.additionCount,
}
for (let i = 0; i < diff.hunks.length; i++) {
const hunk = diff.hunks[i]
const start = side.start(hunk)
if (hit.line < start) {
return {
index: i,
direction: i === 0 ? "down" : "both",
} satisfies { index: number; direction: ExpansionDirections }
}
const end = start + Math.max(side.count(hunk) - 1, -1)
if (hit.line <= end) return
}
return {
index: diff.hunks.length,
direction: "up",
} satisfies { index: number; direction: ExpansionDirections }
}
// ---------------------------------------------------------------------------
// Shared viewer hook
// ---------------------------------------------------------------------------
@@ -167,7 +121,6 @@ type MouseHit = {
type ViewerConfig = {
enableLineSelection: () => boolean
search: () => FileSearchControl | undefined
selectedLines: () => SelectedLineRange | null | undefined
commentedLines: () => SelectedLineRange[]
onLineSelectionEnd: (range: SelectedLineRange | null) => void
@@ -207,7 +160,6 @@ function useFileViewer(config: ViewerConfig) {
wrapper: () => wrapper,
overlay: () => overlay,
getRoot,
shortcuts: config.search()?.shortcuts,
})
// -- selection scheduling --
@@ -407,14 +359,10 @@ function useFileViewer(config: ViewerConfig) {
type Viewer = ReturnType<typeof useFileViewer>
type ModeAdapter = Omit<
ViewerConfig,
"enableLineSelection" | "search" | "selectedLines" | "commentedLines" | "onLineSelectionEnd"
>
type ModeAdapter = Omit<ViewerConfig, "enableLineSelection" | "selectedLines" | "commentedLines" | "onLineSelectionEnd">
type ModeConfig = {
enableLineSelection: () => boolean
search: () => FileSearchControl | undefined
selectedLines: () => SelectedLineRange | null | undefined
commentedLines: () => SelectedLineRange[] | undefined
onLineSelectionEnd: (range: SelectedLineRange | null) => void
@@ -437,7 +385,6 @@ type VirtualStrategy = {
function useModeViewer(config: ModeConfig, adapter: ModeAdapter) {
return useFileViewer({
enableLineSelection: config.enableLineSelection,
search: config.search,
selectedLines: config.selectedLines,
commentedLines: () => config.commentedLines() ?? [],
onLineSelectionEnd: config.onLineSelectionEnd,
@@ -448,32 +395,13 @@ function useModeViewer(config: ModeConfig, adapter: ModeAdapter) {
function useSearchHandle(opts: {
search: () => FileSearchControl | undefined
find: ReturnType<typeof createFileFind>
expand?: (hit: FileSearchReveal) => boolean
}) {
createEffect(() => {
const search = opts.search()
if (!search) return
const handle = {
focus: () => {
opts.find.focus()
},
setQuery: (value: string) => {
opts.find.activate()
opts.find.setQuery(value, { scroll: false })
},
clear: () => {
opts.find.clear()
},
reveal: (hit: FileSearchReveal) => {
opts.find.activate()
return opts.find.reveal(hit)
},
expand: (hit: FileSearchReveal) => opts.expand?.(hit) ?? false,
refresh: () => {
opts.find.activate()
opts.find.refresh()
},
focus: () => opts.find.focus(),
} satisfies FileSearchHandle
search.register(handle)
@@ -563,6 +491,29 @@ function renderViewer<I extends RenderTarget>(opts: {
opts.onReady()
}
function preserve(viewer: Viewer) {
const root = scrollParent(viewer.wrapper)
if (!root) return () => {}
const high = viewer.container.getBoundingClientRect().height
if (!high) return () => {}
const top = viewer.wrapper.getBoundingClientRect().top - root.getBoundingClientRect().top
const prev = viewer.container.style.minHeight
viewer.container.style.minHeight = `${Math.ceil(high)}px`
let done = false
return () => {
if (done) return
done = true
viewer.container.style.minHeight = prev
const next = viewer.wrapper.getBoundingClientRect().top - root.getBoundingClientRect().top
const delta = next - top
if (delta) root.scrollTop += delta
}
}
function scrollParent(el: HTMLElement): HTMLElement | undefined {
let parent = el.parentElement
while (parent) {
@@ -606,7 +557,7 @@ function createLocalVirtualStrategy(host: () => HTMLDivElement | undefined, enab
}
}
function createSharedVirtualStrategy(host: () => HTMLDivElement | undefined, enabled: () => boolean): VirtualStrategy {
function createSharedVirtualStrategy(host: () => HTMLDivElement | undefined): VirtualStrategy {
let shared: NonNullable<ReturnType<typeof acquireVirtualizer>> | undefined
const release = () => {
@@ -616,10 +567,6 @@ function createSharedVirtualStrategy(host: () => HTMLDivElement | undefined, ena
return {
get: () => {
if (!enabled()) {
release()
return
}
if (shared) return shared.virtualizer
const container = host()
@@ -689,7 +636,6 @@ function diffSelectionSide(node: Node | null) {
function ViewerShell(props: {
mode: "text" | "diff"
viewer: ReturnType<typeof useFileViewer>
search: FileSearchControl | undefined
class: string | undefined
classList: ComponentProps<"div">["classList"] | undefined
}) {
@@ -708,7 +654,7 @@ function ViewerShell(props: {
onPointerDown={props.viewer.find.onPointerDown}
onFocus={props.viewer.find.onFocus}
>
<Show when={(props.search?.showBar ?? true) && props.viewer.find.open()}>
<Show when={props.viewer.find.open()}>
<FileSearchBar
pos={props.viewer.find.pos}
query={props.viewer.find.query}
@@ -855,7 +801,6 @@ function TextViewer<T>(props: TextFileProps<T>) {
viewer = useModeViewer(
{
enableLineSelection: () => props.enableLineSelection === true,
search: () => local.search,
selectedLines: () => local.selectedLines,
commentedLines: () => local.commentedLines,
onLineSelectionEnd: (range) => local.onLineSelectionEnd?.(range),
@@ -941,9 +886,7 @@ function TextViewer<T>(props: TextFileProps<T>) {
virtuals.cleanup()
})
return (
<ViewerShell mode="text" viewer={viewer} search={local.search} class={local.class} classList={local.classList} />
)
return <ViewerShell mode="text" viewer={viewer} class={local.class} classList={local.classList} />
}
// ---------------------------------------------------------------------------
@@ -1029,7 +972,6 @@ function DiffViewer<T>(props: DiffFileProps<T>) {
viewer = useModeViewer(
{
enableLineSelection: () => props.enableLineSelection === true,
search: () => local.search,
selectedLines: () => local.selectedLines,
commentedLines: () => local.commentedLines,
onLineSelectionEnd: (range) => local.onLineSelectionEnd?.(range),
@@ -1037,10 +979,7 @@ function DiffViewer<T>(props: DiffFileProps<T>) {
adapter,
)
const virtuals = createSharedVirtualStrategy(
() => viewer.container,
() => local.search?.disableVirtualization !== true,
)
const virtuals = createSharedVirtualStrategy(() => viewer.container)
const large = createMemo(() => {
const before = typeof local.before?.contents === "string" ? local.before.contents : ""
@@ -1074,12 +1013,13 @@ function DiffViewer<T>(props: DiffFileProps<T>) {
return { ...perf, disableLineNumbers: true }
})
const notify = () => {
const notify = (done?: VoidFunction) => {
notifyRendered({
viewer,
isReady: (root) => root.querySelector("[data-line]") != null,
settleFrames: 1,
onReady: () => {
done?.()
setSelectedLines(viewer.lastSelection)
viewer.find.refresh({ reset: true })
local.onRendered?.()
@@ -1090,20 +1030,6 @@ function DiffViewer<T>(props: DiffFileProps<T>) {
useSearchHandle({
search: () => local.search,
find: viewer.find,
expand: (hit) => {
const active = instance as
| ((FileDiff<T> | VirtualizedFileDiff<T>) & {
fileDiff?: FileDiffMetadata
})
| undefined
if (!active?.fileDiff) return false
const next = expansionForHit(active.fileDiff, hit)
if (!next) return false
active.expandHunk(next.index, next.direction)
return true
},
})
// -- render instance --
@@ -1114,6 +1040,9 @@ function DiffViewer<T>(props: DiffFileProps<T>) {
const virtualizer = virtuals.get()
const beforeContents = typeof local.before?.contents === "string" ? local.before.contents : ""
const afterContents = typeof local.after?.contents === "string" ? local.after.contents : ""
const done = preserve(viewer)
onCleanup(done)
const cacheKey = (contents: string) => {
if (!large()) return sampledChecksum(contents, contents.length)
@@ -1138,7 +1067,7 @@ function DiffViewer<T>(props: DiffFileProps<T>) {
containerWrapper: viewer.container,
})
},
onReady: notify,
onReady: () => notify(done),
})
})
@@ -1158,9 +1087,7 @@ function DiffViewer<T>(props: DiffFileProps<T>) {
dragEndSide = undefined
})
return (
<ViewerShell mode="diff" viewer={viewer} search={local.search} class={local.class} classList={local.classList} />
)
return <ViewerShell mode="diff" viewer={viewer} class={local.class} classList={local.classList} />
}
// ---------------------------------------------------------------------------

View File

@@ -1,39 +0,0 @@
import { describe, expect, test } from "bun:test"
import { buildSessionSearchHits, stepSessionSearchIndex } from "./session-review-search"
describe("session review search", () => {
test("builds hits with line, col, and side", () => {
const hits = buildSessionSearchHits({
query: "alpha",
files: [
{
file: "a.txt",
before: "alpha\nbeta alpha",
after: "ALPHA",
},
],
})
expect(hits).toEqual([
{ file: "a.txt", side: "deletions", line: 1, col: 1, len: 5 },
{ file: "a.txt", side: "deletions", line: 2, col: 6, len: 5 },
{ file: "a.txt", side: "additions", line: 1, col: 1, len: 5 },
])
})
test("uses non-overlapping matches", () => {
const hits = buildSessionSearchHits({
query: "aa",
files: [{ file: "a.txt", after: "aaaa" }],
})
expect(hits.map((hit) => hit.col)).toEqual([1, 3])
})
test("wraps next and previous navigation", () => {
expect(stepSessionSearchIndex(5, 0, -1)).toBe(4)
expect(stepSessionSearchIndex(5, 4, 1)).toBe(0)
expect(stepSessionSearchIndex(5, 2, 1)).toBe(3)
expect(stepSessionSearchIndex(0, 0, 1)).toBe(0)
})
})

View File

@@ -1,59 +0,0 @@
export type SessionSearchHit = {
file: string
side: "additions" | "deletions"
line: number
col: number
len: number
}
type SessionSearchFile = {
file: string
before?: string
after?: string
}
function hitsForSide(args: { file: string; side: SessionSearchHit["side"]; text: string; needle: string }) {
return args.text.split("\n").flatMap((line, i) => {
if (!line) return []
const hay = line.toLowerCase()
let at = hay.indexOf(args.needle)
if (at < 0) return []
const out: SessionSearchHit[] = []
while (at >= 0) {
out.push({
file: args.file,
side: args.side,
line: i + 1,
col: at + 1,
len: args.needle.length,
})
at = hay.indexOf(args.needle, at + args.needle.length)
}
return out
})
}
export function buildSessionSearchHits(args: { query: string; files: SessionSearchFile[] }) {
const value = args.query.trim().toLowerCase()
if (!value) return []
return args.files.flatMap((file) => {
const out: SessionSearchHit[] = []
if (typeof file.before === "string") {
out.push(...hitsForSide({ file: file.file, side: "deletions", text: file.before, needle: value }))
}
if (typeof file.after === "string") {
out.push(...hitsForSide({ file: file.file, side: "additions", text: file.after, needle: value }))
}
return out
})
}
export function stepSessionSearchIndex(total: number, current: number, dir: 1 | -1) {
if (total <= 0) return 0
if (current < 0 || current >= total) return dir > 0 ? 0 : total - 1
return (current + dir + total) % total
}

View File

@@ -9,9 +9,6 @@ import { IconButton } from "./icon-button"
import { StickyAccordionHeader } from "./sticky-accordion-header"
import { Tooltip } from "./tooltip"
import { ScrollView } from "./scroll-view"
import { FileSearchBar } from "./file-search"
import type { FileSearchHandle } from "./file"
import { buildSessionSearchHits, stepSessionSearchIndex, type SessionSearchHit } from "./session-review-search"
import { useFileComponent } from "../context/file"
import { useI18n } from "../context/i18n"
import { getDirectory, getFilename } from "@opencode-ai/util/path"
@@ -63,6 +60,8 @@ export type SessionReviewCommentActions = {
export type SessionReviewFocus = { file: string; id: string }
type ReviewDiff = FileDiff & { preloaded?: PreloadMultiFileDiffResult<any> }
export interface SessionReviewProps {
title?: JSX.Element
empty?: JSX.Element
@@ -86,7 +85,7 @@ export interface SessionReviewProps {
classList?: Record<string, boolean | undefined>
classes?: { root?: string; header?: string; container?: string }
actions?: JSX.Element
diffs: (FileDiff & { preloaded?: PreloadMultiFileDiffResult<any> })[]
diffs: ReviewDiff[]
onViewFile?: (file: string) => void
readFile?: (path: string) => Promise<FileContent | undefined>
}
@@ -135,15 +134,10 @@ type SessionReviewSelection = {
export const SessionReview = (props: SessionReviewProps) => {
let scroll: HTMLDivElement | undefined
let searchInput: HTMLInputElement | undefined
let focusToken = 0
let revealToken = 0
let highlightedFile: string | undefined
const i18n = useI18n()
const fileComponent = useFileComponent()
const anchors = new Map<string, HTMLElement>()
const searchHandles = new Map<string, FileSearchHandle>()
const readyFiles = new Set<string>()
const [store, setStore] = createStore<{ open: string[]; force: Record<string, boolean> }>({
open: [],
force: {},
@@ -152,18 +146,12 @@ export const SessionReview = (props: SessionReviewProps) => {
const [selection, setSelection] = createSignal<SessionReviewSelection | null>(null)
const [commenting, setCommenting] = createSignal<SessionReviewSelection | null>(null)
const [opened, setOpened] = createSignal<SessionReviewFocus | null>(null)
const [searchOpen, setSearchOpen] = createSignal(false)
const [searchQuery, setSearchQuery] = createSignal("")
const [searchActive, setSearchActive] = createSignal(0)
const [searchPos, setSearchPos] = createSignal({ top: 8, right: 8 })
const open = () => props.open ?? store.open
const files = createMemo(() => props.diffs.map((d) => d.file))
const diffs = createMemo(() => new Map(props.diffs.map((d) => [d.file, d] as const)))
const files = createMemo(() => props.diffs.map((diff) => diff.file))
const diffs = createMemo(() => new Map(props.diffs.map((diff) => [diff.file, diff] as const)))
const diffStyle = () => props.diffStyle ?? (props.split ? "split" : "unified")
const hasDiffs = () => files().length > 0
const searchValue = createMemo(() => searchQuery().trim())
const searchExpanded = createMemo(() => searchValue().length > 0)
const handleChange = (open: string[]) => {
props.onOpenChange?.(open)
@@ -176,266 +164,8 @@ export const SessionReview = (props: SessionReviewProps) => {
handleChange(next)
}
const clearViewerSearch = () => {
for (const handle of searchHandles.values()) handle.clear()
highlightedFile = undefined
}
const openFileLabel = () => i18n.t("ui.sessionReview.openFile")
const selectionLabel = (range: SelectedLineRange) => {
const start = Math.min(range.start, range.end)
const end = Math.max(range.start, range.end)
if (start === end) return i18n.t("ui.sessionReview.selection.line", { line: start })
return i18n.t("ui.sessionReview.selection.lines", { start, end })
}
const focusSearch = () => {
if (!hasDiffs()) return
setSearchOpen(true)
requestAnimationFrame(() => {
searchInput?.focus()
searchInput?.select()
})
}
const closeSearch = () => {
revealToken++
setSearchOpen(false)
setSearchQuery("")
setSearchActive(0)
clearViewerSearch()
}
const positionSearchBar = () => {
if (typeof window === "undefined") return
if (!scroll) return
const rect = scroll.getBoundingClientRect()
const title = parseFloat(getComputedStyle(scroll).getPropertyValue("--session-title-height"))
const header = Number.isNaN(title) ? 0 : title
setSearchPos({
top: Math.round(rect.top) + header - 4,
right: Math.round(window.innerWidth - rect.right) + 8,
})
}
const searchHits = createMemo(() =>
buildSessionSearchHits({
query: searchQuery(),
files: props.diffs.flatMap((diff) => {
if (mediaKindFromPath(diff.file)) return []
return [
{
file: diff.file,
before: typeof diff.before === "string" ? diff.before : undefined,
after: typeof diff.after === "string" ? diff.after : undefined,
},
]
}),
}),
)
const waitForViewer = (file: string, token: number) =>
new Promise<FileSearchHandle | undefined>((resolve) => {
let attempt = 0
const tick = () => {
if (token !== revealToken) {
resolve(undefined)
return
}
const handle = searchHandles.get(file)
if (handle && readyFiles.has(file)) {
resolve(handle)
return
}
if (attempt >= 180) {
resolve(undefined)
return
}
attempt++
requestAnimationFrame(tick)
}
tick()
})
const waitForFrames = (count: number, token: number) =>
new Promise<boolean>((resolve) => {
const tick = (left: number) => {
if (token !== revealToken) {
resolve(false)
return
}
if (left <= 0) {
resolve(true)
return
}
requestAnimationFrame(() => tick(left - 1))
}
tick(count)
})
const revealSearchHit = async (token: number, hit: SessionSearchHit, query: string) => {
const diff = diffs().get(hit.file)
if (!diff) return
if (!open().includes(hit.file)) {
handleChange([...open(), hit.file])
}
if (!mediaKindFromPath(hit.file) && diff.additions + diff.deletions > MAX_DIFF_CHANGED_LINES) {
setStore("force", hit.file, true)
}
const handle = await waitForViewer(hit.file, token)
if (!handle || token !== revealToken) return
if (searchValue() !== query) return
if (!(await waitForFrames(2, token))) return
if (highlightedFile && highlightedFile !== hit.file) {
searchHandles.get(highlightedFile)?.clear()
highlightedFile = undefined
}
anchors.get(hit.file)?.scrollIntoView({ block: "nearest" })
let done = false
for (let i = 0; i < 4; i++) {
if (token !== revealToken) return
if (searchValue() !== query) return
handle.setQuery(query)
if (handle.reveal(hit)) {
done = true
break
}
const expanded = handle.expand(hit)
handle.refresh()
if (!(await waitForFrames(expanded ? 2 : 1, token))) return
}
if (!done) return
if (!(await waitForFrames(1, token))) return
handle.reveal(hit)
highlightedFile = hit.file
}
const navigateSearch = (dir: 1 | -1) => {
const total = searchHits().length
if (total <= 0) return
setSearchActive((value) => stepSessionSearchIndex(total, value, dir))
}
const inReview = (node: unknown, path?: unknown[]) => {
if (node === searchInput) return true
if (path?.some((item) => item === scroll || item === searchInput)) return true
if (path?.some((item) => item instanceof HTMLElement && item.dataset.component === "session-review")) {
return true
}
if (!(node instanceof Node)) return false
if (searchInput?.contains(node)) return true
if (node instanceof HTMLElement && node.closest("[data-component='session-review']")) return true
if (!scroll) return false
return scroll.contains(node)
}
createEffect(() => {
if (typeof window === "undefined") return
const onKeyDown = (event: KeyboardEvent) => {
const mod = event.metaKey || event.ctrlKey
if (!mod) return
const key = event.key.toLowerCase()
if (key !== "f" && key !== "g") return
if (key === "f") {
if (!hasDiffs()) return
event.preventDefault()
event.stopPropagation()
focusSearch()
return
}
const path = typeof event.composedPath === "function" ? event.composedPath() : undefined
if (!inReview(event.target, path) && !inReview(document.activeElement, path)) return
if (!searchOpen()) return
event.preventDefault()
event.stopPropagation()
navigateSearch(event.shiftKey ? -1 : 1)
}
window.addEventListener("keydown", onKeyDown, { capture: true })
onCleanup(() => window.removeEventListener("keydown", onKeyDown, { capture: true }))
})
createEffect(() => {
diffStyle()
searchExpanded()
readyFiles.clear()
})
createEffect(() => {
if (!searchOpen()) return
if (!scroll) return
const root = scroll
requestAnimationFrame(positionSearchBar)
window.addEventListener("resize", positionSearchBar, { passive: true })
const observer = typeof ResizeObserver === "undefined" ? undefined : new ResizeObserver(positionSearchBar)
observer?.observe(root)
onCleanup(() => {
window.removeEventListener("resize", positionSearchBar)
observer?.disconnect()
})
})
createEffect(() => {
const total = searchHits().length
if (total === 0) {
if (searchActive() !== 0) setSearchActive(0)
return
}
if (searchActive() >= total) setSearchActive(total - 1)
})
createEffect(() => {
diffStyle()
const query = searchValue()
const hits = searchHits()
const token = ++revealToken
if (!query || hits.length === 0) {
clearViewerSearch()
return
}
const hit = hits[Math.min(searchActive(), hits.length - 1)]
if (!hit) return
void revealSearchHit(token, hit, query)
})
onCleanup(() => {
revealToken++
clearViewerSearch()
readyFiles.clear()
searchHandles.clear()
})
const selectionSide = (range: SelectedLineRange) => range.endSide ?? range.side ?? "additions"
const selectionPreview = (diff: FileDiff, range: SelectedLineRange) => {
@@ -499,58 +229,6 @@ export const SessionReview = (props: SessionReviewProps) => {
})
})
const handleReviewKeyDown = (event: KeyboardEvent) => {
if (event.defaultPrevented) return
const mod = event.metaKey || event.ctrlKey
const key = event.key.toLowerCase()
const target = event.target
if (mod && key === "f") {
event.preventDefault()
event.stopPropagation()
focusSearch()
return
}
if (mod && key === "g") {
if (!searchOpen()) return
event.preventDefault()
event.stopPropagation()
navigateSearch(event.shiftKey ? -1 : 1)
}
}
const handleSearchInputKeyDown = (event: KeyboardEvent) => {
const mod = event.metaKey || event.ctrlKey
const key = event.key.toLowerCase()
if (mod && key === "g") {
event.preventDefault()
event.stopPropagation()
navigateSearch(event.shiftKey ? -1 : 1)
return
}
if (mod && key === "f") {
event.preventDefault()
event.stopPropagation()
focusSearch()
return
}
if (event.key === "Escape") {
event.preventDefault()
event.stopPropagation()
closeSearch()
return
}
if (event.key !== "Enter") return
event.preventDefault()
event.stopPropagation()
navigateSearch(event.shiftKey ? -1 : 1)
}
return (
<div data-component="session-review" class={props.class} classList={props.classList}>
<div data-slot="session-review-header" class={props.classes?.header}>
@@ -594,31 +272,10 @@ export const SessionReview = (props: SessionReviewProps) => {
props.scrollRef?.(el)
}}
onScroll={props.onScroll as any}
onKeyDown={handleReviewKeyDown}
classList={{
[props.classes?.root ?? ""]: !!props.classes?.root,
}}
>
<Show when={searchOpen()}>
<FileSearchBar
pos={searchPos}
query={searchQuery}
index={() => (searchHits().length ? Math.min(searchActive(), searchHits().length - 1) : 0)}
count={() => searchHits().length}
setInput={(el) => {
searchInput = el
}}
onInput={(value) => {
setSearchQuery(value)
setSearchActive(0)
}}
onKeyDown={(event) => handleSearchInputKeyDown(event)}
onClose={closeSearch}
onPrev={() => navigateSearch(-1)}
onNext={() => navigateSearch(1)}
/>
</Show>
<div data-slot="session-review-container" class={props.classes?.container}>
<Show when={hasDiffs()} fallback={props.empty}>
<div class="pb-6">
@@ -627,8 +284,7 @@ export const SessionReview = (props: SessionReviewProps) => {
{(file) => {
let wrapper: HTMLDivElement | undefined
const diff = createMemo(() => diffs().get(file))
const item = () => diff()!
const item = createMemo(() => diffs().get(file)!)
const expanded = createMemo(() => open().includes(file))
const force = () => !!store.force[file]
@@ -720,9 +376,6 @@ export const SessionReview = (props: SessionReviewProps) => {
onCleanup(() => {
anchors.delete(file)
readyFiles.delete(file)
searchHandles.delete(file)
if (highlightedFile === file) highlightedFile = undefined
})
const handleLineSelected = (range: SelectedLineRange | null) => {
@@ -839,9 +492,7 @@ export const SessionReview = (props: SessionReviewProps) => {
mode="diff"
preloadedDiff={item().preloaded}
diffStyle={diffStyle()}
expansionLineCount={searchExpanded() ? Number.MAX_SAFE_INTEGER : 20}
onRendered={() => {
readyFiles.add(file)
props.onDiffRendered?.()
}}
enableLineSelection={props.onLineComment != null}
@@ -854,21 +505,6 @@ export const SessionReview = (props: SessionReviewProps) => {
renderHoverUtility={props.onLineComment ? commentsUi.renderHoverUtility : undefined}
selectedLines={selectedLines()}
commentedLines={commentedLines()}
search={{
shortcuts: "disabled",
showBar: false,
disableVirtualization: searchExpanded(),
register: (handle: FileSearchHandle | null) => {
if (!handle) {
searchHandles.delete(file)
readyFiles.delete(file)
if (highlightedFile === file) highlightedFile = undefined
return
}
searchHandles.set(file, handle)
},
}}
before={{
name: file,
contents: typeof item().before === "string" ? item().before : "",

View File

@@ -8,20 +8,6 @@ export type FindHost = {
isOpen: () => boolean
}
type FileFindSide = "additions" | "deletions"
export type FileFindReveal = {
side: FileFindSide
line: number
col: number
len: number
}
type FileFindHit = FileFindReveal & {
range: Range
alt?: number
}
const hosts = new Set<FindHost>()
let target: FindHost | undefined
let current: FindHost | undefined
@@ -112,7 +98,6 @@ type CreateFileFindOptions = {
wrapper: () => HTMLElement | undefined
overlay: () => HTMLDivElement | undefined
getRoot: () => ShadowRoot | undefined
shortcuts?: "global" | "disabled"
}
export function createFileFind(opts: CreateFileFindOptions) {
@@ -120,7 +105,7 @@ export function createFileFind(opts: CreateFileFindOptions) {
let overlayFrame: number | undefined
let overlayScroll: HTMLElement[] = []
let mode: "highlights" | "overlay" = "overlay"
let hits: FileFindHit[] = []
let hits: Range[] = []
const [open, setOpen] = createSignal(false)
const [query, setQuery] = createSignal("")
@@ -161,7 +146,7 @@ export function createFileFind(opts: CreateFileFindOptions) {
const frag = document.createDocumentFragment()
for (let i = 0; i < hits.length; i++) {
const range = hits[i].range
const range = hits[i]
const active = i === currentIndex
for (const rect of Array.from(range.getClientRects())) {
if (!rect.width || !rect.height) continue
@@ -237,7 +222,7 @@ export function createFileFind(opts: CreateFileFindOptions) {
const scan = (root: ShadowRoot, value: string) => {
const needle = value.toLowerCase()
const ranges: FileFindHit[] = []
const ranges: Range[] = []
const cols = Array.from(root.querySelectorAll("[data-content] [data-line], [data-column-content]")).filter(
(node): node is HTMLElement => node instanceof HTMLElement,
)
@@ -250,28 +235,6 @@ export function createFileFind(opts: CreateFileFindOptions) {
let at = hay.indexOf(needle)
if (at === -1) continue
const row = col.closest("[data-line], [data-alt-line]")
if (!(row instanceof HTMLElement)) continue
const primary = parseInt(row.dataset.line ?? "", 10)
const alt = parseInt(row.dataset.altLine ?? "", 10)
const line = (() => {
if (!Number.isNaN(primary)) return primary
if (!Number.isNaN(alt)) return alt
})()
if (line === undefined) continue
const side = (() => {
const code = col.closest("[data-code]")
if (code instanceof HTMLElement) return code.hasAttribute("data-deletions") ? "deletions" : "additions"
const row = col.closest("[data-line-type]")
if (!(row instanceof HTMLElement)) return "additions"
const type = row.dataset.lineType
if (type === "change-deletion") return "deletions"
return "additions"
})() as FileFindSide
const nodes: Text[] = []
const ends: number[] = []
const walker = document.createTreeWalker(col, NodeFilter.SHOW_TEXT)
@@ -305,14 +268,7 @@ export function createFileFind(opts: CreateFileFindOptions) {
const range = document.createRange()
range.setStart(start.node, start.offset)
range.setEnd(end.node, end.offset)
ranges.push({
range,
side,
line,
alt: Number.isNaN(alt) ? undefined : alt,
col: at + 1,
len: value.length,
})
ranges.push(range)
at = hay.indexOf(needle, at + value.length)
}
}
@@ -321,17 +277,12 @@ export function createFileFind(opts: CreateFileFindOptions) {
}
const scrollToRange = (range: Range) => {
const scroll = () => {
const start = range.startContainer
const el = start instanceof Element ? start : start.parentElement
el?.scrollIntoView({ block: "center", inline: "center" })
}
scroll()
requestAnimationFrame(scroll)
const start = range.startContainer
const el = start instanceof Element ? start : start.parentElement
el?.scrollIntoView({ block: "center", inline: "center" })
}
const setHighlights = (ranges: FileFindHit[], currentIndex: number) => {
const setHighlights = (ranges: Range[], currentIndex: number) => {
const api = (globalThis as unknown as { CSS?: { highlights?: any }; Highlight?: any }).CSS?.highlights
const Highlight = (globalThis as unknown as { Highlight?: any }).Highlight
if (!api || typeof Highlight !== "function") return false
@@ -339,37 +290,14 @@ export function createFileFind(opts: CreateFileFindOptions) {
api.delete("opencode-find")
api.delete("opencode-find-current")
const active = ranges[currentIndex]?.range
const active = ranges[currentIndex]
if (active) api.set("opencode-find-current", new Highlight(active))
const rest = ranges.flatMap((hit, i) => (i === currentIndex ? [] : [hit.range]))
const rest = ranges.filter((_, i) => i !== currentIndex)
if (rest.length > 0) api.set("opencode-find", new Highlight(...rest))
return true
}
const select = (currentIndex: number, scroll: boolean) => {
const active = hits[currentIndex]?.range
if (!active) return false
setIndex(currentIndex)
if (mode === "highlights") {
if (!setHighlights(hits, currentIndex)) {
mode = "overlay"
apply({ reset: true, scroll })
return false
}
if (scroll) scrollToRange(active)
return true
}
clearHighlightFind()
syncOverlayScroll()
if (scroll) scrollToRange(active)
scheduleOverlay()
return true
}
const apply = (args?: { reset?: boolean; scroll?: boolean }) => {
if (!open()) return
@@ -393,7 +321,7 @@ export function createFileFind(opts: CreateFileFindOptions) {
setCount(total)
setIndex(currentIndex)
const active = ranges[currentIndex]?.range
const active = ranges[currentIndex]
if (mode === "highlights") {
clearOverlay()
clearOverlayScroll()
@@ -420,23 +348,11 @@ export function createFileFind(opts: CreateFileFindOptions) {
if (current === host) current = undefined
}
const clear = () => {
setQuery("")
clearFind()
}
const activate = () => {
if (opts.shortcuts !== "disabled") {
if (current && current !== host) current.close()
current = host
target = host
}
if (!open()) setOpen(true)
}
const focus = () => {
activate()
if (current && current !== host) current.close()
current = host
target = host
if (!open()) setOpen(true)
requestAnimationFrame(() => {
apply({ scroll: true })
input?.focus()
@@ -450,30 +366,25 @@ export function createFileFind(opts: CreateFileFindOptions) {
if (total <= 0) return
const currentIndex = (index() + dir + total) % total
select(currentIndex, true)
}
setIndex(currentIndex)
const reveal = (targetHit: FileFindReveal) => {
if (!open()) return false
if (hits.length === 0) return false
const active = hits[currentIndex]
if (!active) return
const exact = hits.findIndex(
(hit) =>
hit.side === targetHit.side &&
hit.line === targetHit.line &&
hit.col === targetHit.col &&
hit.len === targetHit.len,
)
const fallback = hits.findIndex(
(hit) =>
(hit.line === targetHit.line || hit.alt === targetHit.line) &&
hit.col === targetHit.col &&
hit.len === targetHit.len,
)
if (mode === "highlights") {
if (!setHighlights(hits, currentIndex)) {
mode = "overlay"
apply({ reset: true, scroll: true })
return
}
scrollToRange(active)
return
}
const nextIndex = exact >= 0 ? exact : fallback
if (nextIndex < 0) return false
return select(nextIndex, true)
clearHighlightFind()
syncOverlayScroll()
scrollToRange(active)
scheduleOverlay()
}
const host: FindHost = {
@@ -486,21 +397,17 @@ export function createFileFind(opts: CreateFileFindOptions) {
onMount(() => {
mode = supportsHighlights() ? "highlights" : "overlay"
if (opts.shortcuts !== "disabled") {
installShortcuts()
hosts.add(host)
if (!target) target = host
}
installShortcuts()
hosts.add(host)
if (!target) target = host
onCleanup(() => {
if (opts.shortcuts !== "disabled") {
hosts.delete(host)
if (current === host) {
current = undefined
clearHighlightFind()
}
if (target === host) target = undefined
hosts.delete(host)
if (current === host) {
current = undefined
clearHighlightFind()
}
if (target === host) target = undefined
})
})
@@ -541,25 +448,20 @@ export function createFileFind(opts: CreateFileFindOptions) {
setInput: (el: HTMLInputElement) => {
input = el
},
setQuery: (value: string, args?: { scroll?: boolean }) => {
setQuery: (value: string) => {
setQuery(value)
setIndex(0)
apply({ reset: true, scroll: args?.scroll ?? true })
apply({ reset: true, scroll: true })
},
clear,
activate,
focus,
close,
next,
reveal,
refresh: (args?: { reset?: boolean; scroll?: boolean }) => apply(args),
onPointerDown: () => {
if (opts.shortcuts === "disabled") return
target = host
opts.wrapper()?.focus({ preventScroll: true })
},
onFocus: () => {
if (opts.shortcuts === "disabled") return
target = host
},
onInputKeyDown: (event: KeyboardEvent) => {