diff --git a/ZB.MOM.WW.Theme/CLAUDE.md b/ZB.MOM.WW.Theme/CLAUDE.md index 2983e36..d9e515c 100644 --- a/ZB.MOM.WW.Theme/CLAUDE.md +++ b/ZB.MOM.WW.Theme/CLAUDE.md @@ -4,6 +4,12 @@ Shared UI kit (Razor Class Library — Technical-Light tokens, IBM Plex, side-ra **Backlog / open work:** lives in Gitea issues on `dohertj2/scadaproj` (`tea issues list --repo dohertj2/scadaproj`) — currently #9 (roll `0.5.0` to the other consumers), #10 (`layout.css` `calc(100vh - 3.3rem)` review). The history below is a record, not a status tracker; file new follow-ups as issues. +## 0.5.1 (2026-11-09, UNPUBLISHED) + +Version bumped in `Directory.Build.props`, not packed or pushed. Fixes Theme-007 (scadaproj#44): +`TechButton` now ORs a caller-splatted `disabled` with `Busy` (it was overwritten by the trailing +`disabled="@Busy"` and dropped whenever `Busy` was false). No API change. Tests: 65 pass. + ## Published 0.5.0 (2026-11-04) Published the tested ignitionoee backlog package to the shared Gitea feed on diff --git a/ZB.MOM.WW.Theme/Directory.Build.props b/ZB.MOM.WW.Theme/Directory.Build.props index 15f1d8a..f17cecb 100644 --- a/ZB.MOM.WW.Theme/Directory.Build.props +++ b/ZB.MOM.WW.Theme/Directory.Build.props @@ -4,7 +4,7 @@ enable enable latest - 0.5.0 + 0.5.1 true diff --git a/ZB.MOM.WW.Theme/src/ZB.MOM.WW.Theme/Components/TechButton.razor b/ZB.MOM.WW.Theme/src/ZB.MOM.WW.Theme/Components/TechButton.razor index a2cf568..cad9d46 100644 --- a/ZB.MOM.WW.Theme/src/ZB.MOM.WW.Theme/Components/TechButton.razor +++ b/ZB.MOM.WW.Theme/src/ZB.MOM.WW.Theme/Components/TechButton.razor @@ -1,6 +1,6 @@ @namespace ZB.MOM.WW.Theme @* Components/TechButton.razor *@ - @@ -33,6 +33,8 @@ /// /// Arbitrary additional HTML attributes splatted onto the underlying /// <button> (e.g. id, @onclick, name, value). + /// A caller-supplied class is merged into the button's classes, and a caller-supplied + /// disabled is OR-ed with — the button is disabled when either asks. /// [Parameter(CaptureUnmatchedValues = true)] public IDictionary? AdditionalAttributes { get; set; } @@ -44,6 +46,13 @@ AdditionalAttributes?.FirstOrDefault(pair => string.Equals(pair.Key, "class", StringComparison.OrdinalIgnoreCase)).Value?.ToString() }.Where(value => !string.IsNullOrWhiteSpace(value))); + // The trailing disabled="..." in the markup overrides a splatted one, so the caller's value has + // to be folded in here or it is silently dropped (Theme-007). Mirrors Blazor's own rendering of + // a splatted attribute: false/null omit it, anything else (true, "disabled", "") renders it. + private bool CallerDisabled => + AdditionalAttributes?.FirstOrDefault(pair => string.Equals(pair.Key, "disabled", StringComparison.OrdinalIgnoreCase)).Value + is { } value && value is not false; + private string VariantClass => Variant switch { ButtonVariant.Secondary => "btn-outline-secondary", diff --git a/ZB.MOM.WW.Theme/tests/ZB.MOM.WW.Theme.Tests/TechButtonExtensionsTests.cs b/ZB.MOM.WW.Theme/tests/ZB.MOM.WW.Theme.Tests/TechButtonExtensionsTests.cs index da423ce..97eb8a9 100644 --- a/ZB.MOM.WW.Theme/tests/ZB.MOM.WW.Theme.Tests/TechButtonExtensionsTests.cs +++ b/ZB.MOM.WW.Theme/tests/ZB.MOM.WW.Theme.Tests/TechButtonExtensionsTests.cs @@ -56,6 +56,42 @@ public sealed class TechButtonExtensionsTests : BunitContext Assert.DoesNotContain(cut.FindAll("span"), x => x.ClassList.Contains("spinner-border")); } + [Theory] + [InlineData(true)] + [InlineData("disabled")] + [InlineData("")] + public void Caller_disabled_is_honoured_when_not_busy(object disabled) + { + // Theme-007 (scadaproj#44): the trailing disabled="@Busy" overwrote the splatted value, so + // a caller's disabled was silently dropped whenever Busy was false. + var cut = Render(p => p + .AddUnmatched("disabled", disabled) + .AddChildContent("Delete")); + + var button = cut.Find("button"); + Assert.True(button.HasAttribute("disabled")); + Assert.Empty(cut.FindAll(".spinner-border")); + } + + [Fact] + public void Caller_disabled_false_leaves_the_button_enabled() + { + var cut = Render(p => p.AddUnmatched("disabled", false).AddChildContent("Go")); + + Assert.False(cut.Find("button").HasAttribute("disabled")); + } + + [Fact] + public void Caller_disabled_toggles_off_on_rerender() + { + var cut = Render(p => p.AddUnmatched("disabled", true).AddChildContent("Go")); + Assert.True(cut.Find("button").HasAttribute("disabled")); + + cut.Render(p => p.AddUnmatched("disabled", false)); + + Assert.False(cut.Find("button").HasAttribute("disabled")); + } + [Fact] public void Lowercase_class_attribute_preserves_required_button_classes() { diff --git a/code-reviews/README.md b/code-reviews/README.md index 86ee6e0..97d4abd 100644 --- a/code-reviews/README.md +++ b/code-reviews/README.md @@ -40,7 +40,7 @@ code-reviews/ ## Summary -10 of 10 libraries reviewed. 47 pending findings across all libraries. +10 of 10 libraries reviewed. 46 pending findings across all libraries. Coverage: Secrets, LocalDb, GalaxyRepository and Overview first reviewed, and Auth, Health, Theme, Audit, Configuration and Telemetry re-reviewed, on 2026-11-09 (#15). Each finding also @@ -50,9 +50,9 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low). |----------|---------------| | Critical | 0 | | High | 0 | -| Medium | 14 | +| Medium | 13 | | Low | 33 | -| **Total** | **47** | +| **Total** | **46** | ## Library Status @@ -67,7 +67,7 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low). | [Overview](Overview/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/3 | 3 | 5 | | [Secrets](Secrets/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/6/13 | 19 | 19 | | [Telemetry](Telemetry/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/3 | 3 | 11 | -| [Theme](Theme/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/1/1 | 2 | 8 | +| [Theme](Theme/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/1 | 1 | 8 | ## Pending Findings @@ -84,7 +84,7 @@ _None open._ _None open._ -### Medium (14) +### Medium (13) | ID | Library | Title | |----|---------|-------| @@ -101,7 +101,6 @@ _None open._ | Secrets-010 | [Secrets](Secrets/findings.md) | Akka replication actor is created only when `ISecretStore` is first resolved; a node that never resolves it never replicates | | Secrets-013 | [Secrets](Secrets/findings.md) | UI add/rotate/delete: if the audit write throws after the store write succeeded, the change is reported and audited as a failure and the resolver cache is not evicted | | Secrets-017 | [Secrets](Secrets/findings.md) | `secret set`/`rotate` take the plaintext on argv; the host builder re-parses argv as configuration and can crash with the value in the error | -| Theme-007 | [Theme](Theme/findings.md) | `TechButton` silently discards a caller-supplied `disabled` attribute | ### Low (33) diff --git a/code-reviews/Theme/findings.md b/code-reviews/Theme/findings.md index ec286b3..9df859b 100644 --- a/code-reviews/Theme/findings.md +++ b/code-reviews/Theme/findings.md @@ -10,7 +10,7 @@ | Last reviewed | 2026-11-09 (re-review; baseline 2026-06-01 at `5f75cd4`) | | Reviewer | Claude (2026-11-09 re-review, scadaproj#15; baseline: Claude automated) | | Commit reviewed | `91fb2b4` | -| Open findings | 2 | +| Open findings | 1 | ## Summary @@ -300,7 +300,7 @@ Resolved in `544a6dd`, 2026-06-01 — added `///` summaries to all nine remainin | Severity | Medium | | Priority | P2 | | Category | Correctness & logic bugs | -| Status | Open | +| Status | Resolved | | Location | `ZB.MOM.WW.Theme/src/ZB.MOM.WW.Theme/Components/TechButton.razor:3` | **Description** @@ -336,7 +336,7 @@ bUnit test for caller `disabled=true` with `Busy=false`. **Resolution** -_Unresolved._ +Resolved 2026-11-09 in the commit tagged `(scadaproj#44)` (Theme `0.5.1`, unpublished) — `TechButton` renders `disabled="@(Busy || CallerDisabled)"`, where `CallerDisabled` reads the splatted `disabled` case-insensitively with Blazor's own splat semantics (false/null omit it; `true`, `"disabled"`, `""` render it). Chose the lookup over a new `[Parameter] bool Disabled` because Blazor binds parameters case-insensitively: a bool parameter would turn an existing string `disabled="disabled"` splat into a runtime cast failure. Regression tests `TechButtonExtensionsTests.Caller_disabled_is_honoured_when_not_busy` (`true`, `"disabled"`, `""`), `Caller_disabled_false_leaves_the_button_enabled` and `Caller_disabled_toggles_off_on_rerender`; the first and third fail without the fix. ### Theme-008 — `nav-state.js` "wired" guard is a DOM attribute that Blazor enhanced-navigation DOM sync strips, so toggle listeners accumulate and the transient active-link reveal is persisted