92 lines
19 KiB
Markdown
92 lines
19 KiB
Markdown
# ScadaBridge UI consistency — first batch review
|
||
|
||
Baseline accepted G1 pilot commit: `cdd3adf6`; Theme 1.0.0 published exact package. This batch follows parent G1 acceptance; it does not change package pins, policies, validators, UTC handling, lazy loading or fleet services. Secrets.Ui adoption awaits SEC07 release. No deploys or live writes.
|
||
|
||
## Scope grouped for review
|
||
|
||
`evidence/sb01-03/source-groups.json` lists every changed path; `source-freeze-sha256.json` records the frozen review source. No SB04+ work started.
|
||
|
||
- **Dialog naming/lifecycle:** DialogHost title/body/prompt ID relationships; AuditDrilldownDrawer modal/name/Escape; native `dialog.js` per-overlay Map state, actual-event synchronous Tab prevention, filtered focusable children, reference-counted inert siblings, scroll-lock ownership, detached-node cleanup, connected-trigger or heading fallback; component disposal invokes deactivation. Existing Confirm/Prompt/Show result semantics and drawer OnClose are preserved. Generic extraction waits for a second consumer.
|
||
- **Label/ID associations:** 126 previously standalone labels associated with native controls across28 files, IDs scoped to component instances; repeated alarm-band keys include the band key. Mutually exclusive numeric/text/Boolean match controls share one relationship safely. Audit multi-select names combine visible field label and current summary; shared table search has a visible label and unique ID. Remaining contextual searches/table cells/move controls receive explicit names without changing bindings/callbacks/conditions.
|
||
- **Intentional caption conversions:** `caption-conversions.json` enumerates files. Composite schema/editor sections, result payloads, console output, button-group captions and list headings use caption divs instead of labels targeting no labelable element. Existing layout classes and content are retained; no output or parameter behavior changed.
|
||
- **Validation relationships:** OPC/Mx existing FieldError selection logic is retained and factored into a helper. Only fields already mapped by RenderFieldError get conditional aria-invalid, describedby and an error ID/alert. Successful removal of Errors removes invalidity and stale description. No new validation rule or validation bypass.
|
||
- **Tests:** existing dialog/drawer/OPC suites extended; new ControlLabelTests checks table search after typing. Strict clipboard test explicitly configures the new lifecycle interop and retains its clipboard payload assertions; no test mode weakening/skips.
|
||
|
||
## Test evidence
|
||
|
||
`red-dialogs.log`: two intended missing-name/modal failures. `red-labels.log` is retained as an invalid initial test fixture (required RowContent omitted); corrected fixture `red-labels-corrected-fixture.log` fails for the intended missing input ID. `red-validation.log` retains a compile error from a duplicate attribute found during the broad scan, then corrected; `red-validation-corrected.log` shows the intended missing aria-invalid failure. Duplicate ParkedMessages attribute was removed; its existing contextual accessible label remains unchanged.
|
||
|
||
Focused related suite: `green-batch.log`104 passed. Intermediate full suite: `full-suite.log`1554 passed/zero skipped before the last validation regression and source naming completion. Final frozen-source suite passed **1555/1555**, zero failures/skips (`final-suite.log`, `sb01-03-final.trx`).
|
||
|
||
## Browser pilot and remaining gates
|
||
|
||
Disposable new fixture source is retained in umbrella `components/ui-theme/validation/scadabridge-ui/host`; actual source RCL ProjectReference, anonymous/synthetic local services only. It binds127.0.0.1:5201, separate from retained G1 canary5199. Existing rigs untouched. `infra/tools/ui_consistency_browser.py` accepts explicit loopback base URL/storage-state/output directory; no real writes.
|
||
|
||
First matrix and one confirmation are retained separately under `browser/` and `browser-confirmation/`. Both passed name/prompt linkage, synchronous first/last Tab and Shift+Tab, hidden/disabled filtering, inert background, Escape, normal trigger focus return, lock cleanup, repeat opening and no-focusable containment at1440/390. **Disconnected-trigger fallback failed**: the native code focused the heading, then immediately removing its temporary tabindex made Chromium blur to BODY. Source now restores that attribute only after blur. No third broad screenshot/polish loop is claimed; A focused no-screenshot technical probe (`focused-fallback.json`) now confirms all three actual hosts restore the disconnected trigger to h1, retain tabindex while focused, clear it on blur, and release locks/inert. Parent lifecycle review remains required.
|
||
|
||
SB03 source inventory does not prove every composite editor's internal accessible name or field help/error relationship at runtime. Remaining work before calling SB03 complete: machine-check frozen associations, contextual duplicate IDs on repeated editors, all existing form help relationships, Monaco composite context, and rendered browser/assistive-tree samples for transport/notification/instance forms. Aggregate form errors must stay aggregate rather than falsely marking all fields invalid. These are explicit pending acceptance items, not covered by passing unit tests.
|
||
|
||
## Lifecycle review refinement
|
||
|
||
The supported policy is **one active overlay**, including the audit drawer and global DialogHost. No arbitrary nested or sibling overlay support is implied by the internal Map/ref-count bookkeeping. Native activation rejects any different active overlay or inert target. C# cancels a rejected service dialog through its unchanged typed cancellation path, or invokes the drawer's existing OnClose callback. DialogService's original second-dialog guard is retained. This prevents descendant Escape bubbling from closing two simultaneously active hosts.
|
||
|
||
A disposed DialogHost unsubscribes, cancels any pending local result, and deactivates its presentation. No new busy-operation semantics are introduced: confirm merely resolves the caller's result; domain work remains owned by the caller after the dialog closes. The adapter does not cancel or retry domain work. Source scan found no remaining callers of removed captureActiveElement/restoreActiveElement/focusTrap globals in src/tests. Browser evidence for the single-active rejection is `single-active-overlay.json`; it programmatically attempts a second local presentation while the real drawer is active, verifies cancellation/one remaining drawer, then verifies one Escape closes it and restores its trigger.
|
||
|
||
Final lifecycle-refined full suite: **1556/1556 passed**, zero skips/failures, `reviewed-suite.log` / `sb01-03-reviewed.trx`. This supersedes1555 for the additional rejection/disposal regression. No first-batch commit until parent source/lifecycle review accepts the frozen diff. Rollback reverts this batch only; accepted G1 package/palette pilot is separate.
|
||
|
||
## Peer finding closure and shared patch adoption
|
||
|
||
Native focus restoration now verifies document.activeElement after attempting the trigger; still-connected disabled/hidden/unfocusable triggers use heading fallback too. `unusable-trigger-red.json` preserves the reproduced disabled/hidden failures, `unusable-trigger-green.json` verifies all four target conditions and clean lock/inert/tabindex restoration. The retained Python test includes a span made unfocusable while still connected, not merely a replacement/disconnection.
|
||
|
||
Published Theme1.0.1 is restored from the official Gitea index into task-local `/tmp/scadabridge-theme101-packages`. Package SHA256995b117b0245577598c91404ed7957c071ebe3413696c8be48b419c9deb1bd5d and runtime DLL parity are in theme101-provenance.json; no stale default cache accepted. Full suite on the patch:1557/1557 passed, zero skips/failures. The added contextual TreeView checkbox regression remains SB03 work; its red failure is retained.
|
||
|
||
## SB03 completed source and bounded rendered acceptance (pending parent review)
|
||
|
||
Accepted dialog source is isolated in87c882ec; published patch pin is20885b61. Remaining SB03 source stays separate. The generic tree checkbox name was replaced with aria-labelledby targeting that row's visible NodeContent; per-tree scope+encoded key makes names unique for repeated trees. The regression renders two trees and resolves each checkbox's unique name target. Selection/keyboard keys are untouched.
|
||
|
||
Repeated ParameterValueForm instances now use unique encoded field IDs, keep visible primitive labels, name composite groups, name list items by canonical path, and link only their existing parse-error map. A red duplicate-ID regression and linked-error correction test passed. FlowDebugger tests' hard-coded param-* ID selectors were migrated to data-param-path while preserving every typed-payload, state, poll, validity and live/replay assertion. The first full integration run failed28 tests on old selectors; it is retained as sb03-final-suite.log. After selector-only corrections all1559 passed.
|
||
|
||
Monaco receives an additive AriaLabel creation option and updates it through existing setEditorOption when changed. All seven consumer callsites supply domain context, including parameter JSON paths. Its actual native accessible textbox (role=textbox) reports the supplied name. The hidden readonly IME textarea is intentionally aria-hidden/tabindex=-1; first rendered detector incorrectly included it, first-results.json preserves that false-positive failure. Corrected rendered results exclude aria-hidden nodes and confirm the actual textbox name. No app UI change was made to fix that measurement.
|
||
|
||
form-help-coverage.json inventories all46 form-text blocks:42 linked field/group descriptions/errors,4 informational notes (log metadata, recompile action explanation, no-running-instance and no-connection empty states). Conditional help/error IDs are emitted only with matching content. Existing OPC/Mx/ServiceLevel validator field matching and parameter parse-error behavior are preserved. Transport passphrase mismatch now announces its existing error and clears describedby/invalid on correction; its existing export test still verifies selection/user identity/export payload. API key/method checkbox groups get group-level guidance, without changing any policy or selected collection. Aggregate errors remain aggregate; fields are not falsely marked invalid by unrelated submission failures.
|
||
|
||
Rendered matrix: actual SMS and SMTP form components plus repeated Mx/parameter/tree and actual Monaco components at1440/390, six cases pass (forms-browser/results.json, six screenshots). No duplicate IDs, unresolved for/describedby targets, unnamed exposed native fields, or browser errors in those samples. Visual inspection confirms labels/forms retain compact incumbent layout. This directly composed synthetic fixture is not authenticated application route coverage; repository reads are empty and every non-Get proxy operation rejects. No save/test-send/verify/trust/deploy was clicked.
|
||
|
||
Source-backed inventory plus rendered representative samples and the full integration suite establish this batch's acceptance; they do not imply an assistive-technology certification or complete deployed role matrix. SB00's full-route baseline and Secrets package adoption remain separate pending gates. Frozen source hashes are sb03-source-freeze.json. No SB04+ implementation yet.
|
||
|
||
### Final SB03 peer correction
|
||
|
||
Parent review found the raw-JSON escape-hatch error was not described by its composite host. The existing parse-error state now sets invalidity/description on the named `.param-json-editor` group; Monaco retains its domain-specific textbox name. The invalid JSON → valid JSON correction regression fails for the missing role/relationship first, then passes with related ParameterValueForm/Monaco/TreeView tests (`raw-json-association-red.log`, `raw-json-association-green.log`). No validator/value projection change. Last full frozen suite1559/1559 is retained; only this narrow relationship and one regression followed it, with parent approval to use focused verification rather than rerun all tests.
|
||
|
||
### Corrective fixture commit after premature SB03 commit
|
||
|
||
Commit915a8a59 was created before the author checked the focused result carefully. Its raw-json-association-green.log actually contains98 passes/1 failure and remains unchanged as historical evidence. The fixture incorrectly expected valid JSON object syntax to erase the existing unresolved-schema error. This was a test-premise error; production validation must retain that error.
|
||
|
||
The corrected regression now asserts invalid syntax is linked, valid object syntax still retains the unknown-declared-type error, and then valid JSON null clears invalidity under InboundApiSchema's existing explicit present-null-is-allowed contract. All prior relationship/domain-name assertions remain. No production source or validator changed in this correction. Focused ParameterValueForm/Monaco/TreeView result:99/99 passed, zero skips/failures (raw-json-association-corrected-green.log). Parent accepted focused verification after the prior1559 full baseline. This paragraph supersedes the premature green claim above; it does not rewrite the red evidence.
|
||
|
||
## SB04–06 review batch
|
||
|
||
SB04's UTC-labelled native inputs now belong to AuditFilterBar. The reusable DateTimeRangeFilter remains unchanged: its DateTime? callback intentionally cannot distinguish invalid raw text from an empty bound. The parent approved using its same native two-field geometry/labels locally so the audit caller can keep correction strings. Valid literals are tagged UTC without browser/server-zone conversion, including DST-date literals. Intentional nullable bounds remain supported; invalid/inverted input returns before OnFilterChanged, preserves strings, and links a recovery error. The query model, presets, filters and backend remain unchanged. New error uses shared field-error s-bad (#b42121), not a local color override. Clear resets raw strings/error together with existing model reset.
|
||
|
||
SB05 dashboard facts settle independently after each read with individual loading/known-zero/unavailable state. Repository reads remain sequential because some adapters share one scoped DbContext. A failed read does not block later reads or replace available facts. Each named region's Retry calls only its read. No domain service, aggregation, polling or authorization change. SB08/SB10/SB11 still own dashboard hierarchy, destination policy and operational-summary changes.
|
||
|
||
SB06 adds contextual Dismiss [title] names to toast close controls and preserves timing/live-region/disposal behavior. Two-toast test proves one dismissal preserves the other message.
|
||
|
||
Evidence sb04-06: red-utc-toast.log6 intended failures; red-dashboard.log6 intended failures. Full Release suite1575/1575 passed, zero skips/failures (full-suite.log/sb04-06.trx). Final audit tests20/20 pass under UTC, America/Indianapolis and Asia/Tokyo; custom, DST, invalid/inverted, correction, nullable and existing preset/filter cases remain green. Only a shared semantic error class and stronger false-zero assertion followed the full run; final-focused.log covers the final audit/dashboard/toast state.
|
||
|
||
First8-case browser matrix retained in browser/results.json. UTC literals/inverted guards passed all4 cases (New_York/Tokyo browser zones ×1440/390). Immediate dashboard retry and one toast count were read before InteractiveServer completion and reported false. The retained confirmation adds outcome-based waits; no source state logic changed for these measurement failures. An intermediate harness quoting typo is retained as confirmation-harness-error.log. Final bounded confirmation8/8 passes (browser-confirmation/results.json): same UTC boundaries, input text retained on inverted range, partial-dashboard Unavailable→known-zero retry with successful facts retained, and named target-only dismissal. All captured dashboard/toast document widths equal viewport width; visual inspection confirms existing density and hierarchy are retained for later SB08 changes.
|
||
|
||
Fixture changes under umbrella validation/scadabridge-ui/host supply only empty read projections and one deliberate first API-key-read failure. All non-read operations reject; no Save/test-send/verify/trust/deploy was performed. Direct actual component composition does not establish application authentication or live backend behavior. Original G1 evidence is unchanged.
|
||
|
||
This batch's production scope is4 Razor/C# source files (AuditFilterBar pair, Dashboard, ToastNotification) plus3 test files; frozen hashes are sb04-06/source-freeze.json. No SB07+ source changes or batch commit before review.
|
||
|
||
### SB04–06 concrete peer corrections
|
||
|
||
The original bUnit invalid-string test did not prove native partial-date behavior. Actual Chromium keyboard entry reproduced value empty + badInput true and the prior onclick Apply emitted an unbounded filter (native-partial-before.json). AuditFilterBar now owns one native form; its only production callsite in AuditLogPage has no surrounding form. Apply submits, Clear is explicitly type=button, and native constraint validation blocks incomplete values before the Blazor submit handler. Tests submit the form instead of directly invoking a button click; all existing filter assertions remain. The input uses step=any to avoid unintentionally rejecting valid seconds. Intentionally cleared optional bounds remain accepted.
|
||
|
||
The native after proof uses actual segmented keyboard entry, not programmatic invalid value assignment: partial input is blocked, focus returns to From, then a genuinely cleared optional bound submits null. Explicit aria-invalid=false initially masked native badInput in Chromium AX. The caller now emits aria-invalid=true only for its own range errors and otherwise omits it; native AX reports invalid=true for a partial native date. Before/after AX proof is retained with native-partial-after.json and native-partial-aria-before.json. No JS validity workaround.
|
||
|
||
Dashboard initial reads alone were sequential, but an early Retry could overlap the next repository read. All initial/retry operations now share a component-owned async gate. The renderer claims each fact's pending state before its first await, coalescing duplicate target requests. A deterministic delayed-Connections/early-Sites-retry regression reproduces the prior3 Sites invocations before Connections completed (expected1); final code waits and makes exactly one recovery read. Counts remain independent, and successful facts are not reloaded. The first async test attempt had CS4014 in NSubstitute verification; compile output is preserved separately, then meaningful red/green evidence follows.
|
||
|
||
Reviewed full Release suite1576/1576 passed, zero skips/failures (reviewed-full-suite.log/sb04-06-reviewed.trx). The only following production change was omitting the false ARIA override; final focused31/31 and final20 audit cases in each of UTC/Indianapolis/Tokyo pass. Native Chromium focused after proof passes including AX invalid=true. These correction checks are narrow technical regressions, not another broad screenshot/polish round. No batch commit or SB07+ expansion until parent accepts the corrected frozen source.
|