Skip to content

Fixes #3031 - #3038

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/3031-rollup-tile-aria
Sep 5, 2026
Merged

erikdarlingdata merged 1 commit into
devfrom
fix/3031-rollup-tile-aria

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Fixes #3031.

The fleet rollup tiles associated each number with the text that says what it counts by reading-order proximity alone. #3031 measured this carefully and its narrow version is the accurate one: the text is in the accessibility tree, in the correct reading order, and the meaning is carried by text rather than colour — nothing was unavailable and there is no WCAG 1.4.1 failure here. What was missing is the programmatic relationship: role, aria-label and aria-describedby were all null on .num, so the association was inferable but not determinable.

What changed

rollup()'s tile helper, once, so no call site can opt a tile in or out. The label and the coverage sub-line carry stable label-derived ids (rollup-deadlocks-recent-lbl, rollup-deadlocks-recent-sub) and the number points at them with aria-describedby. Nothing is added to, removed from, or reordered in the rendered text, and no stylesheet in this wwwroot keys on an id or on an aria-* attribute, so nothing moves visually.

Measured in a browser against the built page, per tile:

tile number's description
Servers … Offline, Blocking (recent) its own label
Deadlocks (recent) Deadlocks (recent), read 0 of 12 servers

rollupTextId de-duplicates within a render. Two tiles sharing a label would otherwise emit a duplicate id, and aria-describedby resolves a duplicate to the first match — a silent mis-association rather than a visible break. Ids are stable across the page's 60s re-render (verified over four consecutive renders) and do not collide with the shell's own ids (app, sidebar, main, statusbar, server-list, view-list).

The label is in aria-describedby, which is a deliberate departure from the shape #3031 sketched

#3031 proposed the sub-line only, giving "0, read 0 of 12 servers". Measured, that shape fixes exactly one tile: six of the seven have no sub-line, so their aria-describedby comes out empty and the number↔label association — which is what the issue's title is about, and what it says every .rollup tile shares — stays proximity-only everywhere. That is the "one tile is the odd one out" outcome the issue argues against, arriving from the other direction. The label is therefore in the description too, so all seven tiles gain the relationship.

The label rides in aria-describedby rather than aria-label / aria-labelledby because .num is a plain div. ARIA prohibits naming role=generic, so a name set there is invalid and may be dropped; a description is a supported property on it. Description is the carrier that works without inventing a widget role for a static figure.

The coverage note stays on title, and that is a judgment, not an oversight

Attaching it to aria-describedby was ruled out when the sub-line was built and it stays ruled out. Measured: the number's description is 40 characters today; adding the note takes it to 617. The note itself is 575 characters for a 12-server fleet with no coverage — FleetDeadlockCoverage.Note is derived, and its WindowNote alone is 372 — so a paragraph naming both windows and every uncovered cause would be announced on every pass over the number. That is worse than the hover it would replace. The short coverage fact under the label is the right accessible carrier.

The note's own availability gap is real and is not addressed here. It sits on title of a non-interactive div, which Chrome exposes as the tile's accessible name but not as a description, and a title on a non-interactive element is not reliably announced. Every way of fixing it properly is a bigger change than this one: announcing it on the number is the thing just ruled out; aria-details has too thin an AT footing to call a fix; and the option that would actually work — a real focusable disclosure that reveals the note as visible text — is a design change to the tile, not an attribute. It wants its own decision.

Two other judgments, stated rather than buried

The sub-line at full coverage stays unconditional. It reads read all 12 servers rather than 12 of 12, which is already the softened form. Making it conditional would put the load-bearing signal in its absence, and would leave "zero, whole fleet measured" and "zero, from a build that reports no coverage at all" looking identical — the same defect one step out. Unchanged deliberately.

critical keyed on total_deadlocks > 0 is still right. The case worth worrying about — real deadlocks the collector could not read — does not render as an unstyled all-clear: partial coverage with a zero count already takes warning through the arm beside it, and only a response making no coverage claim at all renders unstyled. A measured incident outranking an incomplete denominator is correct, and the sub-line under it already says the count may be short. Unchanged.

Same pattern, not in this PR

wwwroot/js/pages/ag.js (rollup(), ~lines 97–103) builds a three-tile .rollup with the identical num/lbl proximity-only pattern and no sub-lines. It is the other half of the convention #3031 describes and it is untouched here, because this lane was scoped to pages/fleet.js for parallel-work safety. The clean fix for both is to hoist rollupTextId and the tile builder into util.js and have both pages call it; doing that inside this PR would widen it into a shared module every page imports.

How this was verified

There is no JS test harness in this repository — no package.json, no jest/vitest/karma/playwright config, no *.test.js anywhere in the tree (swept at unlimited depth, with the identical find invocation positive-controlled against planted files first). So this carries no automated coverage, and the verification was by hand, using the same method #3031's own measurements used: reading the accessibility tree and the resolved ARIA relationships programmatically out of a live browser.

