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 (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 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 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
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 SummaryThis PR adds light- and dark-specific custom color overrides that apply when automatic theme switching resolves the corresponding appearance.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the prior Settings-preview concern is invalid because previewing and applying both target manual mode, while cancellation restores the exact palette captured before Settings opened.
|
| Filename | Overview |
|---|---|
| src/config/theme.rs | Adds deserializable light and dark custom color blocks while retaining the existing shared override fields. |
| src/app/mod.rs | Selects the override block matching the resolved host appearance and applies it after shared colors. |
| src/app/state.rs | Adds palette application support for all mode-specific color tokens. |
| docs/next/website/src/data/config-reference.json | Documents every supported light- and dark-mode custom color key. |
| docs/next/website/src/content/docs/configuration.mdx | Explains mode-specific theme configuration and override precedence. |
| src/main.rs | Extends the generated default configuration example with light and dark override blocks. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Resolve host appearance] --> B{auto_switch enabled?}
B -->|No| C[Manual theme]
B -->|Yes: light| D[Light theme]
B -->|Yes: dark| E[Dark theme]
C --> F[Apply shared overrides]
D --> F
E --> F
F --> G{Resolved mode}
G -->|Manual| H[Final palette]
G -->|Light| I[Apply custom.light]
G -->|Dark| J[Apply custom.dark]
I --> H
J --> H
Reviews (5): 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 |
baff68f to
bd6babf
Compare
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
bd6babf to
1c53cf1
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
just checksteps all pass: fmt, clippy, windows-lint, config-reference parity, integration assets, plugin marketplace, script unittestslive_handoffx2,api_pingpane-info) are pty-timing timeouts that fail identically on unmodified master in the same environmentsrc/config/theme.rscover parsing both blocks, per-mode resolution precedence, and that configs without the new blocks resolve unchangedauto_switchflipping between catppuccin-mocha and catppuccin-latte