refactor(cli): better react patterns for BaseSettingsDialog

[psinha40898]

Summary

Before an alternate buffer version of the settings dialog lands it makes sense to:

• Remove unfortunate react patterns from BaseSettingsDialog
• Consolidate responsive rendering (sliding window) logic into BaseSettingsDialog which will later conditionally render an alternate buffer version

This has the side effect of making AgentConfigDialog responsive and primed for alternate buffer responsiveness as well.

Details

Consolidate rendering in preparation for Alternate Buffer Dialogs:

The logic to make the settings dialogs responsive (in default mode) has been moved to the BaseSettingsDialog. This is where the conditional rendering for alt buffer vs default mode will also live.

Alt buffer changes aside, this makes AgentConfigDialog responsive the same way SettingsDialog is

Cleaned up the following react patterns:

Removed or moved useEffects

React justification: useEffect should not be used to synchronize React state because the effect runs after React commits which can be brittle and is inefficient.
https://react.dev/learn/you-might-not-need-an-effect
When we do need a useEffect, we should check if it makes sense to move into a custom hook. If it does, that is preferred.

• Instead of relying on useEffect and complicated logic to handle active settings when search filters the list, we calculate it on render and rely on the key of the settings instead of the index. This also removes the need to diff against as ref.
• useEffect to enforce focus on Main menu when scope is hidden is now derived on render as it should be
• useEffect to create cursor blink effect moved into the useInlineEditBuffer hook

Custom Hooks and Reducer

React justification: custom hooks help to make components more readable

the code inside them describes what they want to do (...) rather than how to do it.

https://react.dev/learn/reusing-logic-with-custom-hooks

• The sliding window navigation logic has been refactored into useSettingsNavigation.ts which uses the custom hook and reducer pattern. The custom hook makes the component more readable, and the reducer is a better way to handle the deeply coupled states of activeItem and the start of the window.
• The logic for inline editing of settings is consolidated in useInlineEditBuffer.ts. In addition to making the component easier to read, this also consolidates the editingKey cursorPos and buffer states into one reducer.

Minor fixes

• Identical callback that was defined twice in both handleScopeHighlight and handleScopeSelect has been consolidated into one callback

Related Issues

Fixes #21140
Fixes #21203
Related to #15840

How to Validate

Responsiveness:
Open the /settings Dialog
Navigate through it and wrap around fully
Put your active item on a setting, then provide a search filter that the active item will survive
Example: highlight "Dynamic window title" then filter "title"
Note that the active highlight sticks to that item
Put your active item on a setting, then provide a search filter that the active item will not survive
Example: highlight "Dynamic window title" then filter "ignore"
Note that the active highlight switches to the start of the sliding window
Resize the terminal window

Inline editing:
Find a setting that accepts a numerical value that can be typed in
Example: set maximum discovery max dirs to 201 from 200
Enter a new value and save

Repeat steps for /agents config
(agentsEnabled must be set to true under experimental in settings.json)

Pre-Merge Checklist

• Updated relevant documentation and README (if needed)
• Added/updated tests (if needed)
• Noted breaking changes (if any)
• Validated on required platforms/methods:
  • MacOS
    • npm run
    • npx
    • Docker
    • Podman
    • Seatbelt
  • Windows
    • npm run
    • npx
    • Docker
  • Linux
    • npm run
    • npx
    • Docker

[psinha40898]

Currently Gemini CLI shows both top and bottom indicators when the list is scrollable. It could be changed to only show indicators if there is room to scroll in that direction.

This would require updating snapshots and I left it out of this PR because it could sensibly be added separately

[psinha40898]

/gemini review

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 refactors the CLI's dialog components, particularly BaseSettingsDialog, to enhance maintainability and prepare for future features like alternate buffer dialogs. By centralizing responsive rendering logic and abstracting complex state management into custom React hooks, the codebase becomes cleaner and more aligned with modern React best practices. This refactoring also brings immediate benefits by making the AgentConfigDialog responsive to terminal height.

Highlights

