Fix unreadable agent toolbelt chip text after theme change#12970
Conversation
Context-chip colors are derived from the theme when a chip is built, and the agent toolbelt editors cache their chip renderers (rebuilding only on chip-selection changes). So after switching themes the cached colors went stale — e.g. a color built under a light theme (near-black) stayed on the chip when the user switched to a dark theme like Cyber Wave, rendering the value chips dark-on-dark and unreadable. The neighboring action chips were unaffected because they recompute their color on every render. Refresh the chips' colors dynamically when the theme changes: the agent toolbelt editors subscribe to `AppearanceEvent::ThemeChanged` and rebuild the configurator's chips against the live appearance (preserving the current arrangement, including unsaved edits), via the new `ChipConfigurator::refresh_appearance`. Co-Authored-By: Warp <agent@warp.dev>
1b08549 to
0129823
Compare
Cross-checking other places these context chips are rendered surfaced two more spots with the same stale-color-on-theme-change root cause, both caching theme-derived chip renderers without refreshing on a theme switch: - The terminal prompt editor (`prompt/editor_modal.rs`) caches its `SingleZone` chip renderers and had no theme-change handling. - The Settings > Appearance "Input" preview (`settings_view/appearance_page.rs`) cached `context_chips` and explicitly ignored `ThemeChanged`. Both use prompt colors (vs the agent toolbelt's neutral text), so the visible impact is usually milder, but switching from a light theme to a dark one can still leave dark-on-dark chip text. Subscribe both to `AppearanceEvent::ThemeChanged` and rebuild their chips against the live appearance. Not affected: the header toolbar editor (its items recompute their color at render time) and the live terminal input/agent footer surfaces (which already react to theme changes / render fresh). Co-Authored-By: Warp <agent@warp.dev>
| /// (agent toolbelt) is rebuilt this way; `SingleZone` chips are built from | ||
| /// pre-made renderers and are refreshed by their owner. | ||
| pub fn refresh_appearance(&mut self, appearance: &Appearance) { | ||
| if self.layout != ChipConfiguratorLayout::LeftRightZones { |
There was a problem hiding this comment.
Why we are early returning for non LeftRightZones layout? This contract doesn't seem semantically correct given the method signature
There was a problem hiding this comment.
Good call — removed refresh_appearance entirely. As you noted, silently no-op'ing for SingleZone is an unclear contract for a method that reads like it refreshes any configurator. Instead, each agent toolbelt editor now rebuilds its chips via its own existing, layout-appropriate construction path on AppearanceEvent::ThemeChanged:
AgentToolbarInlineEditorreusesreset_from_settings(it persists its arrangement on every edit, so reloading preserves the user's layout) — matching the existingSessionSettingsrefresh right above it.AgentToolbarEditorModalreusesopen_toolbar_items_from_settings, guarded byhas_items()so a closed modal isn't repopulated.
Net behavior is unchanged (chips rebuild against the live theme on a theme switch), but there's no longer a configurator method whose effect depends on a layout it doesn't take as a parameter. Fixed in 15a1a7d.
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR refreshes cached context-chip renderers on theme changes for the agent toolbelt editor, terminal prompt editor, and Appearance settings input preview so theme-derived chip text colors stay readable after switching themes.
Concerns
- For this user-facing UI change, please include screenshots or a screen recording demonstrating the chip text remains readable after switching themes end to end. The provided PR description includes manual verification text but no attached visual evidence, which the Warp review guidance requires for UI-impacting changes.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…gated helper Replace ChipConfigurator::refresh_appearance — which silently no-op'd for the SingleZone layout, an unclear contract for a method that reads like it refreshes any configurator — with rebuilds via each editor's existing, layout-appropriate construction path on AppearanceEvent::ThemeChanged: - AgentToolbarInlineEditor reuses reset_from_settings (it persists its arrangement on every edit, so reloading preserves the user's layout), matching the existing SessionSettings refresh. - AgentToolbarEditorModal reuses open_toolbar_items_from_settings, guarded by has_items() so a closed modal isn't repopulated. Net behavior is unchanged (chips rebuild against the live theme on a theme switch), but there's no longer a configurator method whose effect depends on a layout it doesn't take as a parameter. Co-Authored-By: Warp <agent@warp.dev>
Description
Fixes context-chip text becoming unreadable after a theme switch (reported for the agent toolbelt in the Cyber Wave theme via Slack).
Root cause: Several surfaces cache context-chip renderers whose text color is derived from the theme at build time, and only rebuild on chip-selection changes. After switching themes the cached colors go stale — e.g. a color built under a light theme (near-black) stays on the chip when the user switches to a dark theme, rendering chip text dark-on-dark. (Action/control chips were unaffected because they recompute their color on every render.)
Fix: Refresh the chips' colors dynamically on theme change. Each affected editor/preview subscribes to
AppearanceEvent::ThemeChangedand rebuilds its chips against the live appearance, preserving the current arrangement.Surfaces fixed (after cross-checking all chip render sites):
agent_input_footer/editor.rs) — the originally reported, highest-severity case (neutral text → dark-on-dark). NewChipConfigurator::refresh_appearance.prompt/editor_modal.rs) — cachedSingleZonechips; rebuilds used chips on theme change.settings_view/appearance_page.rs) —handle_appearance_updatenow handlesThemeChangedand rebuilds the preview chips.Checked and not affected: the header toolbar editor (control items recompute color at render time) and the live terminal input / agent footer (already react to theme changes / render fresh).
Testing
computer_use(agent toolbelt editor, Light → Cyber Wave → Dark).#0c1e24; a color built under Light composites to#0f1619, contrast 1.07 (the bug); rebuilt under Cyber Wave it is#9ea5a7, contrast 6.84 (readable, matches the action chips, above WCAG AA 4.5).cargo check/build pass;cargo clippy -p warp --tests -- -D warningsclean; existingcontext_chips::renderertests pass.Agent Mode
Conversation: https://staging.warp.dev/conversation/8107a59b-3e05-40e9-894b-a43c4f41a938
Run: https://oz.staging.warp.dev/runs/019ef64c-af47-7971-b304-e6adcee9caa2
This PR was generated with Oz.