Files

T

Joseph Doherty 9e243493fb ui: Central UI density/consistency sweep + Theme 0.4.1

Applies the family-wide admin-UI cleanup playbook to the Central UI so the
Blazor surfaces stop diverging from the shared kit: buttons are grouped rather
than individually sized, long cell values are contained instead of widening
tables, and hard-coded colours give way to theme tokens.

The headline fix is that MainLayout passed Accent="#2f5fd0" to ThemeShell,
which the kit emits as an inline style on the shell root. Being a descendant of
<html>, it beat the [data-bs-theme="dark"] override for the entire app, so the
dark accent had never rendered. Declaring --accent in site.css :root instead
lets both schemes resolve; light is unchanged because the value already matched
the kit's light default.

Theme pins to 0.4.1, which upstreams the local .btn sizing block verbatim, so
that block is deleted here rather than duplicated. Verified byte-identical
before removal; the repo now declares no --bs-btn-* anywhere.

NOT purely cosmetic, contrary to the sweep's stated scope: four detail-modal
surfaces (NotificationReport, ConfigurationAuditLog, ParkedMessages,
SiteCallsReport) were additionally refactored from holding the selected record
to holding its id and re-resolving from the current page each render, with the
resolve doubling as the visibility gate. A background refresh that drops the
row now closes the modal instead of showing a stale snapshot. This is a
behaviour change and is called out rather than buried: a full-suite run turned
up one intermittent CentralUI failure, CloseButton_DismissesModal, whose stack
(GetRequiredEventBindingEntry during DispatchEventAsync) indicates the handler
was disposed between render and click — a window the previous field-held record
made structurally impossible. Treat the modal lifecycle here as unreviewed.

Build 0/0; suite green apart from that one intermittent failure.

2026-08-11 05:50:12 -04:00

23 KiB

Raw Blame History

Central UI cleanup sweep (2026-08-11)

Applies the family admin-UI cleanup playbook (../scadaproj/admin_ui_cleanup.md) to the ScadaBridge Central UI. That playbook was distilled from two completed rounds — the ignitionoee router and the OtOpcUa AdminUI — both on the same ZB.MOM.WW.Theme 0.3.1 + Bootstrap 5.3 base. Same classes of defect, different instances per app; the discovery phase below was run before anything was changed.

Surface: src/ZB.MOM.WW.ScadaBridge.CentralUI/ (99 .razor files, 43 routable pages) plus src/ZB.MOM.WW.ScadaBridge.Host/Components/App.razor.

0. Where ScadaBridge diverges from the playbook's premises

Recorded first because four of the playbook's assumptions are wrong for this repo, and acting on them unverified would have produced damage rather than cleanup.

Playbook premise	ScadaBridge reality
"Theme 0.3.1 is light-only — don't chase dark mode."	Inverted. This app implements dark mode itself: `site.css` carries a `[data-bs-theme="dark"]` token override block and `App.razor` has a pre-paint inline script reading `localStorage['sb-theme']`. Hard-coded colours and phantom tokens are therefore worse here, not moot.
"Check whether the scoped-CSS bundle is linked — OtOpcUa's was dead."	CLEAN, verified on disk rather than assumed. The RCL bundle carries all 8 scope ids and the Host bundle's entire content is the `@import` of it; `Program.cs` calls `MapStaticAssets()`. Adding a manual `<link>` would double-apply.
"Neither app has bUnit; the final gate is a live browser pass."	Wrong for this repo — `tests/…CentralUI.Tests` references bUnit, and several tests render components. Page-level tests are still source-scan guards that grep `.razor` text.
"The theme's `panel`/`panel-head` shell."	Not used here at all (`grep -c panel-head` = 0). The idiom is Bootstrap `card`/`card-header`/`card-body`.

1. Discovery findings

1a. Foundation / CSS audit