• Responsive Rendering Logic Consolidated: The logic for making settings dialogs responsive to terminal height has been moved from SettingsDialog into the BaseSettingsDialog. This change also makes AgentConfigDialog responsive.
• Improved React Patterns: Several useEffect hooks were removed or refactored. Active settings calculation and focus enforcement are now derived on render, and the cursor blink effect was moved into a custom hook.
• Introduced Custom Hooks: New custom hooks, useSettingsNavigation and useInlineEditBuffer, were created to encapsulate and centralize logic for navigating through settings and handling inline text editing, respectively. This improves component readability and maintainability.

Changelog

• packages/cli/src/ui/components/AgentConfigDialog.test.tsx
  • Added a test case to verify AgentConfigDialog's responsiveness to availableTerminalHeight and list truncation.
• packages/cli/src/ui/components/AgentConfigDialog.tsx
  • Added availableTerminalHeight prop to AgentConfigDialogProps to enable dynamic windowing.
  • Passed the new availableTerminalHeight prop to the BaseSettingsDialog component.
• packages/cli/src/ui/components/DialogManager.tsx
  • Passed the calculated availableTerminalHeight to the AgentConfigDialog component.
• packages/cli/src/ui/components/SettingsDialog.tsx
  • Removed the internal logic for calculating effectiveMaxItemsToShow and showScopeSelection, delegating this responsibility to BaseSettingsDialog.
  • Updated showScopeSelector to directly use hasWorkspace.
  • Passed availableTerminalHeight to BaseSettingsDialog.
• packages/cli/src/ui/components/shared/BaseSettingsDialog.test.tsx
  • Added new test suites to validate the responsiveness of BaseSettingsDialog, including dynamic hiding/showing of the scope selector and item truncation based on availableHeight.
• packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx
  • Replaced direct useEffect and useRef imports with useMemo for optimized rendering.
  • Integrated new custom hooks: useSettingsNavigation for list navigation and useInlineEditBuffer for text editing state.
  • Added availableHeight prop to BaseSettingsDialogProps to enable responsive layout calculations.
  • Implemented responsive logic using useMemo to determine effectiveMaxItemsToShow and finalShowScopeSelector based on available terminal height.
  • Refactored internal state management for active index, scroll offset, editing state, and cursor visibility to leverage the new custom hooks.
  • Updated keyboard event handling to interact with the new custom hooks for navigation and editing actions.
  • Consolidated handleScopeHighlight and handleScopeSelect into a single handleScopeChange callback.
  • Adjusted UI rendering logic to use effectiveFocusSection and finalShowScopeSelector for conditional styling and element visibility.
• packages/cli/src/ui/hooks/useInlineEditBuffer.test.ts
  • Added comprehensive unit tests for the useInlineEditBuffer hook, covering initialization, editing lifecycle, cursor movements, and character manipulation.
• packages/cli/src/ui/hooks/useInlineEditBuffer.ts
  • Created a new custom React hook useInlineEditBuffer to manage the state and logic for inline text editing, including buffer, cursor position, and cursor blinking.
  • Implemented a reducer (editBufferReducer) to handle various editing actions such as moving the cursor, deleting characters, and inserting characters with type validation.
• packages/cli/src/ui/hooks/useSettingsNavigation.test.ts
  • Added unit tests for the useSettingsNavigation hook, verifying navigation, scroll offset adjustments, and active item preservation during list changes.
• packages/cli/src/ui/hooks/useSettingsNavigation.ts
  • Created a new custom React hook useSettingsNavigation to manage the active item index and scroll offset for lists, including logic for wrapping around and adjusting visibility.

Activity

• No specific activity was recorded for this pull request.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request is a significant and well-executed refactor of the settings dialog components. Moving the complex state management for navigation and inline editing into custom hooks (useSettingsNavigation and useInlineEditBuffer) greatly improves the readability and maintainability of BaseSettingsDialog. Consolidating the responsive layout logic into the base component is also a smart move that benefits other dialogs like AgentConfigDialog.

I've found one high-severity issue in the new useSettingsNavigation hook related to a React anti-pattern that could cause subtle bugs. I've left a detailed comment with a suggested fix.

