docs: correct mxgw Bootstrap claim + fold five-app sweep findings into admin_ui_cleanup
CLAUDE.md (Secrets row): - mxgw is NOT Bootstrap-free (libman.json pins bootstrap@5.3.3, App.razor:7 links it ahead of ThemeHead). The scadaproj#2 modal gate had skipped mxgw on that false premise; gate since run and PASS 2026-08-11. - scoped-CSS bundle claim scoped to its 2026-07-19 date; OtOpcUa links one now too, so it is two of four rather than one. admin_ui_cleanup.md — five rounds now (adds mxgw, HistorianGateway, ScadaBridge): - 1a-1: test sources are a fifth definition site, checked BEFORE the ghost list. A class matched only by a test is a selector, never a ghost. Extends to substring and visible-text assertions, so button labels are load-bearing. - 1a-4: two inverted button-sizing instances (literal font-size neutering btn-sm app-wide; a chassis class standing in for the button system). - 1a-5: determine dark scheme per app; where one exists the step inverts. - 1a-6 (new): a token override emitted as an inline style on the shell root defeats every scheme-level :root override. OtOpcUa carries the inert-but- latent instance. - 1c: reword nothing before checking test sources; the docs may be the wrong document, not the page. - 2: btn-sm fold breaks button.btn-*.btn-sm locators; cell-clip is wrong for resizable columns; briefs carry the rubric, not just line numbers. - verification: baseline pre-existing failures by measuring; a suspiciously fast green is a vacuous pass, not a pass. - live gate: mandatory bUnit or not; hit-testability over visibility; measure behaviour where computed values mislead (-webkit-box serializes flow-root); record what could not be exercised. - new: exemptions from a family-wide sweep must re-derive their premise.
This commit is contained in:
Joseph Doherty
+155
-18
@@ -1,17 +1,40 @@
|
||||
# Admin-UI cleanup playbook (`ZB.MOM.WW.Theme` + Bootstrap Blazor apps)
|
||||
|
||||
A reusable sweep recipe for the family's Blazor Server admin UIs, distilled from two completed
|
||||
A reusable sweep recipe for the family's Blazor Server admin UIs, distilled from five completed
|
||||
rounds:
|
||||
|
||||
- **ignitionoee router** — `../ignitionoee/docs/plans/2026-08-05-router-ui-sweeps.md` (+ its
|
||||
render-validation companion plan)
|
||||
- **OtOpcUa AdminUI** — `../OtOpcUa/docs/plans/2026-08-10-adminui-ui-sweeps.md`
|
||||
- **MxAccessGateway dashboard** — `../MxAccessGateway/docs/plans/2026-08-11-dashboard-ui-sweeps.md`
|
||||
- **HistorianGateway dashboard** — `../HistorianGateway/docs/plans/2026-08-11-dashboard-ui-sweep.md`
|
||||
- **ScadaBridge CentralUI** — `../ScadaBridge/docs/plans/2026-08-11-central-ui-cleanup-sweep.md`
|
||||
|
||||
Both apps sit on **ZB.MOM.WW.Theme 0.3.1 + self-hosted Bootstrap 5.3**, and both sweeps found the
|
||||
same *classes* of defect with different instances. Run the discovery phase before assuming which
|
||||
instances your app has — the router had ghost layout classes and the OtOpcUa AdminUI had none, but
|
||||
OtOpcUa had a dead scoped-CSS bundle the router didn't. **Neither app has bUnit; the final gate is
|
||||
always a live browser pass.**
|
||||
All five sit on **ZB.MOM.WW.Theme 0.3.1 + self-hosted Bootstrap 5.3** (mxgw included — an earlier
|
||||
claim that it was Bootstrap-free was false; see the exemption rule below), and all five found the
|
||||
same *classes* of defect with different instances. **Run the discovery phase before assuming which
|
||||
instances your app has.** The spread so far: the router had ghost layout classes and OtOpcUa had
|
||||
none; OtOpcUa had a dead scoped-CSS bundle and mxgw/HistorianGateway have no `*.razor.css` at all
|
||||
while ScadaBridge's bundle was already correctly wired; HistorianGateway's phantom tokens were all
|
||||
fallback-masked and ScadaBridge had zero; ScadaBridge's *pages* were right and its *docs* were
|
||||
wrong, inverting §1c.
|
||||
|
||||
**Determine the test harness per app — do not assume.** The router and OtOpcUa have no bUnit, so a
|
||||
live browser pass is their only gate. HistorianGateway (`bunit 1.40.0`, ten Dashboard test files)
|
||||
and ScadaBridge (`CentralUI.Tests`, 990 tests) both have one, which changes discovery materially
|
||||
(see §1a-1). **A live pass is still mandatory everywhere**, because computed styles and cascade
|
||||
order are exactly what bUnit cannot see.
|
||||
|
||||
### Exemptions need their premise re-derived
|
||||
|
||||
If a family-wide sweep or gate excuses one app on a property claim about that app — "it's
|
||||
Bootstrap-free", "it has no dark mode", "it has no scoped CSS" — **re-derive that property from
|
||||
that repo at sweep time.** It happened once already: the 2026-07-19 `/admin/secrets` modal gate
|
||||
skipped mxgw as "Bootstrap-free so never affected", which was false; mxgw self-hosts Bootstrap
|
||||
5.3.3 and *was* affected, and only escaped because it had taken the fix independently. The
|
||||
exemption reached the right conclusion on false reasoning and nobody re-checked for three weeks.
|
||||
This is the same discipline §1c applies to cited config keys, aimed at scope decisions instead of
|
||||
prose.
|
||||
|
||||
---
|
||||
|
||||
@@ -27,9 +50,32 @@ The theme ships less than pages assume. Check, in order:
|
||||
1. **Shipped-vs-used class matrix.** Extract every CSS class used in `*.razor` markup and check each
|
||||
against (a) the theme package's `staticwebassets/css/*.css`
|
||||
(`~/.nuget/packages/zb.mom.ww.theme/<ver>/`), (b) Bootstrap, (c) the app's own stylesheet, (d)
|
||||
page `<style>` blocks. Anything defined nowhere is a **silent no-op** — the browser ignores it
|
||||
without any error. (Router: 13 ghost classes from the mockups' theme.css that 0.3.1 never
|
||||
shipped. OtOpcUa: zero — it was authored against what ships.)
|
||||
page `<style>` blocks, **(e) the test sources**. Anything defined in none of (a)–(d) is a
|
||||
**silent no-op** — the browser ignores it without any error. (Router: 13 ghost classes from the
|
||||
mockups' theme.css that 0.3.1 never shipped. OtOpcUa: zero — it was authored against what ships.
|
||||
mxgw: 3, one load-bearing. HistorianGateway: 4. ScadaBridge: 26, but only one lost intent.)
|
||||
|
||||
**Check (e) BEFORE writing the ghost list, not as a filter afterwards.** A class matched only by
|
||||
a test is a **selector, not a ghost** — rename it deliberately or leave it, never delete it. The
|
||||
failure mode is nasty: deleting it reds the suite, and the tempting "fix" is to amend the
|
||||
assertions, which quietly destroys the coverage. HistorianGateway's 16 `console-*` classes look
|
||||
exactly like ghosts and are in fact driven by
|
||||
`cut.Find("input.console-tag")` / `button.console-run`, with the Viewer role gate resting on
|
||||
`Assert.DoesNotContain("console-write", cut.Markup)` — five tests. That sweep caught them only
|
||||
because it happened to read the test files while checking something else; ordering luck, not
|
||||
method. Hence: check first.
|
||||
|
||||
**The constraint is not limited to class names.** A bUnit suite can pin markup by *substring*
|
||||
and by *visible text*: `ApiKeysPageRenderTests` asserts `DoesNotContain("secret", …,
|
||||
OrdinalIgnoreCase)` over the list markup, and elsewhere selects with
|
||||
`Find("button:contains('Create API Key')")`. So **button label text is load-bearing too** — a
|
||||
sweep that "improves" wording breaks a suite exactly as a deleted class does. This binds §1c as
|
||||
much as it binds this step; see the note there.
|
||||
|
||||
Corollary, learned the hard way twice: **a dead-code claim derived from grepping `src/` alone is
|
||||
not a dead-code claim.** ScadaBridge nearly deleted `Shared/DataTable.razor` as an orphan —
|
||||
`DataTablePagerTests` renders it. Same mistake as the `console-*` near-deletion, different
|
||||
clothes.
|
||||
2. **Is the scoped-CSS bundle linked?** If any `*.razor.css` exists, the build emits
|
||||
`<App>.bundle.scp.css` — but nothing links it automatically when the head is hand-authored.
|
||||
Compare the `<link>` list in `App.razor` against `obj/…/scopedcss/bundle/`. OtOpcUa's
|
||||
@@ -43,8 +89,45 @@ The theme ships less than pages assume. Check, in order:
|
||||
4. **Button sizing.** Theme 0.3.1 ships **no `.btn` rule**, so Bootstrap's hard-coded
|
||||
`--bs-btn-font-size: 1rem` applies against the theme's 0.9rem body. Count how many buttons carry
|
||||
`btn-sm` vs full-size; modal feet and shared form shells are where full-size accumulates.
|
||||
5. **Dark scheme: don't chase it.** Theme 0.3.1 is light-only (no `prefers-color-scheme` /
|
||||
`data-bs-theme` anywhere). Fix any comment claiming otherwise.
|
||||
(ScadaBridge: 50 full-size + 13 `py-0 px-*` hacks.)
|
||||
|
||||
Two **inverted** instances, where the standard §2 `.btn` block would not have helped:
|
||||
|
||||
- **A literal `font-size` in the app stylesheet** (mxgw `site.css:187`, `font-size: 0.82rem`).
|
||||
Bootstrap sizes `.btn-sm`/`.btn-group-sm` *purely* by redefining `--bs-btn-font-size`, so a
|
||||
literal at equal specificity loaded later flattens every button to one size — `btn-sm` becomes
|
||||
a font-size no-op app-wide and differs only in padding. Tell: `btn-sm` present in quantity but
|
||||
visually indistinguishable.
|
||||
- **A chassis class standing in for the button system** (HistorianGateway: all 13 buttons were
|
||||
`.rail-btn`, the *side-rail* class, and 11 carried inline `style="width:auto"` to undo the
|
||||
`width:100%` a local override imposed). The `.btn` block would have applied to nothing. Fix is
|
||||
the block **plus** moving page buttons onto real `.btn`; the width hacks then delete
|
||||
themselves. Tell: inline sizing hacks clustered on one non-`.btn` class.
|
||||
5. **Dark scheme: determine whether the app has one.** Theme 0.3.1 itself is light-only (no
|
||||
`prefers-color-scheme` / `data-bs-theme`), and the router, OtOpcUa, mxgw and HistorianGateway
|
||||
are all light-only — for those, don't chase it, and fix any comment claiming otherwise.
|
||||
**ScadaBridge is the exception**: it implements its own `[data-bs-theme="dark"]` token block
|
||||
plus a pre-paint localStorage script. Where a second scheme exists this step **inverts** —
|
||||
hard-coded colours and inline token overrides are actively worse, not merely moot, because they
|
||||
are invisible in the scheme you're looking at and broken in the other.
|
||||
6. **Per-app token overrides delivered as inline styles.** Check how the app passes brand tokens to
|
||||
`ThemeShell`. The kit emits `Accent="…"` as an inline `style="--accent: …"` **on the shell
|
||||
root** — a descendant of `<html>` — so it **beats every scheme-level `:root` override below it**.
|
||||
ScadaBridge's `Accent="#2f5fd0"` killed the dark accent app-wide (brand mark, active rail-link
|
||||
border, `.rail-btn`) while being a no-op in light, because that hex already *is* theme.css's
|
||||
light default: invisible in both directions.
|
||||
|
||||
The general rule: **a per-app token override delivered as an inline style on a shell element
|
||||
silently defeats every scheme-level `:root` override.** Declare the token in the app
|
||||
stylesheet's `:root` instead — it then loses to the dark block by source order and wins over
|
||||
theme.css by sheet order.
|
||||
|
||||
Worked example of why to check even when nothing looks wrong: **OtOpcUa passes the identical
|
||||
`Accent="#2f5fd0"`** (`MainLayout.razor:11`) and has no dark scheme at all, so the override is
|
||||
**inert today and latent forever** — it does nothing now and silently kills the dark accent the
|
||||
day someone adds a dark block. That future session will see "dark mode doesn't work" with no
|
||||
reason to suspect a layout parameter set years earlier. ignitionoee passes no `Accent` and is
|
||||
unaffected.
|
||||
|
||||
### 1b. Button inventory
|
||||
|
||||
@@ -77,7 +160,18 @@ Inventory, with file:line:
|
||||
- **KEEP**: state banners (empty/error/truncation/live status), form hints (`form-text`, field
|
||||
hints — they render only in form context), visual-encoding **legends** that are the only decoder
|
||||
of a chip/graphic, workflow cross-links, expander-gated notes.
|
||||
- **Hunt stale claims while you're in there** — both sweeps found prose that was actively *wrong*,
|
||||
- **Before rewording anything, re-read §1a-1(e).** In a repo with bUnit, **visible text is a
|
||||
selector**: `Find("button:contains('Create API Key')")` makes a button's label load-bearing, and
|
||||
`DoesNotContain("secret", …, OrdinalIgnoreCase)` makes an *absent* substring load-bearing. Prose
|
||||
work is the activity most likely to trip this, because improving wording is the whole point of
|
||||
it. Grep the test sources for any string you are about to change.
|
||||
- **The wrong document may be the docs, not the page.** ScadaBridge inverted this section entirely:
|
||||
zero DELETEs, zero RELOCATEs — its pages carried no unconditional documentation at all — but its
|
||||
*docs* were wrong. `Component-Transport.md` claimed notification lists pull their SMTP config
|
||||
(they don't) and listed API keys as exportable (deliberately not; SMS configs were missing
|
||||
instead), plus stale ClusterClient claims in three files. When the page and the doc disagree,
|
||||
establish which one the code supports before assuming the page is the stale one.
|
||||
- **Hunt stale claims while you're in there** — every sweep found prose that was actively *wrong*,
|
||||
not just verbose: milestone labels for shipped features ("F8/F9 pending", "(F3)", "arriving in
|
||||
Batch 2/3"), architecture claims superseded by later phases, dead hyperlinks to repo paths the
|
||||
app doesn't serve, and **config keys that don't exist** (prose said `Authentication:Ldap`, the
|
||||
@@ -162,6 +256,10 @@ foundation branch merges — all branches build independently and compose at mer
|
||||
|
||||
- `<div class="btn-group" role="group">` (`btn-group btn-group-sm` when members are small); fold
|
||||
per-button `btn-sm` into the group class; drop `ms-2`/`me-1` spacers between members.
|
||||
**Budget for the fold breaking test locators**: any selector pinning `button.btn-*.btn-sm` stops
|
||||
matching the moment `btn-sm` moves onto the group. ScadaBridge updated 16 selectors for exactly
|
||||
this and it is a mechanical, expected cost — not a signal that the fold was wrong. Three sweeps
|
||||
hit the fold and only the one with a dense suite noticed, which is why it is written down here.
|
||||
- Mixed `<a class="btn">` + `<button>` members are valid Bootstrap — group them (keep `download`
|
||||
etc. attributes verbatim).
|
||||
- A group whose conditional member doesn't render is fine — keep `@if` inside the group.
|
||||
@@ -184,7 +282,14 @@ foundation branch merges — all branches build independently and compose at mer
|
||||
- **Truncate unbounded text with a safe helper, never a raw slice.** OtOpcUa's
|
||||
`DisplayText.Abbreviate(value, 60)` + the full text on `title` — a raw `@x[..60]` 500s the whole
|
||||
page when the value is shorter (OtOpcUa #504). Keep the full text reachable (tooltip + the
|
||||
row's detail dialog).
|
||||
row's detail dialog). Every app that hit this needed its own helper (HistorianGateway added
|
||||
`Dashboard/DisplayText.cs` with 7 tests pinning the short-value case); ScadaBridge also found
|
||||
three unguarded `[..8]` slices in `Topology.razor` that were a latent
|
||||
`ArgumentOutOfRangeException` on the deploy path, so grep for raw range operators generally, not
|
||||
only in the cells you're truncating.
|
||||
- **CSS clipping is the wrong tool for user-resizable grid columns.** `nowrap` + `max-width` raises
|
||||
the column's min-content width and breaks drag-to-resize. Cap the *string* there instead, and
|
||||
keep CSS clamping for fixed-width columns.
|
||||
- **Inline expander rows → a dialog.** Host the same detail component in a Bootstrap modal
|
||||
(`modal-backdrop fade show` + `modal fade show d-block`, `modal-lg`), one at a time. Lifecycle
|
||||
cautions that made the OtOpcUa version correct:
|
||||
@@ -201,23 +306,55 @@ foundation branch merges — all branches build independently and compose at mer
|
||||
- Discovery scans → plan doc with the full inventories → **one foundation agent + N page-batch
|
||||
agents on disjoint file sets**, all parallel; shared components (form shells, collection editors)
|
||||
are the biggest lever — one file can fix a dozen forms.
|
||||
- Briefs must carry exact `file:line` inventories **and** the instruction *"verify by reading
|
||||
before editing; refuse items the markup contradicts."* Both sweeps had agents correctly refuse
|
||||
brief items (buttons in mutually exclusive `@if` branches can't be grouped; toolbars the
|
||||
inventory over-counted).
|
||||
- Briefs must carry exact `file:line` inventories **and the rubric that produced them**, not just
|
||||
the line numbers — plus the instruction *"verify by reading before editing; refuse items the
|
||||
markup contradicts."* An agent holding only a line list can't tell a mis-derived item from a
|
||||
correct one. Every sweep so far has had agents correctly refuse brief items (buttons in mutually
|
||||
exclusive `@if` branches can't be grouped; toolbars the inventory over-counted), four of
|
||||
ScadaBridge's seven batches did, and one there caught a brief instruction that would have failed
|
||||
the test suite. Treat refusals as the mechanism working, not as friction.
|
||||
- Merge foundation first, then batches; run the **full build + test suite on the merged tree** even
|
||||
if every branch was green in isolation (parallel branches compose only at the full build).
|
||||
- **Baseline any failure you intend to call pre-existing — measure it, don't assert it.** Stash to a
|
||||
clean tree and run it more than once; a test that fails 2-in-3 on the untouched base is not your
|
||||
regression, and you can only know that by running it. Related trap: **a suspiciously fast green
|
||||
is not a pass.** HistorianGateway's `RedundantWritePathLiveTests` "passed in isolation" in 504 ms
|
||||
while the real code path requires a live historian round trip — it was taking an early-return
|
||||
guard and passing *vacuously*, and the full-suite run that failed was the honest one. This
|
||||
inverts the usual reading of "passes in isolation." An unguarded environment-dependent test that
|
||||
fails rather than skips on a dev box is a hygiene defect worth filing on its own, because it
|
||||
trains people to ignore a red suite.
|
||||
- Verification greps on the merged tree: zero remaining phantom-token usages, zero remaining
|
||||
`·` action separators / inline `width:` hacks, `btn-group` count roughly matching the
|
||||
inventory, the bundle link present.
|
||||
|
||||
### Live browser gate (mandatory — these apps have no bUnit)
|
||||
### Live browser gate (mandatory in every app, bUnit or not)
|
||||
|
||||
bUnit cannot see computed styles, cascade order, or whether an element is actually reachable by a
|
||||
pointer — which is precisely what this sweep changes. Two probes worth stealing:
|
||||
**hit-testability**, not just visibility (`document.elementFromPoint` at a confirm button's centre
|
||||
must return the button, not the backdrop — mxgw's modal gate), and **realistic-length injected
|
||||
data** (ScadaBridge injected a 150-char OPC UA node id; HistorianGateway created a key with an
|
||||
88-char display name, which rendered as 40 chars + ellipsis with Revoke still in the viewport).
|
||||
|
||||
**Record what you could not exercise rather than rounding it up.** HistorianGateway proved 2 of its
|
||||
4 truncation sites live and said so: `sf.Error` needs a genuine drain failure, which needs a
|
||||
historian that accepts a connection and *then* rejects the write, and a box with nothing listening
|
||||
dies at setup and never enqueues. That is a rig capability ceiling — don't manufacture it, and
|
||||
don't claim the sites you couldn't reach.
|
||||
|
||||
Rebuild/redeploy the app's dev rig (**all** instances behind the load balancer — round-robin serves
|
||||
you the stale one otherwise), then walk:
|
||||
|
||||
1. Computed-style probes (one `javascript` eval): a `.btn`'s `font-size` vs body; the phantom-token
|
||||
replacement class resolving to real colors; the scoped bundle present in `document.styleSheets`.
|
||||
**Measure behaviour rather than reading a computed value when the two can disagree.** Chrome
|
||||
serializes `-webkit-box` as `display: flow-root`, so a working line-clamp *reads* like a silent
|
||||
failure; ScadaBridge measured instead (40px clamped vs 500px unclamped) and confirmed it works.
|
||||
Reading the value alone would have produced a "fix" for something that was never broken.
|
||||
If the app has a second colour scheme, probe **both** — flip it and confirm the accent tokens
|
||||
move together (ScadaBridge: `rgb(47,95,208)` → `rgb(77,127,232)`, with `--accent-deep` following
|
||||
instead of desyncing).
|
||||
2. Every page leads with data — no unconditional documentation above the fold.
|
||||
3. Seamed `btn-group`s where the inventory says; destructive members red.
|
||||
4. One arm→confirm flow per style: arm → confirm UI appears → **Cancel** (never complete a real
|
||||
|
||||
Reference in New Issue
Block a user