glitter: top bar contrast and padding, tab button parity, debounced diff loads - #363
Merged
Merged
Conversation
Add a leading blank row to renderSegment's block (now 4 rows instead of 3) so the hover/open fill covers the padding along with the label and value rows. hitTest derives topH from renderTopBar's own height, so the extra row propagates to hit-testing with no separate offset change. The column divider between segments is rebuilt to match the new 4-row segment height, since lipgloss.JoinHorizontal pads a shorter block with unstyled filler rows rather than sharing the neighboring background. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a pad row above the Changes/History label row so the tab strip is a 3-row button (pad, label, underline) instead of 2. The pad and label rows are the button: both hover and both click resolve to the same target. The underline row is only the active-tab indicator now -- sidebarHit no longer treats it as part of the button, closing the gap where a click landed on cells that never painted hover. sidebarFixedTopRows moves from 7 to 8 to account for the extra row, and sidebarHit's row arithmetic shifts to match. Existing geometry tests and PTY-level mouse-click tests are updated for the new row offsets; a few pick a different (still-visible) target row since the combined padding from both this change and the top-bar padding change leaves less room for the Changes list at a fixed terminal height. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Uses the half-block technique from renderCommitButton: the pad row's bottom half now carries the segment's own live background (rest, hover, or open) as a half-block foreground over a theme.Bg canvas, instead of painting a full line of that background. The divider's first line gets the same treatment so it doesn't notch against its half-height neighbors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The trailing row after value was still full height while the pad row above the label became a half block in 320ddd9, reading asymmetric. It now mirrors the pad row: the segment's live background paints as the upper half-block's foreground over a theme.Bg canvas, tracking rest/hover/open the same way the pad row does. The divider's last line gets the matching glyph so it doesn't notch against its half-height neighbors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The top bar at rest painted BgSubtle, which is lighter than the board canvas (theme.Bg), so the bar never visually separated from it. Adds theme.TopBarBg (#100D1C), darker than Bg the way GitHub Desktop's toolbar sits darker than its content area, and wires it as the top bar's rest fill in segmentBaseColor and the divider cells so the whole bar reads as one band. BgSubtle is untouched everywhere else (the picker's selected-panel strip, the diff staging hint, the stash strip) since those were never part of this complaint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The prior attempt darkened TopBarBg to separate the bar from the canvas, copying GitHub Desktop's toolbar/content contrast in the wrong direction: that app is light and steps down to a near-black toolbar, but rt's canvas is already near-black, so the only direction with room is up. TopBarBg is now #262038 (lighter than Bg's #161224); HoverBg was too close to the new rest fill to read as a step up, so the bar gets its own TopBarHoverBg (#363058) instead of raising the shared HoverBg other surfaces depend on. Resolved luminance ordering: Surface (open) < TopBarBg (rest) < TopBarHoverBg (hover). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cursorSelectCmd emitted mission:select on every cursor movement, so scrolling fast through the Changes list fired a diff load per row passed over. It now moves the cursor instantly but schedules the select behind a 150ms tea.Tick, gated by a generation counter that each movement bumps: a tick whose captured generation no longer matches the current one is a superseded movement and is a no-op, so a fast scroll settles once instead of thrashing through every row it passed. Clicking a row or its checkbox still emits immediately, since each is a deliberate act rather than a pass-over. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 79 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Comment |
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 free
to 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.
What
Board polish, all from driving
rt glitterand reacting to what it looked like.The top bar now reads as its own band. It was
#1C162Cagainst a#161224canvas, six points lighter, which merged into the board. The first attempt went darker, on the theory that GitHub Desktop's toolbar is darker than its content. That was the wrong lesson from the right reference: GHD is a LIGHT app, so its dark toolbar is a large contrast step. In a dark app the content is already near-black and the only direction with room is up.So the bar gets its own tokens, lifted above the canvas:
Surface#221A35TopBarBg#262038TopBarHoverBg#363058TopBarBgis now 2.37x the canvas's luminance, up from 0.66x. Hover needed its own token because the sharedHoverBgsat only 1.6x above the new rest fill, too close to read as a step;HoverBgitself is untouched, so the rows, diff lines and foldout rows that share it are unaffected. Open deliberately keepsSurface, which is the foldout panel's own colour, so an open segment merges with its dropdown the way GHD's does.Two tests pin the DIRECTION rather than the values, since a value-equality test would have sailed straight past the darker-than-canvas mistake.
Half-row padding, top and bottom. Each segment gained a row of padding above and below, then both were shrunk to half rows with
▄and▀, the same half-block technique the commit button uses for its caps. The glyph's foreground tracks the segment's live background, so a hovered or open segment's fill reaches through the padding rather than leaving a stripe. The column dividers got the matching treatment on both rows; a full-height divider cell against half-height neighbours leaves a visible notch, which the background-coverage tests caught during the build.The Changes/History tabs are a real button. The strip was two rows, both clickable, but only the label row painted hover, so the highlight covered half of its own click target. It is now three rows: a pad row and the label row form the button, both hovering and both clicking, with the underline staying the active-tab indicator and doing neither. A test asserts the hovered cells and the clickable cells are exactly the same set, so changing one without the other fails.
Scrolling no longer thrashes the diff pane. Every cursor move emitted
mission:select, so a ten-row scroll fired ten diff loads and ten repaints. Cursor movement stays instant; the emission now waits 150ms for the cursor to settle. A generation counter carried into each tick makes superseded ticks no-ops, which is what makes it a debounce rather than a throttle: without it a fast scroll still emits a trailing burst for rows you flew past. Clicks stay immediate, since clicking a row is a deliberate act rather than passing over it.Verification
go vet ./...clean;go test -count=1 ./...all packages ok.bash scripts/repo-purity.shok;bun run test:pty4 pass.The debounce is tested deterministically rather than with sleeps: the tick is behind a package-var indirection mirroring the existing
nowFnoverride in that file, so the generation logic is exercised directly. Its tests cover a burst collapsing to one emission carrying the final path, returning to the origin row emitting nothing, clicks staying immediate, and a click mid-debounce not being double-fired by a later stale tick.Two commits on this branch undo each other (darken, then lighten). Left in place deliberately: this squash-merges to one commit, so main never sees the detour, and the second commit message records why the first was wrong.
🤖 Generated with Claude Code