Six assertions, kept separable so each part of the change reds a different line: the number carries a description; the browser resolves every reference (via ariaDescribedByElements, not the probe's own getElementById, so a dangling reference cannot pass for a plausible-looking id string); the description includes the label; no duplicate ids document-wide; the note is absent from the description; the description stays under 200 characters.

Green across four coverage shapes — partial, full, no coverage object, and real deadlocks with partial coverage — driven both through a module harness importing renderFleet directly and through the real index.html shell.

Five mutations of the served source, each proven to have actually landed (the harness refuses to serve a mutation whose target text is absent, so a silently-stale mutation cannot pass as green), each failing a different assertion:

mutation reds
aria-describedby removed "number carries a description", all 7 tiles
ids removed, aria-describedby kept "browser resolves every ref", all 7 — the attribute is still present, so a check that only looked for the attribute would have missed this
sub-line only, no label "description includes the label", all 7; and 6 of 7 lose their description entirely
de-dup guard removed, two tiles sharing a label "no duplicate ids" — rollup-deadlocks-recent-lbl twice
the note attached to the description "note absent" and "description stays short" — 617 characters

The duplicate-label case was also run with the guard in place as a control, and stayed green, so the failure above is attributable to removing the guard rather than to the duplicated label.

Not verified

  • No live Darling service was involved. /api/fleet was stubbed. The payload's field names were taken from DarlingFleetReader.cs's [JsonPropertyName] attributes, and the note string was assembled from the C# WindowNote / PostgresCause constants and confirmed byte-identical to what the Note getter produces for that coverage shape — but no end-to-end request was ever made, and a serializer change that this cross-check would not catch could still surprise it.
  • No assistive technology was driven. The relationships were read out of the DOM and out of Chrome's own IDREF resolution; nothing here proves what NVDA, JAWS or VoiceOver actually say. That distinction matters more than usual for this change: .num is a non-focusable generic element, and screen readers vary in whether they announce a description on one at all. This establishes the programmatic relationship 1.3.1 asks for. It does not promise a changed announcement.
  • Chromium only, one engine. No Firefox or WebKit.
  • Light mode and high-contrast rendering were not tested. The argument that they cannot be affected is the stylesheet grep above, not an observation.
  • The .num elements do not appear as distinct nodes in every accessibility-tree dump — the flattening varies by tool. The numbers' text and order were confirmed present via textContent in reading order.

CHANGELOG

Not committed here, per the lanes' routing. Entry text for the [Unreleased] → ### Fixed block:

  • Web dashboard: each fleet rollup tile's number is now programmatically associated with its label and its coverage sub-line via aria-describedby, rather than by reading-order proximity alone. (#3031)

#3031)

The rollup tiles associated their number with the text that says what it counts by reading-order proximity
alone. The text was in the accessibility tree and in the right order, so nothing was unavailable and no
1.4.1 failure existed - but the relationship was inferable rather than programmatically determinable, on
every .rollup tile rather than on one.

The label and the coverage sub-line now carry stable label-derived ids and the number describes itself with
them, wired once in the tile helper so no call site can opt a tile in or out. The label rides in
aria-describedby rather than aria-label/aria-labelledby because .num is a plain div and ARIA prohibits
naming role=generic.

The 575-character coverage note stays on the tile's title and out of aria-describedby: announcing a
paragraph on every pass over the number is worse than the hover it would replace.
@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown

Reviewed against CONTRIBUTING.md conventions, Lite/Darling parity, correctness, and security.

  • Scope: pure front-end change in Darling/.../wwwroot/js/pages/fleet.js — no T-SQL touched, so the collector-query style rules (AS aliases, OPTION(RECOMPILE), etc.) don't apply here.
  • Lite/Darling parity: not applicable. Lite is a WPF desktop app with no web dashboard, so there's no counterpart file for this change to drift from.
  • Correctness: traced rollupTextId/tile through the el() helper — null subId/props are correctly skipped (el drops v == null props), so a tile with no sub-line gets aria-describedby pointing at just the label id, not "lblId null". usedIds is a fresh Set per rollup(d) call, so re-renders can't leak stale/duplicate ids, and the dedup loop (base-2, base-3, ...) is bounded by the fixed 7-tile call site. Verified no other call site or CSS selector (app.css) keys off .num/.lbl/.sub ids or aria-* attributes, so nothing shifts visually.
  • Security: labels are hardcoded literals at each call site (never user/collector data), and ids/aria attributes are set via setAttribute, not the banned html prop — no injection surface.
  • Out of scope, correctly left alone: ag.js's structurally-identical rollup()/tile helper is untouched, as called out in the PR description; it's a separate function in a separate module so there's no id-collision risk with this change.

No issues found — looks correct and well-scoped.

@erikdarlingdata
erikdarlingdata merged commit c6928a1 into dev Sep 5, 2026
8 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/3031-rollup-tile-aria branch September 5, 2026 20:25
erikdarlingdata added a commit that referenced this pull request Sep 6, 2026
…3045) (#3054)

The AG rollup's three tiles built the number and its label as sibling divs
with nothing tying them together, the defect #3038 fixed on the fleet page.
The label now carries a stable id and the number describes itself with it.

rollupTextId moves from fleet.js to util.js rather than being copied: the
de-duplication rule it carries is subtle (a duplicate id resolves to the
first match, mis-associating silently), AG's labels are all distinct today,
and a local copy would be written without the guard and stay correct until a
fourth tile repeated a label.

The guard derives its population from the shipped modules — every rollup
figure under wwwroot/js must carry a description, with a floor under the
enumeration — because a positive-only check is what let this page survive
#3031.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant