feat(ui): standardize semantic focus colors and enhance history visibility#20745
feat(ui): standardize semantic focus colors and enhance history visibility#20745
Conversation
|
Hi @keithguerin, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
|
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 How to link an issue: Thank you for your understanding and for being a part of our community! |
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 enhances the CLI's theming system by introducing a new command for visualizing theme colors and standardizing how focus and active states are represented visually. These changes improve user experience through better readability and consistent visual cues, while also addressing underlying architectural issues for a more robust color management system. Highlights
Changelog
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
|
|
You already have 7 pull requests open. Please work on getting existing PRs merged before opening more. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new /colors command and standardizes focus/selection colors throughout the UI, which is a great improvement for theme consistency and developer experience. The refactoring to resolve circular dependencies is also a welcome change. However, I've found a critical issue where snapshot tests are failing due to an incomplete mock theme in the test environment for DescriptiveRadioButtonSelect. This needs to be addressed to ensure the tests pass.
| // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html | ||
|
|
||
| exports[`DescriptiveRadioButtonSelect > should render correctly with custom props 1`] = ` | ||
| " 1. Foo Title | ||
| This is Foo. | ||
| ● 2. Bar Title | ||
| This is Bar. | ||
| 3. Baz Title | ||
| This is Baz. | ||
| " | ||
| ERROR Cannot read properties of undefined (reading 'focus') | ||
|
|
||
| src/ui/components/shared/BaseSelectionList.tsx:120:33 | ||
|
|
||
| 117: let numberColor = theme.text.primary; | ||
| 118: | ||
| 119: if (isSelected) { | ||
| 120: titleColor = theme.ui.focus; | ||
| 121: numberColor = theme.ui.focus; | ||
| 122: } else if (item.disabled) { | ||
| 123: titleColor = theme.text.secondary; | ||
|
|
||
| - (src/ui/components/shared/BaseSelectionList.tsx:120:33) | ||
| - at Array.map (<anonymous>)\\t | ||
| - BaseSelectionList (src/ui/components/shared/BaseSelectionList.tsx:111:21) | ||
| -Object.react-stack-bott | ||
| m-frame (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconci | ||
| ler/cjs/react-reconciler.development.js:15859:20) | ||
| -renderWithHoo | ||
| s (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/re | ||
| act-reconciler.development.js:3221:22) | ||
| -updateFunctionComp | ||
| nent (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/c | ||
| js/react-reconciler.development.js:6475:19) | ||
| -beginWor | ||
| (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/react-r | ||
| econciler.development.js:8009:18) | ||
| -runWithFiberIn | ||
| EV (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/r | ||
| eact-reconciler.development.js:1738:13) | ||
| -performUnitOfW | ||
| rk (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/r | ||
| eact-reconciler.development.js:12834:22) | ||
| -workLoopSyn | ||
| (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/reac | ||
| t-reconciler.development.js:12644:41) | ||
| " | ||
| `; | ||
|
|
||
| exports[`DescriptiveRadioButtonSelect > should render correctly with default props 1`] = ` | ||
| "● Foo Title | ||
| This is Foo. | ||
| Bar Title | ||
| This is Bar. | ||
| Baz Title | ||
| This is Baz. | ||
| " | ||
| ERROR Cannot read properties of undefined (reading 'focus') | ||
|
|
||
| src/ui/components/shared/BaseSelectionList.tsx:120:33 | ||
|
|
||
| 117: let numberColor = theme.text.primary; | ||
| 118: | ||
| 119: if (isSelected) { | ||
| 120: titleColor = theme.ui.focus; | ||
| 121: numberColor = theme.ui.focus; | ||
| 122: } else if (item.disabled) { | ||
| 123: titleColor = theme.text.secondary; | ||
|
|
||
| - (src/ui/components/shared/BaseSelectionList.tsx:120:33) | ||
| - at Array.map (<anonymous>)\\t | ||
| - BaseSelectionList (src/ui/components/shared/BaseSelectionList.tsx:111:21) | ||
| -Object.react-stack-bott | ||
| m-frame (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconci | ||
| ler/cjs/react-reconciler.development.js:15859:20) | ||
| -renderWithHoo | ||
| s (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/re | ||
| act-reconciler.development.js:3221:22) | ||
| -updateFunctionComp | ||
| nent (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/c | ||
| js/react-reconciler.development.js:6475:19) | ||
| -beginWor | ||
| (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/react-r | ||
| econciler.development.js:8009:18) | ||
| -runWithFiberIn | ||
| EV (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/r | ||
| eact-reconciler.development.js:1738:13) | ||
| -performUnitOfW | ||
| rk (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/r | ||
| eact-reconciler.development.js:12834:22) | ||
| -workLoopSyn | ||
| (/Users/keithguerin/Documents/gemini-cli_colors/node_modules/react-reconciler/cjs/reac | ||
| t-reconciler.development.js:12644:41) | ||
| " | ||
| `; |
There was a problem hiding this comment.
This snapshot shows that the test for DescriptiveRadioButtonSelect is failing with a TypeError: Cannot read properties of undefined (reading 'focus'). This is a critical issue that breaks the build.
The error originates in BaseSelectionList.tsx at the line titleColor = theme.ui.focus;, which indicates that theme.ui is undefined in the test environment for DescriptiveRadioButtonSelect.
To fix this, the mock for semantic-colors.js in DescriptiveRadioButtonSelect.test.tsx needs to be updated to provide a complete theme object, including the ui and background properties with their respective focus keys. A similar update was made in BaseSelectionList.test.tsx which can be used as a reference.
|
You already have 7 pull requests open. Please work on getting existing PRs merged before opening more. |
|
Thank you for linking an issue! This pull request has been automatically reopened. |
|
Size Change: +13.7 kB (+0.05%) Total Size: 25.9 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
priority/p2
keithguerin
changed the title
keithguerin
force-pushed
the
feat/colors-command
branch
from
1dba73b to
ae3f4ca
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring of the UI's color system. By standardizing on semantic tokens like theme.ui.focus and theme.ui.active, it greatly improves visual consistency for focus and activity states across more than 15 components. The removal of theme.border.focused simplifies the theme structure. I appreciate the thoughtful UX enhancements, such as improving the readability of user messages in the history and replacing the old spinner with a new theme-aware one. The addition of the /colors developer tool is a valuable addition for theme creation and debugging. Furthermore, the architectural improvement to resolve a circular dependency between theme.ts and color-utils.ts enhances code maintainability. The changes are comprehensive, consistent, and significantly elevate the quality and clarity of the UI codebase.
Note: Security Review did not run due to the size of the PR.
jacob314
force-pushed
the
feat/colors-command
branch
3 times, most recently
from
e52a03a to
eb4ab8e
Compare
feat(ui): refine /colors layout and add colorized names and vertical gradients feat(ui): move backgrounds to top and further refine /colors layout feat(ui): fix ESLint unescaped entities in /colors feat(ui): finalize /colors layout and improve interaction explanation feat(ui): use primary color for /colors summary and remove background row gaps feat(ui): make the first column narrower in /colors output feat(ui): refine /colors value column width and prevent text wrap in tests feat(ui): merge /colors table into /theme dialog and restrict to authors feat(ui): refine /theme colors debug visualizer layout and styling feat(ui): simplify colors visualizer bullets and remove redundant usage column feat(ui): narrow background boxes and update title in colors visualizer feat(ui): remove redundant column logic and use flexGrow for name column in colors visualizer refactor(ui): implement recursive color flattening and improve contrast in colors visualizer feat(ui): standardize focus/selection color and integrate visualizer into theme dialog feat(ui): implement standardized selection background with interpolation feat(ui): rename selection to focus and use AccentGreen for focus by default fix(ui): resolve test failures and snapshot mismatches fix(ui): improve user message visibility and refine focus hints feat(ui): replace rainbow shell icon with themed blue spinner feat(ui): make shell icon green when focused and fix DescriptiveRadioButtonSelect test mock chore: re-trigger CI fix(cli): make string-width test environment-agnostic fix(cli): update remaining test mocks for consistent semantic-colors chore: re-trigger CI with latest fixes fix(cli): normalize Header tests and mock CliSpinner to avoid act warnings
jacob314
force-pushed
the
feat/colors-command
branch
from
eb4ab8e to
f9ed83f
Compare
…ility (google-gemini#20745) Co-authored-by: jacob314 <jacob314@gmail.com>
…ility (google-gemini#20745) Co-authored-by: jacob314 <jacob314@gmail.com>
…ility (google-gemini#20745) Co-authored-by: jacob314 <jacob314@gmail.com>
…ility (google-gemini#20745) Co-authored-by: jacob314 <jacob314@gmail.com>
2 participants
Summary
This PR represents a comprehensive refinement of the Gemini CLI's semantic color system and history UI. It standardizes how focus and activity are represented across all components, improves the readability of user interactions, and introduces new developer tools for theme validation.
Details
🎨 Color Standardization & Focus Definition
theme.ui.focus(typically Green) as the single source of truth for interactive focus. Removed the divergenttheme.border.focusedto ensure a consistent "keyboard focus" signal across all inputs, dialogs, and selection lists.theme.ui.active(typically Blue) to represent structural activity or background tasks (e.g., running shell commands) that are not currently capturing keyboard input.📖 History & UX Enhancements
🛠 Developer Tools & Robustness
/colorsslash command and integrated theColorsDisplayvisualizer directly into the/themedialog (visible in dev mode).Related Issues
Standardizes UX and addresses consistency concerns related to #18719 and #18640.
Pre-Merge Checklist
theme.tsandcolor-utils.ts.textUtils.