FSHSP-106 fix(a11y): clear the mechanical axe violations (151 → 79) - #87
Open
LBU4SH wants to merge 15 commits into
Open
FSHSP-106 fix(a11y): clear the mechanical axe violations (151 → 79)#87LBU4SH wants to merge 15 commits into
LBU4SH wants to merge 15 commits into
Conversation
ui-input forwarded aria-expanded to the native input, but that attribute is not allowed on the implicit textbox role: screen readers dropped it, so a ui-datepicker never announced that a calendar could open, nor its state. Promote the input to role="combobox" (the ARIA 1.2 role that accepts aria-expanded) only when ariaExpanded is provided, leaving plain text fields announced as before. ui-datepicker is the only consumer of these attributes in the kit. Clears the 25 aria-allowed-attr violations of the a11y job (136 -> 111 occurrences, 122 -> 97 failing stories), with no new violation and no cascade on aria-required-children, verified by a full rebuild + axe pass.
`min`/`max` are declared with `numberAttribute`, which turns an unset input into NaN rather than null. The spinbutton then rendered aria-valuemin="NaN" and aria-valuemax="NaN" — an invalid ARIA value, so the bound was not just useless but discarded by assistive tech. Emit both attributes only for a finite bound: no bound, no attribute. Clears 11 of the aria-valid-attr-value violations of the a11y job.
The tags listbox only got an accessible name when no visible `label` was set: with a label, the name went to the inner <input> and the listbox announced nothing. Fall back to `label` for the listbox name. In ui-autocomplete the `role="listbox"` also spanned the whole box, including the combobox <input> — a listbox may contain only options/groups. Wrap the option tags alone, with `display: contents` so the layout is untouched (same treatment ui-input-tags already had). Clears 14 aria-input-field-name and 3 aria-required-children violations.
Both pickers rendered `role="gridcell"` buttons straight inside `role="grid"`, with no `role="row"` in between — required by the grid pattern, and present on the day grid. Screen readers got no row structure to walk in the month and year views, and axe flagged the whole grid as invalid. Chunk the cells into the rows the CSS actually renders (3 columns for months, 2 for years) and mark the wrappers `display: contents`, so the rendered grid is byte-for-byte the same. Clears 2 aria-required-parent (22 nodes) and 2 aria-required-children.
aria-label is prohibited on a role-less <span>: the name the consumer passed was simply dropped, so these components had no accessible name at all. Take role="img" when an aria-label is provided — and role="group" for a removable chip, whose × button must not end up inside an img, which hides its children from assistive tech. No label, no role: unlabelled usage is unchanged. Clears the 4 aria-prohibited-attr violations (ui-avatar's badge included, it is a ui-badge instance).
…hable A scrolling region has to be focusable, or its overflowing content can only be reached with a mouse. ui-table: the viewport takes tabindex=0 whenever `scrollable` is on. ui-modal: the body region is always `overflow-y: auto`, but an unconditional tab stop inserts itself into the focus order of every dialog — a focus-trap unit test caught exactly that. It therefore becomes focusable only once it actually overflows AND holds nothing focusable itself; since the content is projected, neither is derivable from the inputs, so both are measured (ResizeObserver). Clears the 6 scrollable-region-focusable violations.
`hasProjectedContent` is detected from the `.ui-button-content` element, but that
element was only rendered `@if (label() || !isIconOnly())` — and `isIconOnly()`
derives from that very detection. A ui-button given an `icon` and nothing but
projected content therefore never rendered the content, stayed in icon-only mode,
and ended up with no accessible name at all. The detection could never succeed:
the element it inspects did not exist.
Always render the wrapper. The SCSS already carried
`.ui-button-content:empty { display: none; }` ("empty → removed, avoids phantom
gap") precisely for this, so a genuine icon-only button is unchanged.
Clears the button-name violation on the Projected Content story — which was not
a story oversight but this bug, rendering an empty button.
…ble headers axe's empty-table-header wants text that is actually present: an aria-label on the <th> does not satisfy it (measured — the violation survived that attempt). The kit had no screen-reader-only utility, while four components hand-roll the same block in their own SCSS (ui-datepicker, ui-select, ui-file-upload, ui-read-only), so add it once to the published helpers sheet. Use it to name the selection and row-expansion columns of the ui-table stories. Verified: both stories now report zero violations, and the headers still measure 56px and 68px — the absolutely-positioned span adds nothing to the column.
… mounted ui-tabs used as a navigation menu (a router-outlet renders the content, see the TabMenu story) and ui-stepper used as a plain "steps only" progress indicator (both explicitly documented usages) still had every tab/step point aria-controls at an id that resolves to nothing — an invalid ARIA value. Each container now tracks the panels actually mounted via contentChildren; panelId() returns null when a step/tab has no matching panel, so the attribute is omitted rather than dangled. Clears 2 of the 3 remaining aria-valid-attr-value violations. Verified: both stories now report zero violations for this rule (ui-tabs/Tab Menu's one remaining violation is color-contrast, unrelated). ui-stepper/Vertical's aria-required-children (tablist containing its own tabpanel) is untouched — needs a design decision (tabs vs. accordion in vertical mode) before a fix, tracked separately on FSHSP-106.
…ttern Vertical interleaves each step's panel right under its own header inside the same ui-step-item (see the Vertical story), so the container's role="tablist" always contained a role="tabpanel" descendant — a tablist containing its own tabpanel is invalid ARIA. No DOM restructuring: vertical switches its role vocabulary to the accordion pattern instead (matching ui-accordion-panel's own role+aria-labelledby pairing) rather than moving panels out of the tablist. - ui-stepper host: role="group" instead of "tablist" in vertical (keeps aria-label legal — a plain div with no role could not carry one, the same aria-prohibited-attr fixed earlier on ui-tag/ui-badge/ui-chip). - ui-step header: no role + aria-expanded instead of role="tab" + aria-selected, in vertical only. - ui-step-panel: role="region" instead of "tabpanel", in vertical only. Horizontal (ui-step-list, a real tablist with no panel inside it) is untouched. No CSS rule keys off aria-selected, so nothing visual moves. Clears the last aria-required-children violation. Verified across Vertical, StepsOnly, Linear, Horizontal and Basic: zero violations on all five.
… the tag pattern Decision made with the team: accept the chip-in-listbox pattern as-is rather than restructure it now. Waived via parameters.a11y.config.rules, read by storybook/test-runner.ts. Restructuring tracked in a follow-up ticket.
…hreshold actions.low.content.default (slate-500 on grey-50, 4.44:1) and navigation.highOutlined.content.active (primary-500 on grey-50, 4.20:1) both missed the 4.5:1 AA threshold by a hair. slate-500 -> slate-600 (5.45:1). Its hover state already sat one step above at slate-600 — left alone, default would now equal hover and the state would lose its visible feedback. hover moves to slate-700 to keep a step between them. primary-500 -> primary-600 (6.00:1) for the active tab. hover shared the exact same value already (also 4.20:1, real defect but not axe-measured since it's a :hover pseudo-class) — moved to primary-600 too, staying in sync with active as before rather than diverging. Verified against the real build: ui-link/Default, ui-segment-control/Basic and ui-tabs/Basic all report zero violations, no visible change beyond the two target texts. Source is Figma-exported (src/design-tokens/semantics.json, each value carries its Figma variableId) — the same change is needed on the Figma variable, or a future export silently reverts this.
c3824a2 added a second `contentChildren` on UiTabs (matching each UiTabPanel by value, to know whether a panel exists) alongside the existing one for UiTab. Two sibling content-children queries on this component break `<ng-content>` projection for every UiTab but the first: labels 2..n rendered empty, leaving tabs with no accessible name at all. The a11y run caught it as `button-name`, 2 stories / 14 nodes (Dynamic, Scrollable). Bisected by restoring only this file to main's version — the defect vanished, confirming the query, not the rest of the branch. Use a single `contentChild` on the UiTabPanels wrapper instead: a presence check is all `panelId()` needs, and it keeps the query count at one. Verified on all seven tabs stories — labels are back, `aria-controls` still resolves to a real panel where panels exist and is still omitted on TabMenu, which has none. Zero violations across the whole ui-tabs suite. Also pins both contracts in the spec (rendered label, aria-controls presence), which nothing asserted before. They would NOT have caught this regression — verified by reintroducing the faulty query, and they still pass: the defect only appears in a real compiled build, not under TestBed/jsdom. Kept because they are cheap and the contracts were untested either way. No CHANGELOG entry: this repairs a defect introduced earlier in this same unmerged branch, so the published behaviour is unchanged.
White on red-500 (#ef4444) is 3.76:1, under the 4.5 AA threshold. red-600 (#dc2626) is 4.83:1 and the 600 step was unused across the whole chain (hover=700, focused=800, pressed=900), so retargeting default onto it needs no new colour and shifts nothing else. Retargets actions.error.surface.default and informative.errorHigh.surface.default, light mode only — the CI job measures nothing else, and dark is left untouched rather than changed unverified. Verified on the real build: ui-alert/Error, ui-button/Error and ui-tag/Error report zero violations. The visible difference is marginal. success and warning are NOT covered here: their own 600 falls short (3.77 and 3.56), so both need a palette decision — either a new intermediate shade or dark text on the bright surface, which would invert the hover ramp. Tracked on FSHSP-106.
White on green-500 (#10b981) is 2.54:1 against a 4.5 threshold, and no light green can pass with white text — green-600 only reaches 3.77 while green-700 is already the hover state, so the ramp had no usable step left. Add green-650 = #0b835c to all three brand modes: the lightest green clearing 4.5 with a real margin (4.76), chosen to keep the visual shift as small as the threshold allows. Retarget actions.success.surface.default and informative.successHigh.surface.default onto it, light mode only. Designer's call: keep white text and darken the surface, rather than switch the text to dark on the bright green. Dark text would have preserved green-500 exactly (6.47:1) but breaks every other state — the surface darkens on hover/focused/pressed, where dark text drops to 2.99/2.14/1.08 — so it would have meant inverting the ramp direction for this family alone. The surface is noticeably darker, which is inherent to WCAG 2 on saturated greens, not a tuning choice. Verified on the real build: ui-alert/Success, ui-button/Success and ui-tag/Success report zero violations. green-650 has no Figma variableId yet — it must be created there, or the next export drops it.
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
1 participant
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.
Premier lot de FSHSP-106 : toutes les violations axe qui ne demandent aucune décision de design — donc rien qui touche aux couleurs, qui sont bloquées sur les variables Figma.
Mesure
Baseline re-mesurée sur
main(605 stories) : 151 occurrences / 135 stories — pas les 136 du ticket, écrit avant que le datepicker gagne 18 stories.aria-allowed-attraria-input-field-namescrollable-region-focusablearia-prohibited-attraria-required-parentempty-table-headerbutton-namearia-valid-attr-valuearia-required-childrennested-interactivecolor-contrast77 stories en échec au lieu de 135, aucune story devenue rouge, aucune erreur de chargement. Lint et les 154 tests unitaires passent.
Un commit par règle, relisible indépendamment.
Deux trouvailles qui ne sont pas des correctifs d'attribut
ui-buttonavalait son contenu projeté (af31015).hasProjectedContentest détecté depuis.ui-button-content, dont le rendu était conditionné àisIconOnly()— qui dérive de cette détection. Unui-buttonavec une icône et du seul contenu projeté ne rendait donc jamais ce contenu, restait en mode icône seule et se retrouvait sans nom accessible. La détection ne pouvait pas aboutir : l'élément qu'elle inspecte n'existait pas. C'était l'origine réelle dubutton-name.Le kit n'avait aucun utilitaire
sr-only(fca6ba9), alors queui-datepicker,ui-select,ui-file-uploadetui-read-onlyrecopient le même bloc dans leur SCSS. Unaria-labelsur un<th>vide ne satisfait pasempty-table-header— axe veut du texte réellement présent, mesuré et confirmé.Point à trancher en revue
fca6ba9est le seul commit qui élargit la surface CSS publiée (.sr-onlydans les helpers, d'où le typefeatet l'entrée enAdded). Il s'annule seul, sans toucher aux sept autres, si vous préférez le sortir en ticket dédié.Réserve
Le
role="combobox"deui-input(079c575) n'a pas été testé au lecteur d'écran. axe valide la légalité de l'attribut, pas l'expérience : reste à vérifier que NVDA/VoiceOver, en mode formulaire, ne captent pas les flèches dont la grille du datepicker a besoin. À faire avant de retirercontinue-on-errordu joba11y.Reste sur le ticket
nested-interactive(11) +aria-required-children(1) — pattern chips-dans-combobox deui-input-tags/ui-autocomplete, etui-stepperverticalaria-valid-attr-value(2) —aria-controlsvers un panneau non monté, demande un registre des panneaux renduscolor-contrast(65) — passe designer, détail des 7 paires de couleurs en commentaire sur FSHSP-106🤖 Generated with Claude Code