feat(theme): add Matrix films inspired theme and improve theme_picker…#2129
feat(theme): add Matrix films inspired theme and improve theme_picker…#2129malsony wants to merge 2 commits into
Conversation
… logic - Add Matrix films inspired color scheme - Refactor theme_picker to use SELECTABLE_THEMES for last-theme lookup instead of hard-coding
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Matrix' theme to the TUI, adding the necessary color constants, theme definitions, and integration logic across the configuration and palette modules. Additionally, the theme picker's navigation logic was refactored for conciseness and its tests were updated to be more dynamic. Review feedback highlighted a readability issue with low-contrast reasoning text in the new theme and suggested adjusting background colors to better preserve visual separation between UI components.
| if theme == ThemeId::Matrix { | ||
| Color::Rgb(0x00, 0x55, 0x00) // #005500 | ||
| } else { | ||
| ui.mode_plan | ||
| } |
There was a problem hiding this comment.
The hardcoded color #005500 for reasoning text in the Matrix theme has very low contrast against the dark background (#000A00), making it difficult to read. Reasoning text is a primary content element and should be easily legible. Using a brighter green, such as the theme's mode_goal color, would significantly improve readability while maintaining the theme's aesthetic.
| if theme == ThemeId::Matrix { | |
| Color::Rgb(0x00, 0x55, 0x00) // #005500 | |
| } else { | |
| ui.mode_plan | |
| } | |
| if theme == ThemeId::Matrix { | |
| ui.mode_goal | |
| } else { | |
| ui.mode_plan | |
| } |
There was a problem hiding this comment.
The reasoning text is not fading to background, actually.
| surface_bg: Color::Rgb(MATRIX_SURFACE_RGB.0, MATRIX_SURFACE_RGB.1, MATRIX_SURFACE_RGB.2), | ||
| panel_bg: Color::Rgb(MATRIX_SURFACE_RGB.0, MATRIX_SURFACE_RGB.1, MATRIX_SURFACE_RGB.2), | ||
| elevated_bg: Color::Rgb(MATRIX_ELEVATED_RGB.0, MATRIX_ELEVATED_RGB.1, MATRIX_ELEVATED_RGB.2), | ||
| composer_bg: Color::Rgb(MATRIX_SURFACE_RGB.0, MATRIX_SURFACE_RGB.1, MATRIX_SURFACE_RGB.2), | ||
| selection_bg: Color::Rgb(MATRIX_SELECTION_RGB.0, MATRIX_SELECTION_RGB.1, MATRIX_SELECTION_RGB.2), | ||
| header_bg: Color::Rgb(MATRIX_SURFACE_RGB.0, MATRIX_SURFACE_RGB.1, MATRIX_SURFACE_RGB.2), | ||
| footer_bg: Color::Rgb(MATRIX_SURFACE_RGB.0, MATRIX_SURFACE_RGB.1, MATRIX_SURFACE_RGB.2), |
There was a problem hiding this comment.
The MATRIX_UI_THEME uses the same color (MATRIX_SURFACE_RGB) for surface_bg, panel_bg, composer_bg, header_bg, and footer_bg. This eliminates the visual separation between the sidebar, composer, and main transcript area, which is a core part of the TUI's layout design. Consider using a slightly different shade (e.g., MATRIX_ELEVATED_RGB) for the panel and composer backgrounds to restore visual hierarchy.
… logic
Add Matrix films inspired color scheme
Refactor theme_picker to use SELECTABLE_THEMES for last-theme lookup instead of hard-coding
Summary
Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresChecklist