Apply required improvements to theme system of BlazorUI (#12303)#12304
Apply required improvements to theme system of BlazorUI (#12303)#12304msynk wants to merge 2 commits intobitfoundation:developfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR enhances the BlazorUI theme system by expanding color and shadow variant properties, introducing utility classes for theme serialization, color role derivation, and SSR support, and refactoring the CSS variable mapper to emit additional dark/light shade variants. CSS variable generation now delegates through a new public utility layer. ChangesTheme System Improvements
Sequence DiagramsequenceDiagram
participant User as User/App
participant Provider as BitThemeProvider
participant Manager as BitThemeManager
participant Utilities as BitThemeUtilities
participant Mapper as BitThemeMapper
participant JS as JS Interop
User->>Provider: Set Theme & ParentTheme
Provider->>Utilities: Merge(Theme, ParentTheme)
Utilities->>Mapper: Merge(theme1, theme2)
Mapper-->>Utilities: merged BitTheme
Utilities-->>Provider: merged BitTheme
Provider->>Utilities: ToCssVariables(merged)
Utilities->>Mapper: MapToCssVariables(theme)
Mapper-->>Utilities: IReadOnlyDictionary<string, string>
Utilities-->>Provider: CSS variables dict
Provider->>Manager: ApplyBitThemeAsync(merged)
Manager->>Utilities: ToCssVariables(merged)
Utilities->>Mapper: MapToCssVariables(theme)
Mapper-->>Utilities: CSS variables
Utilities-->>Manager: CSS variables dict
Manager->>JS: BitThemeApplyBitTheme(variables, element)
JS->>JS: Set CSS custom properties on element
JS-->>Manager: Applied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR improves the BlazorUI theme system by fixing theme merging behavior in BitThemeProvider, filling missing theme tokens (color variants + box shadows), and adding public utilities to map/merge/serialize themes—plus a small SSR helper and accompanying documentation/tests.
Changes:
- Fix
BitThemeProviderto emit CSS variables and cascade a merged theme (child overrides + parent fallback), rather than only the child theme. - Expand theme surface area (additional foreground/background/border variants and box-shadow tokens) and ensure they’re mapped/merged in
BitThemeMapper. - Add public helpers (
BitThemeUtilities,BitThemePresets,BitThemeSerialization,BitThemeColorDerivation,BitThemeSsr), update demo docs, and add contract-style unit tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BlazorUI/Tests/Bit.BlazorUI.Tests/Utils/Theme/BitThemeMapperContractTests.cs | Adds mapper/merge/serialization contract tests, including a SCSS-driven “all referenced tokens are mapped” check. |
| src/BlazorUI/Tests/Bit.BlazorUI.Tests/Bit.BlazorUI.Tests.csproj | Copies theme-variables.scss into test output for the contract test. |
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/ThemingPage.razor | Documents precedence (preset vs inline vars) and introduces new utilities/SSR helper in the demo page. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeUtilities.cs | New public wrapper for mapping a BitTheme to CSS variables and merging themes. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSsr.cs | New SSR “early head script” helper for restoring/resolving bit-theme before CSS loads. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSerialization.cs | New JSON serialize/deserialize helpers for BitTheme. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeProvider.cs | Fixes merged-theme handling so inline CSS vars and cascaded value align. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemePresets.cs | Adds constants for well-known bit-theme preset names. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeMapper.cs | Adds missing CSS var mappings + merge support (general color variants and box shadows) and removes a duplicate typography block. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeManager.cs | Improves docs and routes Apply→CSS-vars mapping via BitThemeUtilities. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeJsExtensions.cs | Updates JS interop signature to accept IReadOnlyDictionary for theme variables. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeColorDerivation.cs | Adds helper to derive variant steps from a single main color without overwriting explicit values. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitTheme/BitThemeColors.cs | Expands BitThemeGeneralColorVariants with dark/light steps for primary/secondary/tertiary. |
| src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitTheme/BitThemeBoxShadows.cs | Adds missing box-shadow fields (callout2, sizes, inner) to the theme model. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSerialization.cs (1)
11-16: ⚡ Quick winConsider adding
PropertyNameCaseInsensitive = truefor robustnessWithout it, deserialization uses case-sensitive matching against the camelCase-transformed property names. JSON produced by a different serializer (e.g.
System.Text.Jsondefault /Newtonsoft.Jsondefault, which both emitPascalCase) will silently drop every field, returning a defaultBitTheme. The docs describe the use case as "sharing brand tokens", which implies possible interop with external producers.🛡️ Proposed fix
private static readonly JsonSerializerOptions Options = new() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, WriteIndented = true, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, + PropertyNameCaseInsensitive = true, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSerialization.cs` around lines 11 - 16, The JsonSerializerOptions defined in BitThemeSerialization (the static readonly Options) is currently case-sensitive and can drop properties when deserializing JSON with PascalCase names; update the Options object to set PropertyNameCaseInsensitive = true so deserialization will match property names regardless of casing (ensuring BitTheme deserialization works with external producers using PascalCase).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeColorDerivation.cs`:
- Around line 24-26: The light variant steps collapse because ScaleV uses
multiplicative scaling (ScaleV(v, 1.08/1.12/1.16)) causing high-v colors to
clamp to white; update BitThemeColorDerivation so the light variants
(variants.Light, variants.LightHover, variants.LightActive) use an additive
brightness offset instead of multiplicative scaling (e.g., replace ScaleV(v,
factor) calls with a method that returns ToHex(h, s, ClampV(v + delta)) or add
an AddV helper to compute v + fixedDelta and clamp to 1.0); keep referencing
ToHex, ScaleV (or new AddV/ClampV) and the variants.* properties so the steps
remain visually distinct for bright colors.
In `@src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSsr.cs`:
- Around line 12-18: The SSR inline script constant InlineHeadScriptBody in
BitThemeSsr.cs doesn't honor the bit-theme-system attribute and so falls back to
light/dark before checking system preference; update the string in
InlineHeadScriptBody to first check if the document element has attribute
'bit-theme-system' (and if so resolve cur via matchMedia to dk/lt), then
fallback to persisted localStorage or bit-theme/bit-theme-default, keeping the
existing lt/dk retrieval and final r.setAttribute('bit-theme', cur) behavior so
server-side first paint matches the client helper logic.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/ThemingPage.razor`:
- Around line 281-284: The CodeBox example contains an escaped-at sequence that
produces invalid Razor: remove the extra escape before BitThemeSsr so the
expression reads as a normal cast; update the snippet in ThemingPage.razor (the
CodeBox showing @((MarkupString)...)) to use BitThemeSsr.InlineHeadScript
without the extra '@' escape so it renders as
@((MarkupString)BitThemeSsr.InlineHeadScript), not
@((MarkupString)@BitThemeSsr.InlineHeadScript).
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSerialization.cs`:
- Around line 11-16: The JsonSerializerOptions defined in BitThemeSerialization
(the static readonly Options) is currently case-sensitive and can drop
properties when deserializing JSON with PascalCase names; update the Options
object to set PropertyNameCaseInsensitive = true so deserialization will match
property names regardless of casing (ensuring BitTheme deserialization works
with external producers using PascalCase).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b36e58d0-269b-4e56-914d-e06688b68b65
📒 Files selected for processing (14)
src/BlazorUI/Bit.BlazorUI/Utils/Theme/BitTheme/BitThemeBoxShadows.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitTheme/BitThemeColors.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeColorDerivation.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeJsExtensions.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeManager.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeMapper.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemePresets.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeProvider.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSerialization.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeSsr.cssrc/BlazorUI/Bit.BlazorUI/Utils/Theme/BitThemeUtilities.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/ThemingPage.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Bit.BlazorUI.Tests.csprojsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Utils/Theme/BitThemeMapperContractTests.cs
|
@coderabbitai re-review |
|
Tip For best results, initiate chat on the files or code changes.
[review] |
| public const string InlineHeadScriptBody = | ||
| "(function(){var r=document.documentElement,k='bit-current-theme',cur;" + | ||
| "if(r.hasAttribute('bit-theme-persist')){cur=localStorage.getItem(k);}" + | ||
| "cur=cur||r.getAttribute('bit-theme')||r.getAttribute('bit-theme-default')||'light';" + |
| /// </summary> | ||
| public const string InlineHeadScriptBody = | ||
| "(function(){var r=document.documentElement,k='bit-current-theme',cur;" + | ||
| "if(r.hasAttribute('bit-theme-persist')){cur=localStorage.getItem(k);}" + |
| /// <summary>Applies <paramref name="bitTheme"/> as CSS custom properties on <paramref name="element"/> (default: body), overriding stylesheet tokens for that subtree.</summary> | ||
| public async ValueTask ApplyBitThemeAsync(BitTheme bitTheme, ElementReference? element = null) | ||
| { | ||
| if (bitTheme is null) return; | ||
|
|
||
| await _js.BitThemeApplyBitTheme(BitThemeMapper.MapToCssVariables(bitTheme), element); | ||
| await _js.BitThemeApplyBitTheme(BitThemeUtilities.ToCssVariables(bitTheme), element); | ||
| } |
| public static IReadOnlyDictionary<string, string> ToCssVariables(BitTheme bitTheme) | ||
| { | ||
| return BitThemeMapper.MapToCssVariables(bitTheme ?? new BitTheme()); | ||
| } | ||
|
|
||
| /// <summary>Merges two themes: <paramref name="overrides"/> wins; missing values fall back to <paramref name="baseline"/>.</summary> | ||
| public static BitTheme Merge(BitTheme overrides, BitTheme baseline) |
| private static readonly JsonSerializerOptions Options = new() | ||
| { | ||
| PropertyNamingPolicy = JsonNamingPolicy.CamelCase, | ||
| WriteIndented = true, | ||
| DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, | ||
| }; | ||
|
|
||
| public static string Serialize(BitTheme theme) | ||
| { | ||
| var node = JsonSerializer.SerializeToNode(theme ?? new BitTheme(), Options); | ||
| if (node is JsonObject root) | ||
| PruneEmptyObjects(root); | ||
| return node?.ToJsonString(Options) ?? "{}"; | ||
| } | ||
|
|
||
| public static BitTheme Deserialize(string json) | ||
| { | ||
| return string.IsNullOrWhiteSpace(json) | ||
| ? new BitTheme() | ||
| : (JsonSerializer.Deserialize<BitTheme>(json, Options) ?? new BitTheme()); | ||
| } |
| public static class BitThemeColorDerivation | ||
| { | ||
| /// <summary>Fills unset <see cref="BitThemeColorVariants"/> fields from <paramref name="mainHex"/>.</summary> | ||
| public static void FillColorRoleFromMain(BitThemeColorVariants variants, string mainHex) |
| @@ -0,0 +1,221 @@ | |||
| using System; | |||
| using System; | |||
| { | ||
| var v = new BitThemeColorVariants(); | ||
| BitThemeColorDerivation.FillColorRoleFromMain(v, "not-a-color"); | ||
| // The catch block silences errors; variants remain null. |
closes #12303
Summary by CodeRabbit
New Features
Documentation