dohertj2/scadaproj
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
main
scadaproj/ZB.MOM.WW.Theme/themeissues.md
T
Joseph Doherty e5a609be83 docs(theme): mark themeissues #6 resolved in 0.3.1 
Interactive-render nav fix (CSS display:none-when-closed + nav-state.js
MutationObserver re-wire) shipped in 0.3.1 and verified — ScadaBridge Central UI
NavCollapseTests now pass. All six issues now resolved (5 fixed, 1 tradeoff).
2026-06-05 08:32:03 -04:00

20 KiB
Raw Permalink Blame History

ZB.MOM.WW.Theme — Known Issues

Issues found in the ZB.MOM.WW.Theme kit that are best fixed once in the kit and re-distributed to every consuming app, rather than worked around per-app. Found while debugging the ScadaBridge Central UI Playwright suite against kit version 0.2.1 (the version ScadaBridge consumed at the time).

All file references below point at the kit source under src/ZB.MOM.WW.Theme/.

RESOLVED in kit 0.3.0 (2026-06-05). Issues 1, 2, 3, and 5 are fixed in the kit and redistributed; Issue 4 is an accepted, documented tradeoff (no code change). See Resolution below for what changed and why.

RESOLVED in kit 0.3.1 (2026-06-05). Issue 6 (collapsible nav non-functional under interactive Blazor render) is fixed — CSS display:none-when-closed backstop + MutationObserver re-wire in nav-state.js. See the Issue 6 resolution note. All six issues are now resolved (5 fixed, 1 accepted tradeoff).

Summary

# Severity Component Issue Status (0.3.0)
1 Medium NavRailSection / nav-state.js No programmatic expanded-state hook (aria-expanded / data-*) on the section toggle. ✅ Fixed
2 Medium nav-state.js The section containing the active link is not auto-expanded on navigation. ✅ Fixed
3 Medium nav-state.js Persistence wires once on DOMContentLoaded; not re-applied after Blazor enhanced navigation / dynamic re-render. ✅ Fixed
4 Low NavRailSection Always-expanded SSR default causes a flash / layout shift of collapsed sections on load. 📄 Accepted tradeoff (documented)
5 Low (optional) LoginCard Heading bakes the localizable — sign in suffix into the product title with no separate hook. ✅ Fixed
6 High NavRailSection / nav-state.js Under interactive Blazor render mode the whole collapsible nav is non-functional: clicking a header doesn't hide items, and nav-state.js never wires (no aria sync, no persistence, no active-reveal). ✅ Fixed (0.3.1)

Resolution (kit 0.3.0)

Shipped in ZB.MOM.WW.Theme 0.3.0 (2026-06-05) and adopted across all three apps.

  • Issue 1 — aria-expanded hook. NavRailSection.razor now renders <summary class="rail-eyebrow-toggle" aria-expanded="…">, computed from Expanded at SSR time, and nav-state.js keeps it in sync with the native <details open> state on restore and on every toggle. Tests/AT can now await a stable attribute instead of inferring from child-link visibility. (bUnit: NavRailSection_summary_aria_expanded_true_when_open / …_false_when_collapsed.)
  • Issue 2 — auto-expand the active section. After restoring saved state, nav-state.js force-opens any details.rail-section that contains an a.rail-link.active. The reveal is transient — it is flagged with data-zbnav-transient before the open flip so the toggle handler skips persistence and the user's saved collapse preference is preserved.
  • Issue 3 — re-apply after enhanced navigation. apply() is now also bound to Blazor's enhancedload event (Blazor.addEventListener('enhancedload', apply)); the per-element data-zbnav-initialized guard keeps re-runs idempotent. Static-SSR consumers keep persistence + active-reveal after enhanced navigations; interactive Server consumers (e.g. ScadaBridge Central UI) are unaffected as before.
  • Issue 4 — SSR flash / CLS: accepted tradeoff (no code change). The kit deliberately keeps client-only persistence to stay render-mode-agnostic, so the server renders every section open and JS collapses the saved-collapsed ones after first paint. The alternative — an inline pre-paint <head> snippet that mutates not-yet-parsed <details> from localStorage — adds a FOUC-script that runs against DOM that does not yet exist, for a Low-severity cosmetic flash. We chose not to take on that complexity/risk. Consumers who care about the flash for a specific layout can add their own pre-paint restore; the kit will not ship one by default. (This paragraph is the documented decision the issue asks for.)
  • Issue 5 — LoginCard heading hook. The product token is now wrapped in <span class="login-product">@Product</span> — sign in, and an optional Heading parameter fully replaces the heading copy when set (for localization / custom wording). Existing "<Product> — sign in" assertions still pass. (bUnit: Product_is_wrapped_in_login_product_span / Heading_overrides_default_heading_when_set.)

