Skip to content

feat(web): keyboard review flow — advance after marking, r/o shortcuts, vim scrolling - #40

Merged
imfing merged 6 commits into
mainfrom
worktree-review-scroll-next-file
Oct 19, 2026
Merged

imfing merged 6 commits into
mainfrom
worktree-review-scroll-next-file

Conversation

@imfing

@imfing imfing commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Marking reviewed advances to the next file. Pressing the shortcut used to leave the viewport stranded: with "hide reviewed" on, CodeView remounts and restored a stale scrollTop (landing at the top or on an unrelated file); with it off, the file just collapsed in place. Now it scrolls to the next file's header like n does. The hide-reviewed path scrolls after the remount via a pending target instead of the saved offset; the collapse path waits two frames so the target offset isn't computed from pre-collapse heights.
  • Fix: the shortcut could mark the wrong file. Right after n, the previous file's sticky header is still in the DOM at about −6px while the current header sits 9px below the scroll area's top, and getCurrentFile only allowed 8px, so it picked the previous file. Tolerance is now 24px (CURRENT_FILE_HEADER_SLOP_PX).
  • r marks a file reviewed (matches the "Reviewed" label). m still works as an undocumented alias.
  • o expands or collapses the current file, so a reviewed (auto-collapsed) file can be opened without un-marking it. Collapsing a reviewed file no longer records a manual collapse, so un-marking it later expands it again.
  • Vim-style scrolling: j/k (60px), d/u (half page, Vimium-style since Ctrl-d/u belong to the browser), gg/G (top/bottom). Single presses scroll smoothly; key repeat scrolls instantly.
  • Shortcuts dialog and tooltips updated.
  • Fix: scroll position wasn't restored on page reload (pre-existing; v0.7.0 does the same). The restore ran two frames after mount, before CodeView had laid out its content, so scrollTop clamped to 0. It now retries each frame until the content is tall enough (3s deadline) and stops if the user scrolls first.
  • Tidy-up: vim keys folded into the keydown switch, scrolling via CodeView's containerRef, a stable empty set so marking reviewed doesn't re-run the scroll-restore effect, toggleReviewed returns the new state, one afterLayout() helper, and comments that restated the code removed.

Testing

  • oxlint and oxfmt --check pass; vite build succeeds.
  • Verified the release build with headless Chrome on a 6-file fixture and on this repo's diff since v0.5.0, with "hide reviewed" both on and off:
    • n n + scroll + r marks the file in view (it used to mark the previous one); repeated r walks c → d → e → f.
    • p back to a reviewed file, o expands it while it stays reviewed, o collapses it, r un-marks it and it expands.
    • j/k/d/u move by the expected amounts; G reaches the exact bottom (166,605px on the large diff), a single g does nothing, and gg returns to the top.
    • Reload restores the exact offset: 4321 / 21,605px on the fixture, 4321 / 86,420px on the large diff (previously 0).

Marking a file reviewed via the keyboard now scrolls to the next file instead
of restoring a stale scroll offset (which could land at the top or on an
unrelated file when hide-reviewed remounts the view). Also widen the
current-file header tolerance so a pinned header isn't mistaken for the
previous file, which made m mark the wrong file.
r matches the Reviewed button label; m still works but is no longer listed.
Lets a reviewed (auto-collapsed) file be reopened from the keyboard without
un-marking it. Collapsing a reviewed file no longer records a manual
collapse, so un-marking it later expands it again.
j/k scroll about three lines, d/u half a page (Vimium-style, since Ctrl-d/u
belong to the browser), gg/G jump to the top/bottom. Single presses scroll
smoothly; key repeat scrolls instantly so holding a key doesn't lag.
- Fold the vim scroll keys into the keydown switch; read clientHeight only for d/u.
- Scroll via CodeView's containerRef instead of the area's firstElementChild.
- Return a stable EMPTY_SET when nothing is hidden so marking reviewed no longer
  re-runs the scroll-restore effect; call scrollToFile from it via useEffectEvent.
- toggleReviewed returns the new state; toggleFileCollapsed uses reviewedNames.
- Share one afterLayout() double-rAF helper; drop comments that restate the code.
The restore ran two frames after mount, before CodeView had laid out its
content, so scrollTop clamped to 0. Retry each frame until the content is
tall enough (or a 3s deadline passes), and stop if the user scrolls first.
@imfing
imfing merged commit 4b26fb0 into main Oct 19, 2026
8 checks passed
@imfing
imfing deleted the worktree-review-scroll-next-file branch October 19, 2026 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant