tweak(ui): combine diffs in review into one group

This commit is contained in:
David Hill
2026-02-18 23:36:02 +00:00
parent 14c0989411
commit ba53c56a21
2 changed files with 390 additions and 341 deletions

View File

@@ -65,14 +65,61 @@
top: -40px; top: -40px;
} }
[data-slot="accordion-trigger"] { [data-slot="session-review-diffs-group"] {
background-color: var(--background-stronger) !important; background-color: var(--background-stronger);
} border-radius: var(--radius-lg);
box-shadow: var(--shadow-xs-border-base);
overflow: clip;
[data-slot="session-review-accordion-item"][data-selected] { [data-component="accordion"] {
[data-slot="session-review-accordion-content"] { gap: 0;
box-shadow: var(--shadow-xs-border-select); }
border-radius: var(--radius-lg);
[data-component="accordion"] [data-slot="accordion-item"] {
overflow: visible;
}
[data-component="accordion"]
[data-slot="accordion-item"]
[data-slot="accordion-header"]
[data-slot="accordion-trigger"] {
border: 0;
border-radius: 0;
box-shadow: none;
background-color: transparent;
&:hover {
background-color: var(--surface-base-hover);
}
&:active {
background-color: var(--surface-base-active);
}
}
[data-component="accordion"]
[data-slot="accordion-item"]
+ [data-slot="accordion-item"]
[data-slot="accordion-header"]
[data-slot="accordion-trigger"] {
border-top: 1px solid var(--border-weak-base);
}
[data-component="accordion"] [data-slot="accordion-item"][data-expanded] [data-slot="accordion-content"] {
border: 0;
border-top: 1px solid var(--border-weak-base);
border-radius: 0;
}
[data-component="sticky-accordion-header"][data-expanded]::before,
[data-slot="accordion-item"][data-expanded] [data-component="sticky-accordion-header"]::before {
top: 0;
}
[data-slot="session-review-accordion-item"][data-selected]
[data-slot="accordion-header"]
[data-slot="accordion-trigger"] {
background-color: var(--surface-base-active);
} }
} }

View File