Issue 1 — No programmatic expanded-state hook on NavRailSection

Severity: Medium · Files: Components/NavRailSection.razor, wwwroot/js/nav-state.js

Symptom. A section's open/closed state is exposed only through the native <details open> boolean attribute on the <details class="rail-section"> element. The <summary class="rail-eyebrow-toggle"> toggle carries no aria-expanded and there is no data-* state attribute. E2E tests (and some older assistive tech) cannot reliably query or await the expanded state — they must infer it from child-link visibility.

Root cause. NavRailSection.razor renders:

<details class="rail-section" open="@Expanded" data-nav-key="@ResolvedKey">
    <summary class="rail-eyebrow-toggle">@Title</summary>
    <div class="rail-section-body">@ChildContent</div>
</details>

There is no attribute that mirrors open in a test- or AT-stable way.

Impact on consumers. Every consumer's UI tests must assert collapse state indirectly (e.g. waiting on a child link to become visible/hidden) instead of awaiting a stable attribute. This was the proximate cause of several stale ScadaBridge nav tests. Native <details>/<summary> is keyboard- and screen-reader-accessible by default, so this is primarily a testability gap (with a modest a11y upside).

Recommended fix. Mirror open onto aria-expanded on the <summary>, kept in sync by nav-state.js (set it during apply() from el.open, and update it inside the existing toggle listener). This gives both tests and AT a stable, queryable attribute without changing the CSS-only collapse mechanism.

Verify. After the fix, summary.rail-eyebrow-toggle exposes aria-expanded="true|false" that flips when the section is toggled and after a reload restores saved state.

Issue 2 — Active section is not auto-expanded on navigation

Severity: Medium · File: wwwroot/js/nav-state.js

Symptom. When a section is collapsed (either because the user previously collapsed it — localStorage zbnav:<key> = "0" — or because a consumer sets Expanded="false") and the user navigates to a route whose link lives inside that section, the section stays collapsed. The active link (a.rail-link.active) is present in the DOM but hidden by the closed <details>, so the nav no longer shows the user where they are.

Root cause. nav-state.js only restores saved open/closed state; it has no concept of the active link. Grep of the kit confirms the only "active" handling is the .rail-link.active CSS rule in wwwroot/css/layout.css — there is no JS that opens the section containing the active link.

Impact on consumers. Loss of the common "navigating into a section reveals it" behavior. A user who collapses a section and then deep-links (or is redirected) into one of its pages lands with the relevant nav group collapsed and the current page's link hidden. (ScadaBridge previously had app-owned auto-expand-on-navigate; the kit cutover dropped it, and the NavigatingIntoCollapsedSection_AutoExpandsIt test now fails because nothing re-expands.)

Recommended fix. In nav-state.js, after restoring saved state, force-open any section that contains the active link:

// after the saved-state restore loop, before wiring is "done":
document.querySelectorAll("details.rail-section a.rail-link.active").forEach(function (link) {
    var sec = link.closest("details.rail-section");
    if (sec && !sec.open) sec.open = true;   // reveal the section the user is in
});

Run this both on initial load and after Blazor navigation (see Issue 3). Decide whether the forced-open should also persist to localStorage or be a transient reveal (recommended: transient — don't overwrite the user's saved preference).

Verify. Collapse a section, navigate to one of its pages (or reload directly on it): the section opens and the active link is visible.

Issue 3 — Persistence wires once and is not re-applied after navigation

Severity: Medium · File: wwwroot/js/nav-state.js

Symptom. apply() runs only on the initial DOMContentLoaded (or first script eval) and guards each element with data-zbnav-initialized. Under Blazor static SSR enhanced navigation — or any dynamic re-render that replaces the <details> nodes — newly inserted sections are never wired: their saved state is not restored and their toggles are not persisted. The active-section logic from Issue 2 would likewise not re-run.

Root cause. The script self-invokes once:

if (document.readyState === "loading")
    document.addEventListener("DOMContentLoaded", apply);
else
    apply();

There is no hook for Blazor's enhanced-navigation lifecycle and no MutationObserver.

