UI: standardize "focus" (fka "selection") color across themes - #17372
UI: standardize "focus" (fka "selection") color across themes#17372keithguerin wants to merge 1 commit into
Conversation
Summary of ChangesHello @keithguerin, 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 introduces a significant improvement to the CLI's user interface by standardizing the selection color across all available themes. By consistently applying the Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request effectively standardizes the selection color across themes by introducing a selection semantic token and mapping it to AccentGreen. The changes are consistently applied across the different theme definitions, the Theme class, and test mocks.
I've left one comment in packages/cli/src/ui/themes/theme.ts regarding a logical redundancy and a potential design improvement in the createCustomTheme function. Addressing this will improve code clarity and maintainability. Overall, this is a good enhancement for theme consistency.
| comment: customTheme.ui?.comment ?? colors.Comment, | ||
| symbol: customTheme.ui?.symbol ?? colors.Gray, | ||
| dark: colors.DarkGray, | ||
| selection: customTheme.status?.success ?? colors.AccentGreen, |
There was a problem hiding this comment.
Tying the selection color to status.success for custom themes might be inflexible, as a user may want to configure them independently. A better long-term solution would be to add a selection property to the ui object in the CustomTheme interface for separate customization.
For the current implementation, there is a logical redundancy. The expression customTheme.status?.success ?? colors.AccentGreen is unnecessarily complex because colors.AccentGreen (defined on line 274) is already configured to use customTheme.status?.success as its primary source.
You can simplify this to make the code cleaner and avoid the redundant check.
| selection: customTheme.status?.success ?? colors.AccentGreen, | |
| selection: colors.AccentGreen, |
1aa6c77 to
9da4c75
Compare
f210812 to
bcea72a
Compare
| "@types/shell-quote": "^1.7.5", | ||
| "@types/tar": "^6.1.13", | ||
| "@types/yargs": "^17.0.32", | ||
| "@vitest/coverage-v8": "^3.2.4", |
There was a problem hiding this comment.
revert changes to all the package.json files. These are spurious
jacob314
left a comment
There was a problem hiding this comment.
Approved once *.json are reverted from the PR and plan.md is reverted.
26a9575 to
eb0fe67
Compare
|
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. You can easily reopen this PR once you have linked it to an issue. How to link an issue: 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! |
20b979b to
e881211
Compare
e881211 to
18c9004
Compare
18c9004 to
9efa855
Compare
9efa855 to
2af88ad
Compare
|
Merged into #20489 to consolidate color changes. |
Summary
Standardize the selection color in the CLI across all themes and UI components using a new
selectionsemantic token. This decouples selection highlighting from thestatus.successcolor.Details
selectionto theSemanticColorsinterface and mapped it toAccentGreenin all base themes (light, dark, ANSI).Themeconstructor andcreateCustomThemeto support the new token, ensuring custom themes can also define their own selection color.SuggestionsDisplay(autocomplete menu for/and@) to use theselectiontoken.SettingsDialogto use theselectiontoken for active items.SessionBrowser(session resume menu) to use theselectiontoken.BaseSelectionList(and by extensionRadioButtonSelect) to use theselectiontoken.DefaultDarkandDefaultLightto explicitly pass their semantic color definitions.Header.test.tsxandno-color.tsmocks to include the new property.Related Issues
None.
How to Validate
npm run buildto ensure the project compiles without errors./,/settings,/resume) and verify that the selection color is consistent and follows the theme's green accent (or custom selection color if defined).Pre-Merge Checklist
Fixes #18719