Phantom var(--…) tokens: CLEAN, zero. All 25 token usages resolve. The two that look undefined (--audit-col-width, --tv-depth) are set at runtime from AuditResultsGrid.razor.cs / audit-grid.js and TreeView.razor. No var(--x, fallback) masks a phantom.
Ghost CSS classes: 26 distinct / 39 occurrences, but only ONE loses intent. form-label-sm (×6, SecuredWrites.razor:44,54,65,76,80,89) — Bootstrap ships .form-label and .col-form-label-sm but not .form-label-sm, so the intended smaller label silently no-opped next to form-select-sm/form-control-sm controls. The other 25 are inert naming hooks whose geometry lives in an inline style= on the same element or is driven by JS interop.
Button sizing. Theme 0.3.1 ships no .btn rule, so Bootstrap's hard-coded --bs-btn-font-size: 1rem applied against the kit's 0.9rem body. 316 .btn elements: 244 explicit btn-sm/btn-lg, 22 sized by an enclosing btn-group-sm, 50 full-size — concentrated in the Transport wizard feet (18) and the shared form-shell Save/Cancel feet (15). 13 further buttons carry py-0 px-* micro-sizing hacks, a symptom of the same gap.
Hard-coded colours — one HIGH. MainLayout.razor:6 passed Accent="#2f5fd0" to ThemeShell. The kit emits that parameter as style="--accent: …" on the shell root, which is a descendant of <html> and therefore beat the [data-bs-theme="dark"] override for the entire app — the dark accent was dead, affecting the brand mark, the active rail-link border and .rail-btn. And it was a no-op in light, because #2f5fd0 is already theme.css's own light default. Also MEDIUM: AlarmTriggerEditor.razor:281-283 baked Bootstrap light subtle tints into a deadband-preview SVG, rendering as bright pastel bars on the dark page.
The playbook's repeated inline width:18px expander hack is essentially absent — the four named tree components size their expanders by class. One hit only, Dialogs/TreeRow.razor:10.

1b. Button inventory

Four of the prior apps' defect classes simply do not exist here: zero · action separators, zero <a href="#"> actions, zero inline style= button sizing hacks, zero navigation-interceptor "must be a button" comments. And btn-group is not zero — 6 correct groups already exist and served as the exemplars.

Real inventory: ~45 grouping candidates (15 dialog feet, 15 form feet, 2 filter bars, 3 pagers, 9 toolbars, ~14 row-action cells); 4 genuine link→action conversions; 4 btn-link Back buttons against 7 siblings already using btn-outline-secondary btn-sm (three idioms for one affordance); ~26 inter-button spacers to drop (and 12 lookalikes that must be kept); exactly one inline arm→confirm flow (TransportExport.razor:499-518), restyle-only.

Biggest levers: DialogHost.razor (1 render site, 23 ConfirmAsync call sites), MonacoEditor.razor (6 files), SchemaBuilder.razor (4 files / 7 instances).

1c. Prose inventory

The UI was already lean: ~12 unconditionally-rendered explainer blocks across 98 files, zero rendered milestone labels, zero dead links, zero wrong config keys, zero wrong numerics. 10 DELETE, 4 RELOCATE-then-delete, ~120 KEEP.

The real defects were in docs/, not on the pages — and per the playbook, wrong facts outrank style:

Component-Transport.md claimed a NotificationList pulls in its SmtpConfiguration. It does not — DependencyResolver only walks ExportSelection.SmtpConfigurationIds. The UI banner was the correct statement and the doc was wrong.
Component-Transport.md listed API keys as exportable in five places. They are deliberately not transportable (see the comment on ExportSelection), and the shipped SMS configs group was missing from the same lists.
Component-Transport.md described a "6-Step" import wizard; the UI renders 5 pills, with Map a sub-section inside Diff.
Component-CentralUI.md (×4) and Component-ClusterInfrastructure.md still said "via ClusterClient" — removed in the gRPC migration's Phase 4.
Component-Commons.md omitted Expression from AlarmTriggerType.
The HiLo partial-merge vs whole-replace override rule existed only in a June plan doc, never in docs/requirements/.

