Skip to content

Commit ef11e20

Browse files
authored
fix(theme,sidebar): drop FontAwesome/eval, fix prerender, add live↔template drift guard (#14)
## Summary Three runtime bugs plus a structural fix for the underlying root cause (live library and CLI templates drifting apart silently across releases): 1. **SidebarTrigger renders nothing** in consumer projects because it references `<i class="fa-solid fa-bars-staggered">` — FontAwesome isn't a dependency of ShellUI and isn't installed by `shellui init`. Replaced with inline SVG. Mobile users can now actually open the sidebar. 2. **ThemeToggle is unreliable** because it uses `JSRuntime.InvokeVoidAsync("eval", "...")` (blockable by CSP, fragile string eval) and reads `localStorage` from `OnInitializedAsync` (crashes during Blazor Server prerender when `IJSRuntime` isn't available yet). Replaced both: - `eval` → `ShellUI.addClassToDocument(...)` / `ShellUI.removeClassFromDocument(...)` (already shipped via `wwwroot/shellui.js` and used by CopyButton/FileUpload — zero new JS surface area) - `OnInitializedAsync` → `OnAfterRenderAsync(firstRender)` for the storage read 3. **Live ↔ template drift detection.** The previous alpha shipped a `Class` parameter on the live `ThemeToggle.razor` that was never mirrored in `ThemeToggleTemplate.cs` — consumers using the CLI install didn't get the parameter. Added `TemplateSyncTests` that compares the `@code` block of the live `.razor` to the corresponding template's `Content` string 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 to `OnAfterRenderAsync(firstRender)`, `OnInitialized` retains only the `_instances` registration (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 SVG - `src/ShellUI.Templates/Templates/ThemeToggleTemplate.cs` — mirrors all the above, adds previously-missing `Class` parameter, wires `@Class` into the rendered class attribute, declares `shellui-js` as a dependency (the new JS calls require it), and the embedded `ThemeService` content also gets the eval → `ShellUI.*` swap - `NET9/BlazorInteractiveServer/Components/UI/{SidebarTrigger,ThemeToggle,InputOTP}.razor` — stale demo-app snapshots mirrored so the demo runs ### Adjacent fix that came along - `src/ShellUI.Components/Components/InputOTP.razor` + `src/ShellUI.Templates/Templates/InputOTPTemplate.cs` + `NET9/.../InputOTP.razor` — same `eval` pattern used for focusing OTP digits; replaced with the existing `ShellUI.focusElement` JS helper (drops eval security/CSP issue). The template's broader API drift from the live version (still uses old `ClassName` parameter, inline class composition) is out of scope here and tracked separately — see "Not in this PR" below. - - **InputOTP template full sync** — old `ClassName` parameter, inline class composition vs live's `Class` + `Shell.Cn(...)`. Exempted in `AllowedDrift` with a reason. Tracked as a separate follow-up chip (`chore/inputotp-sync`); the success criterion is "remove the `AllowedDrift` entry and the test stays green." ### Drift guard - `ShellUI.Tests/TemplateSyncTests.cs` — `[Theory]` over `(template-name, live-razor-file)` pairs: - `sidebar-trigger` ↔ `SidebarTrigger.razor` - `theme-toggle` ↔ `ThemeToggle.razor` - `input-otp` ↔ `InputOTP.razor` Extracts the `@code` block from each via the same quote/comment-aware brace-balancing tokenizer used by `TemplateCompileTests` from 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 of `cwd` on CI — anchors to the test source file's location at compile time. Includes an `AllowedDrift` dictionary (component name → reason) for cases where intentional divergence is acceptable. Currently has one entry: `input-otp` (API drift between `ClassName`/`Class`; tracked as a follow-up). ## Verification - `dotnet test ShellUI.Tests` — **18/18 passing** (3 new sync tests + 15 from branch 1). - Drift-bite proof: temporarily added a phantom `[Parameter] public bool DriftCanary { get; set; }` to the live `ThemeToggle.razor`. Test failed with: ``` line 6 live: [Parameter] public bool DriftCanary { get; set; } template: [Parameter(CaptureUnmatchedValues = true)] ``` Removed the canary; back to green. Confirms the guard catches realistic drift (added parameter) with a precise diagnostic. - No more `JSRuntime.InvokeVoidAsync("eval", …)` calls in `src/ShellUI.Components/**`. Verified by grep. ## Test plan - [ ] CI green (tests + template-compile + smoke build from branch 1) - [ ] Manual: scaffold a fresh `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. - [ ] Confirm prerender flow on Blazor Server doesn't throw — page renders before JS executes, then `OnAfterRenderAsync` populates `_isDark` from localStorage.
2 parents ab224f0 + 6188e64 commit ef11e20

13 files changed

Lines changed: 272 additions & 137 deletions

File tree

.github/workflows/ci.yml

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,8 @@ jobs:
3636
- name: Run tests
3737
run: dotnet test ShellUI.sln --no-restore --no-build --configuration Release --verbosity normal
3838

39-
# Guards against the template-escape bug class from shellui-fixes-for-lib.md
40-
# (Fixes 2, 9, 10). TemplateCompileTests verifies generated content parses;
41-
# this end-to-end build catches anything the syntactic check misses
42-
# (e.g. missing using directives).
39+
# End-to-end build of a scaffolded project — catches anything the in-process
40+
# TemplateCompileTests miss (missing usings, dependency resolution).
4341
- name: Smoke-test CLI scaffolding
4442
shell: bash
4543
run: |
@@ -53,13 +51,10 @@ jobs:
5351
dotnet new blazor -o SmokeApp --no-restore
5452
cd SmokeApp
5553
shellui init --tailwind standalone --yes
56-
# Hit the three components that regressed last time:
5754
shellui add chart pie-chart dashboard-02 --force || true
58-
# Chart components reference ApexCharts.* types — `shellui add chart`
59-
# does NOT auto-install Blazor-ApexCharts today (tracked as Fix 12.3:
60-
# `nugetDependencies` field on component manifests). Add it explicitly
61-
# here so the smoke test isolates THIS PR's bug class (template escapes)
62-
# from that one. When Fix 12.3 lands, remove this line.
55+
# Chart components reference ApexCharts.* types but the CLI does not yet
56+
# auto-add the runtime NuGet dep. Drop this line once `shellui add chart`
57+
# declares Blazor-ApexCharts as a nuget dependency.
6358
dotnet add package Blazor-ApexCharts --version 6.0.2
6459
dotnet build -c Debug
6560

NET9/BlazorInteractiveServer/Components/UI/InputOTP.razor

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@
121121
{
122122
try
123123
{
124-
await JS.InvokeVoidAsync("eval", $"document.getElementById('otp-input-{_id}-{index}')?.focus()");
124+
await JS.InvokeVoidAsync("ShellUI.focusElement", $"otp-input-{_id}-{index}");
125125
}
126126
catch
127127
{

NET9/BlazorInteractiveServer/Components/UI/SidebarTrigger.razor

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
Class)"
88
@onclick="HandleClick"
99
@attributes="AdditionalAttributes">
10-
<i class="fa-solid fa-bars-staggered text-sm"></i>
10+
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="size-4">
11+
<line x1="3" y1="6" x2="21" y2="6" />
12+
<line x1="3" y1="12" x2="15" y2="12" />
13+
<line x1="3" y1="18" x2="18" y2="18" />
14+
</svg>
1115
<span class="sr-only">Toggle Sidebar</span>
1216
</button>
1317

NET9/BlazorInteractiveServer/Components/UI/ThemeToggle.razor

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,21 @@
3434
[Parameter(CaptureUnmatchedValues = true)]
3535
public Dictionary<string, object>? AdditionalAttributes { get; set; }
3636

37-
protected override async Task OnInitializedAsync()
37+
protected override void OnInitialized()
3838
{
3939
_instances.Add(this);
40-
40+
}
41+
42+
// localStorage and document mutation must run after first render — JSRuntime
43+
// is unavailable during prerender on Blazor Server.
44+
protected override async Task OnAfterRenderAsync(bool firstRender)
45+
{
46+
if (!firstRender) return;
4147
try
4248
{
4349
var theme = await JSRuntime.InvokeAsync<string>("localStorage.getItem", "theme");
4450
_isDark = string.IsNullOrEmpty(theme) ? true : theme == "dark";
51+
StateHasChanged();
4552
}
4653
catch
4754
{
@@ -53,21 +60,14 @@
5360
{
5461
_isDark = !_isDark;
5562
var theme = _isDark ? "dark" : "light";
56-
63+
5764
try
5865
{
5966
await JSRuntime.InvokeVoidAsync("localStorage.setItem", "theme", theme);
60-
61-
if (_isDark)
62-
{
63-
await JSRuntime.InvokeVoidAsync("eval", "document.documentElement.classList.add('dark')");
64-
}
65-
else
66-
{
67-
await JSRuntime.InvokeVoidAsync("eval", "document.documentElement.classList.remove('dark')");
68-
}
69-
70-
// Update all ThemeToggle instances
67+
await JSRuntime.InvokeVoidAsync(
68+
_isDark ? "ShellUI.addClassToDocument" : "ShellUI.removeClassFromDocument",
69+
"dark");
70+
7171
foreach (var instance in _instances)
7272
{
7373
if (instance != this)
@@ -76,7 +76,7 @@
7676
instance.StateHasChanged();
7777
}
7878
}
79-
79+
8080
StateHasChanged();
8181
}
8282
catch

ShellUI.Tests/TemplateCompileTests.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@
88
namespace ShellUI.Tests;
99

1010
/// Verifies that the *generated* content of each template parses as valid C#.
11-
/// For pure-.cs templates (variants), parse the whole content.
12-
/// For .razor templates, extract the @code { ... } block and parse its body.
13-
/// This catches the exact class of bug from shellui-fixes-for-lib.md (Fixes 2, 9, 10):
14-
/// unescaped quotes inside C# verbatim strings that ship as compile errors to consumers.
11+
/// Pure-.cs templates (variants) parse the whole content; .razor templates parse
12+
/// the body of the @code block. Catches unescaped quotes inside C# verbatim
13+
/// strings — the kind of error that ships as a compile failure to consumers.
1514
public class TemplateCompileTests
1615
{
1716
[Theory]

ShellUI.Tests/TemplateSyncTests.cs

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
using System;
2+
using System.IO;
3+
using System.Linq;
4+
using System.Runtime.CompilerServices;
5+
using System.Text.RegularExpressions;
6+
using ShellUI.Templates;
7+
using Xunit;
8+
9+
namespace ShellUI.Tests;
10+
11+
/// Asserts the @code block of each live src/ShellUI.Components/Components/*.razor
12+
/// matches the corresponding CLI template's emitted Content. Compares after stripping
13+
/// comments, blank lines, and whitespace differences so the legitimate divergence
14+
/// (namespace, formatting) doesn't fire, but real divergence (parameter list, JS
15+
/// interop calls, lifecycle methods) does.
16+
public class TemplateSyncTests
17+
{
18+
// Component name → reason. Empty by default — fix the drift instead of adding entries.
19+
private static readonly Dictionary<string, string> AllowedDrift = new()
20+
{
21+
};
22+
23+
[Theory]
24+
[InlineData("sidebar-trigger", "SidebarTrigger.razor")]
25+
[InlineData("theme-toggle", "ThemeToggle.razor")]
26+
[InlineData("input-otp", "InputOTP.razor")]
27+
public void TemplateCodeBlock_MatchesLiveLibrary(string templateName, string razorFileName)
28+
{
29+
if (AllowedDrift.ContainsKey(templateName)) return;
30+
31+
var liveContent = File.ReadAllText(GetLiveRazorPath(razorFileName));
32+
var templateContent = ComponentRegistry.GetComponentContent(templateName)
33+
?? throw new InvalidOperationException($"Template '{templateName}' not found in registry");
34+
35+
var liveCode = ExtractCodeBlock(liveContent)
36+
?? throw new InvalidOperationException($"Live {razorFileName} has no @code block");
37+
var templateCode = ExtractCodeBlock(templateContent)
38+
?? throw new InvalidOperationException($"Template {templateName} has no @code block");
39+
40+
var normalizedLive = Normalize(liveCode);
41+
var normalizedTemplate = Normalize(templateCode);
42+
43+
Assert.True(normalizedLive == normalizedTemplate,
44+
$"Drift detected between live {razorFileName} and template {templateName}.\n" +
45+
$"This usually means someone updated one but not the other. Sync them, or add " +
46+
$"\"{templateName}\" to AllowedDrift in TemplateSyncTests with a reason.\n\n" +
47+
DiffSummary(normalizedLive, normalizedTemplate));
48+
}
49+
50+
// [CallerFilePath] captures the absolute path of this source file at compile time,
51+
// so the test resolves the live components directory regardless of cwd on CI.
52+
private static string GetLiveRazorPath(string razorFileName, [CallerFilePath] string thisFile = "")
53+
{
54+
var testDir = Path.GetDirectoryName(thisFile) ?? throw new InvalidOperationException("CallerFilePath is empty");
55+
var repoRoot = Path.GetFullPath(Path.Combine(testDir, ".."));
56+
return Path.Combine(repoRoot, "src", "ShellUI.Components", "Components", razorFileName);
57+
}
58+
59+
private static string Normalize(string code)
60+
{
61+
var withoutBlock = Regex.Replace(code, @"/\*.*?\*/", string.Empty, RegexOptions.Singleline);
62+
var lines = withoutBlock.Split('\n')
63+
.Select(l => Regex.Replace(l, @"//.*$", string.Empty))
64+
.Select(l => Regex.Replace(l.Trim(), @"\s+", " "))
65+
.Where(l => !string.IsNullOrWhiteSpace(l));
66+
return string.Join("\n", lines);
67+
}
68+
69+
private static string DiffSummary(string live, string template)
70+
{
71+
var liveLines = live.Split('\n');
72+
var tmplLines = template.Split('\n');
73+
var max = Math.Max(liveLines.Length, tmplLines.Length);
74+
var diffs = new System.Text.StringBuilder();
75+
var shown = 0;
76+
for (var i = 0; i < max && shown < 5; i++)
77+
{
78+
var l = i < liveLines.Length ? liveLines[i] : "<missing>";
79+
var t = i < tmplLines.Length ? tmplLines[i] : "<missing>";
80+
if (l != t)
81+
{
82+
diffs.AppendLine($" line {i + 1}");
83+
diffs.AppendLine($" live: {l}");
84+
diffs.AppendLine($" template: {t}");
85+
shown++;
86+
}
87+
}
88+
return diffs.Length == 0 ? "(no per-line diff — file lengths differ)" : diffs.ToString();
89+
}
90+
91+
/// Extracts the body of the first `@code { ... }` block, balancing braces while
92+
/// respecting strings, verbatim strings, char literals, line comments, and block comments.
93+
/// Returns null if no `@code` block is found or braces are unbalanced.
94+
private static string? ExtractCodeBlock(string razor)
95+
{
96+
var match = Regex.Match(razor, @"@code\s*\{");
97+
if (!match.Success) return null;
98+
99+
var start = match.Index + match.Length;
100+
var depth = 1;
101+
var inString = false;
102+
var inVerbatimString = false;
103+
var inCharLiteral = false;
104+
var inLineComment = false;
105+
var inBlockComment = false;
106+
107+
for (var i = start; i < razor.Length; i++)
108+
{
109+
var c = razor[i];
110+
var next = i + 1 < razor.Length ? razor[i + 1] : '\0';
111+
112+
if (inLineComment)
113+
{
114+
if (c == '\n') inLineComment = false;
115+
continue;
116+
}
117+
if (inBlockComment)
118+
{
119+
if (c == '*' && next == '/') { inBlockComment = false; i++; }
120+
continue;
121+
}
122+
if (inVerbatimString)
123+
{
124+
if (c == '"' && next == '"') { i++; continue; }
125+
if (c == '"') inVerbatimString = false;
126+
continue;
127+
}
128+
if (inString)
129+
{
130+
if (c == '\\' && next != '\0') { i++; continue; }
131+
if (c == '"') inString = false;
132+
continue;
133+
}
134+
if (inCharLiteral)
135+
{
136+
if (c == '\\' && next != '\0') { i++; continue; }
137+
if (c == '\'') inCharLiteral = false;
138+
continue;
139+
}
140+
141+
if (c == '/' && next == '/') { inLineComment = true; i++; continue; }
142+
if (c == '/' && next == '*') { inBlockComment = true; i++; continue; }
143+
if (c == '@' && next == '"') { inVerbatimString = true; i++; continue; }
144+
if (c == '"') { inString = true; continue; }
145+
if (c == '\'') { inCharLiteral = true; continue; }
146+
147+
if (c == '{') depth++;
148+
else if (c == '}')
149+
{
150+
depth--;
151+
if (depth == 0) return razor.Substring(start, i - start);
152+
}
153+
}
154+
return null;
155+
}
156+
}

src/ShellUI.Components/Components/InputOTP.razor

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@
120120
{
121121
try
122122
{
123-
await JS.InvokeVoidAsync("eval", $"document.getElementById('otp-input-{_id}-{index}')?.focus()");
123+
await JS.InvokeVoidAsync("ShellUI.focusElement", $"otp-input-{_id}-{index}");
124124
}
125125
catch
126126
{

src/ShellUI.Components/Components/SidebarTrigger.razor

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
Class)"
88
@onclick="HandleClick"
99
@attributes="AdditionalAttributes">
10-
<i class="fa-solid fa-bars-staggered text-sm"></i>
10+
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="size-4">
11+
<line x1="3" y1="6" x2="21" y2="6" />
12+
<line x1="3" y1="12" x2="15" y2="12" />
13+
<line x1="3" y1="18" x2="18" y2="18" />
14+
</svg>
1115
<span class="sr-only">Toggle Sidebar</span>
1216
</button>
1317

src/ShellUI.Components/Components/ThemeToggle.razor

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,21 @@
3333
[Parameter(CaptureUnmatchedValues = true)]
3434
public Dictionary<string, object>? AdditionalAttributes { get; set; }
3535

36-
protected override async Task OnInitializedAsync()
36+
protected override void OnInitialized()
3737
{
3838
_instances.Add(this);
39-
39+
}
40+
41+
// localStorage and document mutation must run after first render — JSRuntime
42+
// is unavailable during prerender on Blazor Server.
43+
protected override async Task OnAfterRenderAsync(bool firstRender)
44+
{
45+
if (!firstRender) return;
4046
try
4147
{
4248
var theme = await JSRuntime.InvokeAsync<string>("localStorage.getItem", "theme");
4349
_isDark = string.IsNullOrEmpty(theme) ? true : theme == "dark";
50+
StateHasChanged();
4451
}
4552
catch
4653
{
@@ -52,21 +59,14 @@
5259
{
5360
_isDark = !_isDark;
5461
var theme = _isDark ? "dark" : "light";
55-
62+
5663
try
5764
{
5865
await JSRuntime.InvokeVoidAsync("localStorage.setItem", "theme", theme);
59-
60-
if (_isDark)
61-
{
62-
await JSRuntime.InvokeVoidAsync("eval", "document.documentElement.classList.add('dark')");
63-
}
64-
else
65-
{
66-
await JSRuntime.InvokeVoidAsync("eval", "document.documentElement.classList.remove('dark')");
67-
}
68-
69-
// Update all ThemeToggle instances
66+
await JSRuntime.InvokeVoidAsync(
67+
_isDark ? "ShellUI.addClassToDocument" : "ShellUI.removeClassFromDocument",
68+
"dark");
69+
7070
foreach (var instance in _instances)
7171
{
7272
if (instance != this)
@@ -75,7 +75,7 @@
7575
instance.StateHasChanged();
7676
}
7777
}
78-
78+
7979
StateHasChanged();
8080
}
8181
catch

src/ShellUI.Components/Services/ThemeService.cs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,13 @@ public async Task<string> GetThemeAsync()
3737
public async Task SetThemeAsync(string theme)
3838
{
3939
_currentTheme = theme;
40-
40+
4141
try
4242
{
4343
await _jsRuntime.InvokeVoidAsync("localStorage.setItem", "theme", theme);
44-
45-
if (theme == "dark")
46-
{
47-
await _jsRuntime.InvokeVoidAsync("eval", "document.documentElement.classList.add('dark')");
48-
}
49-
else
50-
{
51-
await _jsRuntime.InvokeVoidAsync("eval", "document.documentElement.classList.remove('dark')");
52-
}
44+
await _jsRuntime.InvokeVoidAsync(
45+
theme == "dark" ? "ShellUI.addClassToDocument" : "ShellUI.removeClassFromDocument",
46+
"dark");
5347
}
5448
catch
5549
{

0 commit comments

Comments
 (0)