diff --git a/Directory.Packages.props b/Directory.Packages.props index 699030d3..6badbfd5 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -103,7 +103,7 @@ - + diff --git a/docs/plans/2026-08-11-central-ui-cleanup-sweep.md b/docs/plans/2026-08-11-central-ui-cleanup-sweep.md new file mode 100644 index 00000000..f7f1a4b7 --- /dev/null +++ b/docs/plans/2026-08-11-central-ui-cleanup-sweep.md @@ -0,0 +1,292 @@ +# 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 `` 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 `` 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 `` 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 `
` 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 `
` 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 `

` title). +- **Identity slam — ~10.** Worst: `Health.razor:208` (name + machine id fused in one ``), + `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* ``, +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: + +1. **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`. +2. **`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. +3. **`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`. +4. **`dc-kebab` restyle (batch 6) — contextual, not accidental.** The page `