Skip to content

Commit ccc97f5

Browse files
committed
Add a11y checks
1 parent 4ee7013 commit ccc97f5

4 files changed

Lines changed: 205 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,14 +277,22 @@ opposite render modes. This is structural, not cosmetic — one plugin instance
277277
**Standing rule: every gate must be proven to fail.** When a step adds a gate, deliberately break
278278
it, record that it went red, then revert. A gate that cannot fail is worse than none.
279279

280-
Current gates (steps 6 + 8): `check:versions`, `fmt:check`, `build`, `check`, `check:exports`,
280+
Current gates (steps 6 + 7 + 8): `check:versions`, `fmt:check`, `build`, `check`, `check:exports`,
281281
`lint`, `test:coverage` (global 80% lines, plus a per-directory 90% lines on `packages/*/src/**`
282282
widened at step 5 from the step-3 `packages/*/src/reactive/**`, which it subsumes; both
283283
non-vacuous, both proved red with no injected dead code), **`conformance`** (237 assertions: 50
284284
static classnames, 50 accessible descriptions, 50 post-flush classnames, 58 action sequences, 19
285285
port-side action sequences, plus alignment/drift/format), `test:ssr` (**both halves**), and
286286
`check` including the examples.
287287

288+
The step-7 a11y gate was proved red and reverted: deleting the `title` binding from
289+
`ValueSelector.tsx` turned **all nine** axe cases red on the **WCAG** assertion (`select-name`, a
290+
level-A violation, not merely a best-practice one) while all three keyboard tests stayed green.
291+
Note the best-practice assertion is an **equality** check against `['label-title-only']`, not a
292+
suppression — RQB labels selectors and text editors with `title` alone and DOM parity is locked, so
293+
that one rule is accepted (recorded under "Known limitations" in `CHANGELOG.md`) while any _other_
294+
best-practice regression still fails.
295+
288296
The four step-6 conformance gates were each proved red and reverted:
289297

290298
1. **DOM parity**` conformance-gate-probe` appended to `ActionElement.tsx`'s class turned

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,22 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
4848
- `README.md` gains a prominent "Requires Solid 2.0" note, documentation links, and an examples
4949
section; new `docs/differences-from-react-querybuilder.md`, `docs/styling.md`, and
5050
`docs/customization.md`.
51+
- Accessibility suite (`src/components/a11y.test.tsx`): `vitest-axe` over all eight conformance
52+
scenarios plus an all-controls independent-combinator case (nine cases), each asserted twice —
53+
WCAG 2.0/2.1 A+AA must be empty, and best-practice must equal exactly `['label-title-only']`, so
54+
any _other_ best-practice regression still fails. Plus keyboard tests: tab order through a rule
55+
row, Enter/Space activation, and the not-toggle label association. It imports
56+
`test/conformance/{scenarios,queries}` rather than duplicating them, so a11y is asserted against
57+
exactly the prop combinations DOM parity is, and it still runs in a fresh clone (both modules are
58+
fixture-independent).
59+
60+
### Known limitations
61+
62+
- **`label-title-only` (axe best-practice) fires on every selector and text editor.** React Query
63+
Builder labels these controls with `title` alone, and full DOM parity is a locked decision for
64+
this port, so adding `aria-label` would break the conformance harness. It is not a WCAG failure:
65+
`title` produces an accessible name, and the level-A `label`/`aria-*` rules pass across all nine
66+
a11y cases. Consumers who need a visible label can supply one through `controlElements`.
5167

5268
### Fixed
5369

