From d22f9b8eb33a4d6ce8b40c0ccdc4e27792bb6e2a Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:09:46 -0600 Subject: [PATCH] fix(session-ui): keep line comment drafts and edits in one state machine (#61816) --- packages/gui-plugins/src/file/view.tsx | 48 +-- .../session-review-comments.spec.ts | 37 ++ .../components/line-comment-annotations.tsx | 344 ++++++------------ .../components/line-comment-machine.test.ts | 334 +++++++++++++++++ .../src/components/line-comment-machine.ts | 254 +++++++++++++ .../src/components/session-review.stories.tsx | 19 +- .../src/components/session-review.tsx | 86 ++--- .../line-comment-annotations-v2.tsx | 143 +++----- .../session-review-file-preview-v2.tsx | 24 +- 9 files changed, 856 insertions(+), 433 deletions(-) create mode 100644 packages/session-ui/src/components/line-comment-machine.test.ts create mode 100644 packages/session-ui/src/components/line-comment-machine.ts diff --git a/packages/gui-plugins/src/file/view.tsx b/packages/gui-plugins/src/file/view.tsx index 2a9f8602f2e..861826147ce 100644 --- a/packages/gui-plugins/src/file/view.tsx +++ b/packages/gui-plugins/src/file/view.tsx @@ -1,10 +1,9 @@ import { createMemo, createSignal, Match, on, onCleanup, onMount, Show, Switch } from "solid-js" -import { createStore } from "solid-js/store" import { Dynamic } from "solid-js/web" import { makeEventListener } from "@solid-primitives/event-listener" import type { FileSearchHandle } from "@opencode/session-ui/file" import { useFileComponent } from "@opencode/ui/context/file" -import { cloneSelectedLineRange, previewSelectedLines } from "@opencode/session-ui/pierre/selection-bridge" +import { previewSelectedLines } from "@opencode/session-ui/pierre/selection-bridge" import { createLineCommentControllerV2 } from "@opencode/session-ui/v2/line-comment-annotations-v2" import { sampledChecksum } from "@opencode/util/encode" import { LineCommentOverflowIcon } from "@opencode/ui/line-comment" @@ -59,8 +58,6 @@ type ScrollPos = { x: number; y: number } type ScrollSyncState = { scroll?: HTMLDivElement; scrollFrame?: number; restoreFrame?: number; pending?: ScrollPos } -type NoteState = { openedComment: string | null; commenting: LineRange | null; selected: LineRange | null } - type FindState = { find: FileSearchHandle | null } function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos: ScrollPos) => void }) { @@ -308,17 +305,6 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio const commentedLines = createMemo(() => fileComments().map((comment) => comment.selection)) - const [note, setNote] = createStore({ openedComment: null, commenting: null, selected: null }) - - const syncSelected = (range: LineRange | null) => { - const p = path() - - if (!p) return - file.selection.set(p, range ? cloneSelectedLineRange(range) : null) - } - - const activeSelection = () => note.selected ?? selectedLines() - const commentsUi = createLineCommentControllerV2({ comments: fileComments, label: ctx.t("ui.lineComment.submit"), @@ -327,15 +313,11 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio items: (query) => file.search(query, { kind: "any" }), }, getSide: selectionSide, - state: { - opened: () => note.openedComment, - setOpened: (id) => setNote("openedComment", id), - selected: () => note.selected, - setSelected: (range) => setNote("selected", range), - commenting: () => note.commenting, - setCommenting: (range) => setNote("commenting", range), - syncSelected, - hoverSelected: syncSelected, + onSyncSelected: (range) => { + const p = path() + + if (!p) return + file.selection.set(p, range) }, onSubmit: ({ comment, selection }) => { const p = path() @@ -388,7 +370,7 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio // A new path, e.g. after the workspace directory changes, drops the comment draft and selection of the old one. const moved = createMemo(on(path, () => ({}), { defer: true })) - createKeyed(moved, () => commentsUi.note.reset()) + createKeyed(moved, () => commentsUi.reset()) // A comment focused elsewhere, e.g. from its composer chip, opens here once this file shows. createKeyed( @@ -403,7 +385,7 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio return target && { target } }, (focus) => { - commentsUi.note.openComment(focus.target.id, focus.target.selection, { cancelDraft: true }) + commentsUi.focusComment(focus.target.id, focus.target.selection) requestAnimationFrame(() => comment.focus.set(null)) }, ) @@ -419,7 +401,7 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio (target) => onCleanup( shared.reveal.register(target.session, target.path, () => { - setNote("selected", null) + commentsUi.clearPassive() scrollSync.queueRestore() }), ), @@ -460,7 +442,7 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio }} enableLineSelection enableGutterUtility - selectedLines={activeSelection()} + selectedLines={commentsUi.selected() ?? selectedLines()} commentedLines={commentedLines()} onRendered={() => { scrollSync.queueRestore() @@ -472,18 +454,8 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio commentsUi.onLineSelected(range) }} onLineSelectionEnd={(range: LineRange | null) => { - if (!range) { - commentsUi.note.select(null) - commentsUi.note.cancelDraft() - - return - } - commentsUi.onLineSelectionEnd(range) }} - onLineNumberSelectionEnd={(range: LineRange | null) => { - commentsUi.onLineNumberSelectionEnd(range) - }} search={search} class="select-text" // Media and previews have their own viewers below; the code view only ever shows text. diff --git a/packages/session-ui/component-tests/session-review-comments.spec.ts b/packages/session-ui/component-tests/session-review-comments.spec.ts index 698462059ab..3ab4bbcc804 100644 --- a/packages/session-ui/component-tests/session-review-comments.spec.ts +++ b/packages/session-ui/component-tests/session-review-comments.spec.ts @@ -48,3 +48,40 @@ story("shows a comment button when a diff line is hovered", async ({ mount }) => await expect(review.getByRole("textbox")).toBeVisible() await expect(review.locator('[data-slot="line-comment-editor-label"]')).toHaveText("Commenting on line 1") }) + +story("keeps typed comment text while other lines and comments are used", async ({ mount, page }) => { + const root = await mount("components-session-review--interactive-comments") + const review = root.locator('[data-component="session-review"]') + const textbox = review.getByRole("textbox") + const firstLine = review.getByText("export const first = 1", { exact: true }) + + await review.locator('[data-line-type="change-addition"] [data-diff-span]').click() + await textbox.fill("first comment") + await textbox.press("Enter") + await expect(review.getByText("first comment", { exact: true })).toBeVisible() + + // The gutter button on another line used to replace the open edit with an empty draft. + await review.getByRole("button", { name: "More options" }).click() + await page.getByRole("menuitem", { name: "Edit" }).click() + await expect(textbox).toHaveValue("first comment") + await textbox.fill("edited comment") + // A click on another line keeps the edit, and the highlight returns to the edited line. + await firstLine.click() + await expect(textbox).toHaveValue("edited comment") + await expect(review.locator('[data-column-number="1"]')).not.toHaveAttribute("data-selected-line") + await firstLine.hover() + await review.getByRole("button", { name: "Comment", exact: true, includeHidden: true }).dispatchEvent("click") + await expect(textbox).toHaveCount(1) + await expect(textbox).toHaveValue("edited comment") + await textbox.press("Enter") + await expect(review.getByText("edited comment", { exact: true })).toBeVisible() + + // Deleting a comment used to discard the draft on another line. + await firstLine.click() + await expect(review.locator('[data-slot="line-comment-editor-label"]')).toHaveText("Commenting on line 1") + await textbox.fill("draft text") + await review.getByRole("button", { name: "More options" }).click() + await page.getByRole("menuitem", { name: "Delete" }).click() + await expect(review.getByText("edited comment", { exact: true })).toHaveCount(0) + await expect(textbox).toHaveValue("draft text") +}) diff --git a/packages/session-ui/src/components/line-comment-annotations.tsx b/packages/session-ui/src/components/line-comment-annotations.tsx index 8377db07ee4..4686d536dd7 100644 --- a/packages/session-ui/src/components/line-comment-annotations.tsx +++ b/packages/session-ui/src/components/line-comment-annotations.tsx @@ -1,11 +1,19 @@ import { type DiffLineAnnotation, type SelectedLineRange } from "@pierre/diffs" -import { createEffect, createMemo, createSignal, onCleanup, Show, type Accessor, type JSX } from "solid-js" -import { createStore } from "solid-js/store" +import { createEffect, createMemo, createSignal, onCleanup, Show, untrack, type Accessor, type JSX } from "solid-js" import { render as renderSolid } from "solid-js/web" import { useI18n } from "@opencode/ui/context/i18n" import { createHoverCommentUtility } from "../pierre/comment-hover" import { cloneSelectedLineRange, formatSelectedLineLabel, lineInSelectedRange } from "../pierre/selection-bridge" import { LineComment, LineCommentEditor, type LineCommentEditorProps } from "./line-comment" +import { + IDLE_LINE_COMMENT_MODE, + transitionLineComment, + viewCommentItem, + viewLineComment, + type LineCommentEffect, + type LineCommentEvent, + type LineCommentMode, +} from "./line-comment-machine" export type LineCommentAnnotationMeta = | { kind: "comment"; key: string; comment: T } @@ -34,15 +42,10 @@ type HoverCommentLine = { side?: "additions" | "deletions" } -export type LineCommentStateProps = { - opened: Accessor - setOpened: (id: T | null) => void - selected: Accessor - setSelected: (range: SelectedLineRange | null) => void - commenting: Accessor - setCommenting: (range: SelectedLineRange | null) => void - syncSelected?: (range: SelectedLineRange | null) => void - hoverSelected?: (range: SelectedLineRange) => void +/** A line-comment mode owned by the caller, such as one file's entry in a multi-file review. Omit it to keep the mode inside the controller. */ +export type LineCommentMachine = { + mode: Accessor + dispatch: (event: LineCommentEvent) => readonly LineCommentEffect[] } export type SavedLineComment = { @@ -51,21 +54,22 @@ export type SavedLineComment = { comment: string } -type LineCommentControllerProps = { +type LineCommentDriverProps = { + machine?: LineCommentMachine + onSyncSelected?: (range: SelectedLineRange | null) => void + onSubmit: (input: { comment: string; selection: SelectedLineRange }) => void + onUpdate?: (input: { id: string; comment: string; selection: SelectedLineRange }) => void + onDelete?: (comment: T) => void +} + +type LineCommentControllerProps = LineCommentDriverProps & { comments: Accessor draftKey: Accessor label: string mention?: LineCommentEditorProps["mention"] - state: LineCommentStateProps - onSubmit: (input: { comment: string; selection: SelectedLineRange }) => void - onUpdate?: (input: { id: string; comment: string; selection: SelectedLineRange }) => void - onDelete?: (comment: T) => void renderCommentActions?: (comment: T, controls: { edit: VoidFunction; remove: VoidFunction }) => JSX.Element editSubmitLabel?: string onDraftPopoverFocusOut?: (event: FocusEvent) => void - getHoverSelectedRange?: Accessor - cancelDraftOnCommentToggle?: boolean - clearSelectionOnSelectionEndNull?: boolean } type LineCommentControllerWithSideProps = LineCommentControllerProps & { @@ -197,19 +201,21 @@ function lineCommentElement(view: Accessor) { /> } > - + {(editor) => ( + + )} ) } @@ -229,147 +235,75 @@ function lineCommentDraftElement(view: Accessor) { ) } -export function createLineCommentState(props: LineCommentStateProps) { - const [state, setState] = createStore<{ draft: string; editing: T | null }>({ - draft: "", - editing: null, - }) +// Runs a line-comment machine and turns its effects into the controller's callbacks. +export function createLineCommentDriver(props: LineCommentDriverProps) { + const machine = props.machine ?? createLocalLineCommentMachine() - const draft = () => state.draft - const setDraft = (value: string) => setState("draft", value) - const editing = () => state.editing - // The updater form stores `value` as is, even when it is itself a function. - const setEditing = (value: T | null) => setState("editing", () => value) + const send = (event: LineCommentEvent, comment?: T) => { + for (const effect of machine.dispatch(event)) { + if (effect.type === "sync-selected") + props.onSyncSelected?.(effect.range ? cloneSelectedLineRange(effect.range) : null) - const toRange = (range: SelectedLineRange | null) => (range ? cloneSelectedLineRange(range) : null) + if (effect.type === "submit-draft") props.onSubmit({ comment: effect.comment, selection: effect.selection }) - const setSelected = (range: SelectedLineRange | null) => { - const next = toRange(range) - props.setSelected(next) - props.syncSelected?.(toRange(next)) + if (effect.type === "submit-edit") + props.onUpdate?.({ id: effect.id, comment: effect.comment, selection: effect.selection }) - return next - } - - const setCommenting = (range: SelectedLineRange | null) => { - const next = toRange(range) - props.setCommenting(next) - - return next - } - - const closeComment = () => { - props.setOpened(null) - } - - const cancelDraft = () => { - setDraft("") - setEditing(null) - setCommenting(null) - } - - const reset = () => { - setDraft("") - setEditing(null) - props.setOpened(null) - props.setSelected(null) - props.setCommenting(null) - } - - const openComment = (id: T, range: SelectedLineRange, options?: { cancelDraft?: boolean }) => { - if (options?.cancelDraft) cancelDraft() - props.setOpened(id) - setSelected(range) - } - - const toggleComment = (id: T, range: SelectedLineRange, options?: { cancelDraft?: boolean }) => { - if (options?.cancelDraft) cancelDraft() - const next = props.opened() === id ? null : id - props.setOpened(next) - setSelected(range) - } - - const openDraft = (range: SelectedLineRange) => { - const next = toRange(range) - setDraft("") - setEditing(null) - closeComment() - setSelected(next) - setCommenting(next) - } - - const openEditor = (id: T, range: SelectedLineRange, value: string) => { - closeComment() - setSelected(range) - props.setCommenting(null) - setEditing(id) - setDraft(value) - } - - const hoverComment = (range: SelectedLineRange) => { - const next = toRange(range) - - if (!next) return - - if (props.hoverSelected) { - props.hoverSelected(next) - - return + if (effect.type === "delete-comment" && comment) props.onDelete?.(comment) } - - setSelected(next) } return { - draft, - setDraft, - editing, - opened: props.opened, - selected: props.selected, - commenting: props.commenting, - isOpen: (id: T) => props.opened() === id, - isEditing: (id: T) => editing() === id, - closeComment, - openComment, - toggleComment, - openDraft, - openEditor, - hoverComment, - cancelDraft, - select: setSelected, - reset, + mode: machine.mode, + send, + view: () => viewLineComment(machine.mode()), + focusComment: (id: string, range: SelectedLineRange) => send({ type: "comment.focused", id, range }), + clearPassive: () => send({ type: "passive.cleared" }), + reset: () => send({ type: "reset" }), } } +function createLocalLineCommentMachine(): LineCommentMachine { + const [mode, setMode] = createSignal(IDLE_LINE_COMMENT_MODE) + + return { + mode, + dispatch: (event) => { + const step = transitionLineComment(untrack(mode), event) + setMode(step.state) + + return step.effects + }, + } +} + +type LineCommentController = { + mode: Accessor + selected: Accessor + focusComment: (id: string, range: SelectedLineRange) => void + clearPassive: VoidFunction + reset: VoidFunction + annotations: Accessor + renderAnnotation: ReturnType< + typeof createManagedLineCommentAnnotationRenderer + >["renderAnnotation"] + renderGutterUtility: ReturnType + onLineSelected: (range: SelectedLineRange | null) => void + onLineSelectionEnd: (range: SelectedLineRange | null) => void +} + export function createLineCommentController( props: LineCommentControllerWithSideProps, -): { - note: ReturnType> - annotations: Accessor>[]> - renderAnnotation: ReturnType< - typeof createManagedLineCommentAnnotationRenderer - >["renderAnnotation"] - renderGutterUtility: ReturnType - onLineSelected: (range: SelectedLineRange | null) => void - onLineSelectionEnd: (range: SelectedLineRange | null) => void -} +): LineCommentController>> export function createLineCommentController( props: LineCommentControllerProps, -): { - note: ReturnType> - annotations: Accessor[]> - renderAnnotation: ReturnType< - typeof createManagedLineCommentAnnotationRenderer - >["renderAnnotation"] - renderGutterUtility: ReturnType - onLineSelected: (range: SelectedLineRange | null) => void - onLineSelectionEnd: (range: SelectedLineRange | null) => void -} +): LineCommentController> export function createLineCommentController( props: LineCommentControllerProps | LineCommentControllerWithSideProps, ) { const i18n = useI18n() - const note = createLineCommentState(props.state) + const driver = createLineCommentDriver(props) + const draftRange = () => driver.view().draftRange const annotations = "getSide" in props @@ -377,7 +311,7 @@ export function createLineCommentController( comments: props.comments, getCommentId: (comment) => comment.id, getCommentSelection: (comment) => comment.selection, - draftRange: note.commenting, + draftRange, draftKey: props.draftKey, getSide: props.getSide, }) @@ -385,7 +319,7 @@ export function createLineCommentController( comments: props.comments, getCommentId: (comment) => comment.id, getCommentSelection: (comment) => comment.selection, - draftRange: note.commenting, + draftRange, draftKey: props.draftKey, }) @@ -394,17 +328,17 @@ export function createLineCommentController( commentElement: lineCommentElement, draftElement: lineCommentDraftElement, renderComment: (comment) => { - const edit = () => note.openEditor(comment.id, comment.selection, comment.comment) + const item = () => viewCommentItem(driver.mode(), comment.id) - const remove = () => { - note.reset() - props.onDelete?.(comment) - } + const edit = () => + driver.send({ type: "edit.opened", id: comment.id, range: comment.selection, text: comment.comment }) + + const remove = () => driver.send({ type: "comment.deleted", id: comment.id }, comment) return { id: comment.id, get open() { - return note.isOpen(comment.id) || note.isEditing(comment.id) + return item().open }, comment: comment.comment, selection: formatSelectedLineLabel(comment.selection, i18n.t), @@ -412,92 +346,52 @@ export function createLineCommentController( return props.renderCommentActions?.(comment, { edit, remove }) }, get editor() { - return note.isEditing(comment.id) + return item().editing ? { get value() { - return note.draft() + return item().editText }, selection: formatSelectedLineLabel(comment.selection, i18n.t), mention: props.mention, - onInput: note.setDraft, - onCancel: note.cancelDraft, - onSubmit: (value: string) => { - props.onUpdate?.({ - id: comment.id, - comment: value, - selection: cloneSelectedLineRange(comment.selection), - }) - note.cancelDraft() - }, + onInput: (text: string) => driver.send({ type: "edit.changed", text }), + onCancel: () => driver.send({ type: "edit.cancelled" }), + onSubmit: (text: string) => driver.send({ type: "edit.submitted", text }), submitLabel: props.editSubmitLabel, } : undefined }, - onMouseEnter: () => note.hoverComment(comment.selection), - onClick: () => { - if (note.isEditing(comment.id)) return - note.toggleComment(comment.id, comment.selection, { cancelDraft: props.cancelDraftOnCommentToggle }) - }, + onMouseEnter: () => driver.send({ type: "comment.hovered", range: comment.selection }), + onClick: () => driver.send({ type: "comment.toggled", id: comment.id, range: comment.selection }), } }, renderDraft: (range) => ({ get value() { - return note.draft() + return driver.view().draftText }, selection: formatSelectedLineLabel(range, i18n.t), mention: props.mention, - onInput: note.setDraft, - onCancel: () => { - note.cancelDraft() - note.select(null) - }, - onSubmit: (comment) => { - props.onSubmit({ comment, selection: cloneSelectedLineRange(range) }) - note.cancelDraft() - }, + onInput: (text) => driver.send({ type: "draft.changed", text }), + onCancel: () => driver.send({ type: "draft.cancelled" }), + onSubmit: (text) => driver.send({ type: "draft.submitted", text }), onPopoverFocusOut: props.onDraftPopoverFocusOut, }), }) - const renderGutterUtility = createLineCommentGutterRenderer({ - label: props.label, - getSelectedRange: () => { - if (note.opened()) return null - - return props.getHoverSelectedRange?.() ?? note.selected() - }, - onOpenDraft: note.openDraft, - }) - - const onLineSelected = (range: SelectedLineRange | null) => { - if (!range) { - note.select(null) - note.cancelDraft() - - return - } - - note.select(range) - } - - const onLineSelectionEnd = (range: SelectedLineRange | null) => { - if (!range) { - if (props.clearSelectionOnSelectionEndNull) note.select(null) - note.cancelDraft() - - return - } - - note.openDraft(range) - } - return { - note, + mode: driver.mode, + selected: () => driver.view().selected, + focusComment: driver.focusComment, + clearPassive: driver.clearPassive, + reset: driver.reset, annotations, renderAnnotation, - renderGutterUtility, - onLineSelected, - onLineSelectionEnd, + renderGutterUtility: createLineCommentGutterRenderer({ + label: props.label, + getSelectedRange: () => driver.view().gutterSelected, + onOpenDraft: (range) => driver.send({ type: "draft.opened", range }), + }), + onLineSelected: (range: SelectedLineRange | null) => driver.send({ type: "line.selected", range }), + onLineSelectionEnd: (range: SelectedLineRange | null) => driver.send({ type: "line.selection-ended", range }), } } diff --git a/packages/session-ui/src/components/line-comment-machine.test.ts b/packages/session-ui/src/components/line-comment-machine.test.ts new file mode 100644 index 00000000000..fa40d24ad4e --- /dev/null +++ b/packages/session-ui/src/components/line-comment-machine.test.ts @@ -0,0 +1,334 @@ +import { describe, expect, test } from "bun:test" +import type { SelectedLineRange } from "@pierre/diffs" +import { + EMPTY_REVIEW_COMMENT_MODES, + IDLE_LINE_COMMENT_MODE, + reviewFileCommentMode, + transitionLineComment, + transitionReviewCommentModes, + viewCommentItem, + viewLineComment, + type LineCommentEvent, + type LineCommentMode, + type LineCommentTransition, + type ReviewCommentModes, +} from "./line-comment-machine" + +const line = (start: number, end = start): SelectedLineRange => ({ start, end, side: "additions" }) + +function replay(events: LineCommentEvent[], state: LineCommentMode = IDLE_LINE_COMMENT_MODE) { + return events.reduce((current, event) => transitionLineComment(current, event).state, state) +} + +function replayFiles(steps: [string, LineCommentEvent][], modes: ReviewCommentModes = EMPTY_REVIEW_COMMENT_MODES) { + return steps.reduce((current, [file, event]) => transitionReviewCommentModes(current, file, event).modes, modes) +} + +describe("transitionLineComment", () => { + test("the gutter button, a line click or a cleared selection keeps the text of an open edit", () => { + const editing = replay([ + { type: "edit.opened", id: "c1", range: line(2), text: "original" }, + { type: "edit.changed", text: "updated" }, + ]) + + expect(transitionLineComment(editing, { type: "draft.opened", range: line(5) })).toEqual({ + state: editing, + effects: [], + }) + + // The viewer already highlighted the clicked lines, so the edit hands its own range back to it. + const clicks: LineCommentEvent[] = [ + { type: "line.selected", range: line(8) }, + { type: "line.selection-ended", range: line(8) }, + { type: "line.selected", range: null }, + { type: "line.selection-ended", range: null }, + ] + + for (const event of clicks) { + const step = transitionLineComment(editing, event) + + expect(step).toEqual({ state: editing, effects: [{ type: "sync-selected", range: line(2) }] }) + expect(viewLineComment(step.state).selected).not.toBe(viewLineComment(editing).selected) + } + + expect(editing).toEqual({ status: "editing", id: "c1", range: line(2), text: "updated" }) + }) + + test("the viewer echoing an opened comment's range keeps the comment open", () => { + // A focused comment used to close at once: its highlight came back as a selection and moved it to `selecting`. + const viewing = transitionLineComment( + { status: "selecting", range: line(9) }, + { type: "comment.focused", id: "c1", range: line(2) }, + ).state + + expect(transitionLineComment(viewing, { type: "line.selected", range: line(2) }).state).toBe(viewing) + expect(transitionLineComment(viewing, { type: "line.selected", range: line(3) }).state).toEqual({ + status: "selecting", + range: line(3), + }) + }) + + test("the viewer echoing a restored range back keeps the same state", () => { + const editing: LineCommentMode = { status: "editing", id: "c1", range: line(2), text: "x" } + const restored = transitionLineComment(editing, { type: "line.selected", range: line(8) }).state + + expect(transitionLineComment(restored, { type: "line.selected", range: line(2) }).state).toBe(restored) + expect(transitionLineComment(restored, { type: "line.selection-ended", range: line(2) }).state).toBe(restored) + }) + + test("a repeated selection end for the same lines keeps the typed draft, and a new range moves it", () => { + const drafting = replay([ + { type: "line.selection-ended", range: line(1, 3) }, + { type: "draft.changed", text: "keep me" }, + ]) + + // The line-number drag used to open the draft again one frame later, which emptied it. + expect(transitionLineComment(drafting, { type: "draft.opened", range: line(1, 3) }).state).toBe(drafting) + expect(transitionLineComment(drafting, { type: "line.selection-ended", range: line(1, 3) }).state).toBe(drafting) + + expect(transitionLineComment(drafting, { type: "line.selection-ended", range: line(1, 5) })).toEqual({ + state: { status: "drafting", range: line(1, 5), text: "keep me" }, + effects: [{ type: "sync-selected", range: line(1, 5) }], + }) + }) + + test("a typed draft survives a cleared selection, and an empty one closes", () => { + const typed = replay([ + { type: "draft.opened", range: line(4) }, + { type: "draft.changed", text: "wip" }, + ]) + + for (const event of [ + { type: "line.selected", range: null }, + { type: "line.selection-ended", range: null }, + ] as const) + expect(transitionLineComment(typed, event)).toEqual({ + state: typed, + effects: [{ type: "sync-selected", range: line(4) }], + }) + + const empty = replay([{ type: "draft.opened", range: line(4) }]) + + expect(transitionLineComment(empty, { type: "line.selection-ended", range: null })).toEqual({ + state: IDLE_LINE_COMMENT_MODE, + effects: [{ type: "sync-selected", range: null }], + }) + }) + + test("deleting one comment keeps a draft, another comment's edit, and closes only the deleted comment", () => { + const drafting: LineCommentMode = { status: "drafting", range: line(20), text: "wip" } + const editingOther: LineCommentMode = { status: "editing", id: "c2", range: line(10), text: "wip" } + + for (const state of [drafting, editingOther]) { + const step = transitionLineComment(state, { type: "comment.deleted", id: "c1" }) + + expect(step.state).toBe(state) + expect(step.effects).toEqual([{ type: "delete-comment", id: "c1" }]) + } + + const own: LineCommentMode[] = [ + { status: "editing", id: "c1", range: line(5), text: "wip" }, + { status: "viewing", id: "c1", range: line(5) }, + ] + + for (const state of own) + expect(transitionLineComment(state, { type: "comment.deleted", id: "c1" })).toEqual({ + state: IDLE_LINE_COMMENT_MODE, + effects: [ + { type: "sync-selected", range: null }, + { type: "delete-comment", id: "c1" }, + ], + }) + }) + + test("clicking a comment never leaves a draft and an open comment at the same time", () => { + const empty = transitionLineComment( + { status: "drafting", range: line(5), text: "" }, + { type: "comment.toggled", id: "c1", range: line(2) }, + ).state + + expect(empty).toEqual({ status: "viewing", id: "c1", range: line(2) }) + expect(viewLineComment(empty).draftRange).toBeNull() + + const typed: LineCommentMode = { status: "drafting", range: line(5), text: "unsent" } + const kept = transitionLineComment(typed, { type: "comment.toggled", id: "c1", range: line(2) }).state + + expect(kept).toBe(typed) + expect(viewCommentItem(kept, "c1").open).toBe(false) + }) + + test("toggling the open comment closes it, and another comment replaces it", () => { + const viewing = replay([{ type: "comment.toggled", id: "c1", range: line(2) }]) + + expect(viewCommentItem(viewing, "c1")).toEqual({ open: true, editing: false, editText: "" }) + expect(transitionLineComment(viewing, { type: "comment.toggled", id: "c2", range: line(7) }).state).toEqual({ + status: "viewing", + id: "c2", + range: line(7), + }) + expect(transitionLineComment(viewing, { type: "comment.toggled", id: "c1", range: line(2) })).toEqual({ + state: IDLE_LINE_COMMENT_MODE, + effects: [{ type: "sync-selected", range: null }], + }) + }) + + test("a focus request opens the comment unless typed text would be lost", () => { + const step = transitionLineComment( + { status: "selecting", range: line(9) }, + { type: "comment.focused", id: "c1", range: line(2) }, + ) + + expect(step).toEqual({ + state: { status: "viewing", id: "c1", range: line(2) }, + effects: [{ type: "sync-selected", range: line(2) }], + }) + expect(transitionLineComment(step.state, { type: "comment.focused", id: "c1", range: line(2) }).state).toBe( + step.state, + ) + + const typed: LineCommentMode = { status: "drafting", range: line(5), text: "unsent" } + const editing: LineCommentMode = { status: "editing", id: "c2", range: line(3), text: "x" } + + expect(transitionLineComment(typed, { type: "comment.focused", id: "c1", range: line(2) }).state).toBe(typed) + expect(transitionLineComment(editing, { type: "comment.focused", id: "c1", range: line(2) }).state).toBe(editing) + }) + + test("hovering a comment previews its lines only while nothing is open", () => { + expect(transitionLineComment(IDLE_LINE_COMMENT_MODE, { type: "comment.hovered", range: line(3) })).toEqual({ + state: { status: "selecting", range: line(3) }, + effects: [{ type: "sync-selected", range: line(3) }], + }) + + const held: LineCommentMode[] = [ + { status: "selecting", range: line(3) }, + { status: "viewing", id: "c1", range: line(2) }, + { status: "drafting", range: line(5), text: "" }, + { status: "editing", id: "c1", range: line(2), text: "x" }, + ] + + for (const state of held) + expect(transitionLineComment(state, { type: "comment.hovered", range: line(3) }).state).toBe(state) + }) + + test("submitting trims the text, rejects blank text and clears the selection first", () => { + const drafting: LineCommentMode = { status: "drafting", range: line(1, 2), text: " note " } + + expect(transitionLineComment(drafting, { type: "draft.submitted", text: " " }).state).toBe(drafting) + expect(transitionLineComment(drafting, { type: "draft.submitted", text: " note " })).toEqual({ + state: IDLE_LINE_COMMENT_MODE, + effects: [ + { type: "sync-selected", range: null }, + { type: "submit-draft", comment: "note", selection: line(1, 2) }, + ], + }) + + const editing: LineCommentMode = { status: "editing", id: "c1", range: line(4), text: "x" } + + expect(transitionLineComment(editing, { type: "edit.submitted", text: "\n" }).state).toBe(editing) + expect(transitionLineComment(editing, { type: "edit.submitted", text: " fixed " })).toEqual({ + state: IDLE_LINE_COMMENT_MODE, + effects: [ + { type: "sync-selected", range: null }, + { type: "submit-edit", id: "c1", comment: "fixed", selection: line(4) }, + ], + }) + }) + + test("cancel, passive clear and reset exit to idle", () => { + const drafting: LineCommentMode = { status: "drafting", range: line(1), text: "x" } + const editing: LineCommentMode = { status: "editing", id: "c1", range: line(4), text: "x" } + const selecting: LineCommentMode = { status: "selecting", range: line(2) } + const viewing: LineCommentMode = { status: "viewing", id: "c1", range: line(4) } + + const cleared: LineCommentTransition = { + state: IDLE_LINE_COMMENT_MODE, + effects: [{ type: "sync-selected", range: null }], + } + + expect(transitionLineComment(drafting, { type: "draft.cancelled" })).toEqual(cleared) + expect(transitionLineComment(editing, { type: "edit.cancelled" })).toEqual(cleared) + expect(transitionLineComment(drafting, { type: "edit.cancelled" }).state).toBe(drafting) + expect(transitionLineComment(editing, { type: "draft.cancelled" }).state).toBe(editing) + + for (const state of [selecting, viewing]) + expect(transitionLineComment(state, { type: "passive.cleared" })).toEqual({ + state: IDLE_LINE_COMMENT_MODE, + effects: [], + }) + + for (const state of [drafting, editing]) + expect(transitionLineComment(state, { type: "passive.cleared" }).state).toBe(state) + + for (const state of [drafting, editing, selecting, viewing]) + expect(transitionLineComment(state, { type: "reset" })).toEqual({ state: IDLE_LINE_COMMENT_MODE, effects: [] }) + }) + + test("events that do not apply return the same state", () => { + const states: LineCommentMode[] = [ + IDLE_LINE_COMMENT_MODE, + { status: "selecting", range: line(2) }, + { status: "viewing", id: "c1", range: line(4) }, + ] + + const events: LineCommentEvent[] = [ + { type: "draft.changed", text: "x" }, + { type: "draft.cancelled" }, + { type: "draft.submitted", text: "x" }, + { type: "edit.changed", text: "x" }, + { type: "edit.cancelled" }, + { type: "edit.submitted", text: "x" }, + ] + + for (const state of states) + for (const event of events) expect(transitionLineComment(state, event).state).toBe(state) + + expect(transitionLineComment(IDLE_LINE_COMMENT_MODE, { type: "reset" }).state).toBe(IDLE_LINE_COMMENT_MODE) + expect(transitionLineComment(IDLE_LINE_COMMENT_MODE, { type: "line.selected", range: null }).state).toBe( + IDLE_LINE_COMMENT_MODE, + ) + }) +}) + +describe("transitionReviewCommentModes", () => { + test("hovering in one file keeps another file's draft and its selected lines", () => { + const modes = replayFiles([ + ["b.ts", { type: "draft.opened", range: line(3) }], + ["b.ts", { type: "draft.changed", text: "wip" }], + ["a.ts", { type: "comment.hovered", range: line(1) }], + ]) + + expect(viewLineComment(reviewFileCommentMode(modes, "b.ts"))).toMatchObject({ + selected: line(3), + draftRange: line(3), + draftText: "wip", + }) + expect(reviewFileCommentMode(modes, "a.ts")).toEqual({ status: "selecting", range: line(1) }) + }) + + test("a selection in one file clears passive selections in the others", () => { + const modes = replayFiles([ + ["a.ts", { type: "line.selected", range: line(1) }], + ["c.ts", { type: "comment.toggled", id: "c1", range: line(5) }], + ]) + + expect(Object.keys(modes)).toEqual(["c.ts"]) + }) + + test("an edit keeps its file active while the user selects elsewhere", () => { + const modes = replayFiles([ + ["d.ts", { type: "edit.opened", id: "d1", range: line(4), text: "edit" }], + ["a.ts", { type: "line.selected", range: line(2) }], + ["a.ts", { type: "line.selected", range: null }], + ]) + + expect(reviewFileCommentMode(modes, "d.ts").status).toBe("editing") + expect(reviewFileCommentMode(modes, "a.ts").status).toBe("idle") + }) + + test("ignored events keep the same map, and the last idle file returns the empty map", () => { + const modes = replayFiles([["a.ts", { type: "line.selected", range: line(1) }]]) + + expect(transitionReviewCommentModes(modes, "a.ts", { type: "line.selected", range: line(1) }).modes).toBe(modes) + expect(transitionReviewCommentModes(modes, "a.ts", { type: "reset" }).modes).toBe(EMPTY_REVIEW_COMMENT_MODES) + }) +}) diff --git a/packages/session-ui/src/components/line-comment-machine.ts b/packages/session-ui/src/components/line-comment-machine.ts new file mode 100644 index 00000000000..20cb1e6932b --- /dev/null +++ b/packages/session-ui/src/components/line-comment-machine.ts @@ -0,0 +1,254 @@ +import type { SelectedLineRange } from "@pierre/diffs" +import { cloneSelectedLineRange } from "../pierre/selection-bridge" + +/** What one file's line comments are doing. A new comment's draft and a saved comment's edit live in different variants, so one never overwrites the other. */ +export type LineCommentMode = + | { readonly status: "idle" } + | { readonly status: "selecting"; readonly range: SelectedLineRange } + | { readonly status: "drafting"; readonly range: SelectedLineRange; readonly text: string } + | { readonly status: "viewing"; readonly id: string; readonly range: SelectedLineRange } + | { readonly status: "editing"; readonly id: string; readonly range: SelectedLineRange; readonly text: string } + +export type LineCommentEvent = + | { readonly type: "line.selected"; readonly range: SelectedLineRange | null } + | { readonly type: "line.selection-ended"; readonly range: SelectedLineRange | null } + | { readonly type: "draft.opened"; readonly range: SelectedLineRange } + | { readonly type: "draft.changed"; readonly text: string } + | { readonly type: "draft.cancelled" } + | { readonly type: "draft.submitted"; readonly text: string } + | { readonly type: "comment.hovered"; readonly range: SelectedLineRange } + | { readonly type: "comment.toggled"; readonly id: string; readonly range: SelectedLineRange } + | { readonly type: "comment.focused"; readonly id: string; readonly range: SelectedLineRange } + | { readonly type: "comment.deleted"; readonly id: string } + | { readonly type: "edit.opened"; readonly id: string; readonly range: SelectedLineRange; readonly text: string } + | { readonly type: "edit.changed"; readonly text: string } + | { readonly type: "edit.cancelled" } + | { readonly type: "edit.submitted"; readonly text: string } + | { readonly type: "passive.cleared" } + | { readonly type: "reset" } + +export type LineCommentEffect = + | { readonly type: "sync-selected"; readonly range: SelectedLineRange | null } + | { readonly type: "submit-draft"; readonly comment: string; readonly selection: SelectedLineRange } + | { + readonly type: "submit-edit" + readonly id: string + readonly comment: string + readonly selection: SelectedLineRange + } + | { readonly type: "delete-comment"; readonly id: string } + +export type LineCommentTransition = { + readonly state: LineCommentMode + readonly effects: readonly LineCommentEffect[] +} + +/** The modes of every file in a multi-file review. Idle files have no entry. */ +export type ReviewCommentModes = Readonly> + +type ActiveLineCommentMode = Exclude + +export const IDLE_LINE_COMMENT_MODE: LineCommentMode = Object.freeze({ status: "idle" }) + +export const EMPTY_REVIEW_COMMENT_MODES: ReviewCommentModes = Object.freeze({}) + +const NO_EFFECTS: readonly LineCommentEffect[] = Object.freeze([]) + +export function transitionLineComment(state: LineCommentMode, event: LineCommentEvent): LineCommentTransition { + switch (event.type) { + case "line.selected": + if (!event.range) return clear(state) + + // The viewer reports every highlight it applies, including the range this mode just handed it. + if (state.status !== "idle" && sameSelectedRange(state.range, event.range)) return stay(state) + + if (state.status === "editing") return restore(state, event.range) + + if (state.status === "drafting") + return enter({ status: "drafting", range: cloneSelectedLineRange(event.range), text: state.text }) + + return enter({ status: "selecting", range: cloneSelectedLineRange(event.range) }) + case "line.selection-ended": + if (!event.range) return clear(state) + + if (state.status === "editing") return restore(state, event.range) + + return openDraft(state, event.range) + case "draft.opened": + return openDraft(state, event.range) + case "draft.changed": + if (state.status !== "drafting" || state.text === event.text) return stay(state) + + return { state: { ...state, text: event.text }, effects: NO_EFFECTS } + case "draft.cancelled": + if (state.status !== "drafting") return stay(state) + + return enter(IDLE_LINE_COMMENT_MODE) + case "draft.submitted": { + const comment = event.text.trim() + + if (state.status !== "drafting" || !comment) return stay(state) + + return enter(IDLE_LINE_COMMENT_MODE, { + type: "submit-draft", + comment, + selection: cloneSelectedLineRange(state.range), + }) + } + + case "comment.hovered": + if (state.status !== "idle" && state.status !== "selecting") return stay(state) + + if (state.status === "selecting" && sameSelectedRange(state.range, event.range)) return stay(state) + + return enter({ status: "selecting", range: cloneSelectedLineRange(event.range) }) + case "comment.toggled": + if (holdsText(state)) return stay(state) + + if (state.status === "viewing" && state.id === event.id) return enter(IDLE_LINE_COMMENT_MODE) + + return enter({ status: "viewing", id: event.id, range: cloneSelectedLineRange(event.range) }) + case "comment.focused": + if (holdsText(state)) return stay(state) + + if (state.status === "viewing" && state.id === event.id && sameSelectedRange(state.range, event.range)) + return stay(state) + + return enter({ status: "viewing", id: event.id, range: cloneSelectedLineRange(event.range) }) + case "comment.deleted": { + const effect: LineCommentEffect = { type: "delete-comment", id: event.id } + + if ((state.status === "viewing" || state.status === "editing") && state.id === event.id) + return enter(IDLE_LINE_COMMENT_MODE, effect) + + return { state, effects: [effect] } + } + + case "edit.opened": + if (holdsText(state)) return stay(state) + + return enter({ status: "editing", id: event.id, range: cloneSelectedLineRange(event.range), text: event.text }) + case "edit.changed": + if (state.status !== "editing" || state.text === event.text) return stay(state) + + return { state: { ...state, text: event.text }, effects: NO_EFFECTS } + case "edit.cancelled": + if (state.status !== "editing") return stay(state) + + return enter(IDLE_LINE_COMMENT_MODE) + case "edit.submitted": { + const comment = event.text.trim() + + if (state.status !== "editing" || !comment) return stay(state) + + return enter(IDLE_LINE_COMMENT_MODE, { + type: "submit-edit", + id: state.id, + comment, + selection: cloneSelectedLineRange(state.range), + }) + } + + case "passive.cleared": + if (state.status !== "selecting" && state.status !== "viewing") return stay(state) + + return { state: IDLE_LINE_COMMENT_MODE, effects: NO_EFFECTS } + case "reset": + if (state.status === "idle") return stay(state) + + return { state: IDLE_LINE_COMMENT_MODE, effects: NO_EFFECTS } + } +} + +/** Applies `event` to `file`. A file that becomes active clears passive selections in the other files and keeps their drafts and edits. */ +export function transitionReviewCommentModes(modes: ReviewCommentModes, file: string, event: LineCommentEvent) { + const current = reviewFileCommentMode(modes, file) + const step = transitionLineComment(current, event) + + if (step.state === current) return { modes, effects: step.effects } + + const next = Object.fromEntries( + Object.entries(modes).filter( + ([other, mode]) => + other !== file && (step.state.status === "idle" || mode.status === "drafting" || mode.status === "editing"), + ), + ) + + if (step.state.status !== "idle") next[file] = step.state + + return { + modes: Object.keys(next).length === 0 ? EMPTY_REVIEW_COMMENT_MODES : next, + effects: step.effects, + } +} + +export function viewLineComment(state: LineCommentMode) { + return { + selected: state.status === "idle" ? null : state.range, + draftRange: state.status === "drafting" ? state.range : null, + draftText: state.status === "drafting" ? state.text : "", + gutterSelected: state.status === "selecting" || state.status === "drafting" ? state.range : null, + } +} + +export function viewCommentItem(state: LineCommentMode, id: string) { + const editing = state.status === "editing" && state.id === id + + return { + open: editing || (state.status === "viewing" && state.id === id), + editing, + editText: editing ? state.text : "", + } +} + +export function reviewFileCommentMode(modes: ReviewCommentModes, file: string): LineCommentMode { + return modes[file] ?? IDLE_LINE_COMMENT_MODE +} + +function sameSelectedRange(a: SelectedLineRange, b: SelectedLineRange) { + return a.start === b.start && a.end === b.end && a.side === b.side && (a.endSide ?? a.side) === (b.endSide ?? b.side) +} + +function openDraft(state: LineCommentMode, range: SelectedLineRange): LineCommentTransition { + if (state.status === "editing") return stay(state) + + if (state.status !== "drafting") return enter({ status: "drafting", range: cloneSelectedLineRange(range), text: "" }) + + if (sameSelectedRange(state.range, range)) return stay(state) + + return enter({ status: "drafting", range: cloneSelectedLineRange(range), text: state.text }) +} + +function clear(state: LineCommentMode): LineCommentTransition { + if (state.status === "idle") return stay(state) + + if (holdsText(state)) return restore(state, null) + + return enter(IDLE_LINE_COMMENT_MODE) +} + +// The viewer highlights a clicked range before the machine sees it. When typed text keeps its range, a fresh copy of +// that range makes the `selectedLines` prop change, so the viewer highlights the held lines again. +function restore(state: ActiveLineCommentMode, range: SelectedLineRange | null): LineCommentTransition { + if (range && sameSelectedRange(state.range, range)) return stay(state) + + return enter({ ...state, range: cloneSelectedLineRange(state.range) }) +} + +// Typed draft text and an open edit are never discarded by a selection, click or focus elsewhere in the file. +function holdsText(state: LineCommentMode) { + if (state.status === "editing") return true + + return state.status === "drafting" && state.text.trim().length > 0 +} + +function enter(state: LineCommentMode, ...effects: LineCommentEffect[]): LineCommentTransition { + return { + state, + effects: [{ type: "sync-selected", range: state.status === "idle" ? null : state.range }, ...effects], + } +} + +function stay(state: LineCommentMode): LineCommentTransition { + return { state, effects: NO_EFFECTS } +} diff --git a/packages/session-ui/src/components/session-review.stories.tsx b/packages/session-ui/src/components/session-review.stories.tsx index 8f7c4bddfa8..f3746ea4565 100644 --- a/packages/session-ui/src/components/session-review.stories.tsx +++ b/packages/session-ui/src/components/session-review.stories.tsx @@ -49,7 +49,7 @@ export const UnifiedDark = { } function InteractiveCommentsStory() { - const [state, setState] = createStore({ comments: [] as SessionReviewComment[] }) + const [state, setState] = createStore<{ comments: SessionReviewComment[]; next: number }>({ comments: [], next: 1 }) const file = "src/review.ts" const diffs = [ @@ -71,9 +71,22 @@ function InteractiveCommentsStory() { diffs={diffs} open={[file]} comments={state.comments} - onLineComment={(comment) => - setState("comments", (comments) => [...comments, { id: `comment-${comments.length + 1}`, ...comment }]) + onLineComment={(comment) => { + setState("comments", (comments) => [...comments, { id: `comment-${state.next}`, ...comment }]) + setState("next", (next) => next + 1) + }} + onLineCommentUpdate={(update) => + setState("comments", (comment) => comment.id === update.id, "comment", update.comment) } + onLineCommentDelete={(target) => + setState("comments", (comments) => comments.filter((comment) => comment.id !== target.id)) + } + lineCommentActions={{ + moreLabel: "More options", + editLabel: "Edit", + deleteLabel: "Delete", + saveLabel: "Save", + }} /> diff --git a/packages/session-ui/src/components/session-review.tsx b/packages/session-ui/src/components/session-review.tsx index bc905c2c5a8..9139f5a9235 100644 --- a/packages/session-ui/src/components/session-review.tsx +++ b/packages/session-ui/src/components/session-review.tsx @@ -12,7 +12,18 @@ import { useFileComponent } from "@opencode/ui/context/file" import { useI18n } from "@opencode/ui/context/i18n" import { getDirectory, getFilename } from "@opencode/util/path" import { checksum } from "@opencode/util/encode" -import { createEffect, createMemo, For, Match, onCleanup, Show, Switch, untrack, type JSX } from "solid-js" +import { + createEffect, + createMemo, + createSignal, + For, + Match, + onCleanup, + Show, + Switch, + untrack, + type JSX, +} from "solid-js" import { createStore } from "solid-js/store" import { Predicate } from "effect" import type { FileDiffInfo } from "@opencode/client/promise" @@ -21,8 +32,14 @@ import { PreloadMultiFileDiffResult } from "@pierre/diffs/ssr" import { type SelectedLineRange } from "@pierre/diffs" import { Dynamic } from "solid-js/web" import { mediaKindFromPath } from "../pierre/media" -import { cloneSelectedLineRange, previewSelectedLines } from "../pierre/selection-bridge" +import { previewSelectedLines } from "../pierre/selection-bridge" import { createLineCommentController } from "./line-comment-annotations" +import { + EMPTY_REVIEW_COMMENT_MODES, + reviewFileCommentMode, + transitionReviewCommentModes, + type LineCommentEvent, +} from "./line-comment-machine" import type { LineCommentEditorProps } from "./line-comment" import { normalize, text, type ViewDiff } from "./session-diff" @@ -139,11 +156,6 @@ function diffId(file: string): string | undefined { return `session-review-diff-${sum}` } -type SessionReviewSelection = { - file: string - range: SelectedLineRange -} - export const SessionReview = (props: SessionReviewProps) => { let scroll: HTMLDivElement | undefined let focusToken = 0 @@ -157,21 +169,20 @@ export const SessionReview = (props: SessionReviewProps) => { open: string[] visible: Record force: Record - selection: SessionReviewSelection | null - commenting: SessionReviewSelection | null - opened: SessionReviewFocus | null }>({ open: [], visible: {}, force: {}, - selection: null, - commenting: null, - opened: null, }) - const selection = () => store.selection - const commenting = () => store.commenting - const opened = () => store.opened + const [commentModes, setCommentModes] = createSignal(EMPTY_REVIEW_COMMENT_MODES) + + const dispatchComment = (file: string, event: LineCommentEvent) => { + const step = transitionReviewCommentModes(untrack(commentModes), file, event) + setCommentModes(step.modes) + + return step.effects + } const open = () => props.open ?? store.open @@ -240,9 +251,7 @@ export const SessionReview = (props: SessionReviewProps) => { const pinned = (file: string) => props.focusedComment?.file === file || props.focusedFile === file || - selection()?.file === file || - commenting()?.file === file || - opened()?.file === file + reviewFileCommentMode(commentModes(), file).status !== "idle" const handleScroll: JSX.EventHandler = (event) => { queue() @@ -300,11 +309,9 @@ export const SessionReview = (props: SessionReviewProps) => { focusToken++ const token = focusToken - setStore("opened", focus) - const comment = (props.comments ?? []).find((c) => c.file === focus.file && c.id === focus.id) - if (comment) setStore("selection", { file: comment.file, range: cloneSelectedLineRange(comment.selection) }) + if (comment) dispatchComment(focus.file, { type: "comment.focused", id: comment.id, range: comment.selection }) const current = open() @@ -451,43 +458,16 @@ export const SessionReview = (props: SessionReviewProps) => { const isDeleted = () => diff().status === "deleted" || (afterText().length === 0 && beforeText().length > 0) - const selectedLines = createMemo(() => { - const current = selection() - - if (!current || current.file !== file) return null - - return current.range - }) - - const draftRange = createMemo(() => { - const current = commenting() - - if (!current || current.file !== file) return null - - return current.range - }) - const commentsUi = createLineCommentController({ comments, label: i18n.t("ui.lineComment.submit"), draftKey: () => file, mention: props.lineCommentMention, - state: { - opened: () => { - const current = opened() - - if (!current || current.file !== file) return null - - return current.id - }, - setOpened: (id) => setStore("opened", id ? { file, id } : null), - selected: selectedLines, - setSelected: (range) => setStore("selection", range ? { file, range } : null), - commenting: draftRange, - setCommenting: (range) => setStore("commenting", range ? { file, range } : null), + machine: { + mode: createMemo(() => reviewFileCommentMode(commentModes(), file)), + dispatch: (event) => dispatchComment(file, event), }, getSide: selectionSide, - clearSelectionOnSelectionEndNull: false, onSubmit: ({ comment, selection }) => { props.onLineComment?.({ file, @@ -693,7 +673,7 @@ export const SessionReview = (props: SessionReviewProps) => { renderGutterUtility={ props.onLineComment ? commentsUi.renderGutterUtility : undefined } - selectedLines={selectedLines()} + selectedLines={commentsUi.selected()} commentedLines={commentedLines()} media={{ mode: "auto", diff --git a/packages/session-ui/src/v2/components/line-comment-annotations-v2.tsx b/packages/session-ui/src/v2/components/line-comment-annotations-v2.tsx index bf5383df4ab..ecb7dc140d0 100644 --- a/packages/session-ui/src/v2/components/line-comment-annotations-v2.tsx +++ b/packages/session-ui/src/v2/components/line-comment-annotations-v2.tsx @@ -2,21 +2,22 @@ import { type SelectedLineRange } from "@pierre/diffs" import { Show, type Accessor, type JSX } from "solid-js" import { createLineCommentAnnotations, + createLineCommentDriver, createLineCommentGutterRenderer, - createLineCommentState, createManagedLineCommentAnnotationRenderer, + type LineCommentMachine, type SavedLineComment, - type LineCommentStateProps, } from "../../components/line-comment-annotations" +import { viewCommentItem } from "../../components/line-comment-machine" import { useI18n } from "@opencode/ui/context/i18n" -import { cloneSelectedLineRange } from "../../pierre/selection-bridge" import { LineCommentEditor, LineComment, type LineCommentEditorMention } from "@opencode/ui/line-comment" type LineCommentControllerV2Props = { comments: Accessor draftKey: Accessor label: string - state: LineCommentStateProps + machine?: LineCommentMachine + onSyncSelected?: (range: SelectedLineRange | null) => void getSide: (range: SelectedLineRange) => "additions" | "deletions" onSubmit: (input: { comment: string; selection: SelectedLineRange }) => void onUpdate?: (input: { id: string; comment: string; selection: SelectedLineRange }) => void @@ -62,17 +63,19 @@ function lineCommentElementV2(view: Accessor) { } > -
event.stopPropagation()}> - -
+ {(editor) => ( +
event.stopPropagation()}> + +
+ )} ) } @@ -95,13 +98,13 @@ function lineCommentDraftElementV2(view: Accessor) { export function createLineCommentControllerV2(props: LineCommentControllerV2Props) { const i18n = useI18n() - const note = createLineCommentState(props.state) + const driver = createLineCommentDriver(props) const annotations = createLineCommentAnnotations({ comments: props.comments, getCommentId: (comment) => comment.id, getCommentSelection: (comment) => comment.selection, - draftRange: note.commenting, + draftRange: () => driver.view().draftRange, draftKey: props.draftKey, getSide: props.getSide, }) @@ -111,12 +114,12 @@ export function createLineCommentControllerV2(props: commentElement: lineCommentElementV2, draftElement: lineCommentDraftElementV2, renderComment: (comment) => { - const edit = () => note.openEditor(comment.id, comment.selection, comment.comment) + const item = () => viewCommentItem(driver.mode(), comment.id) - const remove = () => { - note.reset() - props.onDelete?.(comment) - } + const edit = () => + driver.send({ type: "edit.opened", id: comment.id, range: comment.selection, text: comment.comment }) + + const remove = () => driver.send({ type: "comment.deleted", id: comment.id }, comment) return { id: comment.id, @@ -127,100 +130,54 @@ export function createLineCommentControllerV2(props: return props.renderCommentActions?.(comment, { edit, remove }) }, get editor() { - return note.isEditing(comment.id) + return item().editing ? { get value() { - return note.draft() - }, - onInput: note.setDraft, - onCancel: note.cancelDraft, - onSubmit: (value: string) => { - props.onUpdate?.({ - id: comment.id, - comment: value, - selection: cloneSelectedLineRange(comment.selection), - }) - note.cancelDraft() + return item().editText }, + onInput: (text: string) => driver.send({ type: "edit.changed", text }), + onCancel: () => driver.send({ type: "edit.cancelled" }), + onSubmit: (text: string) => driver.send({ type: "edit.submitted", text }), cancelLabel: i18n.t("ui.lineComment.cancel"), submitLabel: props.editSubmitLabel, mention: props.mention, } : undefined }, - onMouseEnter: () => { - if (note.commenting()) return - note.hoverComment(comment.selection) - }, + onMouseEnter: () => driver.send({ type: "comment.hovered", range: comment.selection }), onClick: () => { - if (props.renderCommentActions || note.isEditing(comment.id)) return - note.toggleComment(comment.id, comment.selection) + if (props.renderCommentActions) return + driver.send({ type: "comment.toggled", id: comment.id, range: comment.selection }) }, } }, - renderDraft: (range) => ({ + renderDraft: () => ({ get value() { - return note.draft() - }, - onInput: note.setDraft, - onCancel: () => { - note.cancelDraft() - note.select(null) - }, - onSubmit: (comment) => { - props.onSubmit({ comment, selection: cloneSelectedLineRange(range) }) - note.cancelDraft() - note.select(null) + return driver.view().draftText }, + onInput: (text) => driver.send({ type: "draft.changed", text }), + onCancel: () => driver.send({ type: "draft.cancelled" }), + onSubmit: (text) => driver.send({ type: "draft.submitted", text }), cancelLabel: i18n.t("ui.lineComment.cancel"), submitLabel: i18n.t("ui.lineComment.submit"), mention: props.mention, }), }) - const renderGutterUtility = createLineCommentGutterRenderer({ - label: props.label, - getSelectedRange: () => { - if (note.opened()) return null - - return note.selected() - }, - onOpenDraft: note.openDraft, - }) - - const onLineSelected = (range: SelectedLineRange | null) => { - if (!range) { - note.select(null) - note.cancelDraft() - - return - } - - note.select(range) - } - - const onLineSelectionEnd = (range: SelectedLineRange | null) => { - if (!range) { - note.cancelDraft() - - return - } - - note.openDraft(range) - } - - const onLineNumberSelectionEnd = (range: SelectedLineRange | null) => { - if (!range) return - note.openDraft(range) - } - return { - note, + mode: driver.mode, + selected: () => driver.view().selected, + focusComment: driver.focusComment, + clearPassive: driver.clearPassive, + reset: driver.reset, annotations, renderAnnotation, - renderGutterUtility, - onLineSelected, - onLineSelectionEnd, - onLineNumberSelectionEnd, + renderGutterUtility: createLineCommentGutterRenderer({ + label: props.label, + getSelectedRange: () => driver.view().gutterSelected, + onOpenDraft: (range) => driver.send({ type: "draft.opened", range }), + }), + onLineSelected: (range: SelectedLineRange | null) => driver.send({ type: "line.selected", range }), + onLineSelectionEnd: (range: SelectedLineRange | null) => driver.send({ type: "line.selection-ended", range }), } } diff --git a/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx b/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx index ddc7e2cf8dd..1745ed98844 100644 --- a/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx +++ b/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx @@ -5,11 +5,10 @@ import { FileIcon } from "@opencode/ui/file-icon" import { useFileComponent } from "@opencode/ui/context/file" import { useI18n } from "@opencode/ui/context/i18n" import { mediaKindFromPath } from "../../pierre/media" -import { cloneSelectedLineRange, previewSelectedLines } from "../../pierre/selection-bridge" +import { previewSelectedLines } from "../../pierre/selection-bridge" import type { FileDiffInfo } from "@opencode/client/promise" import type { PresentationFileContent, PresentationFileDiff } from "../../file-presentation" import { createEffect, createMemo, onCleanup, Show, untrack } from "solid-js" -import { createStore } from "solid-js/store" import { Dynamic } from "solid-js/web" import { normalize, text, type ViewDiff } from "../../components/session-diff" import type { @@ -107,12 +106,6 @@ export function SessionReviewFilePreviewV2(props: SessionReviewFilePreviewV2Prop let scrollRef: HTMLDivElement | undefined let focusToken = 0 - const [store, setStore] = createStore({ - selection: null as SelectedLineRange | null, - commenting: null as SelectedLineRange | null, - opened: null as string | null, - }) - const view = createMemo(() => ({ ...normalize(props.diff), preloaded: "preloaded" in props.diff ? props.diff.preloaded : undefined, @@ -128,14 +121,6 @@ export function SessionReviewFilePreviewV2(props: SessionReviewFilePreviewV2Prop comments, label: i18n.t("ui.lineComment.submit"), draftKey: () => props.file, - state: { - opened: () => store.opened, - setOpened: (id) => setStore("opened", id), - selected: () => store.selection, - setSelected: (range) => setStore("selection", range), - commenting: () => store.commenting, - setCommenting: (range) => setStore("commenting", range), - }, getSide: selectionSide, onSubmit: ({ comment, selection }) => { props.onLineComment?.({ @@ -193,11 +178,9 @@ export function SessionReviewFilePreviewV2(props: SessionReviewFilePreviewV2Prop } untrack(() => { - setStore("opened", focus.id) - const comment = (props.comments ?? []).find((item) => item.file === focus.file && item.id === focus.id) - if (comment) setStore("selection", cloneSelectedLineRange(comment.selection)) + if (comment) commentsUi.focusComment(comment.id, comment.selection) // The diff renders asynchronously, so poll for the comment anchor before // scrolling; clear the focus once handled so revisiting the file does not @@ -252,11 +235,10 @@ export function SessionReviewFilePreviewV2(props: SessionReviewFilePreviewV2Prop if (!lineCommentsEnabled()) return commentsUi.onLineSelectionEnd(range) }} - onLineNumberSelectionEnd={commentsUi.onLineNumberSelectionEnd} annotations={commentsUi.annotations()} renderAnnotation={commentsUi.renderAnnotation} renderGutterUtility={lineCommentsEnabled() ? commentsUi.renderGutterUtility : undefined} - selectedLines={store.selection} + selectedLines={commentsUi.selected()} commentedLines={commentedLines()} media={{ mode: "auto",