Repository navigation
Fixes #3031 - #3038
Merged
Merged
Fixes #3031#3038
Conversation
#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.
|
Reviewed against CONTRIBUTING.md conventions, Lite/Darling parity, correctness, and security.
No issues found — looks correct and well-scoped. |
This was referenced Sep 5, 2026
Merged
Merged
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-labelandaria-describedbywere all null on.num, so the association was inferable but not determinable.What changed
rollup()'stilehelper, 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 witharia-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 anaria-*attribute, so nothing moves visually.Measured in a browser against the built page, per tile:
Deadlocks (recent), read 0 of 12 serversrollupTextIdde-duplicates within a render. Two tiles sharing a label would otherwise emit a duplicate id, andaria-describedbyresolves 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 theiraria-describedbycomes out empty and the number↔label association — which is what the issue's title is about, and what it says every.rolluptile 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-describedbyrather thanaria-label/aria-labelledbybecause.numis a plaindiv. ARIA prohibits namingrole=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 oversightAttaching it to
aria-describedbywas 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.Noteis derived, and itsWindowNotealone 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
titleof a non-interactive div, which Chrome exposes as the tile's accessible name but not as a description, and atitleon 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-detailshas 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 serversrather than12 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.criticalkeyed ontotal_deadlocks > 0is 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 takeswarningthrough 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.rollupwith the identicalnum/lblproximity-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 topages/fleet.jsfor parallel-work safety. The clean fix for both is to hoistrollupTextIdand the tile builder intoutil.jsand 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.jsanywhere in the tree (swept at unlimited depth, with the identicalfindinvocation 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 owngetElementById, 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
renderFleetdirectly and through the realindex.htmlshell.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:
aria-describedbyremovedaria-describedbykeptrollup-deadlocks-recent-lbltwiceThe 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
/api/fleetwas stubbed. The payload's field names were taken fromDarlingFleetReader.cs's[JsonPropertyName]attributes, and the note string was assembled from the C#WindowNote/PostgresCauseconstants and confirmed byte-identical to what theNotegetter 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..numis 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..numelements do not appear as distinct nodes in every accessibility-tree dump — the flattening varies by tool. The numbers' text and order were confirmed present viatextContentin reading order.CHANGELOG
Not committed here, per the lanes' routing. Entry text for the
[Unreleased]→### Fixedblock:aria-describedby, rather than by reading-order proximity alone. (#3031)