Uh oh!
There was an error while loading. Please reload this page.
feat(walkthrough): start at no stop, and mark the stops in the diff - #26
Merged
Conversation
The stepper opened on stop 1 without having scrolled there, so the first press of "next" moved to stop 2 and stop 1 was never shown. It now opens at 0/n with a Start button, and every highlighted hunk says which stop it belongs to instead of all of them claiming to be the current one. A lamp marks where each stop begins — in the gutter and in the file list, with a count when a file is visited more than once — so a reader scrolling the diff can still see what the walkthrough had to say. Clicking a lamp jumps to that stop. Jumping now lands on the stop's lines rather than the top of the file, and sets the active file directly; the scroll observer's threshold used to leave the sidebar pointing at the previous file until the reader scrolled by hand. Both navigators gained a jump-to-first control. Also declares two types that were already being used: the split view's onMouseDown carries the shift key, and ToolbarProps has reviewInProgress. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
…op index Marking a file viewed collapsed it and let the page shorten under the reader, leaving them among files they had already scrolled past. The header is sticky, so it was under the cursor when it was clicked; the collapsed file now goes back to that spot. Only when the reader was inside the file — a file whose header was already in view stays put. The stop index had two clamps: the stepper's own, for display, and the raw value everywhere else. A walkthrough replaced mid-session could therefore show "3/3" in the header with nothing marked current in the diff. It is clamped once now, where it lives, and a new walkthrough resets the position instead of resuming at a stop from the old one. A lamp for a stop starting between two hunks had nowhere to go and silently vanished while the file list kept counting it; it now moves to the first line of the stop the diff renders. And a hunk holding two stops says which one you are on, matching the highlight it already got. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
The check ran after the file had collapsed, by which point the page is shorter, the browser may already have clamped the scroll, and the file no longer looks like the one the reader was inside — so the case worth fixing was the one the guard skipped. The question is now asked while the file is still open, and only the scroll happens afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
The highlight explained itself in a native tooltip, which appeared over the code being reviewed. That tooltip is gone. Hovering the bulb now opens a wheat note that expands to the left — over the old side in split view, so it never covers the new code — carrying the stop's annotation and its whole body. No click needed. One bulb per line rather than per stop: stops that share a line are one thing to say about that block, and their notes are merged into a single note. The dimming keeps its own hover text, which explains a hunk the tool chose to play down and is the one thing a reader cannot infer. Clicking a bulb lands the line at the top of the view rather than the centre, offset by the file's sticky header so the line jumped to is not the one hidden behind it. A stop pointing at unchanged code no longer goes unmarked: the diff expands the gap holding it, the way the reader would by hand. Bounded to gaps of 200 lines, past which the gap's own controls are the better tool. Replaces the lamp-snapping from the previous commit, which put the mark on a nearby line instead of showing the line it was about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six things about the walkthrough, all from watching it get used on #25.
It opened on stop 1 without having gone there. So the first press of "next" moved to stop 2 and
stop 1 was never shown. It now opens at
0/nand the down arrow reads "Start the walkthrough";before it starts, the header shows the walkthrough's topic rather than a stop it hasn't scrolled to.
Every highlighted hunk said "The walkthrough points here." All of them, at once, regardless of
which stop the header was showing — so the copy told you nothing. Each now names its stop
(
Walkthrough stop 3 — the marker), the current one says(you are here), and it gets a strongerhighlight than the rest.
Nothing marked the stops in the diff. A reader who scrolls rather than steps had no way to see
which parts the walkthrough had something to say about. A lamp now sits where each stop begins — in
the gutter, next to the line numbers, in both unified and split view — and in the file list, with a
count when a file is visited more than once. Clicking a lamp jumps to that stop.
Jumping landed on the top of the file and left the sidebar pointing at the previous one until you
scrolled by hand: the active file came from a scroll observer with a threshold the jump didn't cross.
Jumps now scroll to the stop's own lines (
data-new-lineanchors the row) and set the active filedirectly, the way the comment navigation already did.
Jump-to-first, left of the arrows, for both the walkthrough and the comments. Appears only once
you are past the first, since before that it would do nothing.
Also declares two types that were already in use and had drifted: the split view's
onMouseDowncarries the shift key it was already being handed at runtime, and
ToolbarPropshas thereviewInProgressit already reads. Both were type errors in files this PR touches.Tests
51 new, 239 green in
@diffity/ui, 407 across the workspace. The navigation arithmetic, the stoplabels and the mark grouping are pure functions with their own tests; the stepper, the file list, the
gutter lamps and the comment toolbar are covered through jsdom renders.
Two type errors remain in the UI package and are older than this branch —
dashboard.tsxreads aloadingfield that its hook does not return, andtoolbar.tsxpasses a partialGitHubDetails.Left alone rather than guessed at.
🤖 Generated with Claude Code
https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs