Fix unreadable repo switcher text in Code Review pane after theme switch (#10600)#10601
Conversation
The repo / branch switcher dropdown in the Code Review pane caches its text color via `Dropdown::set_font_color` from `theme.sub_text_color(theme.background())` once at construction time. The dropdown view re-renders on theme changes, but it reads back the cached `ColorU` instead of re-deriving from the live theme, so the color goes stale across light/dark switches: e.g. opening the panel in dark mode (default) and then switching to a light theme leaves a near-white label on a near-white background, making the repository name unreadable. Subscribe to `AppearanceEvent::ThemeChanged` from the dropdown's own ViewContext and reapply the freshly derived color. The derivation is extracted into a `repo_dropdown_font_color` helper so the construction site and the subscription stay in lock-step. Fixes warpdotdev#10600. Co-Authored-By: Warp <agent@warp.dev> Co-authored-by: Cursor <cursoragent@cursor.com>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR fixes the Code Review pane repository dropdown text color by re-deriving the cached dropdown font color when the app theme changes. The implementation keeps the initial color derivation and the theme-change path aligned through a shared helper and follows existing Appearance subscription patterns.
Concerns
- None.
Found: 0 critical, 0 important, 0 suggestions
Verdict
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…tch (#10600) (#10601) ## Description Fixes #10600. The repository / branch switcher dropdown at the top of the Code Review pane caches its text color via `Dropdown::set_font_color` from `theme.sub_text_color(theme.background())` once at construction time in `CodeReviewState::new` (`app/src/workspace/view/right_panel.rs`). The dropdown view re-renders on `Appearance` changes, but `Dropdown::render_top_bar` reads back the cached `ColorU` instead of re-deriving it from the live theme, so the color goes stale across light ↔ dark switches: - Open the Code Review pane in dark mode (default) → text color is cached as a near-white (correct against a dark background). - Switch to a light theme → the dropdown re-renders, but the cached near-white label is now painted on a near-white background → unreadable, exactly the symptom reported in #10600. - The reverse direction (light → dark) is the same bug: dark text remains on a now-dark background. The chevron icon already reacts correctly because its `Icon::new(...)` color uses `unwrap_or_else(|| appearance.theme().active_ui_text_color().into())`, which re-derives from the current theme each render. Only the cached `font_color` path is stale. This PR subscribes the dropdown to `AppearanceEvent::ThemeChanged` from its own `ViewContext` and reapplies the freshly derived color via `set_font_color`. The derivation is extracted into a `repo_dropdown_font_color` helper so the construction site and the subscription stay in lock-step. The pattern matches the existing convention used by other selectors in the codebase (e.g. `harness_selector.rs`, `auth_secret_selector.rs`, `update_environment_form.rs`). The subscription is scoped to the dropdown view, so it is dropped automatically when the Code Review state is torn down (e.g. when the panel is closed) — no leak, no impact on pane-group switching where the dropdown is reused. ## Linked Issue Fixes #10600. - [x] The linked issue is labeled \`ready-to-spec\` or \`ready-to-implement\` (triaged bug, implicitly \`ready-to-implement\` per CONTRIBUTING.md). - [x] Where appropriate, screenshots or a short video of the implementation are included below. ## Testing Verified locally with \`./script/run\` against the exact reproduction steps from the issue: 1. Launch Warp (defaults to dark theme). 2. Open two tabs in two different git repositories so the repo switcher is visible (it only renders when \`available_repos.len() > 1\`). 3. Open the Code Review pane and confirm the repository name in the top-left dropdown is readable. 4. Switch \`Settings → Appearance\` to a Light theme **without closing the panel**. - Before this fix: the label is near-white on near-white and effectively invisible. - After this fix: the label re-derives a dark text color and stays readable. 5. Switch back to the dark theme — label updates to a light color, still readable. 6. Close and reopen the panel in light mode — label is correct from the start (sanity check that the construction path also still works). Automated coverage: - \`cargo fmt -- --check\` ✓ - \`cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings\` ✓ - \`cargo nextest run -p warp\` — 4011 / 4012 passing. The single failure (\`settings::init::tests::test_migration_does_not_rerun_when_marker_present\`) reproduces identically on a clean \`master\` checkout in the same workspace and is unrelated to this change (it's a settings-migration test sensitive to local marker state). No new automated test added — the regression is a stale cache in a generic \`Dropdown<A>\` view; asserting it would require either exposing a \`font_color\` getter on the shared component or building a theme-switch UI test harness, both of which feel disproportionate for a 12-line view-construction fix. Happy to wire up coverage in either direction if reviewers prefer. - [x] I have manually tested my changes locally with \`./script/run\`. ### Screenshots / Videos Before: <img width="790" height="670" alt="image" src="https://github.com/user-attachments/assets/e27895fa-7afd-4058-966a-7072b52b26af" /> Fixed: <img width="383" height="173" alt="image" src="https://github.com/user-attachments/assets/bf1de60e-9f5b-458c-90cf-7d718dec6cd0" /> ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-BUG-FIX: Fix unreadable repository switcher text in the Code Review pane after toggling between light and dark themes. --> CHANGELOG-BUG-FIX: Fix unreadable repository switcher text in the Code Review pane after toggling between light and dark themes. Co-Authored-By: Warp <agent@warp.dev> Co-authored-by: Cursor <cursoragent@cursor.com> Made with [Cursor](https://cursor.com) Co-authored-by: Warp <agent@warp.dev> Co-authored-by: Cursor <cursoragent@cursor.com>
…tch (#10600) (#10601) ## Description Fixes #10600. The repository / branch switcher dropdown at the top of the Code Review pane caches its text color via `Dropdown::set_font_color` from `theme.sub_text_color(theme.background())` once at construction time in `CodeReviewState::new` (`app/src/workspace/view/right_panel.rs`). The dropdown view re-renders on `Appearance` changes, but `Dropdown::render_top_bar` reads back the cached `ColorU` instead of re-deriving it from the live theme, so the color goes stale across light ↔ dark switches: - Open the Code Review pane in dark mode (default) → text color is cached as a near-white (correct against a dark background). - Switch to a light theme → the dropdown re-renders, but the cached near-white label is now painted on a near-white background → unreadable, exactly the symptom reported in #10600. - The reverse direction (light → dark) is the same bug: dark text remains on a now-dark background. The chevron icon already reacts correctly because its `Icon::new(...)` color uses `unwrap_or_else(|| appearance.theme().active_ui_text_color().into())`, which re-derives from the current theme each render. Only the cached `font_color` path is stale. This PR subscribes the dropdown to `AppearanceEvent::ThemeChanged` from its own `ViewContext` and reapplies the freshly derived color via `set_font_color`. The derivation is extracted into a `repo_dropdown_font_color` helper so the construction site and the subscription stay in lock-step. The pattern matches the existing convention used by other selectors in the codebase (e.g. `harness_selector.rs`, `auth_secret_selector.rs`, `update_environment_form.rs`). The subscription is scoped to the dropdown view, so it is dropped automatically when the Code Review state is torn down (e.g. when the panel is closed) — no leak, no impact on pane-group switching where the dropdown is reused. ## Linked Issue Fixes #10600. - [x] The linked issue is labeled \`ready-to-spec\` or \`ready-to-implement\` (triaged bug, implicitly \`ready-to-implement\` per CONTRIBUTING.md). - [x] Where appropriate, screenshots or a short video of the implementation are included below. ## Testing Verified locally with \`./script/run\` against the exact reproduction steps from the issue: 1. Launch Warp (defaults to dark theme). 2. Open two tabs in two different git repositories so the repo switcher is visible (it only renders when \`available_repos.len() > 1\`). 3. Open the Code Review pane and confirm the repository name in the top-left dropdown is readable. 4. Switch \`Settings → Appearance\` to a Light theme **without closing the panel**. - Before this fix: the label is near-white on near-white and effectively invisible. - After this fix: the label re-derives a dark text color and stays readable. 5. Switch back to the dark theme — label updates to a light color, still readable. 6. Close and reopen the panel in light mode — label is correct from the start (sanity check that the construction path also still works). Automated coverage: - \`cargo fmt -- --check\` ✓ - \`cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings\` ✓ - \`cargo nextest run -p warp\` — 4011 / 4012 passing. The single failure (\`settings::init::tests::test_migration_does_not_rerun_when_marker_present\`) reproduces identically on a clean \`master\` checkout in the same workspace and is unrelated to this change (it's a settings-migration test sensitive to local marker state). No new automated test added — the regression is a stale cache in a generic \`Dropdown<A>\` view; asserting it would require either exposing a \`font_color\` getter on the shared component or building a theme-switch UI test harness, both of which feel disproportionate for a 12-line view-construction fix. Happy to wire up coverage in either direction if reviewers prefer. - [x] I have manually tested my changes locally with \`./script/run\`. ### Screenshots / Videos Before: <img width="790" height="670" alt="image" src="https://github.com/user-attachments/assets/e27895fa-7afd-4058-966a-7072b52b26af" /> Fixed: <img width="383" height="173" alt="image" src="https://github.com/user-attachments/assets/bf1de60e-9f5b-458c-90cf-7d718dec6cd0" /> ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-BUG-FIX: Fix unreadable repository switcher text in the Code Review pane after toggling between light and dark themes. --> CHANGELOG-BUG-FIX: Fix unreadable repository switcher text in the Code Review pane after toggling between light and dark themes. Co-Authored-By: Warp <agent@warp.dev> Co-authored-by: Cursor <cursoragent@cursor.com> Made with [Cursor](https://cursor.com) Co-authored-by: Warp <agent@warp.dev> Co-authored-by: Cursor <cursoragent@cursor.com>
Description
Fixes #10600.
The repository / branch switcher dropdown at the top of the Code Review pane caches its text color via
Dropdown::set_font_colorfromtheme.sub_text_color(theme.background())once at construction time inCodeReviewState::new(app/src/workspace/view/right_panel.rs). The dropdown view re-renders onAppearancechanges, butDropdown::render_top_barreads back the cachedColorUinstead of re-deriving it from the live theme, so the color goes stale across light ↔ dark switches:The chevron icon already reacts correctly because its
Icon::new(...)color usesunwrap_or_else(|| appearance.theme().active_ui_text_color().into()), which re-derives from the current theme each render. Only the cachedfont_colorpath is stale.This PR subscribes the dropdown to
AppearanceEvent::ThemeChangedfrom its ownViewContextand reapplies the freshly derived color viaset_font_color. The derivation is extracted into arepo_dropdown_font_colorhelper so the construction site and the subscription stay in lock-step. The pattern matches the existing convention used by other selectors in the codebase (e.g.harness_selector.rs,auth_secret_selector.rs,update_environment_form.rs).The subscription is scoped to the dropdown view, so it is dropped automatically when the Code Review state is torn down (e.g. when the panel is closed) — no leak, no impact on pane-group switching where the dropdown is reused.
Linked Issue
Fixes #10600.
Testing
Verified locally with `./script/run` against the exact reproduction steps from the issue:
Automated coverage:
No new automated test added — the regression is a stale cache in a generic `Dropdown` view; asserting it would require either exposing a `font_color` getter on the shared component or building a theme-switch UI test harness, both of which feel disproportionate for a 12-line view-construction fix. Happy to wire up coverage in either direction if reviewers prefer.
Screenshots / Videos
Before:
Fixed:
Agent Mode
CHANGELOG-BUG-FIX: Fix unreadable repository switcher text in the Code Review pane after toggling between light and dark themes.
Co-Authored-By: Warp agent@warp.dev
Co-authored-by: Cursor cursoragent@cursor.com
Made with Cursor