Skip to content

feat: improve terminal visual quality#142

Open
khaliqgant wants to merge 4 commits into
mainfrom
claude/terminal-visual-tweaks
Open

feat: improve terminal visual quality#142
khaliqgant wants to merge 4 commits into
mainfrom
claude/terminal-visual-tweaks

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

Summary

Pure visual improvements to the xterm.js terminal — no functional or architectural changes.

  • lineHeight: 1.2 and letterSpacing: 0.5 for better readability
  • cursorStyle: 'bar' instead of block for a modern feel
  • scrollback: 10000 (was default 1000)
  • fastScrollModifier: 'alt' and macOptionIsMeta: true enabled
  • Tab switching changed from instant display:none to 150ms opacity fade (transition-opacity)

These changes were cherry-picked from PR #139 (relay-8.3-headless-runtime). #139 is held pending the V1 structured output parsing work — this PR extracts the safe, mergeable visual improvements.


Generated by Claude Code

- lineHeight 1.2 and letterSpacing 0.5 for better readability
- cursorStyle 'bar' instead of block for modern feel
- scrollback increased to 10,000 lines
- alt fast-scroll and macOptionIsMeta enabled
- tab switching changed from instant display:none to 150ms opacity fade

https://claude.ai/code/session_01KXU1uAUwx3L82TMLnAmU4z
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 7, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 7, 2026

Review Change Stack

Warning

Review limit reached

@khaliqgant, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 30 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 00053f2a-0050-4cc7-bc1c-26769006c188

📥 Commits

Reviewing files that changed from the base of the PR and between 6ae8157 and 99f907e.

📒 Files selected for processing (2)
  • src/renderer/src/components/terminal/TerminalPane.tsx
  • src/renderer/src/hooks/use-terminal.ts
📝 Walkthrough

Walkthrough

Terminal visibility now uses CSS opacity transitions (containers remain mounted and inactive ones are inert), and xterm initialization options were adjusted for line height, letter spacing, cursor style, scrollback, and macOS modifier behavior.

Changes

Terminal rendering and configuration improvements

