Skip to content

Commit aff6aab

Browse files
michaelfaithmichaelfaith
committed
feat(label-has-associated-control): add option for enforcing label's htmlFor matches control's id
This change adds an option to the `label-has-associated-control` rule, enforcing that the label's htmlFor attribute matches the associated control's id attribute. Previously, the only validation done on htmlFor was that it was on the label component and had text. There was no attempt to cross-check that value against any attribute on the associated control. Not, when the option is enabled, cases where they don't match will report. I also took the opportunity to update the error messages so that each assert type gets an error message with verbiage specific to the assertion. (not sure if this should be called out as a separate feature in the changelog?). Note: the current implementation only checks the first instance it finds of child component that matches each control component type. It assumes that there won't be any acceptable cases where a label would have multiple inputs nested beneath it. Let me know if that assumption doesn't hold. Closes:
1 parent bfb0e9e commit aff6aab

File tree

6 files changed

+418
-63
lines changed

6 files changed

+418
-63
lines changed

‎__tests__/src/rules/label-has-associated-control-test.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const errorMessages = {
2727
nesting: 'A form label must have an associated control as a descendant.',
2828
either: 'A form label must either have a valid htmlFor attribute or a control as a descendant.',
2929
both: 'A form label must have a valid htmlFor attribute and a control as a descendant.',
30+
htmlForShouldMatchId: 'A form label must have a htmlFor attribute that matches the id of the associated control.',
3031
};
3132
const expectedErrors = {};
3233
Object.keys(errorMessages).forEach((key) => {
@@ -58,6 +59,7 @@ const htmlForValid = [
5859
{ code: '<label htmlFor="js_id" aria-label="A label" />' },
5960
{ code: '<label htmlFor="js_id" aria-labelledby="A label" />' },
6061
{ code: '<div><label htmlFor="js_id">A label</label><input id="js_id" /></div>' },
62+
{ code: '<div><label htmlFor={inputId}>A label</label><input id={inputId} /></div>' },
6163
{ code: '<label for="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], settings: attributesSettings },
6264
{ code: '<label for="js_id" aria-label="A label" />', settings: attributesSettings },
6365
{ code: '<label for="js_id" aria-labelledby="A label" />', settings: attributesSettings },
@@ -128,6 +130,12 @@ const alwaysValid = [
128130
{ code: '<input type="hidden" />' },
129131
];
130132

133+
const shouldHtmlForMatchIdValid = [
134+
{ code: '<label htmlFor="js_id" aria-label="A label"><span><span><input id="js_id" /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }] },
135+
{ code: '<div><label htmlFor="js_id">A label</label><input id="js_id" /></div>', options: [{ shouldHtmlForMatchId: true }] },
136+
{ code: '<div><label htmlFor={inputId} aria-label="A label" /><input id={inputId} /></div>', options: [{ shouldHtmlForMatchId: true }] },
137+
];
138+
131139
const htmlForInvalid = (assertType) => {
132140
const expectedError = expectedErrors[assertType];
133141
return [
@@ -164,6 +172,13 @@ const nestingInvalid = (assertType) => {
164172
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
165173
];
166174
};
175+
const shouldHtmlForMatchIdInvalid = [
176+
{ code: '<label htmlFor="js_id" aria-label="A label"><span><span><input /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
177+
{ code: '<label htmlFor="js_id" aria-label="A label"><span><span><input name="js_id" /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
178+
{ code: '<div><label htmlFor="js_id">A label</label><input /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
179+
{ code: '<div><label htmlFor="js_id">A label</label><input name="js_id" /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
180+
{ code: '<div><label htmlFor={inputId} aria-label="A label" /><input name={inputId} /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
181+
];
167182

168183
const neverValid = (assertType) => {
169184
const expectedError = expectedErrors[assertType];
@@ -266,3 +281,21 @@ ruleTester.run(ruleName, rule, {
266281
assert: 'both',
267282
})).map(parserOptionsMapper),
268283
});
284+
285+
// shouldHtmlForMatchId
286+
ruleTester.run(ruleName, rule, {
287+
valid: parsers.all([].concat(
288+
...shouldHtmlForMatchIdValid,
289+
))
290+
.map(ruleOptionsMapperFactory({
291+
assert: 'htmlFor',
292+
}))
293+
.map(parserOptionsMapper),
294+
invalid: parsers.all([].concat(
295+
...shouldHtmlForMatchIdInvalid,
296+
))
297+
.map(ruleOptionsMapperFactory({
298+
assert: 'htmlFor',
299+
}))
300+
.map(parserOptionsMapper),
301+
});

‎__tests__/src/util/getChildComponent-test.js‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
import test from 'tape';
2+
3+
import getChildComponent from '../../../src/util/getChildComponent';
4+
import JSXAttributeMock from '../../../__mocks__/JSXAttributeMock';
5+
import JSXElementMock from '../../../__mocks__/JSXElementMock';
6+
import JSXExpressionContainerMock from '../../../__mocks__/JSXExpressionContainerMock';
7+
8+
test('mayContainChildComponent', (t) => {
9+
t.equal(
10+
getChildComponent(
11+
JSXElementMock('div', [], [
12+
JSXElementMock('div', [], [
13+
JSXElementMock('span', [], []),
14+
JSXElementMock('span', [], [
15+
JSXElementMock('span', [], []),
16+
JSXElementMock('span', [], [
17+
JSXElementMock('span', [], []),
18+
]),
19+
]),
20+
]),
21+
JSXElementMock('span', [], []),
22+
JSXElementMock('img', [
23+
JSXAttributeMock('src', 'some/path'),
24+
]),
25+
]),
26+
'FancyComponent',
27+
5,
28+
),
29+
undefined,
30+
'no FancyComponent returns undefined',
31+
);
32+
33+
t.test('contains an indicated component', (st) => {
34+
const inputMock = JSXElementMock('input');
35+
st.equal(
36+
getChildComponent(
37+
JSXElementMock('div', [], [
38+
inputMock,
39+
]),
40+
'input',
41+
),
42+
inputMock,
43+
'returns input',
44+
);
45+
46+
const FancyComponentMock = JSXElementMock('FancyComponent');
47+
st.equal(
48+
getChildComponent(
49+
JSXElementMock('div', [], [
50+
FancyComponentMock,
51+
]),
52+
'FancyComponent',
53+
),
54+
FancyComponentMock,
55+
'returns FancyComponent',
56+
);
57+
58+
st.equal(
59+
getChildComponent(
60+
JSXElementMock('div', [], [
61+
JSXElementMock('div', [], [
62+
JSXElementMock('FancyComponent'),
63+
]),
64+
]),
65+
'FancyComponent',
66+
),
67+
undefined,
68+
'FancyComponent is outside of default depth, should return undefined',
69+
);
70+
71+
st.equal(
72+
getChildComponent(
73+
JSXElementMock('div', [], [
74+
JSXElementMock('div', [], [
75+
FancyComponentMock,
76+
]),
77+
]),
78+
'FancyComponent',
79+
2,
80+
),
81+
FancyComponentMock,
82+
'FancyComponent is inside of custom depth, should return FancyComponent',
83+
);
84+
85+
st.equal(
86+
getChildComponent(
87+
JSXElementMock('div', [], [
88+
JSXElementMock('div', [], [
89+
JSXElementMock('span', [], []),
90+
JSXElementMock('span', [], [
91+
JSXElementMock('span', [], []),
92+
JSXElementMock('span', [], [
93+
JSXElementMock('span', [], [
94+
JSXElementMock('span', [], [
95+
FancyComponentMock,
96+
]),
97+
]),
98+
]),
99+
]),
100+
]),
101+
JSXElementMock('span', [], []),
102+
JSXElementMock('img', [
103+
JSXAttributeMock('src', 'some/path'),
104+
]),
105+
]),
106+
'FancyComponent',
107+
6,
108+
),
109+
FancyComponentMock,
110+
'deep nesting, returns FancyComponent',
111+
);
112+
113+
st.end();
114+
});
115+
116+
const MysteryBox = JSXExpressionContainerMock('mysteryBox');
117+
t.equal(
118+
getChildComponent(
119+
JSXElementMock('div', [], [
120+
MysteryBox,
121+
]),
122+
'FancyComponent',
123+
),
124+
MysteryBox,
125+
'Indeterminate situations + expression container children - returns component',
126+
);
127+
128+
t.test('Glob name matching - component name contains question mark ? - match any single character', (st) => {
129+
const FancyComponentMock = JSXElementMock('FancyComponent');
130+
st.equal(
131+
getChildComponent(
132+
JSXElementMock('div', [], [
133+
FancyComponentMock,
134+
]),
135+
'Fanc?Co??onent',
136+
),
137+
FancyComponentMock,
138+
'returns component',
139+
);
140+
141+
st.equal(
142+
getChildComponent(
143+
JSXElementMock('div', [], [
144+
JSXElementMock('FancyComponent'),
145+
]),
146+
'FancyComponent?',
147+
),
148+
undefined,
149+
'returns undefined',
150+
);
151+
152+
st.test('component name contains asterisk * - match zero or more characters', (s2t) => {
153+
s2t.equal(
154+
getChildComponent(
155+
JSXElementMock('div', [], [
156+
FancyComponentMock,
157+
]),
158+
'Fancy*',
159+
),
160+
FancyComponentMock,
161+
'returns component',
162+
);
163+
164+
s2t.equal(
165+
getChildComponent(
166+
JSXElementMock('div', [], [
167+
FancyComponentMock,
168+
]),
169+
'*Component',
170+
),
171+
FancyComponentMock,
172+
'returns component',
173+
);
174+
175+
s2t.equal(
176+
getChildComponent(
177+
JSXElementMock('div', [], [
178+
FancyComponentMock,
179+
]),
180+
'Fancy*C*t',
181+
),
182+
FancyComponentMock,
183+
'returns component',
184+
);
185+
186+
s2t.end();
187+
});
188+
189+
st.end();
190+
});
191+
192+
t.test('using a custom elementType function', (st) => {
193+
const CustomInputMock = JSXElementMock('CustomInput');
194+
st.equal(
195+
getChildComponent(
196+
JSXElementMock('div', [], [
197+
CustomInputMock,
198+
]),
199+
'input',
200+
2,
201+
() => 'input',
202+
),
203+
CustomInputMock,
204+
'returns the component when the custom elementType returns the proper name',
205+
);
206+
207+
st.equal(
208+
getChildComponent(
209+
JSXElementMock('div', [], [
210+
JSXElementMock('CustomInput'),
211+
]),
212+
'input',
213+
2,
214+
() => 'button',
215+
),
216+
undefined,
217+
'returns undefined when the custom elementType returns a wrong name',
218+
);
219+
220+
st.end();
221+
});
222+
223+
t.end();
224+
});

‎docs/rules/label-has-associated-control.md‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ The simplest way to achieve an association between a label and an input is to wr
2828

2929
All modern browsers and assistive technology will associate the label with the control.
3030

31-
### Case: The label is a sibling of the control.
31+
### Case: The label is a sibling of the control
3232

3333
In this case, use `htmlFor` and an ID to associate the controls.
3434

@@ -37,7 +37,7 @@ In this case, use `htmlFor` and an ID to associate the controls.
3737
<input type="text" id={domId} />
3838
```
3939

40-
### Case: My label and input components are custom components.
40+
### Case: My label and input components are custom components
4141

4242
You can configure the rule to be aware of your custom components.
4343

@@ -49,14 +49,14 @@ You can configure the rule to be aware of your custom components.
4949

5050
And the configuration:
5151

52-
```json
52+
```js
5353
{
54-
"rules": {
55-
"jsx-a11y/label-has-associated-control": [ 2, {
56-
"labelComponents": ["CustomInputLabel"],
57-
"labelAttributes": ["label"],
58-
"controlComponents": ["CustomInput"],
59-
"depth": 3,
54+
rules: {
55+
'jsx-a11y/label-has-associated-control': [ 2, {
56+
labelComponents: ['CustomInputLabel'],
57+
labelAttributes: ['label'],
58+
controlComponents: ['CustomInput'],
59+
depth: 3,
6060
}],
6161
}
6262
}
@@ -89,15 +89,16 @@ To restate, **every ID needs to be deterministic**, on the server and the client
8989

9090
This rule takes one optional object argument of type object:
9191

92-
```json
92+
```js
9393
{
94-
"rules": {
95-
"jsx-a11y/label-has-associated-control": [ 2, {
96-
"labelComponents": ["CustomLabel"],
97-
"labelAttributes": ["inputLabel"],
98-
"controlComponents": ["CustomInput"],
99-
"assert": "both",
100-
"depth": 3,
94+
rules: {
95+
'jsx-a11y/label-has-associated-control': [ 2, {
96+
labelComponents: ['CustomLabel'],
97+
labelAttributes: ['inputLabel'],
98+
controlComponents: ['CustomInput'],
99+
assert: 'both',
100+
depth: 3,
101+
shouldHtmlForMatchId: true,
101102
}],
102103
}
103104
}
@@ -113,14 +114,18 @@ This rule takes one optional object argument of type object:
113114

114115
`depth` (default 2, max 25) is an integer that determines how deep within a `JSXElement` label the rule should look for text content or an element with a label to determine if the `label` element will have an accessible label.
115116

117+
`shouldHtmlForMatchId` (default `false`) is a boolean dictating whether the `htmlFor` attribute on a label element should be validated as matching the `id` attributed on an associated control (nested or sibling as described in the cases above).
118+
116119
### Fail
120+
117121
```jsx
118122
function Foo(props) {
119123
return <label {...props} />
120124
}
121125
```
122126

123127
### Succeed
128+
124129
```jsx
125130
function Foo(props) {
126131
const {
@@ -133,12 +138,14 @@ function Foo(props) {
133138
```
134139

135140
### Fail
141+
136142
```jsx
137143
<input type="text" />
138144
<label>Surname</label>
139145
```
140146

141147
### Succeed
148+
142149
```jsx
143150
<label>
144151
<input type="text" />
@@ -147,6 +154,7 @@ function Foo(props) {
147154
```
148155

149156
## Accessibility guidelines
157+
150158
- [WCAG 1.3.1](https://www.w3.org/WAI/WCAG21/Understanding/info-and-relationships)
151159
- [WCAG 3.3.2](https://www.w3.org/WAI/WCAG21/Understanding/labels-or-instructions)
152160
- [WCAG 4.1.2](https://www.w3.org/WAI/WCAG21/Understanding/name-role-value)

0 commit comments

Comments
 (0)