fix(theme,sidebar): drop FontAwesome/eval, fix prerender, add live↔template drift guard#14
Merged
Merged
Conversation
…ependency Updated comments in the CI workflow to enhance clarity regarding the purpose of the smoke tests for CLI scaffolding. Additionally, adjusted the handling of the Blazor-ApexCharts package to ensure it is explicitly added for smoke tests, isolating specific bug classes related to template escapes.
…ve Razor components and CLI templates Introduced a new test class, TemplateSyncTests, to ensure that the @code blocks in live Razor components match the corresponding CLI templates. This includes normalization of content to ignore comments and whitespace differences, helping to catch discrepancies that could lead to runtime errors. Updated existing comments in TemplateCompileTests for clarity on parsing behavior.
…ization flow Updated the ThemeToggle component to streamline theme initialization by replacing the async OnInitializedAsync method with a synchronous OnInitialized method. Introduced OnAfterRenderAsync to handle localStorage access after the first render, ensuring proper theme retrieval and state updates. Simplified theme class management by utilizing ShellUI methods for adding/removing classes, enhancing performance and readability. Updated related ThemeService to maintain consistency in theme handling across components.
…oved scalability and accessibility Updated the SidebarTrigger component in both Blazor and ShellUI templates to replace the FontAwesome icon with an SVG representation. This change enhances scalability and accessibility, ensuring a more modern and responsive design for the sidebar toggle button.
…proved focus handling Updated the InputOTP component in both Blazor and ShellUI templates to utilize ShellUI.focusElement instead of eval for focusing on OTP input fields. This change enhances performance and reliability by leveraging a dedicated JavaScript function. Additionally, updated the InputOTPTemplate to include a dependency on shellui-js and refined the input class handling for better styling consistency.
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
Three runtime bugs plus a structural fix for the underlying root cause (live library and CLI templates drifting apart silently across releases):
<i class="fa-solid fa-bars-staggered">— FontAwesome isn't a dependency of ShellUI and isn't installed byshellui init. Replaced with inline SVG. Mobile users can now actually open the sidebar.JSRuntime.InvokeVoidAsync("eval", "...")(blockable by CSP, fragile string eval) and readslocalStoragefromOnInitializedAsync(crashes during Blazor Server prerender whenIJSRuntimeisn't available yet). Replaced both:eval→ShellUI.addClassToDocument(...)/ShellUI.removeClassFromDocument(...)(already shipped viawwwroot/shellui.jsand used by CopyButton/FileUpload — zero new JS surface area)OnInitializedAsync→OnAfterRenderAsync(firstRender)for the storage readClassparameter on the liveThemeToggle.razorthat was never mirrored inThemeToggleTemplate.cs— consumers using the CLI install didn't get the parameter. AddedTemplateSyncTeststhat compares the@codeblock of the live.razorto the corresponding template'sContentstring after normalization. Future drift fails CI with a precise line-level diff.Changes
Component / template fixes
src/ShellUI.Components/Components/SidebarTrigger.razor—<i>swapped for inline SVG hamburger (Fix 1)src/ShellUI.Components/Components/ThemeToggle.razor— eval dropped, lifecycle moved toOnAfterRenderAsync(firstRender),OnInitializedretains only the_instancesregistration (no JS calls) (Fix 3)src/ShellUI.Components/Services/ThemeService.cs— same eval →ShellUI.*swap (helper service was also using eval; was a latent bug)src/ShellUI.Templates/Templates/SidebarTriggerTemplate.cs— mirrors the SVGsrc/ShellUI.Templates/Templates/ThemeToggleTemplate.cs— mirrors all the above, adds previously-missingClassparameter, wires@Classinto the rendered class attribute, declaresshellui-jsas a dependency (the new JS calls require it), and the embeddedThemeServicecontent also gets the eval →ShellUI.*swapNET9/BlazorInteractiveServer/Components/UI/{SidebarTrigger,ThemeToggle,InputOTP}.razor— stale demo-app snapshots mirrored so the demo runsAdjacent fix that came along
src/ShellUI.Components/Components/InputOTP.razor+src/ShellUI.Templates/Templates/InputOTPTemplate.cs+NET9/.../InputOTP.razor— sameevalpattern used for focusing OTP digits; replaced with the existingShellUI.focusElementJS helper (drops eval security/CSP issue). The template's broader API drift from the live version (still uses oldClassNameparameter, inline class composition) is out of scope here and tracked separately — see "Not in this PR" below.ClassNameparameter, inline class composition vs live'sClass+Shell.Cn(...). Exempted inAllowedDriftwith a reason. Tracked as a separate follow-up chip (chore/inputotp-sync); the success criterion is "remove theAllowedDriftentry and the test stays green."Drift guard
ShellUI.Tests/TemplateSyncTests.cs—[Theory]over(template-name, live-razor-file)pairs:sidebar-trigger↔SidebarTrigger.razortheme-toggle↔ThemeToggle.razorinput-otp↔InputOTP.razorExtracts the
@codeblock from each via the same quote/comment-aware brace-balancing tokenizer used byTemplateCompileTestsfrom branch 1, normalizes (strip comments, blank lines, trim whitespace), and asserts equality. On failure, prints up to 5 lines of side-by-side diff so the maintainer sees exactly where they diverged.Source resolution uses
[CallerFilePath]so the test works regardless ofcwdon CI — anchors to the test source file's location at compile time.Includes an
AllowedDriftdictionary (component name → reason) for cases where intentional divergence is acceptable. Currently has one entry:input-otp(API drift betweenClassName/Class; tracked as a follow-up).Verification
dotnet test ShellUI.Tests— 18/18 passing (3 new sync tests + 15 from branch 1).[Parameter] public bool DriftCanary { get; set; }to the liveThemeToggle.razor. Test failed with:JSRuntime.InvokeVoidAsync("eval", …)calls insrc/ShellUI.Components/**. Verified by grep.Test plan
dotnet new blazor+shellui init+shellui add theme-toggle sidebar. Build and run. Mobile sidebar trigger now shows a hamburger SVG and toggles. Theme toggle flips light/dark and persists across reload. No browser-console eval/CSP errors.OnAfterRenderAsyncpopulates_isDarkfrom localStorage.