Layer / File(s) Summary
xterm initialization options
src/renderer/src/hooks/use-terminal.ts
xterm is configured with lineHeight 1.2, letterSpacing, cursorStyle 'bar', scrollback 10000, fastScrollModifier 'alt', and macOptionIsMeta enabled.
Terminal pane opacity-based visibility
src/renderer/src/components/terminal/TerminalPane.tsx
Terminal project containers remain mounted and toggle visibility via CSS transitions (opacity + pointerEvents), with inert={!active} and aria-hidden applied for inactive containers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Soft fades replace the sudden blink and snap,
Terminals settle gently in their tabbed lap,
Taller lines and narrower cursors glide,
Scrollback deep where secrets hide,
Alt dances on Mac — a quiet, polished tap.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: visual improvements to the terminal with CSS transitions and xterm.js configuration updates.
Description check ✅ Passed The description is directly related to the changeset, clearly detailing the visual improvements made and explaining their origin from PR #139.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/terminal-visual-tweaks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the terminal configuration by adjusting font settings, cursor style, scrollback limits, and key modifiers, and transitions the terminal pane hiding mechanism from display toggling to an opacity-based fade. The review feedback highlights three key issues: setting macOptionIsMeta: true globally can break typing for non-US macOS keyboard layouts; using only opacity to hide inactive terminals keeps them in the render tree, which can cause WebGL context limit issues and performance overhead; and increasing the scrollback buffer to 10,000 lines significantly increases memory usage, suggesting a more conservative default like 3,000.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/renderer/src/hooks/use-terminal.ts Outdated
cursorStyle: 'bar',
scrollback: 10000,
fastScrollModifier: 'alt',
macOptionIsMeta: true,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Setting macOptionIsMeta: true globally can break basic typing for macOS users with non-US keyboard layouts (such as German, French, Swedish, etc.). On these layouts, the Option key is required to type essential characters like [, ], {, }, |, \, and @. When macOptionIsMeta is enabled, pressing these key combinations sends an Escape sequence (Meta prefix) instead of the character, making the terminal unusable for these users.\n\nIt is highly recommended to default this to false and make it a user-configurable setting (similar to VS Code's terminal.integrated.macOptionIsMeta).

Suggested change
macOptionIsMeta: true,
macOptionIsMeta: false,

Comment on lines +1082 to +1087
className="absolute inset-0 transition-opacity duration-150 ease-out"
style={{
opacity: active ? 1 : 0,
pointerEvents: active ? 'auto' : 'none'
}}
aria-hidden={!active}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using only opacity: active ? 1 : 0 to hide inactive terminals keeps them fully active in the browser's render and compositing trees. This means the browser continues to perform layout, paint, and compositing calculations for all inactive terminals, leading to unnecessary GPU memory usage and CPU overhead.\n\nFurthermore, because each terminal uses the WebglAddon (which allocates a WebGL context), keeping all of them in the DOM without hiding them can quickly exceed the browser's WebGL context limit (typically 8–16 active contexts), causing context loss and rendering fallback errors.\n\nTo resolve this, we can transition the visibility property alongside opacity. In CSS, visibility transitions elegantly: it becomes visible immediately when showing, but delays becoming hidden until the opacity transition completes. This allows the fade-out animation to play fully while ensuring that inactive terminals are completely removed from the paint/compositing tree once hidden.\n\nNote: This requires changing transition-opacity to transition-all so that Tailwind transitions both opacity and visibility.

                className="absolute inset-0 transition-all duration-150 ease-out"
                style={{
                  opacity: active ? 1 : 0,
                  visibility: active ? 'visible' : 'hidden',
                  pointerEvents: active ? 'auto' : 'none'
                }}
                aria-hidden={!active}

Comment thread src/renderer/src/hooks/use-terminal.ts Outdated
letterSpacing: 0.5,
cursorBlink: true,
cursorStyle: 'bar',
scrollback: 10000,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Increasing the scrollback buffer from 1000 to 10000 lines increases the memory footprint of each terminal instance by up to 10x.\n\nSince all terminal instances for all agents stay mounted in the React tree in tabbed mode (to preserve scroll state), having multiple active agents with large scrollback buffers can significantly increase memory usage in the Electron renderer process. This could exacerbate memory pressure and potential renderer crashes during long sessions.\n\nConsider keeping a more conservative default (e.g., 3000 or 5000) or making this configurable so users can adjust it based on their system resources.

Suggested change
scrollback: 10000,
scrollback: 3000,

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 7, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@agent-relay-code
Copy link
Copy Markdown
Contributor

Fixed one PR regression in TerminalPane.tsx: inactive opacity-hidden terminal panes are now inert, so their tabIndex=0 terminal descendants are not reachable by keyboard focus while aria-hidden.

Validation run:

  • npm test passed: 88 tests.
  • npm run build passed.
  • npx tsc -p tsconfig.web.json --noEmit still fails in unrelated existing renderer files outside this PR (StatusBar, DiffPane, GraphView, settings, syntax highlighter); no errors were reported in the touched terminal files.

I removed local verification artifacts (node_modules, out, optional bin/relayfile-mount) and restored the generated MCP resource manifest after build.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Re-trigger cubic

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 7, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 34e1816 to this PR. The notes below describe what changed.

Addressed the live bot findings in the current checkout:

  • use-terminal.ts: reduced scrollback from 10000 to 3000.
  • use-terminal.ts: changed macOptionIsMeta to false.
  • TerminalPane.tsx: inactive tab panes now transition visibility with opacity, while keeping pointerEvents, aria-hidden, and inert.

Validation:

  • npm test passed: 88 tests.
  • npm run build passed.
  • npx tsc -p tsconfig.web.json --noEmit still fails on existing unrelated renderer type errors outside the touched terminal files.

I also removed local verification artifacts after the run: node_modules, out, and bin/relayfile-mount.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/renderer/src/hooks/use-terminal.ts">

<violation number="1" location="src/renderer/src/hooks/use-terminal.ts:361">
P1: Values inverted relative to PR description intent. The PR description states scrollback is being increased to 10000 and macOptionIsMeta enabled, but the diff reduces scrollback from 10000 to 3000 and disables macOptionIsMeta from true to false. These look like the values were accidentally swapped — likely the author meant to keep scrollback: 10000 and macOptionIsMeta: true as described, or vice versa.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

letterSpacing: 0.5,
cursorBlink: true,
cursorStyle: 'bar',
scrollback: 3000,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Values inverted relative to PR description intent. The PR description states scrollback is being increased to 10000 and macOptionIsMeta enabled, but the diff reduces scrollback from 10000 to 3000 and disables macOptionIsMeta from true to false. These look like the values were accidentally swapped — likely the author meant to keep scrollback: 10000 and macOptionIsMeta: true as described, or vice versa.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/renderer/src/hooks/use-terminal.ts, line 361:

<comment>Values inverted relative to PR description intent. The PR description states scrollback is being increased to 10000 and macOptionIsMeta enabled, but the diff reduces scrollback from 10000 to 3000 and disables macOptionIsMeta from true to false. These look like the values were accidentally swapped — likely the author meant to keep scrollback: 10000 and macOptionIsMeta: true as described, or vice versa.</comment>

<file context>
@@ -358,9 +358,9 @@ export function useTerminal(
         cursorBlink: true,
         cursorStyle: 'bar',
-        scrollback: 10000,
+        scrollback: 3000,
         fastScrollModifier: 'alt',
-        macOptionIsMeta: true,
</file context>
Suggested change
scrollback: 3000,
scrollback: 10000,

opacity:0 + pointerEvents:none does not remove hidden tabs from the
browser tab order — the xterm.js textarea remains focusable. Adding
the inert attribute (via ref callback to avoid TypeScript attribute
gaps) makes the entire inactive container and all its descendants
unfocusable, matching the behaviour of the previous display:none while
preserving the 150ms opacity fade animation.

https://claude.ai/code/session_01KXU1uAUwx3L82TMLnAmU4z
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 7, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

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.

2 participants