fix(cli): stabilize copy mode to prevent flickering and cursor resets#22584
fix(cli): stabilize copy mode to prevent flickering and cursor resets#22584mattKorwel merged 7 commits intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the user experience in Alternate Buffer Mode, particularly on macOS and Windows, by addressing UI instability during text selection. By implementing a "Freeze Frame" strategy, the changes prevent screen flickering, cursor resets, and layout shifts, ensuring a perfectly static and stable environment for native terminal text selection and copying. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Size Change: +1.14 kB (0%) Total Size: 26.3 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
the
status/need-issue
mattKorwel
force-pushed
the
mk-copy-mode
branch
from
c0e9f76 to
109ddfd
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a "Freeze Frame" approach to stabilize the UI in copy mode, preventing flickering and cursor resets. The changes involve locking the layout height of the controls area, pausing UI updates like the memory usage display, and hiding components like the footer and input prompt when copy mode is active. The implementation is logical and addresses the problem effectively. I've identified one potential race condition in how the control height is cached and have provided a suggestion to improve its robustness. Overall, this is a solid fix for a tricky terminal UI issue.
mattKorwel
left a comment
mattKorwel
left a comment
There was a problem hiding this comment.
🤖 Automated Review Assessment (via Gemini CLI)
Functional verification and behavioral analysis for this PR have identified a transient CI issue but verified the core stabilization logic.
✅ Key Improvements
- Copy Mode Stabilization: The fixes for flickering and cursor resets were verified in a TTY environment. The PTY management framework correctly handles the stabilization.
- Build & Review: The core build and automated review passed successfully.
⚠️ Observation: CI Status
- CI Status Failed: GitHub Actions failed during the run. However, Local Diagnostics (lint/typecheck) passed. This indicates the CI failure is likely a transient environment issue (e.g., resource timeout) and not a regression in the code.
Recommendation: Approve once CI is re-run and passing.
gemini-cli
bot
added
priority/p1
|
This goes too far. Now you have no way of copying text from the InputPrompt. Instead of these changes I would suggest removing the logic in InputPrompt setting the cursor when in selection mode. |
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
Here is a review from /review-frontend regarding the mk-copy-mode changes:
Summary
The core approach to resolving the flickering and cursor resets is solid. By tracking and freezing the stableControlsHeight in AppContainer.tsx when entering Copy Mode, you successfully prevent the terminal height from fluctuating, which is what typically triggers the scroll resets in the BaseSelectionList.
However, there are a couple of unintended UI regressions and dead code paths that were introduced to the presentation components as part of this fix. These should be cleaned up.
Feedback
1. CopyModeWarning.tsx introduces a permanent empty line
The component was changed to always render a <Box height={1}> even when copyModeEnabled is false, whereas it previously returned null.
This creates a permanent blank line directly above the Composer when the user is not in copy mode (which is the default state).
Because the <Footer /> (which is 1 line tall) is explicitly unmounted during copy mode, and the CopyModeWarning (which is also 1 line tall) appears in its place, the total lines rendered naturally remain the same. The stableControlsHeight in AppContainer.tsx also enforces the container boundaries. Therefore, padding the UI with an empty Box is unnecessary and degrades the standard visual layout.
Action: Revert CopyModeWarning.tsx back to returning null when copyModeEnabled is false. You will also need to update the snapshot tests in App.test.tsx.snap to reflect the removal of the empty line.
2. Dead code in MemoryUsageDisplay.tsx and Footer.tsx
You added an isActive prop to MemoryUsageDisplay to clear the setInterval when in copy mode, passing it down from Footer as isActive={!uiState.copyModeEnabled}.
However, if you look at Composer.tsx, the entire <Footer /> component is conditionally unmounted:
{!uiState.copyModeEnabled && <Footer />}Because the Footer is completely removed from the React tree during copy mode, MemoryUsageDisplay is also unmounted. When a component unmounts, its useEffect cleanup automatically runs (clearing the interval). Therefore, isActive will always be true whenever MemoryUsageDisplay is mounted, rendering the prop completely useless.
Action: Remove the isActive prop from MemoryUsageDisplay and Footer.
3. DefaultAppLayout.tsx and AppContainer.tsx
The logic to maintain a consistent paddingBottom={1} during alternate buffer mode and the core stableControlsHeight math in AppContainer.tsx are excellent and should remain as implemented.
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
Please revert the accidental change to MemoryUsageDisplay before committing.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
mattKorwel
force-pushed
the
mk-copy-mode
branch
from
1e32ba3 to
23d7bd2
Compare
|
/gemini review |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
…#22584) Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…google-gemini#22584) Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2 participants
Problem
On macOS and Windows, users in Alternate Buffer Mode (fullscreen) experienced significant flickering and cursor resets when attempting to highlight text for copying. This was primarily caused by:
MemoryUsageDisplayupdated every 2 seconds, and theInputPromptmaintained an active cursor. In many terminals, any screen update or cursor movement during a selection forces the terminal to reset the selection or scroll back to the bottom.Ctrl+S) triggered a sequence of component unmounts (Footer, InputPrompt) and visibility changes (CopyModeWarning). These changes caused the history area (MainContent) to resize and the terminal to jump, breaking the user's focus and selection.Fix
This PR implements a "Freeze Frame" approach to Copy Mode:
stableControlsHeightinAppContainer. When Copy Mode is active, the height of the bottom controls area is locked to its last known size, preventing the chat history from resizing or jumping.CopyModeWarningto always occupy a 1-line box, even when hidden, to eliminate the 1-line layout shift when entering/exiting.InputPromptto prevent cursor interference.Footerto eliminate re-renders from status items.paddingBottomin the root layout to avoid shifts in Alternate Buffer mode.These changes ensure that once a user enters Copy Mode, the screen is perfectly static, allowing the terminal's native selection logic to function without interference.
Verification
Related Issues:
Note to reviewer: I will attach a screen recording demonstrating the fix in the comments below.