Plus one vestigial UI field: SiteForm.razor collects a per-site Akka Address that has had no runtime consumer since gRPC Phase 4 — only writers remain; every central→site dial resolves GrpcNodeAAddress/GrpcNodeBAddress.

Out-of-scope doc drift found while verifying the above (NOT fixed here)

Chasing the UI prose's citations surfaced a wider ClusterClient de-drift job that this sweep deliberately did not take on — it is a docs initiative, not a UI cleanup, and silently expanding into it would have made this change unreviewable. Recorded so it does not get lost:

docs/requirements/HighLevelReqs.md:49,51,52,405,523 still describes ClusterClient/ClusterClientReceptionist as the live command/control transport, including "Central creates a ClusterClient per site using both Akka addresses as contact points". All of that went in the migration's Phase 4.
docs/requirements/Component-ManagementService.md contradicts itself: :27 says the ManagementActor is "not advertised via ClusterClientReceptionist", while :16, :21, and the whole :48 "ClusterClientReceptionist Registration" section say it registers and that failover routes through ClusterClient. :272 and :278 repeat the claim.
docs/requirements/Component-NotificationOutbox.md:31 labels the site→central store-and-forward edge "(ClusterClient)" in its diagram.
docs/requirements/Component-Commons.md:292 — a namespace-tree comment reading "HTTP/ClusterClient management commands".

Component-Host.md is already correct (it has an explicit REQ-HOST-6a "no ClusterClientReceptionist" section) and is the model for what the others should say.

1d. Density / layout

The scan that matters most, and the one that fires hardest here.

