feat: add per-mode theme overrides - #2324
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe theme configuration now supports light- and dark-appearance custom color overrides. Automatic theme switching applies active mode overrides after shared custom and legacy accent colors. Tests and documentation cover the new behavior. ChangesTheme appearance overrides
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR adds per-mode theme overrides while preserving existing behavior for configurations that do not use them; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant HostAppearance
participant resolve_effective_theme
participant resolve_palette_for_theme_name
participant Palette
HostAppearance->>resolve_effective_theme: select light or dark appearance
resolve_effective_theme->>resolve_palette_for_theme_name: pass active mode overrides
resolve_palette_for_theme_name->>Palette: apply shared custom colors
Palette->>Palette: apply active mode-specific colors
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @aneym, thanks for your interest in contributing! Herdr automatically admits focused bug fixes from contributors who are not maintainers when the title uses Contributors who are not maintainers may submit only focused bug fixes. If this pull request fixes a bug, rename it to use a conventional Feature requests, behavior changes, and other proposals belong in GitHub Discussions and require maintainer approval before a pull request. If this gate classified the pull request incorrectly, reply and tag a maintainer listed in Patch size: 7 changed files, 391 changed lines. See https://github.com/herdrdev/herdr/blob/master/CONTRIBUTING.md for the contribution policy. |
kangal-bot
added
ai-review
Greptile SummaryThe PR adds light- and dark-specific custom color overrides that activate with automatic appearance switching while preserving existing shared overrides.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/app/mod.rs | Resolves the active appearance-specific override after shared colors while retaining manual-mode behavior. |
| src/app/state.rs | Adds complete palette application for all supported mode-specific color tokens. |
| src/config/theme.rs | Defines and tests deserialization of light and dark custom color subtables. |
| docs/next/website/src/data/config-reference.json | Documents the complete light and dark override key sets. |
| docs/next/website/src/content/docs/configuration.mdx | Explains configuration syntax and override precedence. |
| src/main.rs | Adds commented per-mode override examples to the generated default configuration. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Built-in theme palette] --> B[Apply theme.custom]
B --> C{auto_switch enabled?}
C -->|No| D[Effective palette]
C -->|Yes, light| E[Apply theme.custom.light]
C -->|Yes, dark| F[Apply theme.custom.dark]
E --> D
F --> D
Reviews (7): Last reviewed commit: "feat: add per-mode theme overrides" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
66ee3b8 to
d1a4e65
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config/theme.rs (1)
126-146: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd
sidebar_bgto mode-specific overrides.
ModeThemeColorsomitssidebar_bg, althoughCustomThemeColorsandPalettesupport it. A user cannot set a light-only or dark-only sidebar background.Add
sidebar_bgtoModeThemeColors. Apply it inPalette::with_mode_overrides. Add parsing and palette-resolution coverage.Proposed fix
--- a/src/config/theme.rs +++ b/src/config/theme.rs pub struct ModeThemeColors { pub accent: Option<String>, pub panel_bg: Option<String>, + pub sidebar_bg: Option<String>, pub surface0: Option<String>,--- a/src/app/state.rs +++ b/src/app/state.rs if let Some(c) = &custom.panel_bg { self.panel_bg = parse_color(c); } + if let Some(c) = &custom.sidebar_bg { + self.sidebar_bg = parse_color(c); + }Based on supplied downstream context,
src/app/state.rs:641-692has the same omission.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1cefb19c-6dd0-416f-9a93-164b72966a75
📒 Files selected for processing (7)
docs/next/website/src/content/docs/configuration.mdxdocs/next/website/src/data/config-reference.jsonsrc/app/mod.rssrc/app/state.rssrc/config.rssrc/config/theme.rssrc/main.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/next/website/src/content/docs/configuration.mdx
- docs/next/website/src/data/config-reference.json
- src/main.rs
- src/app/state.rs
- src/app/mod.rs
- src/config.rs
d1a4e65 to
baff68f
Compare
|
Added |
bd6babf to
1c53cf1
Compare
1c53cf1 to
47b056c
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
allow [theme.custom.light] and [theme.custom.dark] blocks so custom overrides can differ per resolved appearance when auto_switch flips. precedence: theme defaults, then [theme.custom], then the block for the active mode. configs without the new blocks resolve unchanged. refs herdrdev#837
47b056c to
14e5036
Compare
3 participants
Summary
[theme.custom]applies one set of overrides in both modes, so withauto_switcha value tuned for the dark theme washes out when the light theme activates. #837 proposed per-mode blocks and @ogulcancelik approved a PR there.This adds
[theme.custom.light]and[theme.custom.dark]. Resolution order: theme defaults, then[theme.custom], then the block matching the resolved appearance. Configs that do not use the new blocks resolve exactly as they do today.Both panes are the same seeded layout rendered with
catppuccin-latteactive; the only difference is the[theme.custom.light]block.Refs #837. #2032 asks for the same capability from a full-theme-pair angle; this shape covers it through
[theme.custom.light]/[theme.custom.dark]alone.Test plan
master(ffc4e263).sidebar_bg/new sidebar row color tokens.live_handoffx2 andapi_pingpane-info); rerunning the CI suite with exactly those three excluded passed all 3,329 remaining Rust tests plus fmt, clippy, UI architecture, integration assets, and plugin marketplace checks.just windows-lint: passed.