Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 106 additions & 9 deletions crates/tui/src/palette.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,63 @@ pub const DRACULA_UI_THEME: UiTheme = UiTheme {
tool_failed: Color::Rgb(0xff, 0x55, 0x55), // red
};

/// "Terminal" theme: lets the host terminal's color scheme show through
/// instead of painting any RGB surface. Backgrounds use `Color::Reset`
/// (the terminal's own default bg) and most text uses `Color::Reset`
/// (terminal's own default fg). Accents are ANSI named colors so they
/// also inherit the user's terminal palette (Solarized, Nord, custom
/// schemes, etc.) rather than DeepSeek brand RGB.
pub const TERMINAL_UI_THEME: UiTheme = UiTheme {
name: "terminal",
// Mode is reported as Dark to avoid the dark→light cell remap kicking
// in; the terminal-theme cell remap already normalizes everything to
// `Color::Reset`, and we never want a second pass overwriting that.
mode: PaletteMode::Dark,
surface_bg: Color::Reset,
panel_bg: Color::Reset,
elevated_bg: Color::Reset,
composer_bg: Color::Reset,
selection_bg: Color::Reset,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using Color::Reset for selection_bg means that selected text or list items will have the same background color as the default terminal background. This makes selections (such as text highlighted for copying or selected menu items) completely invisible or extremely difficult to distinguish.

Consider using a standard ANSI color like Color::DarkGray or Color::Gray for selection_bg to ensure selections are visually distinct while still respecting the user's terminal palette.

Suggested change
selection_bg: Color::Reset,
selection_bg: Color::DarkGray,

header_bg: Color::Reset,
footer_bg: Color::Reset,
text_dim: Color::Reset,
text_hint: Color::Reset,
text_muted: Color::Reset,
text_body: Color::Reset,
text_soft: Color::Reset,
Comment on lines +847 to +851
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Setting text_dim, text_hint, text_muted, and text_soft all to Color::Reset collapses the entire text hierarchy of the TUI. Since text_body is also Color::Reset, all text (including timestamps, muted secondary info, hints, and main body text) will render in the exact same default terminal foreground color, making the interface much harder to scan.

To preserve the visual hierarchy while inheriting the terminal's color scheme, consider using standard ANSI grayscale colors like Color::DarkGray for dim/hint text and Color::Gray for muted/soft text.

Suggested change
text_dim: Color::Reset,
text_hint: Color::Reset,
text_muted: Color::Reset,
text_body: Color::Reset,
text_soft: Color::Reset,
text_dim: Color::DarkGray,
text_hint: Color::DarkGray,
text_muted: Color::Gray,
text_body: Color::Reset,
text_soft: Color::Reset,

border: Color::Reset,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using Color::Reset for border renders all panel and modal borders in the default terminal foreground color (same as body text). This can make borders visually loud and distracting.

Consider using Color::DarkGray or Color::Gray for border to keep the layout clean and unobtrusive while still inheriting the terminal's palette.

Suggested change
border: Color::Reset,
border: Color::DarkGray,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Invisible selection with Color::Reset selection background

selection_bg: Color::Reset means the highlighted item in any list/picker looks identical to un-highlighted items — both use the terminal's default background. A user navigating the theme picker or any other selectable list while on the Terminal theme will have no visual cue about what is currently selected. Even a named ANSI color (Color::Blue, Color::DarkGray) would preserve the terminal-transparent aesthetic while keeping selections legible.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Devin

accent_primary: Color::Blue,
accent_secondary: Color::Cyan,
accent_action: Color::Yellow,
error_fg: Color::Red,
error_hover: Color::Red,
error_surface: Color::Reset,
error_border: Color::Red,
error_text: Color::Red,
warning: Color::Yellow,
success: Color::Green,
info: Color::Cyan,
mode_agent: Color::Blue,
mode_yolo: Color::Red,
// Magenta keeps Plan visually distinct from `status_warning` (yellow)
// so the mode indicator and warning chip don't collide on themes that
// render both in the status row.
mode_plan: Color::Magenta,
mode_goal: Color::Green,
// DarkGray gives "Ready" a low-contrast but still distinguishable hue
// versus default body text (which is `Color::Reset` on this theme).
status_ready: Color::DarkGray,
status_working: Color::Cyan,
status_warning: Color::Yellow,
diff_added_fg: Color::Green,
diff_deleted_fg: Color::Red,
diff_added_bg: Color::Reset,
diff_deleted_bg: Color::Reset,
tool_running: Color::Cyan,
tool_success: Color::Green,
tool_failed: Color::Red,
};

