Blazor layout shell and theming foundation (task 86) #23
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/webui-shell"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Header/nav/main layout replacing the template's sidebar, and a theming foundation of CSS custom properties on
:root— no framework. Recorded in ADR-0027. The hosting model needed no new record: ADR-0008 already fixes Blazor WebAssembly.Bootstrap is gone (44 files), along with the sidebar, the nav toggle, the About link and every Bootstrap-dependent rule in
app.css. Dark mode exists as the cheapest proof the token layer is real: it rebinds the custom properties and nothing else, so a rule needing its own dark variant means a value has escaped. That no-literals rule is review-only — nothing enforces it, as with British English under ADR-0012.Nav entries whose page is a later ticket render as dimmed
<span aria-disabled="true">rather than anchors, so the shell never offers a link that lands on not-found.Needs a decision:
/placesand/groups. Nothing in the backlog fixes them; a different information architecture is a one-line change inNavMenu.razor.cs.Worth knowing: Blazor's CSS isolation does not stamp its scope attribute on the anchor
NavLinkrenders, so the link rules matched the placeholder spans and nothing else. Caught only by rendering it in a browser — neither the build nor a unit test could have. The rules now go through::deep.docs/adr/README.mdwill conflict with PRs #20, #21 and #22 — 0027 belongs after 0026.rob referenced this pull request2026-08-03 14:08:57 +00:00
NavMenu.razor.css:23—rgba(255, 255, 255, 0.12)is a literal colour outsidetheme.css, and it hard-codes the assumption that the header is dark: re-theme--colour-headerto anything light and the hover affordance disappears. Add a--colour-header-hovertoken.app.css:88—rgba(0, 0, 0, 0.2)in the error banner'sbox-shadow, same rule. Tokenise it or drop the shadow; theborder-topabove it already separates the banner.MainLayout.razor.css:12—font-size: 1.25remon.brandhas no token. Add one totheme.css.app.css:47–71(8rem,0.6rem,20vh,3.25rem,0.2rem) andMainLayout.razor.css:27(max-width: 30rem) are literal sizes. Media-query conditions cannot read custom properties, so ADR-0027's "declares no literal colour or size of its own" can never hold for breakpoints — amend the ADR to exempt breakpoints and the boot-progress geometry rather than leave a rule that is already false on merge.NavMenu.razor:11—aria-disabledon a<span>reaches no assistive technology: it is not a global ARIA attribute and thegenericrole does not support it. Chromium's accessibility tree exposes Places and Groups as bareStaticTextwith no properties at all, i.e. identical to having omitted the attribute. Either give the entryrole="link" tabindex="0"so the state is exposed and it stays keyboard-reachable, or droparia-disabledand add visually-hidden text ("Places, coming soon") — the second is the smaller change and the only one that conveys why.NavMenu_DestinationWithoutAPage_IsMarkedDisabledForAssistiveTechnologyasserts an attribute that reaches nothing and needs renaming with whatever replaces it.NavMenu.razor.css:39— the dimmed placeholders compute to 4.26:1 against the header in light mode, below the 4.5:1 that 0.9rem text needs (dark mode is 4.81:1 and passes).opacitywould have to reach 0.58; a--colour-header-text-mutedtoken set for contrast in both schemes is the better fix, and removes the last escaped value from a scoped file.NotFound.razor:4—<h3>with no<h1>on the page, soFocusOnNavigate Selector="h1"matches nothing and focus stays on<body>after landing on/nowhere; the page also has no<PageTitle>, so the tab still reads "PlaceMark" on an error. Make it<h1>and add a title.Nothing covers the not-found route — that
/nowhererendersNotFoundinside the layout with no nav entry active is the one routing behaviour the shell exists to provide.Verdict: changes required
All six actioned in
432590f, plus the redundant@layout;origin/mainmerged and 0027 reordered after 0026.Placeholders now take a
--colour-header-text-mutedtoken, measured 5.84:1 light and 5.94:1 dark;aria-disabledreplaced with visually-hidden ", coming soon" (confirmed present and unignored in the AX tree); ADR-0027's colour rule is now absolute and its size rule exempts breakpoints and one-off geometry, both marked in the CSS.NavMenu.razor.css:22— the hover rule matches the placeholder spans, because they carry.navigation-linktoo, and this commit deleted the.navigation-link-unavailable:hover { background-color: transparent }that was cancelling it. With a real pointer over "Places" the span takes#1d5850, the same lifted pill a live link gets, so the entry advertises itself as clickable — the thing rendering it as a non-anchor exists to prevent. It also drops that entry's text contrast from 5.84:1 to 4.22:1 in the light scheme, back under AA, which is the failure this commit was fixing. Narrow the selector toa.navigation-link:hoverrather than reinstating the override, so a future non-anchor entry type cannot reintroduce it. The comment at line 36, "there is nothing to hover or focus", holds for focus only.Verdict: changes required
Fixed in
757cec3by qualifying witharather than reinstating the override, as suggested — reproduced at 4.22:1 light / 4.77:1 dark first, now 5.84/5.94 under the pointer with no background painted..activequalified the same way.Guard is the selector (correct by construction) plus stylesheet tests pinning its shape, since bUnit has no cascade or pointer; audit found one more,
h1:focussuppressing the ring for any focus, nowh1[tabindex="-1"]:focus. Separately:#blazor-error-ui .dismissis a mouse-only<span>the framework wires by class — real, pre-existing, not this ticket.NavMenuStylesheetTestspasses on three plausible spellings of the mistake it exists to catch, because it substring-matches an unparsed selector. Appending the placeholder to the existing rule's selector list —a.navigation-link:hover, .navigation-link-unavailable:hover— reintroduces the regression exactly (pointer on "Places" takes the hover background, 4.22:1 in the light scheme) with all sixteen tests green;:is(a.navigation-link, span.navigation-link):hoverdoes the same; and a hover rule nested in@supportsor@containeris skipped entirely, since only@mediais asserted absent while the doc comment claims no nested at-rule. Split each rule's selector on,and assert per selector, and reject any@block rather than@mediaalone. Comments containing braces are genuinely handled.app.css:118—#blazor-error-ui .dismiss { cursor: pointer }is the same defect the ADR paragraph above it now describes: the span carries no listener on itself, its ancestors or the document, and a real click leaves the banner atdisplay: block, so it promises an interaction it does not have, mouse-only, with no keyboard route to dismiss. Wire it inindex.htmlor drop the cursor.Verdict: changes required
Guard fixed in
359bce1— per-selector after splitting on commas, any@refused, and functional pseudo-classes rejected before the split (:is()carries its own commas). All three of your mutations now fail, each on the check meant for it, as do the three from the previous round.On
.dismiss: I get the opposite result, and I think the probe is the difference. Blazor attaches that handler lazily inside the function that unhides the banner, so a hand-unhidden banner has no listener and a dead click — exactly what you measured. Forcing a real unhandled exception givesdismiss.onclick= the framework's dismiss function, aclicklistener on the span, and a click that setsdisplay: none. So the cursor is honest and the missing part is the keyboard route (#139, premise unchanged). Left it in place with a comment recording this; happy to be shown wrong if you can dismiss it with the banner raised by hand.Withdrawing the
.dismissfinding — it was wrong, and my method produced the result I reported. Raising the banner with a genuine unhandled exception (thrown from an@onclickhandler, never touchingstyle.display): before it,dismiss.onclickunbound andgetEventListeners[]; after it,dismiss.onclickandreload.onclickboth bound, listeners["click"], and a real mousedown/mouseup on the glyph setsdisplay: none. The mechanism is as you describe —blazor.webassembly.js10.0.10 has one function that doese.style.display="block"and then, behind a run-once flag,querySelectorAll("#blazor-error-ui .reload"/".dismiss").forEach(e=>e.onclick=…), reachable only from the runtime's error callback. Unhiding by hand cannot bind them, socursor: pointeris honest and removing it would be the inverse defect. The keyboard gap is confirmed real (tabIndex -1, no role, never focused across twelve tabs) and #139 covers it as written.NavMenuStylesheetTests.cs:76— the functional-pseudo-class refusal filters selector lists on_interactionPseudoClassesonly, so it never covers the.activerule, and:is(span.navigation-link, a.navigation-link).activepasses: the comma split leavesa.navigation-link).active, which satisfies the qualifier. Whether it is caught depends only on the order the arguments are written — with the anchor listed first it fails. Include.activein the filter for that third test; that turns mutation into a failure, verified.Answering the question as posed: hard to fool now, not merely harder. All three of my earlier spellings are caught, as are
@container,:not(), and hovering the nav wrapper to reach every child. What still passes besides the above isa.navigation-link:hover ~ .navigation-link-unavailable, which does paint the spans but requires deliberately writing a sibling combinator, and an attribute selector carrying the qualifier as a string value — the residual limit is that the guard cannot identify a selector's subject, which is inherent to not having a CSS parser and is worth a sentence in the remarks rather than more machinery.Verdict: changes required
Fixed in
ce4b18f— both tests now filter on one list of affordance markers (the interaction pseudo-classes plus.active). Reproduced first::is(span…, a…).activewas 17/17 green before, fails now, and fails in either argument order.Recorded the combinator and attribute-value survivors in the remarks beside the existing
app.cssconcession, as the inherent limit they are — a substring check cannot identify a selector's subject without a real parser, so this is a guard against a plausible slip, not a proof.Nothing to action.
Verdict: mergeable