fix: make comment review UI scrollable and dark-mode aware - #119
fix: make comment review UI scrollable and dark-mode aware#119benglewis wants to merge 4 commits into
Conversation
|
| Topic | Details | |||
|---|---|---|---|---|
| Themed interaction prompts | Standardize interactive prompt rendering and status feedback across integration setup, post-review actions, merge confirmation, pull-request selection, walkthrough updates, and specification-review prompts by sharing themed indicators and item renderers.Modified files (15)
Latest Contributors(0)
| |||
| Scrollable review flows | Keep comment, diff, chat, and requirement content within the available terminal height using ScrollableViewport, reserved-row layout tracking, resize handling, compact chrome, keyboard scrolling, follow-content behavior, and navigation ownership for autocomplete. Apply the layout architecture across review, issue, PR chat, and specification-review flows.Modified files (13)
Latest Contributors(0)
| |||
| Adaptive terminal theme | Resolve terminal-aware palettes through getTheme, supporting dark/light detection, explicit theme selection, NO_COLOR, per-color environment overrides, theme files, and readable foreground/background combinations. Replace hardcoded rendering colors in diffs, review status displays, chat tooling, and related review UI while preserving legacy color exports.Modified files (15)
Latest Contributors(0)
| |||
| Appearance documentation | Document the new appearance and scrolling contract, including navigation keys, compact-window behavior, theme configuration, override precedence, and colorless rendering options.Modified files (1)
Latest Contributors(0)
|
|
Your organization's Advanced Security usage limit has been reached. To continue using Advanced Security reviews, please upgrade your plan or increase your usage limits in your account settings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d41ded101b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const requirementHeader = ( | ||
| <Box flexDirection="column" flexShrink={0}> | ||
| <Box marginBottom={1}> | ||
| <Text color={MAIN_COLOR} bold> | ||
| Unmet requirement ({currentIndex + 1}/{unmetRequirements.length}) |
There was a problem hiding this comment.
Duplicate requirement headers drift independently
SpecReviewBrowser.requirementHeader duplicates the substantial requirement-header structure in MetRequirementBrowser (see src/pages/SpecReview/MetRequirementBrowser.tsx:213-248), so changes must be synchronized manually and can leave the review screens inconsistent — should we extract a shared RequirementHeader with explicit props for label/count and verdict/explanation presentation, leaving those differences to the callers?
Want Baz to fix this for you? Activate Fixer
The PR comments review screen rendered its comment, diff and conversation at full height, so on a short window the input box and hints were pushed off screen. Diff rows also set a background color without a matching text color, which left them unreadable on dark terminals. - Add a ScrollableViewport that clips content to the rows the terminal has left and scrolls with arrows / PgUp / PgDn / Ctrl+U / Ctrl+D, following new chat output unless the user scrolled up. - Add a ScreenLayout context so chrome that must stay visible (banner, selected PR line, chat input) reports its height and the viewport can size itself to what remains. The banner and the input hints collapse in short windows, where their rows cost more than they are worth. - Use it for the comments browser, PR chat and the requirement browsers. - Add a theme layer that adapts to the terminal background, always pairing a background with a readable foreground. It can be forced or turned off (BAZ_THEME, NO_COLOR) and every color can be overridden through BAZ_COLOR_* or a theme.json file. Without colors, the lines a comment points at are shown in bold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
d41ded1 to
e7cfb7d
Compare
- Restart scrolling from the top when the reviewed item changes, so /next does not open the next comment partway down (scrollResetKey). - Never let the viewport exceed the window: the height is clamped to the rows left after chrome, and the status line yields its row to content when there is almost nothing to spare (which also avoids Ink's mismeasurement of a one-row viewport). - Stop scrolling the conversation while the mention list owns the arrow keys, and size that list to the rows the terminal can spare. - Keep a background only when it has a readable foreground: a background with no color of its own borrows the detected palette, and turning a foreground off with BAZ_COLOR_*_FG=none drops its background too. - Ignore non-string values in a theme file instead of letting them erase defaults or crash theme-name parsing. - Apply lineNumberFg to diff gutters, except on highlighted rows where the row's own foreground is what stays readable. - Route the remaining hardcoded colors through the theme, including "black" review states that were invisible on dark terminals, so NO_COLOR and BAZ_THEME=none really do produce no color. - Add a semantic `text` color for places that must not inherit the terminal's own foreground. - Fix "No diff related to the issue!" grammar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The indicator renderer was byte-identical in all nine SelectInput prompts and the item renderer in eight, so a styling or colorless-mode change had to be repeated in every one. Extract both into SelectRenderers and pass them through; ReviewMenu keeps its own item renderer for the inline chat box and completion marks, and uses the shared indicator. Also reword a test comment for clarity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The 8-footer-row case left a 2-row budget, so it exercised the status line giving up its row but never the collapse itself. Run the case over footer sizes that leave budgets of 2, 1, 0 and -1, asserting the chrome stays whole and content disappears once there is no room for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Problem
Two issues with the PR comments review UI:
backgroundColorwithout a matching text color, so light pastel backgrounds were painted under the terminal's own light foreground. Colors were hardcoded, with no way to adapt or turn them off.Changes
Scrolling
ScrollableViewportclips its content to the rows the terminal has left and scrolls with↑/↓,PgUp/PgDnandCtrl+U/Ctrl+D. A status line reports the visible range (↑↓ lines 12-30 of 84). It follows new chat output, unless the user has scrolled up.ScreenLayoutcontext: chrome that must stay on screen (banner, selected-PR line, chat input) measures itself and reports its height, so the viewport sizes itself to whatever is left. No fixed root height, so screens without a viewport still overflow into the terminal's own scrollback rather than being silently truncated.Theming
src/theme/theme.tsresolves a theme once per process:COLORFGBGwhere available, dark assumed otherwise),BAZ_THEME=auto|dark|light|none,BAZ_TERMINAL_BACKGROUND=dark|light, andNO_COLORsupport,BAZ_COLOR_*env vars or atheme.json($BAZ_THEME_FILE,./.baz/,~/.baz/,~/.config/baz/), withnoneto unstyle a single element."cyan"/"magenta"/"red"colors in the review path now come from the theme, so they honorNO_COLOR.theme/colors.tskeeps its exports, now theme-derived, so untouched screens follow along.Documented under Appearance in the README.
Testing
npm run cicd(lint, format, 192 tests, build) passes.src/theme/theme.spec.ts) and viewport behavior — clipping, chrome staying on screen, arrow/PgDn scrolling, clamping at the end, no hint when the window is tall (src/components/ScrollableViewport.spec.tsx).dist/UI in a real pty at 8-18 rows: the frame fits the window at every size, realPgDnscrolls from the comment through to the diff, and the input box and hints stay visible. Verified dark (#4a4324bg +#ffe9a8fg on the commented lines), light (#fff59d+#3a2f00) andNO_COLOR(no color codes, bold selection) output.Note: the live comments screen needs a logged-in session, so end-to-end verification used the compiled components with fixture data rather than a real PR.
🤖 Generated with Claude Code