From 9e243493fbcb89c62abee2579251d27adc20d45d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 11 Aug 2026 05:50:12 -0400 Subject: [PATCH] ui: Central UI density/consistency sweep + Theme 0.4.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 , 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. --- Directory.Packages.props | 2 +- .../2026-08-11-central-ui-cleanup-sweep.md | 292 ++++++++ docs/requirements/Component-CentralUI.md | 10 +- .../Component-ClusterInfrastructure.md | 2 +- docs/requirements/Component-Commons.md | 2 +- docs/requirements/Component-TemplateEngine.md | 2 +- docs/requirements/Component-Transport.md | 17 +- .../Components/Audit/AuditFilterBar.razor | 10 +- .../Components/Audit/AuditResultsGrid.razor | 2 +- .../Audit/AuditResultsGrid.razor.cs | 28 + .../Dialogs/NodeBrowserDialog.razor | 36 +- .../Dialogs/TestBindingsDialog.razor | 14 +- .../Components/Dialogs/TreeRow.razor | 32 +- .../Forms/MxGatewayEndpointEditor.razor | 128 ++-- .../Forms/OpcUaEndpointEditor.razor | 659 +++++++++--------- .../Components/Layout/MainLayout.razor | 9 +- .../Components/Pages/Admin/ApiKeyForm.razor | 8 +- .../Components/Pages/Admin/ApiKeys.razor | 19 +- .../Pages/Admin/LdapMappingForm.razor | 6 +- .../Components/Pages/Admin/LdapMappings.razor | 8 +- .../Components/Pages/Admin/SiteForm.razor | 80 ++- .../Components/Pages/Admin/Sites.razor | 6 +- .../Pages/Audit/ConfigurationAuditLog.razor | 71 +- .../Pages/Audit/ExecutionTreePage.razor | 8 +- .../Components/Pages/Dashboard.razor | 1 - .../Pages/Deployment/CreateAreaDialog.razor | 6 +- .../Pages/Deployment/Deployments.razor | 19 +- .../Pages/Deployment/InstanceConfigure.razor | 127 ++-- .../Pages/Deployment/InstanceCreate.razor | 6 +- .../Pages/Deployment/MoveAreaDialog.razor | 6 +- .../Pages/Deployment/MoveInstanceDialog.razor | 6 +- .../Pages/Deployment/Topology.razor | 17 +- .../Pages/Design/ApiMethodForm.razor | 162 +++-- .../Pages/Design/ConnectionCertificates.razor | 8 +- .../Pages/Design/DataConnectionForm.razor | 53 +- .../Pages/Design/DbConnectionForm.razor | 4 +- .../Pages/Design/ExternalSystemForm.razor | 4 +- .../Pages/Design/SchemaLibrary.razor | 23 +- .../Pages/Design/SharedScriptForm.razor | 24 +- .../Pages/Design/TemplateAlarmDialog.razor | 6 +- .../Design/TemplateAttributeDialog.razor | 6 +- .../Pages/Design/TemplateCreate.razor | 4 +- .../Pages/Design/TemplateEdit.razor | 190 ++--- .../TemplateNativeAlarmSourceDialog.razor | 6 +- .../Pages/Design/TemplateScriptDialog.razor | 6 +- .../Components/Pages/Design/Templates.razor | 16 +- .../Pages/Design/TransportExport.razor | 43 +- .../Pages/Design/TransportImport.razor | 44 +- .../Pages/Monitoring/AlarmSummary.razor | 9 +- .../Pages/Monitoring/EventLogs.razor | 33 +- .../Components/Pages/Monitoring/Health.razor | 571 ++++++++------- .../Pages/Monitoring/ParkedMessages.razor | 97 ++- .../Notifications/NotificationListForm.razor | 7 +- .../Notifications/NotificationLists.razor | 56 +- .../Notifications/NotificationReport.razor | 90 ++- .../Notifications/SmsConfiguration.razor | 12 +- .../Notifications/SmtpConfiguration.razor | 33 +- .../Pages/Operations/SecuredWrites.razor | 48 +- .../Pages/SiteCalls/SiteCallsReport.razor | 73 +- .../Pages/SiteCalls/SiteCallsReport.razor.cs | 24 +- .../Shared/AlarmTriggerEditor.razor | 9 +- .../Components/Shared/ComposeIntoDialog.razor | 8 +- .../Components/Shared/DialogHost.razor | 14 +- .../Components/Shared/MonacoEditor.razor | 22 +- .../Shared/MoveDataConnectionDialog.razor | 8 +- .../Components/Shared/MoveFolderDialog.razor | 6 +- .../Shared/MoveTemplateDialog.razor | 6 +- .../Shared/RenameFolderDialog.razor | 8 +- .../Components/Shared/SchemaBuilder.razor | 2 +- .../wwwroot/css/site.css | 63 +- .../Admin/LdapMappingCrudTests.cs | 10 +- .../Design/M9SurfaceTests.cs | 2 +- .../Design/TemplateCrudTests.cs | 15 +- .../SiteCrudTests.cs | 8 +- .../NodeBrowserDialogSearchTests.cs | 2 +- .../NodeBrowserDialogSelectionTests.cs | 8 +- .../Pages/Design/TransportExportPageTests.cs | 7 +- 77 files changed, 2197 insertions(+), 1292 deletions(-) create mode 100644 docs/plans/2026-08-11-central-ui-cleanup-sweep.md 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 `