Impact on consumers. Static-SSR consumers (the kit explicitly targets "works in static SSR") lose nav persistence after the first enhanced navigation. Interactive Blazor Server consumers (such as ScadaBridge Central UI) are largely unaffected, because the rail is prerendered once and then patched in place over the SignalR circuit, so the original <details> elements and their listeners survive — which is why ScadaBridge's persistence appears to work today. The kit should still be correct for its static-SSR audience.

Recommended fix. Also re-run apply() on Blazor's enhanced-load event (and keep the per-element init guard so it stays idempotent):

if (window.Blazor && Blazor.addEventListener) {
    Blazor.addEventListener('enhancedload', apply);
}

Optionally add a MutationObserver on the rail container as a framework-agnostic backstop.

Verify. On a static-SSR host, expand/collapse a section, perform an enhanced navigation to another page and back, and confirm the saved state is still restored and toggles still persist.

Issue 4 — Always-expanded SSR default flashes / shifts layout on load

Severity: Low · File: Components/NavRailSection.razor

Symptom. NavRailSection.Expanded defaults to true, so every section renders open in the server HTML. nav-state.js only collapses the saved-collapsed sections after the script runs, producing a brief flash of fully-expanded nav and a layout shift (CLS) on each load for users who keep sections collapsed.

Root cause. State lives in localStorage and is applied by JS post-render, while the server-rendered default is unconditionally expanded. The server has no knowledge of the saved state at render time.

Impact on consumers. Cosmetic flash / minor CLS on initial load; more noticeable with many sections collapsed.

Recommended fix (pick one).

  • Inline a tiny restore snippet in <head> (via ThemeHead) that sets each <details>'s open from localStorage before first paint; or
  • Accept the tradeoff and document it (the kit deliberately chose client-only persistence to stay render-mode-agnostic).

Verify. With several sections saved-collapsed, reload and confirm no expanded-then-collapse flash.

Issue 5 — LoginCard heading couples the product name and "— sign in" (optional)

Severity: Low (optional) · File: Components/LoginCard.razor

Symptom. The card heading is <h1 class="login-title">@Product &mdash; sign in</h1>. The (localizable) — sign in suffix is baked into the product title with no separate hook, so consumers can't restyle/override the heading copy or assert the product token in isolation without string-matching the whole heading.

Impact on consumers. Minor: per-app heading customization and exact-text test assertions are awkward (must match "<Product> — sign in" rather than the product alone).

Recommended fix (optional). Wrap the product in a span and/or expose an override:

<h1 class="login-title"><span class="login-product">@Product</span> &mdash; sign in</h1>

or add an optional Heading parameter that, when set, replaces the default heading entirely.

Issue 6 — Collapsible nav is non-functional under interactive Blazor render mode

Severity: High · Files: Components/NavRailSection.razor, wwwroot/js/nav-state.js, wwwroot/css/layout.css · Status: ✅ Fixed in 0.3.1

Resolution (kit 0.3.1, 2026-06-05). Both recommended parts shipped, so the collapsible nav now works under interactive render modes as well as static SSR:

  1. CSS robust collapse — layout.css hides the body explicitly when closed instead of relying on the native ::details-content content-hiding (which an interactive framework desyncs): .rail-section:not([open]) > .rail-section-body { display: none; }.
  2. Render-mode-agnostic re-wire — nav-state.js adds a MutationObserver on document.documentElement (childList + subtree) that re-runs apply() whenever details.rail-section nodes are added/replaced, so the interactive runtime's re-render gets wired (aria sync, data-zbnav-initialized, localStorage persistence, active-reveal). The existing enhancedload hook (Issue 3) is kept for static-SSR enhanced navigation.

Verified live in ScadaBridge Central UI (global @rendermode InteractiveServer): the Playwright NavCollapseTests (toggle-hides-items, persistence-survives-reload, deep-link-auto-reveal) now pass against 0.3.1.

This corrects Issue 3's note, which claimed interactive Blazor Server consumers are "largely unaffected because the rail is patched in place." Direct observation of the live ScadaBridge Central UI (global @rendermode InteractiveServer) shows that is false — the kit's <details>/JS nav does not work under interactive render modes at all.

Symptom. In an app that renders the rail under an interactive render mode (InteractiveServer, InteractiveWebAssembly, or InteractiveAuto), the collapsible nav is visually and functionally dead:

  1. Clicking a section header toggles the chevron (▶/▼) but does not hide the section's items — the links stay fully visible under a "collapsed" chevron.
  2. aria-expanded never changes, localStorage is never written, the saved state is not restored on reload, and the active-section auto-reveal (Issue 2) does not fire.

