Repository navigation
style(desktop): Files changed as cards that share one edge, with a quiet fold and a clearer header - #3794
Conversation
…-cli does Minimize inline comments folded each open thread to its head row, and left resolved and outdated threads as they were, since those already stand folded; on a pull request whose threads were resolved it changed nothing to see. It now means what stage-cli's means: each line's threads draw as one chip at the end of the line, in a row of no height, so the diff reads as if the line held no comments. The chip names who wrote on the line, how many comments, and how many are open; pressing it shows or hides the line's threads, and the tab's find reads them and opens the line it brings into view. I turns the setting on and off inside Files changed. Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
…on no line a row for their chip Two findings from review. The chip's fold was drawn anew each time it opened or closed, so a find that opened a resolved thread's own fold inside it had that fold drawn closed again, and a keyboard press lost the focus; the fold is now kept and told whether it is open. And the chip of threads on no line, above the diff or under a file's picture, floated up over what stood before it; it now takes a row of its own. Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
…iet fold and a clearer header Each file's code area kept room for a scroll bar up and down, which a Mac that always shows scroll bars drew as a strip at the end of every line, so the rows and the bands between hunks stopped short of the file's header. The code area keeps none now, and the header, the bands, the rows, and the threads share one left and one right edge. Each file is a card on the panel's ground, rounded whole and set apart by the room between files; its header stuck at the top stands square, the card's rounded top scrolled away. The fold is a compact quiet mark with no fill while its file is open. The header names the file's folder quietly and its name in the full ink, cutting the folder first where the row runs short; its letter stands on a tint of its own colour; and Viewed says V in its hint. The bands between hunks are 24 px and quieter, and the tree's search stands level with the first card. Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
…anged-design Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
Adds I to Files changed's shortcut table (Review group) and to its dispatch record, after #3795. Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
Viewed's hint now reads V from Files changed's shortcut table (#3795). Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com>
| <span className="diff-list-viewed"> | ||
| <Checkbox checked={viewed} onChange={onToggle}> | ||
| {LIST_WORDS.VIEWED} | ||
| </Checkbox> | ||
| </span> |
There was a problem hiding this comment.
🟡 Viewed shortcut missing from checkbox
When the Viewed checkbox takes focus, Tooltip places its shortcut on the surrounding span. The nested Checkbox input receives focus instead, so its shortcut is not announced.
Learn more
A tooltip attaches aria-keyshortcuts and its description to the element it wraps. Here that element is a span, but the interactive Viewed control is the nested input rendered by Checkbox. Keyboard focus reaches the input, which has neither attribute. The shortcut remains visible in the pill but is absent from the focused control's accessibility metadata.
Example: A screen-reader user tabs to a file's Viewed checkbox. The tooltip advertises V visually, while the focused input announces only “Viewed, checkbox.”
Recommended fix: Attach the shortcut to the checkbox input, either by extending Checkbox to forward the needed attributes and tooltip events or by passing the shortcut through explicitly. Add an accessibility assertion for the focused input.
Was this helpful? React with 👍 or 👎 to provide feedback.
| <Tooltip | ||
| label={viewed ? LIST_WORDS.UNMARK_VIEWED : LIST_WORDS.MARK_VIEWED} | ||
| chord={VIEWED_CHORD} | ||
| > |
There was a problem hiding this comment.
🟡 Viewed hint lacks a behavior test
The new Tooltip changes Viewed's interaction without a test that fails when the hint is absent. Existing Files changed tests cover marking and V separately, but not this hint.
Learn more
The Viewed checkbox gains a hover and focus hint with a label that depends on its checked state. No test was added in this PR, and Files changed tests verify the checkbox and the V action separately without checking the new hint. The testing rule in AGENTS.md requires a test that fails without every behavior change.
Example: Removing this tooltip leaves the existing tests green, even though a file no longer displays “Mark as viewed V” on hover.
Recommended fix: Add a focused panel test that hovers or focuses the Viewed control, verifies the hint and its V chord, and checks that marking the file changes the hint to “Mark as not viewed.”
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
The developer asked for a design pass on Files changed:
This PR is stacked on #3789, since it touches the same chip rules.
Why the edges didn't line up. Pierre gives every file's code area
scrollbar-gutter: stable, and that area only scrolls sideways (overflow-x: scroll). On a Mac set to show scroll bars always, or in any Chromium drawing classic bars, that reserves a ~10 px strip at the end of every line. So the rows and the "N unmodified lines" bands stopped short of the header, which sits outside that box. Measured in Electron at 900 px: header 0–626, rows 0–616. The list's own scrollbar then added a second strip.What changes
[data-code] { scrollbar-gutter: auto }is set in the diff's shadow CSS. Header, hunk bands, code rows and thread cards now share one left and one right edge. Lines wider than the panel still scroll sideways.gap/paddingTop, so the virtualizer measures them).IconButton: no fill at rest or while open. The filled square was@luke/ui'saria-expanded="true"look for menu buttons. It's still a real button witharia-expandedand its;key.@luke/ui's Checkbox, now with a hint: "Mark as viewed V".hunkSeparatorHeight) so the list stays measured.SHORTCUT_KEYgainsVfor the hint, with a note that bare V is Files changed's.Screenshots
From Electron under xvfb with classic scrollbars, as on a Mac with "Show scroll bars: Always". PR #2908, with Minimize inline comments on: a minimized thread chip on line 356, the collapsed "347 unmodified lines" gap, and a folded file. Before is #3789.
Evidence
tsc --noEmitfor settings, panel and desktop: clean../scripts/oxlint.sh: 0 errors.pnpm knip: clean../scripts/repository-checks.sh: passed. Every new CSS variable is a defined--luke-*role.biome check: no errors.vitest run --project panel --project desktop --project settings: 234 files, 2353 tests passed.Invariants
as.PRIVACY.mdunchanged.Made with Luke