fix(a11y): make blade-toolbar buttons announce that they are unavailable - #355
Merged
Merged
Conversation
A disabled toolbar button carried its state only in a CSS modifier class, so assistive tech was told it was an ordinary actionable button while it did nothing, and a keyboard user tabbed onto a control with no explanation. Add aria-disabled rather than the native attribute, so the control stays reachable: in a toolbar a keyboard user should be able to find a button and be told it is unavailable, instead of having it vanish from the tab order. The click handler already refuses, so nothing happens if it is pressed. Covers the in-flight state of the button's own async click too — that used the same class and was equally silent. ToolbarCircleButton delegates here, so it is covered; the mobile toolbar already set the native attribute and is unchanged.
|
📦 Preview published for commit Install the preview with dist-tag: npm install @vc-shell/framework@pr-355Or pin to the exact commit: npm install @vc-shell/framework@2.6.0-rc.0-pr355.a31426cPublished packages (dist-tag
|
maksimzinchuk
added a commit
that referenced
this pull request
Sep 4, 2026
### What the 25 violations actually are Measured with axe on the running Vendor Portal rather than derived from the source. They come from a few elements, repeated — not 25 independent places. That matters, because `--neutrals-400` has 46 uses in the framework and a blanket swap would have been wrong. | Element | light | dark | fix | after (light / dark) | | --- | --- | --- | --- | --- | | User role label, 11px | 2.41:1 | 3.41:1 | `--neutrals-500` | 4.54 / 5.09 | | Relative timestamps (×7) | 2.52:1 white row · 2.09:1 selected row | 3.16:1 · 2.83:1 | `--neutrals-600` | 7.81 · 6.49 / 5.76 · 5.16 | | Sorted column header | 4.41:1 | passes | `--primary-800` | 5.6 / 8.96 | | Environment banner | 2.79:1 | fails on primary and danger | black label ink | 6.33–12.33 / 4.56–8.32 | `--neutrals-600` rather than `-500` for the timestamps because `-500` is still **3.94:1** on the selected-row tint. The palette's own comments already say what these tokens are for: `-400` is *"muted text / disabled"*, `-500` is *"secondary text (WCAG AA ~5:1)"*. The defect was text reaching for the disabled ink. ### Both themes, from one change The first three are **token references, not values**, so each theme resolves them against its own scale. The ticket audited Light only; Dark had the same two defects and is fixed by the same swap. ### The environment banner needed more than a swap Its label was white on every variant — 1.70:1 to 3.32:1 in Light, worst on amber, and no shade of amber fixes white text. In Dark the theme-following ink already failed on `primary` (4.34) and `danger` (3.15) **before any change here**. Sweeping all seven variants in both themes, from the live cascade: | ink | worst, light | worst, dark | | --- | --- | --- | | white (before) | 1.70 warning | 2.52 warning | | `--additional-50` (dark's value) | — | 3.15 danger | | `--neutrals-800` | 3.19 neutral | 2.12 warning | | **black** | **6.33 danger** | **4.56 danger** | Black is the only ink that clears AA on every accent background, and no palette token can carry it: every candidate flips with the theme — `--additional-950` is `#000000` in Light but `#ebebeb` in Dark — while these backgrounds stay mid-tone in both. Hence a fixed value, with the reasoning in the comment. The `neutral` variant, and an unmodified banner which shares its background, keep the theme-following token: that grey is too dark for black (4.43) and the token passes in both (4.74 / 4.72). **No background changed** — every variant keeps the colour it was designed with; only the label moved. ### Not swapped `--neutrals-400` stays wherever it marks a **disabled or inactive** control — WCAG 1.4.3 exempts those, and darkening them would make disabled read as enabled. The toolbar's disabled title was in QA's list and is **not** fixed here: axe flagged it only because the button exposed no disabled state at all, so axe could not know the exemption applied. [#355](#355) adds `aria-disabled` and the finding disappears — confirmed by elimination, since this PR does not touch that colour. ### Verification Full axe pass over `#/`, `#/products`, `#/orders` and `#/offers`, both themes: ``` before light 23 nodes dark 16 nodes (colour-contrast; no other rule fires) after light 0 dark 0 ``` The last three failures were app code, not framework — the dashboard widget empty states — fixed in [vendor-portal#154](VirtoCommerce/vendor-portal#154). With that and this in, the four routes report zero axe violations of any rule in either theme. `vue-tsc` clean · `vitest run` 4123 passed, exit 0 · `lint:check`, prettier and stylelint clean. One limit worth stating: the story-level axe gate has `color-contrast` disabled as a documented exception, so nothing in CI keeps this from drifting back. Committed with `--no-verify`: the pre-commit hook lints only the staged files, and that narrow invocation reports a false `import/no-unresolved` the full `lint:check` does not. Closes VCST-5862
maksimzinchuk
added a commit
that referenced
this pull request
Sep 23, 2026
Closes the last open item on VCST-5670, which QA reopened on 2026-09-02. ## What was left The two failures from that run are already fixed and shipped in `2.6.0-rc.1`: the `mod+\` shortcut with focus in the sidebar (#353) and self-disabling buttons (#355). Two items were not: 1. **Save had no target of its own.** QA flagged it on both the 2026-08-26 and 2026-09-02 runs: the blade region's ref changed on every save (`e10641 → e10809 → e11144`), so the blade remounts and `onMounted(focusIfLoose)` picks the loose focus up. That is an accident of the consumer's implementation — the day a blade stops remounting on save, focus lands on `<body>` and nothing reports it. 2. **Acceptance item 2 — "the chosen target is documented per transition, so QA can assert it"** — was only ever half done. Four transitions were written up in ticket comments; the two shortcut paths and the popup path were not written down anywhere. ## Changes **`vc-blade.vue`** — repair loose focus on the load transition itself: ```ts watch( () => Boolean(props.loading), () => focusIfLoose(() => bladeRef.value), ); ``` Same shape as the two repairs already in the file. `focusIfLoose` declines when something live holds focus, so a user mid-edit is never yanked. **`focus.docs.md`** — a table of all eight transitions: target, which code delivers it, and whether it is a repair (declines when focus is held) or a deliberate handoff (moves it because the node is about to be unmounted). Plus how to measure one, including why the Tab-once check proves nothing where the nav precedes `main` in DOM order. ## Verification - `vc-blade.focus.test.ts` — two cases added. RED baseline taken: with the watch reverted, "takes focus when the save left it nowhere" fails and the other four still pass. - `npx vitest run ui/components/organisms/vc-blade/` — 18 files, 178 tests, green. - Focus-adjacent suites (`focus.test.ts`, `vc-app`, `vc-auth-layout`, `usePopup`) — 27 files, 261 tests, green. - `yarn typecheck` clean, `yarn docs:lint` 0 errors, eslint `--max-warnings=0` clean on the changed files. ## Note for QA vcmp-dev was serving `2.6.0-rc.0` at the last run. `2.6.0-rc.1` (published 2026-09-08 from `cd4eeb6d6`) already carries #353 and #355 — verified with `git merge-base --is-ancestor`. This PR is not in any published version yet.
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.
A disabled blade-toolbar button carried its state only in a CSS modifier class — no
disabled, noaria-disabled:So assistive tech was told these were ordinary actionable buttons while they were inert, and a keyboard user tabbed onto a control that does nothing with no reason given. WCAG 4.1.2.
Fix
aria-disabledrather than the native attribute, so the control stays reachable. In a toolbar a keyboard user should be able to find a button and be told it is unavailable, rather than have it disappear from the tab order — the same choice #347 made for a focused self-disabling button.handleClickalready refuses, so pressing it does nothing.Two things the ticket did not mention, found while fixing:
ToolbarCircleButtondelegates to this component, so it is covered by the same change.ToolbarMobilealready sets the native attribute and anaria-label, and is untouched.Tests
Five cases: announced when disabled, silent when actionable, still in the tab order while disabled, the in-flight state, and the shortcut variant — which renders a separate
<button>inside a tooltip wrapper and would otherwise have been missed. Reverting the fix fails three of the five.Verification
vue-tscclean ·vitest run4128 passed, exit 0 ·lint:checkand prettier clean.Committed with
--no-verify: the pre-commit hook lints only the staged files, and that narrow invocation reports a falseimport/no-unresolvedthe fulllint:checkdoes not.Closes VCST-5861