Overall, this is a high-quality refactoring that modernizes the codebase and adheres to better React practices. Once the suggested change is addressed, this PR will be in excellent shape.

[gemini-code-assist]

Code Review

This pull request is a significant and well-executed refactoring of the settings dialog components. It successfully centralizes responsive logic into BaseSettingsDialog and introduces custom hooks (useSettingsNavigation, useInlineEditBuffer) to encapsulate complex state and logic, which greatly improves code readability and maintainability. The use of a reducer in useInlineEditBuffer is an excellent pattern for managing the edit state. The changes align well with modern React best practices. I've found one area for improvement in the useSettingsNavigation hook to make it even more robust and aligned with React's principles.

[gemini-cli]

Hi there! Thank you for your contribution to Gemini CLI.

To improve our contribution process and better track changes, we now require all pull requests to be associated with an existing issue, as announced in our recent discussion and as detailed in our CONTRIBUTING.md.

This pull request is being closed because it is not currently linked to an issue. Once you have updated the description of this PR to link an issue (e.g., by adding Fixes #123 or Related to #123), it will be automatically reopened.

How to link an issue:
Add a keyword followed by the issue number (e.g., Fixes #123) in the description of your pull request. For more details on supported keywords and how linking works, please refer to the GitHub Documentation on linking pull requests to issues.

Thank you for your understanding and for being a part of our community!

[gemini-code-assist]

Code Review

This pull request is a significant and well-executed refactoring of the BaseSettingsDialog and related components. The introduction of custom hooks like useSettingsNavigation and useInlineEditBuffer greatly improves the state management logic, making it more robust, readable, and aligned with modern React patterns. Consolidating the responsive layout calculations into the base component is a smart move that simplifies SettingsDialog and extends responsiveness to AgentConfigDialog. The code is clean, well-tested, and the changes effectively address the goals outlined in the description. I have reviewed the changes thoroughly and found no issues.

[jacob314]

Thanks for this excellent refactor! The code is significantly cleaner and more aligned with our React best practices. Moving complex state management into custom hooks (useInlineEditBuffer and useSettingsNavigation) and removing brittle useEffect synchronizations is a huge improvement. The use of a reducer for the text editing state is spot-on. I also really appreciate the attention to detail with the cursor blinking effect—resetting the interval when the buffer or cursor changes makes the editing experience feel much more responsive and polished.

I did notice a couple of small issues related to the responsive height calculation that need to be addressed:

1. Missing Footer Height in Responsive Layout Calculation (Potential Bug)

In the original SettingsDialog, the height of the restart prompt was factored into the responsive calculation (const RESTART_PROMPT_HEIGHT = showRestartPrompt ? 1 : 0). Now that the sliding-window logic has moved to BaseSettingsDialog, this logic has been lost.

BaseSettingsDialog accepts footerContent: ReactNode, but does not know how much vertical space it consumes. When a footer appears (e.g., the 1-line restart prompt in SettingsDialog, or the "Changes saved automatically" text in AgentConfigDialog), effectiveMaxItemsToShow is not reduced, causing the dialog to overflow availableTerminalHeight by 1 row. This can trigger terminal scrolling or flickering.

Recommendation: Please add an optional footerHeight?: number prop to BaseSettingsDialog and add it to baseFixedHeight to ensure the layout remains constrained.

2. Unconditional Spacer Height

There is an unconditional spacer <Box height={1} /> rendered below the search section. The calculation SEARCH_SECTION_HEIGHT = searchEnabled ? 5 : 0 does not account for this spacer remaining when searchEnabled is false. This means the calculation is off by 1 row when search is disabled. Since you are actively touching this height math, it would be great to fix this minor pre-existing issue.

Overall, this is a fantastic PR. Once the height calculations are dialed in, this will be ready to go!

[psinha40898]

Thank you for the review! Since the parent dialog should own the footer I turned the prop into an object.

[jacob314]

Approved.
One existing issue we should prioritize fixing which I found while testing this refactor is you now cannot type the letter k for a setting option that is free for text.
For example select "Plan Directory"
and start typing a directory name. If it contains the letters j or k that won't work and will navigate the selection list instead.