feat(ui): add browser/refresh/lock icons, drop selection-state active variants - #1201
Conversation
… variants Add three monoline chrome icons for the upcoming embedded-browser tab (PR1): browser (globe), refresh (circular arrow), lock (padlock). Each was generated from an imagegen sheet, traced via potrace + sharp, and fitted into the shared 0 0 20 20 viewBox the same way as the rest of the registry. The same change removes the three selection-state "active" icon variants that were never visually useful: tab/toggle selection is already shown by the chip background, so review-active / terminal-active / status-active only added a redundant corner badge. Dropping them lets RightPanelShellIcon lose its `active` prop and RightPanelTabStrip lose the now-unused `activeTab` prop. sidebar-active is intentionally kept -- it is the sidebar / right-panel on-state toggle, not a selection badge. Verification: - eslint (changed files) + turbo typecheck: pass - e2e icon-viewbox-fit @smoke: pass -- every glyph incl. browser/refresh/lock renders inside 0..20 (no clipping) - right-panel-titlebar: Status (selected) and Review chips render correctly with no active badge, confirmed from the captured frame. The snap's own assertion is flaky locally on a pre-existing file-tree keybind race (mod+\ first-press drop) unrelated to this change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR removes active-state-driven icon switching across the session UI. Icon registry adds new glyphs and removes ChangesActive-State Icon Removal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. Comment |
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/components/status-popover.tsx, packages/app/src/pages/session/right-panel-tab-strip.tsx, packages/app/src/pages/session/session-side-panel.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
There was a problem hiding this comment.
Code Review
This pull request simplifies icon rendering by removing active-state icons (review-active, status-active, and terminal-active) along with their associated conditional logic in components like StatusPopover and RightPanelTabStrip. Additionally, it adds new icons for browser, lock, and refresh to the icon library. There are no review comments, so I have no feedback to provide.
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.
Summary
Add three monoline chrome icons to the registry —
browser(globe),refresh(circular arrow),lock(padlock) — and remove the three selection-state "active" icon variants:review-active,terminal-active,status-active.Why
The three new icons are needed by the upcoming embedded-browser tab (the first shippable PR of #1186):
browsermarks the tab,refreshandlocksit in the address bar. Landing the icon work here keeps that PR focused on the WebContentsView wiring.The
*-activevariants were never visually useful: tab/toggle selection is already shown by the chip background, so each one only added a redundant corner badge. Removing them letsRightPanelShellIcondrop itsactiveprop andRightPanelTabStripdrop the now-unusedactiveTabprop (net −8 lines).sidebar-activeis intentionally kept — it is the sidebar / right-panel on-state toggle, not a selection badge.Each new glyph was generated from an imagegen sheet, traced via potrace + sharp, and fitted into the shared
0 0 20 20viewBox the same way as the rest of the registry.Related Issue
Part of #1186 (OpenCLI / embedded browser). Icon-prep PR ahead of the embedded-browser tab.
Human Review Status
Pending
Review Focus
icon.tsx: the 3 new glyph paths (keyshape fit inside the 20×20 viewBox) and the 3 deletions.right-panel-tab-strip.tsx: removal of theactive/activeTabprops — confirm no other caller relied on them (the upstreamactiveTabmemo is still used by the review body).Risk Notes
Low. The only behavior change is that chip/toggle selection no longer swaps to a badge icon — selection is shown by the chip background, which is the intended design.
sidebar-activeis kept. No platform/packaging/docs/dependency surface was touched (the two conditional checklist items for those are left unticked for that reason).How To Verify
Screenshots or Recordings
Chip surface verified from a captured Chromium frame (local e2e artifact, not inlined): the right-panel tab strip shows the Status chip selected and the Review chip at rest, both rendering their icon with no active corner badge — selection is conveyed by the chip background only. The three new glyphs are not yet wired into any surface (that happens in the embedded-browser PR); their geometry is verified in Chromium by the
icon-viewbox-fitsmoke test above.Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
Refactor
Style