Unbounded free-text columns — ~30 cells. Worst: SecuredWrites.razor:218 (@row.ExecutionError, raw device exception text, last of 11 columns, six text-nowrap neighbours), TransportImport.razor:265 (@item.FieldDiffJson, externally-produced JSON in the only <pre> in the codebase without pre-wrap, inside a colspan="6" row), and ConnectionCertificates.razor:63-64 (two adjacent remote X.509 DNs, bare, columns 1–2 of 7). Notable pattern: several rows had already bounded their LastError correctly but left a sibling column bare.
Sections running together — 8 pages. Worst: Health.razor:234-434 (five <h6> groups in one card body per site) and TemplateEdit.razor:436 (four heading+table groups in one card body, and that card is emitted above the page's own <h4> title).
Identity slam — ~10. Worst: Health.razor:208 (name + machine id fused in one <strong>), NotificationLists.razor:77 (recipient name + contact in one chip, uncapped count, one cell).
Raw slices. No unguarded ones in markup. Three unguarded on toast paths (Topology.razor:905,963,964) plus a parser bug at ParameterValueForm.razor:382.
Detail surfaces holding the row OBJECT rather than its id — 5. Latent today (none of those pages is on a timer) but wrong, and mandatory to fix before any lands on a timer page.

Reference pages already doing it right: Admin/Sites.razor (stacked identity), Monitoring/ParkedMessages.razor (line-clamped error + drawer).

1e. Tree tables — verdict: REJECT

Do not port the router's generic TreeTable/TreeVisibility. Zero rowspan, zero indent-by-padding table rows, zero repeated path-prefix columns, zero faked group-header rows. The only hierarchy-ish shape (dotted composed member names in InstanceConfigure.razor) is one column, flat for non-composed instances, carries per-row edit controls, and already has a proper tree presentation in DebugView.razor. The six nav/picker trees (TreeView, TemplateFolderTree, NodeBrowserDialog+TreeRow, ExecutionTree, SchemaBuilder) are the "different animal" the playbook says to leave alone. An unused component would be an orphan.

2. Changes made

Foundation (landed first — page batches reference it by name)

src/ZB.MOM.WW.ScadaBridge.CentralUI/wwwroot/css/site.css (already loaded after <ThemeHead />, so cascade order needed no change):

The .btn / .btn-sm Bootstrap CSS-variable override block, verbatim from the playbook, with the upstreaming header comment. Normalizes every unsized button app-wide without touching call sites. Deliberately overrides the variables only — btn-group seam and radius machinery depends on the box properties.
--accent moved into :root here, and Accent="#2f5fd0" removed from MainLayout.razor's ThemeShell. As a :root declaration it loses to the dark block (equal specificity, later in the same file) and wins over theme.css (equal specificity, earlier sheet). Light mode is byte-identical because the value already matched theme.css's own light default.
.form-label-sm — the one ghost class that lost intent, given the rule it always implied.
Table-containment utilities the page batches use: .cell-clip (+ -sm / -lg width modifiers), .cell-clamp-2, .detail-pre. Modelled on the existing .parked-error-clamp pattern. House rule: every clip/clamp is paired with a title so the full value stays reachable.
AlarmTriggerEditor.razor deadband SVG fills → var(--bad-bg) / var(--ok-bg).

Docs (landed before the page deletions, so no deletion raced its relocation): Component-Transport.md (API keys not transportable + the recovery hint, SMS configs added, the SMTP dependency-edge claim corrected, 6-Step → 5-Step, manifest samples and CLI synopsis), Component-CentralUI.md (ClusterClient → gRPC command channel ×4, Akka address marked legacy), Component-ClusterInfrastructure.md (ClusterClient → gRPC), Component-Commons.md (AlarmTriggerType.Expression), Component-TemplateEngine.md (the HiLo partial-merge rule).

Page batches

Seven batches on disjoint file sets, each briefed with its own file:line inventory and the standing instruction to verify by reading before editing and refuse items the markup contradicts. Behavior preserved byte-for-byte throughout: @onclick, disabled, bindings, AuthorizeView gates, data-test/data-testid hooks, and the arm→confirm two-step.

Batch	Files	Highlights
1 — Transport + Ops	SecuredWrites, TransportImport/Export, ConnectionCertificates, TestBindingsDialog	The two worst unbounded cells bounded; 5 prose deletions + the 2 relocated banners; the app's only inline arm→confirm restyled (two-step preserved)
2 — Monitoring	Health, EventLogs, ParkedMessages, AlarmSummary	Health per-site rework (see refusal below); identity slam split; EventLogs expander re-keyed off the loop index onto the event GUID; ParkedMessages drawer converted from row-object to row-id + re-resolve
3 — Admin + form shells	6 Admin pages, both endpoint editors, 4 Design forms	SiteForm/DataConnectionForm/ApiMethodForm split into one card per group; the 7-group OpcUaEndpointEditor given `fieldset`/`legend`; Back-link idiom unified; Akka address relabelled legacy
4 — Shared + dialogs	DialogHost, MonacoEditor, SchemaBuilder + 14 dialogs	`DialogHost` footer grouped — one edit reaching 23 confirm call sites; 4 Monaco toolbar links → a real button group across 6 files; 12 dialog feet grouped
5 — Reports/Audit/Notifications	SiteCalls, 6 Notifications pages, 2 Audit pages, AuditResultsGrid/FilterBar, Dashboard	Row-action groups (anchor + buttons, destructive red); recipient chip slam capped at 5 + "N more"; three object-holding detail surfaces converted to id + re-resolve
6 — Deployment + Design	Deployments, InstanceCreate, Topology, TemplateCreate, SchemaLibrary, SharedScriptForm, Templates, DataConnections, AuditLogPage	Deployment identity split; three unguarded `[..8]` revision-hash slices fixed (a real latent `ArgumentOutOfRangeException` on the deploy path)
7 — The two guard-pinned giants	InstanceConfigure, TemplateEdit	TemplateEdit's inherited-members card moved below the page title (ordering bug) and split into four carded groups; all 67 pinned substrings re-verified present, both negative assertions still absent

Refusals worth recording (the briefs were wrong, the agents were right)

The playbook's instruction to verify by reading before editing and refuse what the markup contradicts earned its place four times:

Health per-site groups (batch 2) — brief premise false. The brief said "five headings stack flush in one card body" with "metadata paragraphs between heading and table". Neither was true: the headings sit in four col-md-6 grid columns, three already behind collapse toggles, and no such paragraph exists. Following it literally would have nested two cards inside each of two collapse regions with the toggles floating outside any boundary. The agent restructured to one card per grid column, promoting each existing collapse toggle into a real card-header.
ApiMethodForm "Real I/O" strings (batch 3) — brief would have broken the suite. The brief said to preserve "Real I/O" there. TestRunWarningTests asserts the opposite: ApiMethodForm must not contain it (the Inbound API sandbox has no side-effect surface). Verified against the test.
AuditResultsGrid Target column (batch 5) — .cell-clip was the wrong tool. white-space: nowrap
- max-width raises a cell's min-content width, which would stop a resized column shrinking below the text width and break drag-to-resize at exactly the narrow widths the feature exists for. Fixed with a character cap instead, mirroring the grid's own TruncateError.
dc-kebab restyle (batch 6) — contextual, not accidental. The page <style> block drives a hover-reveal with !important padding and an explicit colour override that btn-outline-secondary would fight. It is also the only kebab of 12 living in a hover-reveal tree row.

Two smaller ones: DataConnections' toolbar has just one plain button once the dropdown is excluded, so a btn-group there would be a no-op wrapper (no change made); and NodeBrowserDialog's search label was reverted by its batch on discovering two tests selected it by tag — completed afterwards at the orchestration level, where the tests could be updated in the same change.

One brief error I introduced and all three affected agents caught independently: I specified class="mono" for stacked machine ids. .mono does exist (theme.css:117, with tabular-nums) — the agents' stated reason was wrong — but the app uses Bootstrap's font-monospace everywhere and has zero mono usages, so their substitution is the correct outcome for consistency.

3. Verification

dotnet build ZB.MOM.WW.ScadaBridge.slnx → 0 warnings, 0 errors.

dotnet test ZB.MOM.WW.ScadaBridge.slnx → 8,750 passing, including CentralUI.Tests 990/990 (the bUnit + source-scan guard suites that pin this UI).

Two non-issues, both confirmed rather than assumed:

SiteRuntime.Tests 1 failure / Host.Tests 1 failure — different tests on different runs, and both projects pass in isolation on re-run (540/540, 473/473). The sweep touched zero files in either project. Flakes.
CentralUI.PlaywrightTests 159 failures — environmental and pre-existing. docker/docker-compose.yml sets ScadaBridge__Security__Auth__DisableLogin: "true" on both central nodes (the SEC-36 GLAuth rotation workaround, uncommitted and not part of this change). Every test expecting a login flow fails regardless of this sweep; the visible errors are login-throttle lockouts and a 429.

Test selectors updated (markup changed underneath them)

Folding per-member btn-sm into the group class — which the playbook mandates — moves the size class off the button and invalidates locators pinning it. Also the two node-picker labels became buttons. Updated, each with a comment naming this sweep:

TransportExportPageTests.cs ×2 — assertions on data-testids carried by deleted prose banners. The neighbouring Assert.Empty(...group-api-keys) and Assert.NotNull(...group-sites) survive and carry the real intent.
TemplateCrudTests.cs ×4, SiteCrudTests.cs ×4, LdapMappingCrudTests.cs ×5, M9SurfaceTests.cs ×1 — button.btn-success.btn-sm / btn-outline-danger.btn-sm → size class dropped.
NodeBrowserDialogSelectionTests.cs ×2, NodeBrowserDialogSearchTests.cs ×1 — anchor → button.btn-link.

Merged-tree verification greps

btn-group count 6 → 43; 0 groups left with a redundant member btn-sm (5 were normalized at merge).
35 .cell-clip/.cell-clamp-2 usages, 35 carrying a title — the house rule held across all seven batches.
javascript:void(0) — 0 remaining in live markup (2 mentions survive inside comments explaining the removal).
Every pinned guard-test substring present; both negative assertions still absent.

4. Live browser gate

Rig redeployed via docker/deploy.sh — all 8 nodes, both central instances, so Traefik round-robin cannot serve a stale one. Both /health/ready 200; node-a active.

#	Check	Result
1	Computed-style probes	PASS — body 14.4px vs `.btn-sm` 12.48px (0.78rem override live); `.form-label-sm` 12.8px (was a silent no-op); `.cell-clip` block/352px/nowrap/hidden; `.detail-pre` pre-wrap + 300px; scoped bundle and site.css both in `document.styleSheets`
1a	`.cell-clamp-2` genuinely clamping	PASS — computed `display` serializes as `flow-root`, which looked like a silent failure; measured instead: 40px clamped vs 500px unclamped. Working
1b	Dark accent (the headline fix)	PASS — brand mark `rgb(47,95,208)` light → `rgb(77,127,232)` dark; `--accent-deep` now flips with it (`#1e3f99` → `#7aa0ef`) instead of desyncing. The inline `--accent` on the shell root is gone
2	Every page leads with data	PASS — Dashboard leads with KPI tiles; Export Bundle leads with the step indicator + artifact tree; all 10 deleted prose blocks absent
3	Seamed `btn-group`s, destructive red	PASS — measured seam gap −1px (overlapping borders); Site Calls row group renders `View audit history │ Retry │ Discard` as one control with Discard red
4	Arm→confirm, then Cancel	PASS — trigger is now an outline-danger button (was a bare red text link); arming flips it solid and reveals the guarded warning + seamed `Yes, export without encryption │ Cancel`; Cancel disarms, stays on Encrypt, no export performed
4a	`DialogHost` grouped footer (23 call sites)	PASS — Discard on a parked notification opens `Cancel │ Delete` seamed, danger labelling intact, backdrop covering; Cancel closed cleanly with all 31 parked rows intact and no destructive write
5	Scoped-bundle resurrections	N/A — the bundle was already correctly linked here (discovery 1a-2); nothing to resurrect
6	Legends / hints / empty states still render	PASS — ParkedMessages empty state ("Nothing has failed enough to give up on at this site"), the Encrypt-step secret-fields banner, and SiteForm's legacy-address `form-text` all render
7	Density with realistic faulted data	PASS — exercised against a genuinely unhealthy rig: 31 parked notifications with live error text, 47 stuck site calls, 1 parked call. No horizontal scroll on any page; zero elements overflowing the viewport
7a	Clip actually engages under pressure	PASS — injecting a 150-char OPC UA node id into a clipped cell truncates it (1084px content in a 352px box) while the table grows only 75px and the viewport never overflows. Uncontained, that value would have pushed the actions column off-screen
7b	Sections read as separate cards	PASS — Health's three KPI groups and its four per-site groups now have real card boundaries; SiteForm renders as `Edit Site` / `Node A` / `Node B`
7c	Identity stacking	PASS — Health shows `Test Plant A` with `site-a` in mono beneath (was fused into one `<strong>`)

Not exercised live: the ParkedMessages drawer re-key — site-a has no parked store-and-forward rows (the 31 parked are central-side notifications). It is covered by the build and by CentralUI.Tests; the equivalent re-key on NotificationReport was exercised through the confirm path above.

Rig note: docker/docker-compose.yml carries an uncommitted DisableLogin: "true" from the earlier EWS live-gate session (SEC-36). It predates this sweep and was left untouched.

23 KiB Raw Blame History Unescape Escape