Root cause. The kit nav is a static-SSR / CSS-only design (NavRailSection's own comment: "works in static SSR"). Under an interactive render mode, Blazor's runtime owns and re-renders the <details>/<summary> DOM after it adopts the prerendered markup. Two independent consequences, both observed live:

  • Native collapse is defeated. On the live page a closed section has details.open === false and its ::details-content computes content-visibility: hidden, yet the .rail-section-body and its links remain laid out and visible (measured non-zero height / non-null offsetParent). Blazor's management of the native <details> desyncs the browser's built-in content-hiding. The body's display value (flex/block/grid/inline) makes no difference — only an explicit display: none actually hides it.
  • nav-state.js never wires the live DOM. The interactive <details> elements have no data-zbnav-initialized attribute, i.e. wire() never ran on them: apply() runs on DOMContentLoaded against the prerendered nodes, which Blazor then replaces, and the only re-run hook (enhancedload, added for Issue 3) does not fire under interactive render modes. So aria sync, localStorage persistence, and active-reveal are all inert.

This matters for the kit's stated goal: per the normalization notes, nav-expand persistence was promoted into the kit at 0.2.0 "so all three apps share one persistence mechanism." One of the three consumers (ScadaBridge Central UI) is interactive Blazor Server, where that mechanism silently does nothing.

Verified evidence (live, global InteractiveServer). On a logged-in dashboard: data-zbnav-initialized absent on every details.rail-section; after clicking a header, details.open === false but the section's link still reports clientHeight: 33 and a non-null offsetParent; setting .rail-section-body { display:none } is the only thing that hides it; localStorage has no zbnav:* keys before or after toggling.

Recommended fix (two parts — both belong in the kit).

  1. Make the collapse render-mode-robust (CSS). Don't rely solely on the native <details> content-hiding; hide the body explicitly when closed:

    /* layout.css — robust across render modes; native ::details-content hiding
   is unreliable once an interactive framework manages the <details>. */
.rail-section:not([open]) > .rail-section-body { display: none; }

    (Verified live: this is exactly what hides the items.)

  2. Make persistence/aria/reveal work under interactive render. enhancedload is static-SSR-only; also wire after the interactive runtime has (re)rendered. Options, in preference order:

    • Re-run apply() from Blazor's post-render hooks — Blazor.addEventListener('afterStarted', …) (interactive WASM/Server boot) and re-apply on circuit/render updates; and/or
    • Add a MutationObserver on the rail container that calls apply() when details.rail-section nodes are added/replaced (framework-agnostic backstop — covers interactive re-renders, enhanced nav, and dynamic nav alike);
    • Or ship an explicitly interactive NavRailSection variant (a small Blazor component with an @onclick toggle and [Parameter] bool Expanded two-way state) for consumers that render interactively — which is what NavRailSection's own comment already gestures at ("Apps that want cookie-persisted expand state keep their own interactive NavSection"). If the kit's intent is that interactive apps bring their own section component, say so loudly in the docs and have the CSS-only one degrade gracefully (part 1 still applies so at least the visual collapse works).

Verify. In an interactive-render host: clicking a header hides the section's items; the summary's aria-expanded flips; localStorage gets a zbnav:<key> entry; the state survives a reload; and deep-linking into a collapsed section reveals it.

Consumer note (ScadaBridge). Resolved on 0.3.1: ScadaBridge's Central UI consumes ZB.MOM.WW.Theme 0.3.1, and the Playwright NavCollapseTests (toggling, persistence, auto-reveal) now pass — the NavCollapseWiredAsync gate (which waits for data-zbnav-initialized on every details.rail-section) is satisfied under interactive render, so those tests run unskipped and green.

Not kit bugs — expected consumer adaptations

For the avoidance of doubt, the following are not theme issues; they are the normal cost of adopting the kit and belong in each consumer's own tests/markup:

  • Login markup moved from a hand-rolled <h4>ScadaBridge</h4> + Sign In button to the kit's LoginCard (h1.login-title reading "<Product> — sign in", button labelled Sign in). Consumers must update selectors/text accordingly.
  • Nav moved from app-owned button.nav-section-toggle + aria-expanded + a scadabridge_nav cookie to the kit's <details.rail-section> + <summary> + localStorage (zbnav:<key>). Collapsed sections now keep their children in the DOM (hidden), and sections default to expanded, not collapsed — so DOM-presence-based "hidden" assertions and "collapsed by default" assumptions must be rewritten around visibility and the <details open> state.

These are being handled in the ScadaBridge Playwright suite separately.

Reference in New Issue View Git Blame Copy Permalink