fix(theme): move ThemeProvider into interactive render boundary (#239) - #243
Closed
mpaulosky wants to merge 4 commits into
Closed
fix(theme): move ThemeProvider into interactive render boundary (#239)#243mpaulosky wants to merge 4 commits into
mpaulosky wants to merge 4 commits into
Conversation
Root cause: ThemeProvider was in App.razor (static SSR context). Static components never execute OnAfterRenderAsync, so JS interop calls to themeManager.getColor/getBrightness never fired. Interactive children calling SetColor() via cascading parameter updated a static instance — StateHasChanged() was a no-op and the cascade never re-propagated. Fix: - Move ThemeProvider from App.razor into Routes.razor, which renders with @rendermode="InteractiveServer". ThemeProvider now correctly executes OnAfterRenderAsync on first render, reads persisted colour and brightness from localStorage, and propagates them to all theme-aware children. - Add explicit selected="@(CurrentColor == X)" on each <option> in ThemeColorDropdownComponent for robustness (works in both static initial render HTML and interactive re-renders). - Add try-catch to SetColor/SetBrightness in ThemeProvider.razor.cs to tolerate circuit disconnects during JS invocation. Also fix pre-existing build blocker: - Pin Snappier to 1.3.1 in Directory.Packages.props (NU1903 / GHSA-pggp-6c3x-2xmx) - Add direct PackageReference in Web.csproj and AppHost.csproj Tests: Architecture 15/15 · bUnit 87/87 · Domain 42/42 Closes #239 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
squad
Contributor
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes theme color/brightness persistence by moving ThemeProvider into the Blazor InteractiveServer render boundary so JS interop runs and cascading values re-propagate across navigation/reloads.
Changes:
- Move
ThemeProviderwrapper fromApp.razortoRoutes.razor(inside@rendermode="InteractiveServer"). - Improve theme UI/state robustness (dropdown option selection markup + JS interop resilience in
ThemeProvider). - Add/extend bUnit + architecture tests and pin/add
Snappierto address NU1903 vulnerability build blocking.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Web/Components/App.razor | Removes ThemeProvider wrapper so it can be relocated into the interactive subtree. |
| src/Web/Components/Routes.razor | Wraps <Router> with ThemeProvider inside the interactive boundary. |
| src/Web/Components/Theme/ThemeProvider.razor.cs | Adds try/catch around JS writes for resilience. |
| src/Web/Components/Theme/ThemeColorDropdownComponent.razor | Adds per-option selected to better reflect current theme in initial markup. |
| tests/Web.Tests.Bunit/Components/Theme/ThemeColorPersistenceTests.cs | Adds persistence/cascade-focused bUnit coverage for issue #239. |
| tests/Web.Tests.Bunit/Components/Layout/MainLayoutThemeTests.cs | Adds layout surface assertions for primary palette classes + ThemeProvider cascade integration. |
| tests/Architecture.Tests/ThemeLayerTests.cs | Adds layout namespace test and theme dependency constraints. |
| Directory.Packages.props | Pins Snappier to 1.3.1 under central package management. |
| src/Web/Web.csproj | Adds direct Snappier reference. |
| src/AppHost/AppHost.csproj | Adds direct Snappier reference. |
| .squad/agents/legolas/history.md | Documents the fix and related context. |
| .squad/agents/gimli/history.md | Documents test work/context for the theme persistence effort. |
Comment on lines
+53
to
+60
| try | ||
| { | ||
| await Js.InvokeVoidAsync("themeManager.setColor", color); | ||
| } | ||
| catch | ||
| { | ||
| // Ignore JS errors (circuit disconnect, localStorage unavailable) | ||
| } |
Comment on lines
+67
to
+74
| try | ||
| { | ||
| await Js.InvokeVoidAsync("themeManager.setBrightness", brightness); | ||
| } | ||
| catch | ||
| { | ||
| // Ignore JS errors (circuit disconnect, localStorage unavailable) | ||
| } |
| // Act | ||
| var result = Types.InAssembly(layoutAssembly) | ||
| .That() | ||
| .ResideInNamespace("MyBlog.Web.Components.Layout") |
Comment on lines
+1
to
+2
| @using MyBlog.Web.Components.Theme | ||
|
|
Comment on lines
+83
to
+86
| // Act | ||
| cut.InvokeAsync(() => cut.Instance.SetColor(newColor)); | ||
|
|
||
| // Assert — JS setColor is invoked, which writes to localStorage (read back on next page load) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #243 +/- ##
==========================================
+ Coverage 78.64% 79.09% +0.45%
==========================================
Files 43 43
Lines 721 727 +6
Branches 112 112
==========================================
+ Hits 567 575 +8
+ Misses 108 106 -2
Partials 46 46
🚀 New features to boost your workflow:
|
mpaulosky
pushed a commit
that referenced
this pull request
May 7, 2026
Merge decision inbox (aragorn-precommit-gate) into decisions.md. Update Legolas and Gimli history with session work on #239. - Legolas: Fixed theme color selector persistence by moving ThemeProvider into interactive render boundary (Routes.razor) - Gimli: Added comprehensive test coverage for persistence (264 lines of tests across 2 files) - Fix: Improved ThemeColorDropdownComponent binding and error handling - Bonus: Pinned Snappier 1.3.1 for security (NU1903 / GHSA-pggp-6c3x-2xmx) Test results: Architecture 15/15 ✅ | bUnit 87/87 ✅ | Domain 42/42 ✅ PR #243 opened and ready for review. Closes #239 (pending PR merge) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
🔄 CHANGES_REQUESTED — Routing fix cycle This PR is not ready to merge yet. Blocking artifacts:
Per lockout, the original PR authors should not handle this revision cycle. Routed to: Sam (Backend / .NET) Please correct the blocking test artifacts, push to |
Architecture.Tests/ThemeLayerTests.cs: - Replace tautological LayoutComponents_ShouldResideIn_LayoutNamespace test (filtered on namespace then asserted the same namespace — zero protection) with ThemeComponents_ShouldNotDependOn_LayoutComponents, enforcing that theme components cannot take a dependency on layout (one-way coupling rule). Web.Tests.Bunit/Components/Theme/ThemeColorPersistenceTests.cs: - Promote SetColor and SetBrightness write-path tests from void to async Task and await cut.InvokeAsync(...) so the full InvokeVoidAsync path completes before assertions run — pins the async write path correctly. - Also fix ThemeProvider_AfterSetColorRed test (same unawaited pattern). - Add ThemeProvider_SetColor_SilentlySwallowsJsException_AndKeepsNewColor: uses JSRuntimeMode.Strict with themeManager.setColor unplanned so bUnit throws on the InvokeVoidAsync call; exercises the catch branch, asserts CurrentColor is updated and no exception propagates. - Add ThemeProvider_SetBrightness_SilentlySwallowsJsException_AndKeepsNewBrightness: same pattern for SetBrightness / themeManager.setBrightness. No production code changes required — ThemeProvider.razor.cs catch blocks are already correct; the Strict-mode test approach is sufficient to exercise them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Gate rerun complete: this branch is no longer mergeable against dev. After PR #242 landed, PR #243 now conflicts on Directory.Packages.props, src/Web/Components/App.razor, src/Web/Components/Routes.razor, and src/Web/Components/Theme/ThemeProvider.razor.cs. Rebasing would also have to keep the newer dev-side fixes rather than this branch's stale artifacts, including the no-@rendermode NavMenu boundary from #242, the narrowed JS exception handling plus themeManager.markInitialized() in ThemeProvider, and the ThemeRenderBoundaryTests coverage that this branch would otherwise drop. Closing as superseded by the already-merged dev work; no further revision cycle is needed on this PR. |
Owner
Author
|
Closing as superseded by PR #242 and the current dev branch state. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the theme color selector not persisting across navigation and page reloads.
Working as Legolas (Frontend/Blazor specialist)
Root Cause
ThemeProviderwas placed inApp.razor(static SSR context). Static Blazor components never executeOnAfterRenderAsync, so JS interop calls tothemeManager.getColor/getBrightnessnever fired. Interactive children callingSetColor()via cascading parameter updated a static parent instance —StateHasChanged()was a no-op and the cascade never re-propagated.Fix
Move
ThemeProviderfromApp.razorintoRoutes.razor— which is rendered with@rendermode="InteractiveServer"fromApp.razor. ThemeProvider now correctly executesOnAfterRenderAsyncon first interactive render, reads the persisted color/brightness from localStorage, and propagates via cascading value to all theme-aware children.Changes
App.razor<ThemeProvider>wrapper (ThemeProvider moves to Routes)Routes.razor<Router>in<ThemeProvider>(inside interactive boundary)ThemeColorDropdownComponent.razorselected="@(CurrentColor == X)"on each<option>for robustnessThemeProvider.razor.csSetColor/SetBrightnessfor circuit disconnect resilienceDirectory.Packages.propsSnappier 1.3.1(pre-existing build blocker NU1903)Web.csproj+AppHost.csprojTests
Notes
The anti-FOUC IIFE in
App.razor <head>intentionally remains in place — it runs during static pre-render before Blazor hydration to prevent theme flash on load.Closes #239