pub const GRUVBOX_DARK_UI_THEME: UiTheme = UiTheme {
name: "gruvbox-dark",
mode: PaletteMode::Dark,
Expand Down Expand Up @@ -874,6 +931,7 @@ pub const GRUVBOX_DARK_UI_THEME: UiTheme = UiTheme {
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ThemeId {
System,
Terminal,
Whale,
WhaleLight,
Grayscale,
Expand All @@ -891,6 +949,7 @@ impl ThemeId {
pub fn from_name(value: &str) -> Option<Self> {
match normalize_theme_name(value)? {
"system" => Some(Self::System),
"terminal" => Some(Self::Terminal),
"dark" => Some(Self::Whale),
"light" => Some(Self::WhaleLight),
"grayscale" => Some(Self::Grayscale),
Expand All @@ -908,6 +967,7 @@ impl ThemeId {
pub const fn name(self) -> &'static str {
match self {
Self::System => "system",
Self::Terminal => "terminal",
Self::Whale => "dark",
Self::WhaleLight => "light",
Self::Grayscale => "grayscale",
Expand All @@ -923,6 +983,7 @@ impl ThemeId {
pub const fn display_name(self) -> &'static str {
match self {
Self::System => "System",
Self::Terminal => "Terminal",
Self::Whale => "Whale (Dark)",
Self::WhaleLight => "Whale Light",
Self::Grayscale => "Grayscale",
Expand All @@ -938,6 +999,7 @@ impl ThemeId {
pub const fn tagline(self) -> &'static str {
match self {
Self::System => "Follow terminal background (COLORFGBG / macOS appearance)",
Self::Terminal => "Inherit terminal colors fully (transparent surfaces, ANSI accents)",
Self::Whale => "Whale dark — deep navy & gold",
Self::WhaleLight => "DeepSeek light, paper-ish",
Self::Grayscale => "Color-minimal high contrast",
Expand All @@ -956,6 +1018,7 @@ impl ThemeId {
pub fn ui_theme(self) -> UiTheme {
match self {
Self::System => UiTheme::detect(),
Self::Terminal => TERMINAL_UI_THEME,
Self::Whale => UI_THEME,
Self::WhaleLight => LIGHT_UI_THEME,
Self::Grayscale => GRAYSCALE_UI_THEME,
Expand All @@ -970,6 +1033,7 @@ impl ThemeId {
/// Themes shown in the `/theme` picker, in display order.
pub const SELECTABLE_THEMES: &[ThemeId] = &[
ThemeId::System,
ThemeId::Terminal,
ThemeId::Whale,
ThemeId::WhaleLight,
ThemeId::Grayscale,
Expand Down Expand Up @@ -1012,6 +1076,7 @@ impl UiTheme {
pub fn normalize_theme_name(value: &str) -> Option<&'static str> {
match value.trim().to_ascii_lowercase().as_str() {
"" | "auto" | "system" | "default" => Some("system"),
"terminal" | "term" | "transparent" | "follow-terminal" | "inherit" => Some("terminal"),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 term and follow-terminal aliases lack test coverage

The existing test only exercises transparent and inherit. The term and follow-terminal aliases introduced in the same match arm have no assertions, so a future typo or refactor could silently break them.

Fix in Devin

"dark" | "whale" | "whale-dark" => Some("dark"),
"light" | "whale-light" => Some("light"),
"grayscale" | "greyscale" | "gray" | "grey" | "mono" | "monochrome" | "black-white"
Expand Down Expand Up @@ -1189,7 +1254,11 @@ const fn theme_diff_deleted_bg(ui: &UiTheme) -> Color {
pub const fn theme_remap_active(theme: ThemeId) -> bool {
matches!(
theme,
ThemeId::CatppuccinMocha | ThemeId::TokyoNight | ThemeId::Dracula | ThemeId::GruvboxDark
ThemeId::Terminal
| ThemeId::CatppuccinMocha
| ThemeId::TokyoNight
| ThemeId::Dracula
| ThemeId::GruvboxDark
)
}

Expand Down Expand Up @@ -1677,14 +1746,15 @@ fn rgb_to_ansi256(r: u8, g: u8, b: u8) -> u8 {
mod tests {
use super::{
ACCENT_REASONING_LIVE, ColorDepth, DEEPSEEK_INK, DEEPSEEK_RED, DEEPSEEK_SKY,
DEEPSEEK_SLATE, GRAYSCALE_BORDER, GRAYSCALE_ELEVATED, GRAYSCALE_PANEL, GRAYSCALE_REASONING,
GRAYSCALE_SURFACE, GRAYSCALE_TEXT_BODY, GRAYSCALE_TEXT_HINT, GRAYSCALE_TEXT_SOFT,
GRAYSCALE_UI_THEME, LIGHT_BORDER, LIGHT_ELEVATED, LIGHT_PANEL, LIGHT_REASONING,
LIGHT_SURFACE, LIGHT_TEXT_BODY, LIGHT_TEXT_HINT, LIGHT_UI_THEME, PaletteMode,
SURFACE_REASONING, SURFACE_REASONING_TINT, TEXT_BODY, TEXT_HINT, TEXT_REASONING,
TEXT_TOOL_OUTPUT, UI_THEME, WHALE_REASONING_TEXT_RGB, WHALE_REASONING_TINT_RGB,
WHALE_TEXT_BODY_RGB, adapt_bg, adapt_bg_for_palette_mode, adapt_color,
adapt_fg_for_palette_mode, blend, luma, nearest_ansi16, normalize_hex_rgb_color,
DEEPSEEK_SLATE, DIFF_ADDED, DIFF_ADDED_BG, GRAYSCALE_BORDER, GRAYSCALE_ELEVATED,
GRAYSCALE_PANEL, GRAYSCALE_REASONING, GRAYSCALE_SURFACE, GRAYSCALE_TEXT_BODY,
GRAYSCALE_TEXT_HINT, GRAYSCALE_TEXT_SOFT, GRAYSCALE_UI_THEME, LIGHT_BORDER, LIGHT_ELEVATED,
LIGHT_PANEL, LIGHT_REASONING, LIGHT_SURFACE, LIGHT_TEXT_BODY, LIGHT_TEXT_HINT,
LIGHT_UI_THEME, PaletteMode, SURFACE_REASONING, SURFACE_REASONING_TINT, TERMINAL_UI_THEME,
TEXT_BODY, TEXT_HINT, TEXT_REASONING, TEXT_TOOL_OUTPUT, ThemeId, UI_THEME,
WHALE_REASONING_TEXT_RGB, WHALE_REASONING_TINT_RGB, WHALE_TEXT_BODY_RGB, adapt_bg,
adapt_bg_for_palette_mode, adapt_bg_for_theme, adapt_color, adapt_fg_for_palette_mode,
adapt_fg_for_theme, blend, luma, nearest_ansi16, normalize_hex_rgb_color,
normalize_theme_name, parse_hex_rgb_color, pulse_brightness, reasoning_surface_tint,
rgb_to_ansi256, theme_label_for_mode, ui_theme_from_settings,
};
Expand Down Expand Up @@ -1770,12 +1840,39 @@ mod tests {
assert_eq!(normalize_theme_name("system"), Some("system"));
assert_eq!(normalize_theme_name("default"), Some("system"));
assert_eq!(normalize_theme_name("whale"), Some("dark"));
assert_eq!(normalize_theme_name("transparent"), Some("terminal"));
assert_eq!(normalize_theme_name("inherit"), Some("terminal"));
assert_eq!(normalize_theme_name("black-white"), Some("grayscale"));
assert_eq!(normalize_theme_name("mono"), Some("grayscale"));
assert_eq!(normalize_theme_name("solarized"), None);
Comment on lines 1840 to 1847
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale test name after appending terminal-alias assertions

Terminal alias assertions (transparent, inherit) were added to a test named theme_names_normalize_common_grayscale_aliases. The name no longer describes what the test actually covers, making it harder to locate the right test when a future alias change breaks it. Consider renaming it or splitting it into a dedicated theme_names_normalize_terminal_aliases test.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Devin

assert_eq!(theme_label_for_mode(PaletteMode::Grayscale), "grayscale");
}

#[test]
fn terminal_theme_resets_surfaces_and_remaps_direct_palette_constants() {
assert_eq!(ThemeId::from_name("terminal"), Some(ThemeId::Terminal));
assert_eq!(TERMINAL_UI_THEME.surface_bg, Color::Reset);
assert_eq!(TERMINAL_UI_THEME.footer_bg, Color::Reset);
assert_eq!(TERMINAL_UI_THEME.text_body, Color::Reset);

assert_eq!(
adapt_bg_for_theme(DEEPSEEK_INK, ThemeId::Terminal, &TERMINAL_UI_THEME),
Color::Reset
);
assert_eq!(
adapt_bg_for_theme(DIFF_ADDED_BG, ThemeId::Terminal, &TERMINAL_UI_THEME),
Color::Reset
);
assert_eq!(
adapt_fg_for_theme(TEXT_BODY, ThemeId::Terminal, &TERMINAL_UI_THEME),
Color::Reset
);
assert_eq!(
adapt_fg_for_theme(DIFF_ADDED, ThemeId::Terminal, &TERMINAL_UI_THEME),
Color::Green
);
}

#[test]
fn light_palette_has_quiet_layer_separation() {
assert_eq!(LIGHT_SURFACE, Color::Rgb(246, 248, 251));
Expand Down
7 changes: 4 additions & 3 deletions crates/tui/src/tui/theme_picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ mod tests {
let mut v = ThemePickerView::new("system".to_string());
let action = v.handle_key(key(KeyCode::Down));
assert!(matches!(action, ViewAction::Emit(_)));
assert_eq!(selected_name(&action), Some(ThemeId::Whale.name()));
assert_eq!(selected_name(&action), Some(ThemeId::Terminal.name()));
}

#[test]
Expand All @@ -337,6 +337,7 @@ mod tests {
v.handle_key(key(KeyCode::Down));
v.handle_key(key(KeyCode::Down));
v.handle_key(key(KeyCode::Down));
v.handle_key(key(KeyCode::Down));
v.handle_key(key(KeyCode::Down)); // -> CatppuccinMocha
let action = v.handle_key(key(KeyCode::Enter));
match action {
Expand Down Expand Up @@ -376,8 +377,8 @@ mod tests {
#[test]
fn digit_jumps_to_row() {
let mut v = ThemePickerView::new("system".to_string());
let action = v.handle_key(key(KeyCode::Char('5')));
// Row 5 (1-indexed) -> index 4 -> CatppuccinMocha
let action = v.handle_key(key(KeyCode::Char('6')));
// Row 6 (1-indexed) -> index 5 -> CatppuccinMocha
assert_eq!(
selected_name(&action),
Some(ThemeId::CatppuccinMocha.name())
Expand Down
Loading