@@ -319,385 +319,387 @@ export const SessionReview = (props: SessionReviewProps) => {
</div> </div>
<div data-slot="session-review-container" class={props.classes?.container}> <div data-slot="session-review-container" class={props.classes?.container}>
<Show when={hasDiffs()} fallback={props.empty}> <Show when={hasDiffs()} fallback={props.empty}>
<Accordion multiple value={open()} onChange={handleChange}> <div data-slot="session-review-diffs-group">
<For each={props.diffs}> <Accordion multiple value={open()} onChange={handleChange}>
{(diff) => { <For each={props.diffs}>
let wrapper: HTMLDivElement | undefined {(diff) => {
let wrapper: HTMLDivElement | undefined
const expanded = createMemo(() => open().includes(diff.file)) const expanded = createMemo(() => open().includes(diff.file))
const [force, setForce] = createSignal(false) const [force, setForce] = createSignal(false)
const comments = createMemo(() => (props.comments ?? []).filter((c) => c.file === diff.file)) const comments = createMemo(() => (props.comments ?? []).filter((c) => c.file === diff.file))
const commentedLines = createMemo(() => comments().map((c) => c.selection)) const commentedLines = createMemo(() => comments().map((c) => c.selection))
const beforeText = () => (typeof diff.before === "string" ? diff.before : "") const beforeText = () => (typeof diff.before === "string" ? diff.before : "")
const afterText = () => (typeof diff.after === "string" ? diff.after : "") const afterText = () => (typeof diff.after === "string" ? diff.after : "")
const changedLines = () => diff.additions + diff.deletions const changedLines = () => diff.additions + diff.deletions
const tooLarge = createMemo(() => { const tooLarge = createMemo(() => {
if (!expanded()) return false if (!expanded()) return false
if (force()) return false if (force()) return false
if (isImageFile(diff.file)) return false if (isImageFile(diff.file)) return false
return changedLines() > MAX_DIFF_CHANGED_LINES return changedLines() > MAX_DIFF_CHANGED_LINES
}) })
const isAdded = () => diff.status === "added" || (beforeText().length === 0 && afterText().length > 0) const isAdded = () => diff.status === "added" || (beforeText().length === 0 && afterText().length > 0)
const isDeleted = () => const isDeleted = () =>
diff.status === "deleted" || (afterText().length === 0 && beforeText().length > 0) diff.status === "deleted" || (afterText().length === 0 && beforeText().length > 0)
const isImage = () => isImageFile(diff.file) const isImage = () => isImageFile(diff.file)
const isAudio = () => isAudioFile(diff.file) const isAudio = () => isAudioFile(diff.file)
const diffImageSrc = dataUrlFromValue(diff.after) ?? dataUrlFromValue(diff.before) const diffImageSrc = dataUrlFromValue(diff.after) ?? dataUrlFromValue(diff.before)
const [imageSrc, setImageSrc] = createSignal<string | undefined>(diffImageSrc) const [imageSrc, setImageSrc] = createSignal<string | undefined>(diffImageSrc)
const [imageStatus, setImageStatus] = createSignal<"idle" | "loading" | "error">("idle") const [imageStatus, setImageStatus] = createSignal<"idle" | "loading" | "error">("idle")
const diffAudioSrc = dataUrlFromValue(diff.after) ?? dataUrlFromValue(diff.before) const diffAudioSrc = dataUrlFromValue(diff.after) ?? dataUrlFromValue(diff.before)
const [audioSrc, setAudioSrc] = createSignal<string | undefined>(diffAudioSrc) const [audioSrc, setAudioSrc] = createSignal<string | undefined>(diffAudioSrc)
const [audioStatus, setAudioStatus] = createSignal<"idle" | "loading" | "error">("idle") const [audioStatus, setAudioStatus] = createSignal<"idle" | "loading" | "error">("idle")
const [audioMime, setAudioMime] = createSignal<string | undefined>(undefined) const [audioMime, setAudioMime] = createSignal<string | undefined>(undefined)
const selectedLines = createMemo(() => { const selectedLines = createMemo(() => {
const current = selection() const current = selection()
if (!current || current.file !== diff.file) return null if (!current || current.file !== diff.file) return null
return current.range return current.range
}) })
const draftRange = createMemo(() => { const draftRange = createMemo(() => {
const current = commenting() const current = commenting()
if (!current || current.file !== diff.file) return null if (!current || current.file !== diff.file) return null
return current.range return current.range
}) })
const [draft, setDraft] = createSignal("") const [draft, setDraft] = createSignal("")
const [positions, setPositions] = createSignal<Record<string, number>>({}) const [positions, setPositions] = createSignal<Record<string, number>>({})
const [draftTop, setDraftTop] = createSignal<number | undefined>(undefined) const [draftTop, setDraftTop] = createSignal<number | undefined>(undefined)
const getRoot = () => { const getRoot = () => {
const el = wrapper const el = wrapper
if (!el) return if (!el) return
const host = el.querySelector("diffs-container") const host = el.querySelector("diffs-container")
if (!(host instanceof HTMLElement)) return if (!(host instanceof HTMLElement)) return
return host.shadowRoot ?? undefined return host.shadowRoot ?? undefined
}
const updateAnchors = () => {
const el = wrapper
if (!el) return
const root = getRoot()
if (!root) return
const next: Record<string, number> = {}
for (const item of comments()) {
const marker = findMarker(root, item.selection)
if (!marker) continue
next[item.id] = markerTop(el, marker)
}
setPositions(next)
const range = draftRange()
if (!range) {
setDraftTop(undefined)
return
} }
const marker = findMarker(root, range) const updateAnchors = () => {
if (!marker) { const el = wrapper
setDraftTop(undefined) if (!el) return
return
const root = getRoot()
if (!root) return
const next: Record<string, number> = {}
for (const item of comments()) {
const marker = findMarker(root, item.selection)
if (!marker) continue
next[item.id] = markerTop(el, marker)
}
setPositions(next)
const range = draftRange()
if (!range) {
setDraftTop(undefined)
return
}
const marker = findMarker(root, range)
if (!marker) {
setDraftTop(undefined)
return
}
setDraftTop(markerTop(el, marker))
} }
setDraftTop(markerTop(el, marker)) const scheduleAnchors = () => {
} requestAnimationFrame(updateAnchors)
}
const scheduleAnchors = () => { createEffect(() => {
requestAnimationFrame(updateAnchors) comments()
} scheduleAnchors()
})
createEffect(() => { createEffect(() => {
comments() const range = draftRange()
scheduleAnchors() if (!range) return
}) setDraft("")
scheduleAnchors()
})
createEffect(() => { createEffect(() => {
const range = draftRange() if (!open().includes(diff.file)) return
if (!range) return if (!isImage()) return
setDraft("") if (imageSrc()) return
scheduleAnchors() if (imageStatus() !== "idle") return
}) if (isDeleted()) return
createEffect(() => { const reader = props.readFile
if (!open().includes(diff.file)) return if (!reader) return
if (!isImage()) return
if (imageSrc()) return
if (imageStatus() !== "idle") return
if (isDeleted()) return
const reader = props.readFile setImageStatus("loading")
if (!reader) return reader(diff.file)
.then((result) => {
setImageStatus("loading") const src = dataUrl(result)
reader(diff.file) if (!src) {
.then((result) => { setImageStatus("error")
const src = dataUrl(result) return
if (!src) { }
setImageSrc(src)
setImageStatus("idle")
})
.catch(() => {
setImageStatus("error") setImageStatus("error")
return })
} })
setImageSrc(src)
setImageStatus("idle")
})
.catch(() => {
setImageStatus("error")
})
})
createEffect(() => { createEffect(() => {
if (!open().includes(diff.file)) return if (!open().includes(diff.file)) return
if (!isAudio()) return if (!isAudio()) return
if (audioSrc()) return if (audioSrc()) return
if (audioStatus() !== "idle") return if (audioStatus() !== "idle") return
const reader = props.readFile const reader = props.readFile
if (!reader) return if (!reader) return
setAudioStatus("loading") setAudioStatus("loading")
reader(diff.file) reader(diff.file)
.then((result) => { .then((result) => {
const src = dataUrl(result) const src = dataUrl(result)
if (!src) { if (!src) {
setAudioStatus("error")
return
}
setAudioMime(normalizeMimeType(result?.mimeType))
setAudioSrc(src)
setAudioStatus("idle")
})
.catch(() => {
setAudioStatus("error") setAudioStatus("error")
return })
} })
setAudioMime(normalizeMimeType(result?.mimeType))
setAudioSrc(src)
setAudioStatus("idle")
})
.catch(() => {
setAudioStatus("error")
})
})
const handleLineSelected = (range: SelectedLineRange | null) => { const handleLineSelected = (range: SelectedLineRange | null) => {
if (!props.onLineComment) return if (!props.onLineComment) return
if (!range) { if (!range) {
setSelection(null) setSelection(null)
return return
}
setSelection({ file: diff.file, range })
} }
setSelection({ file: diff.file, range }) const handleLineSelectionEnd = (range: SelectedLineRange | null) => {
} if (!props.onLineComment) return
const handleLineSelectionEnd = (range: SelectedLineRange | null) => { if (!range) {
if (!props.onLineComment) return setCommenting(null)
return
}
if (!range) { setSelection({ file: diff.file, range })
setCommenting(null) setCommenting({ file: diff.file, range })
return
} }
setSelection({ file: diff.file, range }) const openComment = (comment: SessionReviewComment) => {
setCommenting({ file: diff.file, range }) setOpened({ file: comment.file, id: comment.id })
} setSelection({ file: comment.file, range: comment.selection })
}
const openComment = (comment: SessionReviewComment) => { const isCommentOpen = (comment: SessionReviewComment) => {
setOpened({ file: comment.file, id: comment.id }) const current = opened()
setSelection({ file: comment.file, range: comment.selection }) if (!current) return false
} return current.file === comment.file && current.id === comment.id
}
const isCommentOpen = (comment: SessionReviewComment) => { return (
const current = opened() <Accordion.Item
if (!current) return false value={diff.file}
return current.file === comment.file && current.id === comment.id id={diffId(diff.file)}
} data-file={diff.file}
data-slot="session-review-accordion-item"
return ( data-selected={props.focusedFile === diff.file ? "" : undefined}
<Accordion.Item >
value={diff.file} <StickyAccordionHeader>
id={diffId(diff.file)} <Accordion.Trigger>
data-file={diff.file} <div data-slot="session-review-trigger-content">
data-slot="session-review-accordion-item" <div data-slot="session-review-file-info">
data-selected={props.focusedFile === diff.file ? "" : undefined} <FileIcon node={{ path: diff.file, type: "file" }} />
> <div data-slot="session-review-file-name-container">
<StickyAccordionHeader> <Show when={diff.file.includes("/")}>
<Accordion.Trigger> <span data-slot="session-review-directory">{`\u202A${getDirectory(diff.file)}\u202C`}</span>
<div data-slot="session-review-trigger-content"> </Show>
<div data-slot="session-review-file-info"> <span data-slot="session-review-filename">{getFilename(diff.file)}</span>
<FileIcon node={{ path: diff.file, type: "file" }} /> <Show when={props.onViewFile}>
<div data-slot="session-review-file-name-container"> <button
<Show when={diff.file.includes("/")}> data-slot="session-review-view-button"
<span data-slot="session-review-directory">{`\u202A${getDirectory(diff.file)}\u202C`}</span> type="button"
</Show> onClick={(e) => {
<span data-slot="session-review-filename">{getFilename(diff.file)}</span> e.stopPropagation()
<Show when={props.onViewFile}> props.onViewFile?.(diff.file)
<button }}
data-slot="session-review-view-button" >
type="button" <Icon name="eye" size="small" />
onClick={(e) => { </button>
e.stopPropagation() </Show>
props.onViewFile?.(diff.file) </div>
}} </div>
> <div data-slot="session-review-trigger-actions">
<Icon name="eye" size="small" /> <Switch>
</button> <Match when={isAdded()}>
</Show> <span data-slot="session-review-change" data-type="added">
{i18n.t("ui.sessionReview.change.added")}
</span>
</Match>
<Match when={isDeleted()}>
<span data-slot="session-review-change" data-type="removed">
{i18n.t("ui.sessionReview.change.removed")}
</span>
</Match>
<Match when={isImage()}>
<span data-slot="session-review-change" data-type="modified">
{i18n.t("ui.sessionReview.change.modified")}
</span>
</Match>
<Match when={true}>
<DiffChanges changes={diff} />
</Match>
</Switch>
<Icon name="chevron-grabber-vertical" size="small" />
</div> </div>
</div> </div>
<div data-slot="session-review-trigger-actions"> </Accordion.Trigger>
</StickyAccordionHeader>
<Accordion.Content data-slot="session-review-accordion-content">
<div
data-slot="session-review-diff-wrapper"
ref={(el) => {
wrapper = el
anchors.set(diff.file, el)
scheduleAnchors()
}}
>
<Show when={expanded()}>
<Switch> <Switch>
<Match when={isAdded()}> <Match when={isImage() && imageSrc()}>
<span data-slot="session-review-change" data-type="added"> <div data-slot="session-review-image-container">
{i18n.t("ui.sessionReview.change.added")} <img data-slot="session-review-image" src={imageSrc()} alt={diff.file} />
</span>
</Match>
<Match when={isDeleted()}>
<span data-slot="session-review-change" data-type="removed">
{i18n.t("ui.sessionReview.change.removed")}
</span>
</Match>
<Match when={isImage()}>
<span data-slot="session-review-change" data-type="modified">
{i18n.t("ui.sessionReview.change.modified")}
</span>
</Match>
<Match when={true}>
<DiffChanges changes={diff} />
</Match>
</Switch>
<Icon name="chevron-grabber-vertical" size="small" />
</div>
</div>
</Accordion.Trigger>
</StickyAccordionHeader>
<Accordion.Content data-slot="session-review-accordion-content">
<div
data-slot="session-review-diff-wrapper"
ref={(el) => {
wrapper = el
anchors.set(diff.file, el)
scheduleAnchors()
}}
>
<Show when={expanded()}>
<Switch>
<Match when={isImage() && imageSrc()}>
<div data-slot="session-review-image-container">
<img data-slot="session-review-image" src={imageSrc()} alt={diff.file} />
</div>
</Match>
<Match when={isImage() && isDeleted()}>
<div data-slot="session-review-image-container" data-removed>
<span data-slot="session-review-image-placeholder">
{i18n.t("ui.sessionReview.change.removed")}
</span>
</div>
</Match>
<Match when={isImage() && !imageSrc()}>
<div data-slot="session-review-image-container">
<span data-slot="session-review-image-placeholder">
{imageStatus() === "loading"
? i18n.t("ui.sessionReview.image.loading")
: i18n.t("ui.sessionReview.image.placeholder")}
</span>
</div>
</Match>
<Match when={!isImage() && tooLarge()}>
<div data-slot="session-review-large-diff">
<div data-slot="session-review-large-diff-title">
{i18n.t("ui.sessionReview.largeDiff.title")}
</div> </div>
<div data-slot="session-review-large-diff-meta"> </Match>
{i18n.t("ui.sessionReview.largeDiff.meta", { <Match when={isImage() && isDeleted()}>
limit: MAX_DIFF_CHANGED_LINES.toLocaleString(), <div data-slot="session-review-image-container" data-removed>
current: changedLines().toLocaleString(), <span data-slot="session-review-image-placeholder">
})} {i18n.t("ui.sessionReview.change.removed")}
</span>
</div> </div>
<div data-slot="session-review-large-diff-actions"> </Match>
<Button size="normal" variant="secondary" onClick={() => setForce(true)}> <Match when={isImage() && !imageSrc()}>
{i18n.t("ui.sessionReview.largeDiff.renderAnyway")} <div data-slot="session-review-image-container">
</Button> <span data-slot="session-review-image-placeholder">
{imageStatus() === "loading"
? i18n.t("ui.sessionReview.image.loading")
: i18n.t("ui.sessionReview.image.placeholder")}
</span>
</div> </div>
</div> </Match>
</Match> <Match when={!isImage() && tooLarge()}>
<Match when={!isImage()}> <div data-slot="session-review-large-diff">
<Dynamic <div data-slot="session-review-large-diff-title">
component={diffComponent} {i18n.t("ui.sessionReview.largeDiff.title")}
preloadedDiff={diff.preloaded} </div>
diffStyle={diffStyle()} <div data-slot="session-review-large-diff-meta">
onRendered={() => { {i18n.t("ui.sessionReview.largeDiff.meta", {
props.onDiffRendered?.() limit: MAX_DIFF_CHANGED_LINES.toLocaleString(),
scheduleAnchors() current: changedLines().toLocaleString(),
}} })}
enableLineSelection={props.onLineComment != null} </div>
onLineSelected={handleLineSelected} <div data-slot="session-review-large-diff-actions">
onLineSelectionEnd={handleLineSelectionEnd} <Button size="normal" variant="secondary" onClick={() => setForce(true)}>
selectedLines={selectedLines()} {i18n.t("ui.sessionReview.largeDiff.renderAnyway")}
commentedLines={commentedLines()} </Button>
before={{ </div>
name: diff.file!, </div>
contents: typeof diff.before === "string" ? diff.before : "", </Match>
}} <Match when={!isImage()}>
after={{ <Dynamic
name: diff.file!, component={diffComponent}
contents: typeof diff.after === "string" ? diff.after : "", preloadedDiff={diff.preloaded}
}} diffStyle={diffStyle()}
/> onRendered={() => {
</Match> props.onDiffRendered?.()
</Switch> scheduleAnchors()
}}
<For each={comments()}> enableLineSelection={props.onLineComment != null}
{(comment) => ( onLineSelected={handleLineSelected}
<LineComment onLineSelectionEnd={handleLineSelectionEnd}
id={comment.id} selectedLines={selectedLines()}
top={positions()[comment.id]} commentedLines={commentedLines()}
onMouseEnter={() => setSelection({ file: comment.file, range: comment.selection })} before={{
onClick={() => { name: diff.file!,
if (isCommentOpen(comment)) { contents: typeof diff.before === "string" ? diff.before : "",
setOpened(null) }}
return after={{
} name: diff.file!,
contents: typeof diff.after === "string" ? diff.after : "",
openComment(comment)
}}
open={isCommentOpen(comment)}
comment={comment.comment}
selection={selectionLabel(comment.selection)}
/>
)}
</For>
<Show when={draftRange()}>
{(range) => (
<Show when={draftTop() !== undefined}>
<LineCommentEditor
top={draftTop()}
value={draft()}
selection={selectionLabel(range())}
onInput={setDraft}
onCancel={() => setCommenting(null)}
onSubmit={(comment) => {
props.onLineComment?.({
file: diff.file,
selection: range(),
comment,
preview: selectionPreview(diff, range()),
})
setCommenting(null)
}} }}
/> />
</Show> </Match>
)} </Switch>
<For each={comments()}>
{(comment) => (
<LineComment
id={comment.id}
top={positions()[comment.id]}
onMouseEnter={() => setSelection({ file: comment.file, range: comment.selection })}
onClick={() => {
if (isCommentOpen(comment)) {
setOpened(null)
return
}
openComment(comment)
}}
open={isCommentOpen(comment)}
comment={comment.comment}
selection={selectionLabel(comment.selection)}
/>
)}
</For>
<Show when={draftRange()}>
{(range) => (
<Show when={draftTop() !== undefined}>
<LineCommentEditor
top={draftTop()}
value={draft()}
selection={selectionLabel(range())}
onInput={setDraft}
onCancel={() => setCommenting(null)}
onSubmit={(comment) => {
props.onLineComment?.({
file: diff.file,
selection: range(),
comment,
preview: selectionPreview(diff, range()),
})
setCommenting(null)
}}
/>
</Show>
)}
</Show>
</Show> </Show>
</Show> </div>
</div> </Accordion.Content>
</Accordion.Content> </Accordion.Item>
</Accordion.Item> )
) }}
}} </For>
</For> </Accordion>
</Accordion> </div>
</Show> </Show>
</div> </div>
</div> </div>