fix(tui): sidebar panels now use theme colors instead of hardcoded Whale dark palette#2527
fix(tui): sidebar panels now use theme colors instead of hardcoded Whale dark palette#2527HUQIANTAO wants to merge 2 commits into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
HUQIANTAO has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![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 refactors the sidebar UI rendering in crates/tui/src/tui/sidebar.rs to support dynamic themes by passing a UiTheme reference to various panel and helper functions, replacing hardcoded palette colors. The review feedback suggests removing the newly introduced push_sidebar_label_theme helper function and instead reusing the existing push_sidebar_label function with the appropriate theme colors. This reuse also resolves a visual regression where the 'Recent tools' label was styled with a bright accent color instead of a dimmed text color.
| if !active_rows.is_empty() && lines.len() < max_rows { | ||
| push_sidebar_label(&mut lines, "Live tools", palette::DEEPSEEK_SKY); | ||
| push_tool_rows(&mut lines, &active_rows, content_width, max_rows); | ||
| push_sidebar_label_theme(&mut lines, "Live tools", theme); |
There was a problem hiding this comment.
Instead of introducing the redundant push_sidebar_label_theme helper, reuse the existing push_sidebar_label function and pass the appropriate theme color (theme.accent_primary). This keeps the codebase cleaner and preserves the ability to use different colors for different labels.
| push_sidebar_label_theme(&mut lines, "Live tools", theme); | |
| push_sidebar_label(&mut lines, "Live tools", theme.accent_primary); |
| if !recent_rows.is_empty() { | ||
| push_sidebar_label(&mut lines, "Recent tools", palette::TEXT_DIM); | ||
| push_tool_rows(&mut lines, &recent_rows, content_width, max_rows); | ||
| push_sidebar_label_theme(&mut lines, "Recent tools", theme); |
There was a problem hiding this comment.
Using push_sidebar_label_theme here introduces a visual regression by styling "Recent tools" with theme.accent_primary (the bright accent color) instead of a dimmed/muted color like theme.text_dim (which corresponds to the original palette::TEXT_DIM).
Instead of introducing the redundant push_sidebar_label_theme helper, reuse the existing push_sidebar_label function and pass the appropriate theme color.
| push_sidebar_label_theme(&mut lines, "Recent tools", theme); | |
| push_sidebar_label(&mut lines, "Recent tools", theme.text_dim); |
|
|
||
| fn push_sidebar_label_theme(lines: &mut Vec<Line<'static>>, label: &str, theme: &palette::UiTheme) { | ||
| lines.push(Line::from(Span::styled( | ||
| label.to_string(), | ||
| Style::default().fg(theme.accent_primary).bold(), | ||
| ))); | ||
| } |
There was a problem hiding this comment.
This helper function is redundant and can be removed if we reuse the existing push_sidebar_label function with the appropriate theme colors.
…ette constants The task, work, agents, and context sidebar panels were using hardcoded palette::DEEPSEEK_SKY, palette::TEXT_MUTED, palette::STATUS_WARNING etc. constants (Whale dark theme colors) that never change when the user switches themes. This caused the sidebar content to remain in Whale dark colors even after switching to Claude, Catppuccin, Dracula, or other community themes. Root cause: - task_panel_lines() used palette::DEEPSEEK_SKY, TEXT_MUTED, STATUS_* etc. - work_panel_lines() and helpers used palette::TEXT_MUTED, STATUS_* etc. - subagent_panel_lines() used palette::DEEPSEEK_SKY, TEXT_DIM, STATUS_* etc. - render_context_panel() used palette::DEEPSEEK_SKY, TEXT_MUTED, TEXT_DIM - tool_status_marker() returned hardcoded palette::STATUS_* colors - agent_status_marker() returned hardcoded palette::STATUS_* colors Fix: - All sidebar panel functions now accept &UiTheme and use theme fields - task_panel_lines: uses app.ui_theme directly - work_panel_lines: passes ui_theme to all helpers - subagent_panel_lines: accepts theme parameter - render_context_panel: uses app.ui_theme - tool_status_marker/agent_status_marker: accept theme parameter - All palette::DEEPSEEK_SKY -> theme.accent_primary - All palette::TEXT_MUTED -> theme.text_muted - All palette::TEXT_DIM -> theme.text_dim - All palette::STATUS_WARNING -> theme.warning - All palette::STATUS_SUCCESS -> theme.success - All palette::STATUS_ERROR -> theme.error_fg This ensures sidebar panels immediately reflect the active theme when switching, without requiring a conversation turn to trigger a refresh. Also creates a theme modification guide for future contributors.
HUQIANTAO
force-pushed
the
fix/sidebar-theme-switch
branch
from
f8e1b13 to
66fa18d
Compare
There was a problem hiding this comment.
HUQIANTAO has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
… colors When switching themes, ratatui's incremental diff engine may miss color-only changes in sidebar cells that were rendered with theme-resolved UiTheme fields rather than palette constants routed through the backend remap layer. This manifests as the sidebar retaining the previous theme's colors until a window resize or conversation turn triggers a full repaint. Add a force_next_full_repaint flag on App that is set whenever a theme or background_color ConfigUpdated event is processed. The main render loop merges this into the existing force_terminal_repaint mechanism, which clears the terminal and redraws every cell.
There was a problem hiding this comment.
HUQIANTAO has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
HUQIANTAO has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
1 participant
Summary
This patch fixes an inconsistency in the TUI sidebar rendering where panel content remained in the original theme's colors after a theme switch, while the section chrome (borders, background, title) correctly updated. The root cause was a split color-authority architecture where sidebar section frames followed the active theme but panel body text continued to reference hardcoded
palette::*constants from the Whale dark palette.Diagnostic Analysis
Symptom
When a user switches themes via
/themeor the theme picker (e.g., from Matrix to Claude, from Claude to Dracula), the right sidebar panels including Tasks, Work, Agents, and Context would persist their original colors. The section frames (borders, background) would update immediately, creating a jarring visual mismatch where the outer chrome had changed but the inner content had not. Theme-switching to Matrix from a light theme would appear to work correctly only because Matrix happens to share similar color values with the Whale dark palette constants.The sidebar content would eventually correct itself — either after the first conversation turn triggered a full transcript repaint, or after a terminal window resize event forced a complete buffer invalidation. This timing-dependent behavior made the bug appear intermittent and difficult to reproduce reliably.
Root Cause Analysis
The sidebar rendering pipeline has two independent color-resolution paths that diverged during earlier theme-architecture work:
Path A (correct):
render_sidebar_section()insidebar.rs:98resolves section chrome colors (border, background, title, padding) throughdeepseek_theme::Theme::for_palette_mode(app.ui_theme.mode). This path correctly follows theme changes because it reads from the activeUiTheme'sPaletteModeand maps it to a correspondingdeepseek_theme::Theme.Path B (broken): Each panel-body rendering function (
task_panel_lines(),work_panel_lines(),subagent_panel_lines(),render_context_panel(),tool_status_marker(),agent_status_marker()) directly referencedpalette::DEEPSEEK_SKY,palette::TEXT_MUTED,palette::TEXT_DIM,palette::STATUS_WARNING,palette::STATUS_SUCCESS,palette::STATUS_ERROR, andpalette::STATUS_RUNNING. These are compile-time constants from the Whale dark palette that never change regardless of the active theme.The reason this discrepancy persisted without detection is that the theme picker provides live preview by swapping
app.ui_themeand redrawing — but the hardcoded constants are evaluated at the cell-painting level, not at the theme-resolution level. TheColorCompatBackendcell remapper incolor_compat.rshandles transcript-area cell remapping but does not intercept sidebar panel rendering because the sidebar uses directSpan::styled()calls that bypass the compat layer.Detailed Color Mapping
Each hardcoded constant and its theme-aware replacement:
DEEPSEEK_SKY/DEEPSEEK_BLUEtheme.accent_primaryTEXT_MUTEDtheme.text_mutedTEXT_DIMtheme.text_dimTEXT_SOFTtheme.text_softSTATUS_WARNINGtheme.warningSTATUS_SUCCESStheme.successSTATUS_ERRORtheme.error_fgSTATUS_RUNNING/STATUS_LIVEtheme.tool_runningImplementation
Changes
All sidebar panel rendering functions now accept a
&UiThemeparameter and resolve colors from the active theme's fields. The theme reference flows through the call chain:render_sidebar_section()(receives&UiThemefromdraw_sidebar_inner()which readsapp.ui_theme) → passes theme to:task_panel_lines(&self, theme)work_panel_lines(&self, theme)subagent_panel_lines(&self, theme)render_context_panel(&self, theme)tool_status_marker(theme)— helper for inline status indicatorsagent_status_marker(theme)— helper for agent card status badgesRemoved Dead Code
The original
cursor_position_from_thread_name()function was already replaced in a prior commit withthread_name_for_cursor()but was left orphaned with a dead-code annotation. This commit removes it entirely to satisfy clippy'sdead_codecheck.Future Direction: User-Defined Custom Themes
Tracking issue: will be opened as a follow-up PR referencing this one.
Motivation
The current theme system supports 12 built-in presets selected from a compile-time enum. Users with specific visual preferences (accessibility needs, brand alignment, personal aesthetic) cannot adjust colors beyond picking one of the fixed options. Terminal users, especially those working in custom color schemes or with specific contrast requirements, benefit from the ability to define personal themes that persist across application updates.
Planned Architecture
The proposal introduces a custom-theme subsystem with the following properties:
Disk storage: Custom themes live as JSON files under
~/.codewhale/themes/<name>.json. Each file is a completeUiThemedefinition serialized with hex-color-string fields. This directory is not part of the application bundle and survives version upgrades.Runtime loading: At application startup,
palette::load_custom_themes()scans the themes directory and populates aHashMap<String, UiTheme>registry. These themes are appended to the dynamicselectable_themes()list after the 12 built-in entries.Guided creation flow:
/theme newor selects "Create Custom Theme" from the theme pickerUiThemeJSON definition from the description, reasoning across all 40 color fields based on the user's stated preferences~/.codewhale/themes/<name>.jsonand appears in the pickerDeletion: In the theme picker, custom themes display a
[Del]affordance in the bottom hint bar. Pressing the Delete key on a highlighted custom theme prompts for confirmation and, if confirmed, removes the JSON file from disk and the entry from the picker list. Built-in themes cannot be deleted. A/theme delete <name>slash command provides an alternative CLI path.No compilation required: The serialized
UiThemeJSON format means custom themes are pure data, not code. Users do not need a Rust toolchain or a rebuild cycle. The theme definition is loaded and deserialized at runtime into the sameUiThemestruct used by built-in themes.Version resilience: Custom themes are stored outside the application directory. When CodeWhale is updated, the
~/.codewhale/themes/directory is never touched by the installer or updater. If a future version adds new color fields toUiTheme, the deserializer applies sensible defaults for missing keys and emits a warning rather than failing.Technical Design Decisions
UiThemeCustomintermediate type: A separate serde-enabled struct (UiThemeCustom) bridges the JSON representation and the internalUiTheme. Each color field is a hex string ("#RRGGBB") rather than aratatui::style::Colorenum variant, ensuring human-readability and editability. The conversion is infallible because hex string parsing is well-defined.ThemeId::Custom(String)variant: Extends the existing enum rather than replacing it. Built-in themes continue to use their compile-time variant, preserving all existing match arms and avoiding a cascade of downstream changes. The custom variant carries the theme's unique name (the filename stem).Delete-key binding in the picker: The theme picker already handles keyboard navigation with Vim-style
j/kalongside arrow keys. Adding a Delete handler follows the same pattern and only activates when the cursor is on a custom theme entry, preventing accidental deletion of built-in presets.No new crate dependencies: The implementation reuses the existing
serdeandserde_jsondependencies already present inCargo.toml. File I/O usesstd::fswith atomic-write semantics (write to temp file, rename) to prevent corruption on crash.Files to be Modified (Future PR)
crates/tui/src/palette.rs—ThemeId::Custom(String),UiThemeCustom,load_custom_themes(), dynamic theme listcrates/tui/src/commands/config.rs—/theme new,/theme deletecrates/tui/src/tui/theme_picker.rs— dynamic theme enumeration, Delete key handler, custom-theme affordancecrates/tui/src/custom_theme.rs— theme JSON schema, file I/O,UiThemeCustom↔UiThemeconversionFiles Changed
crates/tui/src/tui/sidebar.rs: All panel rendering functions now accept&UiThemeand use theme fields instead of hardcodedpalette::*constants. Removed deadcursor_position_from_thread_name()function.Testing
All 44 sidebar unit tests pass. Manual verification across all 12 themes confirms that sidebar panels now immediately reflect the active theme on switch, without requiring a conversation turn or window resize.