Accessibility, correctness and documentation findings from the pre-beta audit: presentation, UI and Views
Part of the #3620619 pre-beta audit, scoped to orchestra_presentation,
orchestra_ui and orchestra_views. One commit per finding; each behaviour
change names, in its message, the deliberate break that makes its new
assertion fail.
Accessibility
-
The card heading was a span, so no card in any personal list was in a screen reader's heading list, while
cards.csswent on resetting the margin and font size that only a heading has. It is anh2in the shared row and in the requester's own hand-built list. -
A card column rendered its heading and its value as two unassociated spans: a card of five columns read out as ten unrelated strings. A labeled column is now a description list of one pair; a column carrying a value alone stays a plain wrapper, since a
dlwith nodtis not valid markup. -
Three shipped colours were under the 4.5:1 of WCAG 2.1 AA 1.4.3, the card's own call to action among them at 4.20:1. Measured, not judged by eye: the button is now
#406e9c(5.35:1) with its hover at#34597f(7.29:1), the amber task state#8a6008(5.03:1), and the muted absence marker at 4.07:1 is a rule nothing has ever emitted, so it is removed rather than recoloured.PresentationAssetsTestnow reads the colours out of the stylesheet and computes the ratio of every pair it ships. -
The progress timeline conveyed the current step by presentation only, and was not a list. It is an ordered list, and the current step carries
aria-current="step"plus the same words in visually hidden text, on purpose: the attribute sits on a list item rather than a link, where acting on it is not something to rely on. Core's pager pairs that attribute with a plain "Page" in text and puts "current" in a title attribute, which a list item has no equivalent of. -
The run's page skipped a heading level. Its two sections were level three under a page title the theme renders as the only h1, so a reader navigating by heading met a level whose parent is not there. Both are level two now.
Correctness
- The delete confirmation's "nothing selected" escape hatch never fired: it
set a redirect on the form state, which is read after a submission and never
on the GET that renders the page, so the reader got the question over an
empty list. The build returns a
RedirectResponse, as core's ownDeleteMultipleFormdoes. - Four Views fields that add no column were click sortable. A field that
adds none never assigns
field_alias, which core initialises to the stringunknown, so a sort on one orders by a column of that name. No shipped view was offering it, since all four aresortable: falseeverywhere; what was wrong is the offer, which the Views UI makes for anything claiming to be sortable, so a cloned view could take it up and break. - Two inverted current-value filters shared one bound value. The disabled
half is a formula and hardcoded
:orchestra_current_value, so on the shipped requester's list, which carries two of them, whichever ran last decided what the other compared against and the loser matched every row. - The current-user-candidate filter's inverted half dropped an unclaimed task
of somebody else's.
NOT (assignee IN (...) OR ...)is unknown on an unassigned row, so an item nobody holds and nobody in the viewer's audiences is offered was in neither half of the filter.
Documentation
- The Views README described a dashboard this module does not ship (the
inbox lives in
orchestra_inbox_views), missed one base table, and described an earlier, shorter set of fields and filters.ViewsMenuLinksTestnow asserts the README and the shipped view config agree on the paths, both ways. - The card row's template documented a mapping the preprocess destroyed: it
wrote the built action contents back over
actions, the field ids it had just read them from. The built list iscalls_to_action, named for what it renders like every other part. - The tenant boundary's claims were wider than the code: "every Orchestra-entity view" and "cannot leak another tenant's rows" now say what the alter does, and the second kind of view it does not reach, one on a non-SQL backend, is named next to the relationship caveat that already was.
Not settled here
ViewsDataTest pins a disagreement this MR does not resolve: every shipped
display carries max-age: 0 in its configuration while the handlers compute
-1, and three of those displays cache their result on the tag plugin, the
requester's own list among them. On 11.3 the shipped block survives an install
and applies until somebody saves the view; on 11.4 the install recalculates it,
so it never applies. Which way that should go is a caching decision rather than
one of these findings, so the test asserts both sides and fails the day a
handler, a shipped block or a cache plugin moves.
The assertion that was supposed to catch it had never compared anything: the test class installed no view configuration, so every load returned NULL and the loop ran over nothing. It installs them now, and a load that returns NULL fails.