docs/differences-from-react-querybuilder.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,17 @@ FullCombinator>`), and `QueryBuilderProps` is the same conditional type React Qu
129129
Solid components are plain functions with no compile-time prop enumeration, so there is no
130130
non-conditional base interface.
131131

132+
`QueryBuilder` is generic over `RG`, `F`, `O`, and `C`; `Rule` and `RuleGroup` over `F` and `O`
133+
(`RuleProps`/`RuleGroupProps` carry no `RG` parameter upstream either — the group type is fixed by
134+
`RuleGroupTypeAny` on `RuleGroupProps.ruleGroup`). `SimpleQueryBuilderProps`,
135+
`SimpleQueryBuilderPropsIC`, `SimpleRuleProps`, and `SimpleRuleGroupProps` are the
136+
default-parameter aliases.
137+
138+
Two problems the Vue port had do **not** arise here, because a Solid component is a plain generic
139+
function rather than a compiled single-file component: there is no compiler macro or `generic=`
140+
attribute to declare the parameters, and the emitted props carry no `Record<string, unknown>` index
141+
signature — so no component needs a `widenedProps` re-widening cast.
142+
132143
## 7. Reactivity
133144

134145
React Query Builder's hooks (`useQueryBuilder`, `useRule`, `useRuleGroup`, `useValueEditor`, …) are
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
/**
2+
* Accessibility coverage for the rendered tree.
3+
*
4+
* The scenario list is the same one the conformance harness uses, so a11y is asserted against
5+
* exactly the prop combinations DOM parity is asserted against. `scenarios.tsx` and `queries.ts`
6+
* are self-contained — they carry no dependency on the downloaded fixture files — so this suite
7+
* runs under the default `bun run test` config in a fresh clone.
8+
*/
9+
10+
import type { RuleGroupType } from '@react-querybuilder/core';
11+
import { TestID } from '@react-querybuilder/core';
12+
import { render, within } from '@solidjs/testing-library';
13+
import userEvent from '@testing-library/user-event';
14+
import { flush } from 'solid-js';
15+
import { describe, expect, it } from 'vitest';
16+
import { axe } from 'vitest-axe';
17+
import { queries } from '../../test/conformance/queries.js';
18+
import { fields, scenarios } from '../../test/conformance/scenarios.jsx';
19+
import type { QueryBuilderProps } from '../types/index.js';
20+
import { QueryBuilder } from './QueryBuilder.jsx';
21+
22+
/**
23+
* WCAG 2.0/2.1 level A and AA. Axe's "best-practice" rules are asserted separately, because the
24+
* ported DOM knowingly violates one of them — see {@link acceptedBestPracticeViolations}.
25+
*/
26+
const wcagTags = ['wcag2a', 'wcag2aa', 'wcag21a', 'wcag21aa'];
27+
28+
/**
29+
* `label-title-only` fires on every selector and text editor in the tree: React Query Builder
30+
* labels them with `title` alone, and full DOM parity is a locked decision for this port, so
31+
* adding `aria-label` here would break the conformance harness. It is a best-practice rule, not
32+
* a WCAG failure — `title` does produce an accessible name, which is why the `label`/`aria-*`
33+
* rules at level A pass. Consumers who need a visible label can supply one through
34+
* `controlElements`.
35+
*
36+
* The list is asserted rather than suppressed, so any *other* best-practice regression still
37+
* fails. It holds for the `multiValue` scenario too: the multiselect and radio-group editors are
38+
* new to `schemaVersion` 2, and upstream cleared its own axe run only after giving the bound-pair
39+
* editors a `title`, which this port reproduces.
40+
*/
41+
const acceptedBestPracticeViolations = ['label-title-only'];
42+
43+
/**
44+
* `vitest-axe`'s `toHaveNoViolations` matcher is not registered in this project's setup file, so
45+
* assert on the results directly.
46+
*/
47+
const expectNoViolations = async (container: Element): Promise<void> => {
48+
const wcag = await axe(container, { runOnly: wcagTags });
49+
expect(wcag.violations.map(v => `${v.id}: ${v.help}`)).toEqual([]);
50+
51+
const bestPractice = await axe(container, { runOnly: ['best-practice'] });
52+
expect(bestPractice.violations.map(v => v.id).toSorted()).toEqual(acceptedBestPracticeViolations);
53+
};
54+
55+
/** The scenario props are deliberately loose (see `scenarios.tsx`); widen once, here. */
56+
const renderScenario = (props: Record<string, unknown>): HTMLElement =>
57+
render(() => <QueryBuilder {...(props as QueryBuilderProps)} />).container as HTMLElement;
58+
59+
describe('accessibility', () => {
60+
for (const scenario of scenarios) {
61+
// One query per scenario is enough: the scenarios vary the controls, and the fixture queries
62+
// vary only the tree shape. `nested` exercises groups, rules, and depth at once.
63+
const query = (scenario.query ??
64+
queries[
65+
scenario.queries?.includes('nested') ? 'nested' : scenario.queries![0]
66+
]) as RuleGroupType;
67+
68+
it(`has no axe violations: ${scenario.name}`, async () => {
69+
const container = renderScenario({ ...scenario.props, defaultQuery: query });
70+
// Effects (notably the value-editor reset) run on the next flush; axe must see the settled
71+
// tree, not the first paint.
72+
flush();
73+
74+
await expectNoViolations(container);
75+
});
76+
}
77+
78+
it('has no axe violations with every control and an independent-combinator query', async () => {
79+
const container = renderScenario({
80+
fields,
81+
defaultQuery: queries.icNested,
82+
showNotToggle: true,
83+
showCloneButtons: true,
84+
showLockButtons: true,
85+
showShiftActions: true,
86+
showMuteButtons: true,
87+
showUndoRedo: true,
88+
});
89+
flush();
90+
91+
await expectNoViolations(container);
92+
});
93+
});
94+
95+
describe('keyboard navigation', () => {
96+
it('reaches every control in a rule row in document order', async () => {
97+
const container = renderScenario({
98+
fields,
99+
defaultQuery: queries.flat,
100+
showShiftActions: true,
101+
showCloneButtons: true,
102+
showLockButtons: true,
103+
});
104+
flush();
105+
106+
// The second rule, so that neither shift button is disabled and the whole row is tabbable.
107+
// The first rule's "shift up" is disabled, which tab skips, making the expected order
108+
// position-dependent.
109+
const rule = within(container).getAllByTestId(TestID.rule)[1];
110+
const expected = [
111+
TestID.shiftActions,
112+
TestID.shiftActions,
113+
TestID.fields,
114+
TestID.operators,
115+
TestID.valueEditor,
116+
TestID.cloneRule,
117+
TestID.lockRule,
118+
TestID.removeRule,
119+
];
120+
121+
rule.querySelectorAll<HTMLElement>('button, select, input')[0].focus();
122+
123+
const reached: string[] = [];
124+
for (let i = 0; i < expected.length; i++) {
125+
const active = document.activeElement as HTMLElement | null;
126+
expect(active && rule.contains(active)).toBe(true);
127+
reached.push(active!.closest('[data-testid]')!.getAttribute('data-testid')!);
128+
await userEvent.tab();
129+
}
130+
131+
expect(reached).toEqual(expected);
132+
// The next tab leaves the rule entirely.
133+
expect(rule.contains(document.activeElement)).toBe(false);
134+
});
135+
136+
it('activates a button control with the keyboard', async () => {
137+
const container = renderScenario({ fields, defaultQuery: queries.singleRule });
138+
const scoped = within(container);
139+
140+
scoped.getByTestId(TestID.addRule).focus();
141+
await userEvent.keyboard('{Enter}');
142+
flush();
143+
144+
expect(scoped.getAllByTestId(TestID.rule)).toHaveLength(2);
145+
146+
scoped.getAllByTestId(TestID.removeRule)[1].focus();
147+
await userEvent.keyboard(' ');
148+
flush();
149+
150+
expect(scoped.getAllByTestId(TestID.rule)).toHaveLength(1);
151+
});
152+
153+
it('associates the not-toggle label with its checkbox', async () => {
154+
const container = renderScenario({
155+
fields,
156+
defaultQuery: queries.singleRule,
157+
showNotToggle: true,
158+
});
159+
160+
const checkbox = within(container).getByLabelText('Not');
161+
expect(checkbox).toHaveAttribute('type', 'checkbox');
162+
163+
checkbox.focus();
164+
await userEvent.keyboard(' ');
165+
flush();
166+
167+
expect(checkbox).toBeChecked();
168+
});
169+
});

0 commit comments

Comments
 (0)