Skip to content

fix: make comment review UI scrollable and dark-mode aware - #119

Open
benglewis wants to merge 4 commits into
baz-scm:mainfrom
benglewis:fix/comments-ui-scroll-and-theme
Open

fix: make comment review UI scrollable and dark-mode aware#119
benglewis wants to merge 4 commits into
baz-scm:mainfrom
benglewis:fix/comments-ui-scroll-and-theme

Conversation

@benglewis

Copy link
Copy Markdown
Contributor

Problem

Two issues with the PR comments review UI:

  1. Small windows are unusable. The comment, its diff and the conversation were rendered at full height, so on a short (but wide) window the input box and its hints were pushed off screen and the terminal scrolled the frame away. Nothing could be scrolled back either.
  2. Dark terminals are unreadable. Diff rows and the file header set a backgroundColor without 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

  • New ScrollableViewport clips its content to the rows the terminal has left and scrolls with /, PgUp/PgDn and Ctrl+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.
  • New ScreenLayout context: 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.
  • Chrome adapts too: below 24 rows the banner drops to one line and the input hints collapse onto one line — their rows cost more than they are worth in a short window.
  • Applied to the comments browser, PR chat, and the met/unmet requirement browsers.

Theming

  • New src/theme/theme.ts resolves a theme once per process:
    • adapts to the terminal background (COLORFGBG where available, dark assumed otherwise),
    • BAZ_THEME=auto|dark|light|none, BAZ_TERMINAL_BACKGROUND=dark|light, and NO_COLOR support,
    • every color overridable via BAZ_COLOR_* env vars or a theme.json ($BAZ_THEME_FILE, ./.baz/, ~/.baz/, ~/.config/baz/), with none to unstyle a single element.
  • A background is now always paired with a readable foreground — the actual dark-mode fix. Without colors, the lines a comment points at are bold instead.
  • Hardcoded "cyan"/"magenta"/"red" colors in the review path now come from the theme, so they honor NO_COLOR.
  • theme/colors.ts keeps 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.
  • New tests: theme resolution/detection/overrides (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).
  • Manually drove the compiled dist/ UI in a real pty at 8-18 rows: the frame fits the window at every size, real PgDn scrolls from the comment through to the diff, and the input box and hints stay visible. Verified dark (#4a4324 bg + #ffe9a8 fg on the commented lines), light (#fff59d + #3a2f00) and NO_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

@baz-reviewer

baz-reviewer Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Generated description

Improve terminal review flows by adding measured, keyboard-navigable scrolling that preserves chat inputs, headers, and other persistent chrome in short windows. Centralize adaptive theming across review, diff, prompt, integration, and selector components, with readable foreground/background pairing, terminal detection, configuration overrides, colorless operation, and comprehensive tests and documentation.

Topics
TopicDetails
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)
  • src/components/SelectRenderers.tsx
  • src/flows/Integration/IntegrationPrompt.tsx
  • src/flows/Integration/IntegrationProviderSelector.tsx
  • src/flows/Review/PostReviewPrompt.tsx
  • src/flows/Review/ReviewMenu.tsx
  • src/pages/PRSelector/MergeConfirmationPrompt.tsx
  • src/pages/PRSelector/PullRequestCard.tsx
  • src/pages/PRSelector/PullRequestSelector.tsx
  • src/pages/PRSelector/PullRequestSelectorContainer.tsx
  • src/pages/PRWalkthrough/UpdateAvailablePrompt.tsx
  • src/pages/SpecReview/ReviewStatusPrompt.tsx
  • src/pages/SpecReview/SpecReviewErrorPrompt.tsx
  • src/pages/SpecReview/TriggerSpecReviewPrompt.tsx
  • src/pages/integrations/CredentialsFlow.tsx
  • src/pages/integrations/OAuthFlow.tsx
Latest Contributors(0)
UserCommitDate
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)
  • src/components/HeaderDisplay.tsx
  • src/components/MentionAutocomplete.tsx
  • src/components/ScrollableViewport.spec.tsx
  • src/components/ScrollableViewport.tsx
  • src/components/layout/ScreenLayout.tsx
  • src/flows/Review/Review.tsx
  • src/hooks/useTerminalSize.ts
  • src/pages/IssueBrowser.tsx
  • src/pages/PRChat/PRChat.tsx
  • src/pages/SpecReview/MetRequirementBrowser.tsx
  • src/pages/SpecReview/SpecReviewBrowser.tsx
  • src/pages/chat/ChatDisplay.tsx
  • src/pages/chat/ChatInput.tsx
Latest Contributors(0)
UserCommitDate
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)
  • src/components/DiffDisplay.tsx
  • src/components/DiffDisplayContainer.tsx
  • src/components/PullRequestReview.tsx
  • src/components/SelectRenderers.tsx
  • src/flows/Review/ReviewMenu.tsx
  • src/pages/PRChat/PRChat.tsx
  • src/pages/PROverview/PullRequestOverview.tsx
  • src/pages/SpecReview/MetRequirementBrowser.tsx
  • src/pages/SpecReview/SpecReviewBrowser.tsx
  • src/pages/chat/ChatDisplay.tsx
  • src/pages/chat/ChatInput.tsx
  • src/pages/chat/ToolCallDisplay.tsx
  • src/theme/colors.ts
  • src/theme/theme.spec.ts
  • src/theme/theme.ts
Latest Contributors(0)
UserCommitDate
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)
  • README.md
Latest Contributors(0)
UserCommitDate

Review this PR on Baz | Customize your next review

@baz-reviewer

baz-reviewer Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

⚠️ Advanced Security cannot run on this PR.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/components/ScrollableViewport.tsx
Comment thread src/pages/chat/ChatDisplay.tsx
Comment thread src/components/ScrollableViewport.tsx
Comment thread src/theme/colors.ts
Comment thread src/theme/theme.ts Outdated
Comment thread src/theme/theme.ts Outdated
Comment thread src/components/ScrollableViewport.tsx Outdated
Comment on lines +216 to 220
const requirementHeader = (
<Box flexDirection="column" flexShrink={0}>
<Box marginBottom={1}>
<Text color={MAIN_COLOR} bold>
Unmet requirement ({currentIndex + 1}/{unmetRequirements.length})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Severity

Want Baz to fix this for you? Activate Fixer

Comment thread src/components/DiffDisplayContainer.tsx Outdated
Comment thread src/components/DiffDisplay.tsx Outdated
Comment thread src/pages/chat/ChatDisplay.tsx
Comment thread src/components/PullRequestReview.tsx
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>
@benglewis
benglewis force-pushed the fix/comments-ui-scroll-and-theme branch from d41ded1 to e7cfb7d Compare August 20, 2026 17:27
- 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>
Comment thread src/flows/Integration/IntegrationPrompt.tsx Outdated
Comment thread src/components/ScrollableViewport.spec.tsx Outdated
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>
Comment thread src/components/ScrollableViewport.spec.tsx Outdated
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>
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