diff --git a/packages/panel/src/ai-elements/fold.tsx b/packages/panel/src/ai-elements/fold.tsx index 09cdba5261..1283f962e2 100644 --- a/packages/panel/src/ai-elements/fold.tsx +++ b/packages/panel/src/ai-elements/fold.tsx @@ -19,19 +19,26 @@ import type { ComponentProps, MouseEvent, ReactNode } from "react"; * summary row is `Disclosure` itself, not this. */ -/** Note that the element's own `open` is not a prop: the state is this component's, and a fold meant to start open says `defaultOpen`. */ +/** Note that the element's own `open` is not a prop: a fold meant to start open says `defaultOpen`, and one whose holder keeps its state says `open` and hears `onOpenChange`, each press and each toggle a find makes. */ export type FoldProps = Omit, "open"> & { defaultOpen?: boolean; + open?: boolean | undefined; + onOpenChange?: ((open: boolean) => void) | undefined; }; export function Fold({ className, children, defaultOpen = false, + open, + onOpenChange, onToggle, ...props }: FoldProps): ReactNode { - const { state, details } = useDisclosureFold(defaultOpen); + const { state, details } = useDisclosureFold( + defaultOpen, + open === undefined ? undefined : { open, onOpenChange: onOpenChange ?? (() => undefined) }, + ); return (
.diff-thread-chip-row:only-child:not([open], [data-above])) { + padding-block: 0; +} + +.diff-thread-chip-row { + position: relative; +} + +.diff-thread-chip-row:not([open], [data-above]) { + height: 0; +} + +.diff-thread-chip { + display: flex; + align-items: center; + gap: var(--luke-icon-gap); + width: fit-content; + margin-inline-start: auto; + padding: var(--luke-space-2) var(--luke-space-6); + border: 1px solid var(--luke-color-border); + border-radius: var(--luke-control-radius); + background: var(--luke-color-surface); + color: var(--luke-color-text-muted); + font-size: var(--luke-type-caption-size); +} + +.diff-thread-chip:hover { + color: var(--luke-color-text); +} + +.diff-thread-chip-row:not([open], [data-above]) > .diff-thread-chip { + position: absolute; + right: 0; + bottom: var(--luke-space-2); +} + +.diff-thread-chip-row[open] > .diff-thread-chip { + margin-bottom: var(--luke-space-4); +} + +.diff-thread-chip > svg { + width: var(--luke-space-12); + height: var(--luke-space-12); +} + +.diff-thread-chip-faces { + display: inline-flex; +} + +.diff-thread-chip-faces > .actor-mark + .actor-mark { + margin-inline-start: calc(-1 * var(--luke-space-4)); +} + +.diff-thread-chip-resolved { + color: var(--luke-color-complete); +} + +.diff-thread-chip-threads { + display: flex; + flex-direction: column; + gap: var(--luke-space-6); +} + .diff-inline-thread { --luke-focus-ring-offset: var(--luke-focus-ring-inset); max-width: 720px; diff --git a/packages/panel/src/tasks/agent-changes-tab.test.ts b/packages/panel/src/tasks/agent-changes-tab.test.ts index c6228b152e..580f28ff63 100644 --- a/packages/panel/src/tasks/agent-changes-tab.test.ts +++ b/packages/panel/src/tasks/agent-changes-tab.test.ts @@ -30,6 +30,7 @@ import { FILES_CHANGED_SHORTCUTS, type ShortcutChord, } from "@luke/settings/shortcuts"; +import { DisclosureFind } from "@luke/ui"; import { Effect } from "effect"; import * as AtomRegistry from "effect/reactivity/AtomRegistry"; import { act, createElement, type ReactElement } from "react"; @@ -40,6 +41,7 @@ import { useAppCommand, useAppKeymap } from "../app-commands.js"; import type { SideBundles } from "../bundle-loader.js"; import type { DiffListProps, DiffViewModule } from "../diff-view/contract.js"; import type { FileTreeModule } from "../file-tree/contract.js"; +import { findRanges, reveal } from "../find-matches.js"; import { fakeAppBridge, fakeAppBridgeCalls } from "../testing/app-bridge.js"; import { handSettings } from "../testing/app-state.js"; import { onRegistry } from "../testing/copy-feedback.js"; @@ -244,6 +246,8 @@ interface DrawOptions { readonly beside?: ReactElement; readonly repository?: string; readonly control?: typeof control; + /** Whether a find looks through the tab, which draws its folds' closed bodies hidden. */ + readonly finding?: boolean; /** Whether the view is the Pull request tab's chosen one; it is unless told. */ readonly shown?: boolean; } @@ -263,7 +267,13 @@ async function redraw(options: DrawOptions = {}): Promise { root?.render( onRegistry( registry, - onSideBundles(bundles, createElement("div", null, tab, options.beside ?? null)), + onSideBundles( + bundles, + createElement(DisclosureFind, { + finding: options.finding ?? false, + children: createElement("div", null, tab, options.beside ?? null), + }), + ), ), ), ); @@ -682,7 +692,13 @@ test("? inside the view opens a sheet of its keys by group, which Escape or ? ag ["Clear file focus", "Esc"], ], ], - ["Review", [["Toggle viewed and go to next file", "V"]]], + [ + "Review", + [ + ["Toggle viewed and go to next file", "V"], + ["Minimize or show inline comments", "I"], + ], + ], ]); press(sheet, { key: "Escape" }); @@ -1480,6 +1496,74 @@ test("a refused mark leaves a fold chosen since, as Collapse all chose it", asyn ]); }); +test("I turns Minimize inline comments on and off inside Files changed, each line's threads then one chip that a find still reads and opens", async () => { + // Each thread drawn as a resolved one is, folded closed under its head. + const threads: ChangesThreads = { + list: [seatsThread(2, "why-42")], + draw: (thread) => + createElement( + "details", + { className: "drawn-card" }, + createElement("summary", null, "head"), + createElement("p", { className: "drawn-thread" }, thread.comments[0]?.body), + ), + }; + await drawTab({ threads }); + const note = () => lastDrawn().renderAnnotation("src/seats.ts", `${PAGE}#why-42`); + assert.equal(note().querySelector(".drawn-thread")?.textContent, "why-42"); + + press(listBox(), { key: "i" }); + await flush(); + const chip = + note().querySelector(".diff-thread-chip") ?? assert.fail("a chip stands"); + assert.equal(chip.getAttribute("aria-label"), "1 comment by octo · 1 open, show"); + assert.equal(note().querySelector(".drawn-thread"), null, "minimized, the thread is put away"); + press(document.body, { key: "i" }); + await flush(); + assert.ok(note().querySelector(".diff-thread-chip"), "outside the view, I is not the view's"); + + // A find looking through the tab reads the put-away thread's words, and opens the line it brings into view. + await redraw({ threads, finding: true }); + const slot = note(); + document.body.append(slot); + const [match] = findRanges(slot, "why-42"); + assert.ok(match, "the find reads the minimized thread"); + // Note that jsdom lays nothing out, so a range is measured where the find scrolls it into view as standing nowhere. + Range.prototype.getBoundingClientRect = () => new DOMRect(); + act(() => reveal(match, slot)); + await flush(); + assert.equal( + note().querySelector(".diff-thread-chip")?.getAttribute("aria-label"), + "1 comment by octo · 1 open, hide", + ); + assert.equal( + note().querySelector(".drawn-card")?.open, + true, + "the found words' own fold stays open", + ); + + press(listBox(), { key: "i" }); + await flush(); + assert.equal(note().querySelector(".diff-thread-chip"), null, "I again draws every thread whole"); +}); + +test("minimized, the threads on no line take a row of their own for their chip, which has no line to float over", async () => { + const threads: ChangesThreads = { + list: [seatsThread(2, "why-42"), seatsThread(null, "was-3")], + draw: (thread) => createElement("p", null, thread.comments[0]?.body), + }; + await drawTab({ threads }); + press(listBox(), { key: "i" }); + await flush(); + const [outdated, current] = lastDrawn().files[0]?.annotations ?? []; + const chipOf = (key: string | undefined) => + lastDrawn() + .renderAnnotation("src/seats.ts", key ?? "") + .querySelector(".diff-thread-chip-row"); + assert.equal(chipOf(outdated?.key)?.hasAttribute("data-above"), true, "above the diff"); + assert.equal(chipOf(current?.key)?.hasAttribute("data-above"), false, "on its line"); +}); + /** The seats file at the pull request's head, ten lines, its fifth changed, and its patch as GitHub sends it. */ const HEAD_TEXT = `${["1", "2", "3", "4", "five", "6", "7", "8", "9", "10"].join("\n")}\n`; const BASE_TEXT = HEAD_TEXT.replace("five", "5"); diff --git a/packages/panel/src/tasks/agent-changes-tab.tsx b/packages/panel/src/tasks/agent-changes-tab.tsx index 97fd7f54f1..27002106ef 100644 --- a/packages/panel/src/tasks/agent-changes-tab.tsx +++ b/packages/panel/src/tasks/agent-changes-tab.tsx @@ -40,7 +40,7 @@ import { ImageIcon, RotateCwIcon, } from "lucide-react"; -import { type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { Fragment, type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useAct } from "../act.js"; import { appPlatform } from "../app-platform.js"; import { useCopyToast } from "../copy-feedback.js"; @@ -94,6 +94,7 @@ import { useFileViewSettings } from "./file-view-settings.js"; import { FilesChangedKeysSheet } from "./files-changed-keys.js"; import { ImageDiff, type ImageSides, imageDrawable } from "./image-diff.js"; import { LineComment, type PendingReview } from "./line-comment.js"; +import { MinimizedThreads } from "./minimized-threads.js"; import { PullRequestMark } from "./pull-request-mark.js"; import { OpenedBy, ReviewersLine } from "./pull-request-people.js"; import { PullRequestWriteNote } from "./pull-request-writes.js"; @@ -340,6 +341,62 @@ function AboveThreads({ ); } +/** Which of a file's lines show their threads while Minimize inline comments is on, and the door that shows or hides one line's. */ +interface FileMinimized { + readonly shown: (side: DiffSelection["side"], line: number) => boolean; + readonly show: (side: DiffSelection["side"], line: number, shown: boolean) => void; +} + +/** + * A file's threads on their lines while Minimize inline comments is on, as + * stage-cli draws them: each line's threads as one chip at the line's end + * (`MinimizedThreads`), standing in a row of no height, and under it, once + * the chip shows them, the threads as they are drawn otherwise. A line a + * comment is being written on always shows its threads, since the comment + * stands among them. + */ +function minimizedNotes( + placed: ReturnType["placed"], + draw: ChangesThreads["draw"] | undefined, + minimized: FileMinimized, + comment: DiffNote | undefined, +): DiffNote[] { + const lines = new Map>(); + for (const each of placed) { + const side = lines.get(each.side) ?? new Map(); + lines.set(each.side, side.set(each.line, [...(side.get(each.line) ?? []), each])); + } + return [...lines].flatMap(([side, onSide]) => + [...onSide].flatMap(([line, threads]) => { + const drawn = threads.map((each) => ( + {draw?.(each.thread, THREAD_PLACE.ON_LINE)} + )); + const [first] = threads; + if (first === undefined) return []; + // A line a comment is being written on shows its threads as they are, the comment among them. + const writing = comment?.side === side && comment.line === line; + return writing + ? threads.map((each, at) => ({ key: each.key, side, line, content: drawn[at] })) + : [ + { + key: first.key, + side, + line, + content: ( + each.thread)} + shown={minimized.shown(side, line)} + onShown={(shown) => minimized.show(side, line, shown)} + > + {drawn} + + ), + }, + ]; + }), + ); +} + /** * A file of the list as the bundle is handed it: its patch, its fold, its * header, and its notes: each review thread under its line, and those its @@ -361,6 +418,8 @@ function entryOf( comment: DiffNote | undefined, /** Whether the lines between its hunks may be opened, and why they could not be where a read was refused. */ gaps: { readonly open: boolean; readonly refused: ExpansionRefusal | undefined }, + /** Which of the file's lines show their threads while Minimize inline comments is on, and the door that shows or hides one; none while it is off. */ + minimized: FileMinimized | undefined, ): DiffListEntry { const path = file.path; const image = imageTypeOfPath(path) === undefined || images === undefined ? undefined : images; @@ -370,12 +429,30 @@ function entryOf( // Note that a pictured file draws no lines, so its threads stand above it, as on a binary file. const drawnAs = pictured === undefined ? file : { ...file, patch: null }; const { placed, above } = threadsOn(threads, drawnAs); - const notes: DiffNote[] = placed.map((each) => ({ - key: each.key, - side: each.side, - line: each.line, - content: draw?.(each.thread, THREAD_PLACE.ON_LINE), - })); + const notes: DiffNote[] = + minimized === undefined + ? placed.map((each) => ({ + key: each.key, + side: each.side, + line: each.line, + content: draw?.(each.thread, THREAD_PLACE.ON_LINE), + })) + : minimizedNotes(placed, draw, minimized, comment); + // The threads above the diff, as one list, or minimized to a chip of their own. + const aboveOf = (unplaced: typeof above, side: DiffSelection["side"]) => { + const list = ; + if (minimized === undefined) return list; + return ( + each.thread)} + shown={minimized.shown(side, FILE_NOTE_LINE)} + onShown={(shown) => minimized.show(side, FILE_NOTE_LINE, shown)} + above + > + {list} + + ); + }; // A pictured image, or a file with no hunks, stands as what is drawn in the place of its lines. const notice = fileNotice(file); const lead = @@ -392,7 +469,7 @@ function entryOf( content: ( <> {lead} - {above.length === 0 ? null : } + {above.length === 0 ? null : aboveOf(above, DIFF_SELECTION_SIDE.ADDITIONS)} ), }); @@ -404,7 +481,7 @@ function entryOf( key: ABOVE_KEY[side], side, line: FILE_NOTE_LINE, - content: , + content: aboveOf(unplaced, side), }); } if (lead === undefined && gaps.refused !== undefined) { @@ -492,6 +569,7 @@ function useEntries( images: ImageSides | undefined, commenting: ListCommenting | undefined, texts: FileTexts, + minimizedOf: (path: string) => FileMinimized | undefined, ): readonly DiffListEntry[] { const kept = useRef(new Map()); const next = new Map(); @@ -499,16 +577,18 @@ function useEntries( const own = threads?.list.filter((thread) => thread.path === file.path) ?? []; const comment = commenting?.note?.path === file.path ? commenting.note.content : undefined; const gaps = { open: texts.expandable(file), refused: texts.refused.get(file.path) }; + const minimized = minimizedOf(file.path); const inputs = [ ...entryInputs(file, list, own, threads?.draw, images, comment), gaps.open, gaps.refused, + minimized, ]; const was = kept.current.get(file.path); const entry = was !== undefined && sameInputs(was.inputs, inputs) ? was.entry - : entryOf(file, list, own, threads?.draw, images, comment, gaps); + : entryOf(file, list, own, threads?.draw, images, comment, gaps, minimized); next.set(file.path, { inputs, entry }); return entry; }); @@ -516,6 +596,57 @@ function useEntries( return entries; } +/** The lines that show their threads while Minimize inline comments is on: by file, by side, by line. */ +type ShownLines = ReadonlyMap>>; + +/** No line showing its threads. */ +const NONE_SHOWN: ShownLines = new Map(); + +/** + * Which lines show their threads while Minimize inline comments is on, + * each file's answer the same object while none of its lines was shown or + * hidden since, so the files whose lines did not change keep their entries; + * none while it is off, and every line minimized again each time it is + * turned on, as stage-cli does. + */ +function useMinimizedLines(minimize: boolean): (path: string) => FileMinimized | undefined { + const [shown, setShown] = useState<{ readonly of: boolean; readonly lines: ShownLines }>({ + of: minimize, + lines: NONE_SHOWN, + }); + const lines = shown.of === minimize ? shown.lines : NONE_SHOWN; + const toggle = useCallback( + (path: string, side: DiffSelection["side"], line: number, open: boolean) => + setShown((was) => { + const all = was.of === minimize ? was.lines : NONE_SHOWN; + const sides = all.get(path) ?? new Map>(); + const onSide = new Set(sides.get(side)); + if (open) onSide.add(line); + else onSide.delete(line); + return { of: minimize, lines: new Map(all).set(path, new Map(sides).set(side, onSide)) }; + }), + [minimize], + ); + const kept = useRef( + new Map< + string, + { readonly of: unknown; readonly toggle: unknown; readonly answer: FileMinimized } + >(), + ); + return (path) => { + if (!minimize) return undefined; + const own = lines.get(path); + const held = kept.current.get(path); + if (held !== undefined && held.of === own && held.toggle === toggle) return held.answer; + const answer: FileMinimized = { + shown: (side, line) => own?.get(side)?.has(line) === true, + show: (side, line, open) => toggle(path, side, line, open), + }; + kept.current.set(path, { of: own, toggle, answer }); + return answer; + }; +} + /** The key a comment being written stands under its lines by; a review thread's note is keyed by its page, which this never is. */ const COMMENT_KEY = "line-comment"; @@ -820,7 +951,9 @@ function FilesList({ boxRef: (element: HTMLDivElement | null) => void; }): React.JSX.Element { const look = useDiffLook(roomy); - const entries = useEntries(list, threads, images, commenting, texts); + const { settings } = useFileViewSettings(); + const minimizedOf = useMinimizedLines(settings.diffMinimizeComments); + const entries = useEntries(list, threads, images, commenting, texts, minimizedOf); const query = list.query.trim(); return (
@@ -965,7 +1098,10 @@ export function AgentChangesTab({ control.readPullRequestViewed, pullRequest === null ? undefined : reads.current.count, ); - const list = useChangesList(changed.files, viewedFiles.marks); + const viewSettings = useFileViewSettings(); + const minimize = () => + viewSettings.change("diffMinimizeComments", !viewSettings.settings.diffMinimizeComments); + const list = useChangesList(changed.files, viewedFiles.marks, minimize); const answered = reads.current.count; // Note that the images are named afresh only with an answer, because every changed image's entry is drawn from them. const images = useMemo( diff --git a/packages/panel/src/tasks/agent-pull-request-tab.test.ts b/packages/panel/src/tasks/agent-pull-request-tab.test.ts index faf1876509..0790001965 100644 --- a/packages/panel/src/tasks/agent-pull-request-tab.test.ts +++ b/packages/panel/src/tasks/agent-pull-request-tab.test.ts @@ -2860,7 +2860,7 @@ test("the title's edit is kept as the focus leaves its field, as it is on Enter" assert.equal(tab.querySelector('[aria-label="Pull request title"]'), null, "the field closes"); }); -test("Files changed draws each review thread under its line, its comments through the overview's own drawing, folded while Minimize inline comments is on", async () => { +test("Files changed draws each review thread under its line, its comments through the overview's own drawing, and while Minimize inline comments is on, as one chip at the line's end that shows it", async () => { const review = { kind: CONVERSATION_ENTRY.REVIEW, by: CHARLES_ON_GITHUB, @@ -2925,7 +2925,20 @@ test("Files changed draws each review thread under its line, its comments throug ); assert.equal(note.querySelector("details")?.open, true, "a current thread stands open"); await minimize(); - assert.equal(note.querySelector("details")?.open, false, "minimized, the thread stands folded"); + const chip = note.querySelector(".diff-thread-chip") ?? assert.fail("a chip stands"); + assert.equal(chip.getAttribute("aria-label"), "1 comment by Charles Pan · 1 open, show"); + assert.equal( + note.querySelector(".pull-request-thread-comment"), + null, + "minimized, no thread is drawn", + ); + act(() => chip.click()); + await settle(); + assert.ok(note.querySelector(".pull-request-thread-comment"), "the chip shows the line's thread"); + assert.equal( + note.querySelector(".diff-thread-chip")?.getAttribute("aria-label"), + "1 comment by Charles Pan · 1 open, hide", + ); }); test("a review thread on its line in Files changed is replied on and resolved from under its comments, as in the Conversation", async () => { diff --git a/packages/panel/src/tasks/agent-pull-request-tab.tsx b/packages/panel/src/tasks/agent-pull-request-tab.tsx index 42701a0408..10523867fc 100644 --- a/packages/panel/src/tasks/agent-pull-request-tab.tsx +++ b/packages/panel/src/tasks/agent-pull-request-tab.tsx @@ -103,7 +103,6 @@ import { useConversationFilters, } from "./conversation-filters.js"; import { PierreDiff, useDiffLook } from "./file-diff.js"; -import { useFileViewSettings } from "./file-view-settings.js"; import type { PendingReview } from "./line-comment.js"; import { blockedWords, MergeActions } from "./merge-box.js"; import { @@ -966,9 +965,9 @@ function ThreadComments({ /** * A review thread drawn on its line in Files changed, as GitHub's own diff * draws one: whether it was resolved or a later push outdated it, at its - * head, which folds it closed until opened, as Minimize inline comments - * folds every one; then its comments. An outdated one, whose line is gone, - * names the line it was made on. + * head, which folds it closed until opened; then its comments. An outdated + * one, whose line is gone, names the line it was made on. Minimize inline + * comments draws a line's threads as a chip instead (`minimized-threads.tsx`). */ function InlineThread({ thread, @@ -982,15 +981,12 @@ function InlineThread({ now: number; picture: GitHubPictureDrawer; }): React.JSX.Element { - const { settings } = useFileViewSettings(); const labels = threadLabels(thread); const lines = place === THREAD_PLACE.ABOVE ? threadLines(thread) : undefined; - const folded = labels.length > 0 || settings.diffMinimizeComments; const first = thread.comments[0]; return (
- {/* Note that the fold is drawn anew as the setting changes, so minimizing folds every thread at once. */} - + {first === undefined ? null : } {lines === undefined ? null : {lines}} diff --git a/packages/panel/src/tasks/changes-list.tsx b/packages/panel/src/tasks/changes-list.tsx index b8001cac11..7d778f50c2 100644 --- a/packages/panel/src/tasks/changes-list.tsx +++ b/packages/panel/src/tasks/changes-list.tsx @@ -36,7 +36,8 @@ import type { DiffListDoor } from "./diff-list.js"; * next and the previous file, `;` collapses or expands the file stood on, * ⇧; collapses every file the filter keeps, or expands them all once all * are collapsed, V marks the file stood on viewed on GitHub, or unmarks it, - * and steps on to the next, Escape lets go of the file stood on, and `?` + * and steps on to the next, I turns Minimize inline comments on or off, as + * stage-cli's does, Escape lets go of the file stood on, and `?` * opens the sheet that lists them all (`files-changed-keys.tsx`). Each is a * row of `@luke/settings/shortcuts`'s Files changed table, which the press is * matched against and the sheet and the Keyboard shortcuts page list, so a @@ -170,6 +171,8 @@ export interface ChangesList { export function useChangesList( files: readonly CodingAgentChangedFile[], marks?: ViewedMarks | undefined, + /** Turns Minimize inline comments on or off, as I does; none leaves I to the window. */ + onMinimize?: (() => void) | undefined, ): ChangesList { const [query, onQuery] = useState(""); const [chosen, setChosen] = useState>(new Map()); @@ -286,8 +289,8 @@ export function useChangesList( const onFile = (run: (path: string) => void) => current === undefined ? undefined : () => run(current); // What each of the view's keys does as the list stands, none where it has - // nothing to act on: every key but `?` needs a file to stand on, and V the - // marks read. Note that the record is total over the table, so a key + // nothing to act on: every key but `?` needs a file to stand on, V the + // marks read, and I a Minimize inline comments to turn. Note that the record is total over the table, so a key // added to the table does not build until it is handled here. const keyRuns = () => ({ @@ -307,6 +310,8 @@ export function useChangesList( markViewed(path); stepFrom(path, 1); }), + [FILES_CHANGED_KEY.MINIMIZE_COMMENTS]: + onMinimize === undefined ? undefined : onFile(() => onMinimize()), [FILES_CHANGED_KEY.SHOW_SHORTCUTS]: () => setKeysShown(true), }) satisfies Record void) | undefined>; const onKeyDown = (event: React.KeyboardEvent) => { diff --git a/packages/panel/src/tasks/minimized-threads.tsx b/packages/panel/src/tasks/minimized-threads.tsx new file mode 100644 index 0000000000..987724d6c8 --- /dev/null +++ b/packages/panel/src/tasks/minimized-threads.tsx @@ -0,0 +1,100 @@ +import type { ReviewThread } from "@luke/features/coding-agent-wire"; +import { actorName } from "@luke/ui/actor-model"; +import { CircleCheckIcon, CircleIcon } from "lucide-react"; +import type { ReactNode } from "react"; +import { ACTOR_SIZE, ActorMark } from "../actor-ref.js"; +import { Fold, FoldBody, FoldSummary } from "../ai-elements/fold.js"; +import { Tooltip } from "../tooltip.js"; +import { useNamer } from "./pull-request-people.js"; + +/** + * minimized-threads.tsx -- a line's review threads drawn small while Minimize inline comments is on, as stage-cli draws them: one chip at the end of the line, the faces of who wrote on it, how many comments, and whether every thread is resolved, which shows the threads or puts them away. + * + * The chip stands in a row of no height of its own, floated up over the end + * of the line it is on, so the diff reads as if the line held no comments; + * the threads, once shown, are drawn under the chip as they are while + * nothing is minimized. Shown or put away, the line is told the view + * (`onShown`), which keeps it so while the setting stands. + */ + +/** How many faces a chip draws before the rest go uncounted, as stage-cli's does. */ +const CHIP_FACES = 4; + +/** What a chip says of a line's threads, and what pressing it does. */ +const CHIP_WORDS = { + comments: (count: number) => (count === 1 ? "1 comment" : `${count} comments`), + open: (count: number) => (count === 0 ? "all resolved" : `${count} open`), + SHOW: "show", + HIDE: "hide", + SHOW_HINT: "Show comments", + HIDE_HINT: "Hide comments", +} as const; + +/** Everyone who wrote on the threads, each once, in the order they first wrote. */ +function writersOf(threads: readonly ReviewThread[]) { + const seen = new Map(); + for (const comment of threads.flatMap((thread) => thread.comments)) { + if (!seen.has(comment.by.login)) seen.set(comment.by.login, comment.by); + } + return [...seen.values()]; +} + +/** + * One line's threads minimized: the chip, a fold's summary, which says who + * wrote on them, how many comments, and how many are still open, said whole + * to a screen reader with what pressing it does ("3 comments by dana, sam · + * 1 open, show"); and the threads it folds, drawn under it once shown. Note + * that a fold is what lets the tab's find reach the threads while they are + * put away, and open the line whose words it brings into view. + */ +export function MinimizedThreads({ + threads, + shown, + onShown, + above = false, + children, +}: { + threads: readonly ReviewThread[]; + /** Whether the threads stand above the diff, on no line, where the chip takes a row of its own rather than floating over a line. */ + above?: boolean; + /** Whether the threads stand under the chip. */ + shown: boolean; + /** Told the threads were shown or put away, by the chip or by the find. */ + onShown: (shown: boolean) => void; + /** The threads as they are drawn while nothing is minimized. */ + children: ReactNode; +}): React.JSX.Element { + const name = useNamer(); + const writers = writersOf(threads).map(name); + const count = threads.reduce((sum, thread) => sum + thread.comments.length + thread.more, 0); + const open = threads.filter((thread) => !thread.resolved).length; + const label = [ + `${CHIP_WORDS.comments(count)} by ${writers.map((writer) => actorName(writer)).join(", ")}`, + `${CHIP_WORDS.open(open)}, ${shown ? CHIP_WORDS.HIDE : CHIP_WORDS.SHOW}`, + ].join(" · "); + return ( + + + + {open === 0 ? ( + + + {children} + + ); +} diff --git a/packages/panel/src/tasks/pull-request-people.tsx b/packages/panel/src/tasks/pull-request-people.tsx index e62ccb68ea..4f1578e108 100644 --- a/packages/panel/src/tasks/pull-request-people.tsx +++ b/packages/panel/src/tasks/pull-request-people.tsx @@ -51,7 +51,7 @@ const REVIEW_GLYPHS = { } as const satisfies Record; /** Names the people as the members around the caller say. */ -function useNamer(): (person: GitHubPerson) => Actor { +export function useNamer(): (person: GitHubPerson) => Actor { const { members, readerId, former } = useMemberDirectory(); return (person) => githubActor(person, { members, readerId, former }); } diff --git a/packages/settings/src/shortcuts.ts b/packages/settings/src/shortcuts.ts index 82249cd768..20a03aeee5 100644 --- a/packages/settings/src/shortcuts.ts +++ b/packages/settings/src/shortcuts.ts @@ -590,6 +590,7 @@ export const FILES_CHANGED_KEY = { TOGGLE_ALL_FILES: "files-changed-toggle-all-files", LET_GO: "files-changed-let-go", MARK_VIEWED: "files-changed-mark-viewed", + MINIMIZE_COMMENTS: "files-changed-minimize-comments", SHOW_SHORTCUTS: "files-changed-show-shortcuts", } as const; @@ -605,8 +606,9 @@ interface FilesChangedShortcut { * Files changed's keys: the one table the view's handler matches a press * against, and its `?` sheet and the Keyboard shortcuts page list, so what * is listed is what answers. Escape lets go of the file stood on only while - * one is, since otherwise it is the window's layered Escape, and V answers - * only once the member's viewed marks are read. + * one is, since otherwise it is the window's layered Escape, V answers + * only once the member's viewed marks are read, and I only where Minimize + * inline comments can be turned. */ export const FILES_CHANGED_SHORTCUTS = { [FILES_CHANGED_KEY.NEXT_FILE]: { @@ -633,6 +635,10 @@ export const FILES_CHANGED_SHORTCUTS = { label: "Toggle viewed and go to next file", chord: { key: SHORTCUT_KEY.V, plain: true }, }, + [FILES_CHANGED_KEY.MINIMIZE_COMMENTS]: { + label: "Minimize or show inline comments", + chord: { key: SHORTCUT_KEY.I, plain: true }, + }, [FILES_CHANGED_KEY.SHOW_SHORTCUTS]: { label: "Show keyboard shortcuts", chord: { key: SHORTCUT_KEY.SLASH, plain: true, shift: true }, @@ -656,7 +662,10 @@ export const FILES_CHANGED_SHORTCUT_GROUPS = [ FILES_CHANGED_KEY.LET_GO, ], }, - { title: "Review", keys: [FILES_CHANGED_KEY.MARK_VIEWED] }, + { + title: "Review", + keys: [FILES_CHANGED_KEY.MARK_VIEWED, FILES_CHANGED_KEY.MINIMIZE_COMMENTS], + }, ] as const satisfies readonly { title: string; keys: readonly FilesChangedKey[] }[]; /** The Keyboard shortcuts page's group of Files changed's keys, and its settings search's words for them. */