fix(desktop): minimize inline comments to one chip per line, as stage-cli does

Merged
DA
18h ago
+454-33

Summary

The developer wasn't sure "Minimize inline comments" was working.

Cause: this isn't a regression. The setting worked as #3245 built it: it folded each open thread to its head row, live, both from the popover and from Settings › Appearance › Diffs. But resolved and outdated threads are always folded already, so on a pull request whose threads were mostly resolved the toggle changed nothing you could see. And even when it folded something, a minimized thread still took up a full row. stage-cli's setting of the same name means something different, and the developer asked for stage-cli's meaning.

What it does now (as stage-cli does):

  • While the setting is on, each line's threads, resolved ones included, are drawn as one chip at the end of the line, in a row of zero height. The diff reads as if the line had no comments.
  • The chip shows the faces of who wrote on the line, the comment count, and an open or resolved mark.
  • The chip is a real control: it's a disclosure fold's <summary>, so it's keyboard-reachable. It has an accessible name, e.g. "3 comments by Dana, Sam · 1 open, show". Its tooltip reads "Show comments" or "Hide comments".
  • Pressing the chip shows or hides that line's threads. The choice holds until the setting changes; turning it back on minimizes every line again, as stage-cli does.
  • A line with a comment being written on it always shows its threads.
  • The threads above the diff (outdated, or on lines no hunk shows) get a chip of their own.
  • I turns the setting on and off, but only while focus is inside Files changed, as J/K do. Plain I was free: the app only uses ⇧I (Inbox) and ⌥I. It is a row of FILES_CHANGED_SHORTCUTS (Review group, "Minimize or show inline comments"), added after #3795, so the ? sheet and the Keyboard shortcuts page list it.
  • ⌘F still reads the words of a minimized line's threads, because the fold's body is drawn hidden while a find looks through the tab. A find that brings a match into view opens that line.
  • Layout: Pierre re-measures the annotation row as the chip opens and closes. I checked in the harness:
    • The next line moves 107 px down when the thread is shown, and back when it's hidden.
    • The list's scroll height follows (+136 px when shown).
    • With the list scrolled, the sticky file header stays at offset 0 through open and close.
  • InlineThread no longer folds by the setting. Resolved and outdated threads still start folded.

Screenshots

PR #2908's build.mjs with a review thread on line 356, Minimize inline comments on, before and after:

lightdark
before (main)
after

The chip pressed: the line's thread is shown under it.

Evidence

  • tsc --noEmit for panel and desktop: clean.
  • ./scripts/oxlint.sh: 0 errors.
  • pnpm knip: clean.
  • ./scripts/repository-checks.sh: passed.
  • biome check: no errors. The 17 warnings are the same as on main.
  • vitest run --project panel --project desktop: 230 files, 2336 tests, all passed.
  • Tests:
    • agent-pull-request-tab.test.ts: the #3245 test now expects, once minimized, the chip with its accessible name and no thread body, then the thread shown after pressing the chip. It fails on main, which draws the folded head instead.
    • agent-changes-tab.test.ts (new):
      • I turns minimize on inside the view, and does nothing outside it.
      • While a find looks through the tab, findRanges reads the minimized thread's words, and reveal opens the line.
      • I again draws every thread whole.

Invariants

  • Checks green locally, as listed above.
  • JSON Schema goldens unchanged.
  • Behaviour change tested.
  • No new as.
  • No raw async primitives.
  • No discarded Effect.
  • Test count +1.
  • PRIVACY.md unchanged: the chip's words sit in Files changed, which is already left out of the screen recording.
  • Deps unchanged.

Made with Luke


Devin Review