Skip to content

Commit b6fad78

Browse files
authored
fix: checkbox a11y (#1116)
* fix: remove aria-checked, fix multiinput id changing every render * fix: storyshots * fix: indeterminate * fix: checked and defaultChecked stories * fix: tests
1 parent 63de9ea commit b6fad78

File tree

7 files changed

+66
-44
lines changed

7 files changed

+66
-44
lines changed

src/Forms/Checkbox.js

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,6 @@ import shortId from '../utils/shortId';
77
import React, { useEffect, useRef } from 'react';
88
import 'fundamental-styles/dist/checkbox.css';
99

10-
const getCheckStatus = (checked, indeterminate) => {
11-
if (indeterminate) {
12-
return 'mixed';
13-
} else if (checked) {
14-
return 'true';
15-
} else {
16-
return 'false';
17-
}
18-
};
19-
2010
/** With a **Checkbox**, all options are visible and the user can make one or more selections.
2111
This component can also be disabled and displayed in a row */
2212

@@ -71,9 +61,9 @@ const Checkbox = React.forwardRef(({
7161
ref={ref}>
7262
<input
7363
{...inputProps}
74-
aria-checked={getCheckStatus(checked, indeterminate)}
75-
checked={checked || defaultChecked}
64+
checked={checked}
7665
className={classes}
66+
defaultChecked={defaultChecked}
7767
disabled={disabled}
7868
id={checkId}
7969
name={name}

src/Forms/Checkbox.test.js

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ describe('<Checkbox />', () => {
1414
onChange: () => {}
1515
});
1616

17-
expect(element.find('input').props().checked).toBe(true);
18-
expect(element.find('input').props()['aria-checked']).toBe('true');
17+
expect(element.find('input').getDOMNode().checked).toBe(true);
1918
});
2019

2120
test('should add checked attribute if defaultChecked is true', () => {
@@ -24,29 +23,24 @@ describe('<Checkbox />', () => {
2423
onChange: () => {}
2524
});
2625

27-
expect(element.find('input').props().checked).toBe(true);
28-
29-
// TODO: The aria-checked does not reflect the current state of the checkbox if it is uncontrolled
30-
// expect(element.find('input').props()['aria-checked']).toBe('true');
26+
expect(element.find('input').getDOMNode().checked).toBe(true);
3127
});
3228

33-
test('should add aria-checked attribute of "mixed" if indeterminate is true', () => {
29+
test('should add indeterminate property when indeterminate is passed', () => {
3430
let element = setup({
3531
indeterminate: true,
36-
defaultChecked: true,
3732
onChange: () => {}
3833
});
3934

40-
expect(element.find('input').props().checked).toBe(true);
41-
expect(element.find('input').props()['aria-checked']).toBe('mixed');
35+
expect(element.find('input').getDOMNode().indeterminate).toBe(true);
4236
});
4337

4438
test('should set disabled to true when passed', () => {
4539
let element = setup({
4640
disabled: true
4741
});
4842

49-
expect(element.find('input').props().disabled).toBe(true);
43+
expect(element.find('input').getDOMNode().disabled).toBe(true);
5044
});
5145

5246
test('should add inline class when inline is passed', () => {

src/Forms/__stories__/Checkbox.stories.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ export const primary = () => (<Checkbox>Default Checkbox</Checkbox>);
1616
export const indeterminate = () => (
1717
<Checkbox indeterminate>Text Option</Checkbox>
1818
);
19+
export const controlledChecked = () => (
20+
<Checkbox checked={boolean('checked (controlled)', false)}>Text Option</Checkbox>
21+
);
22+
export const defaultChecked = () => (
23+
<Checkbox defaultChecked>Text Option</Checkbox>
24+
);
1925
export const disabled = () => (
2026
<Checkbox disabled>Text Option</Checkbox>
2127
);
@@ -46,5 +52,5 @@ export const dev = () => (
4652
})}>Text Option</Checkbox>
4753
);
4854

49-
55+
controlledChecked.parameters = { docs: { disable: true } };
5056
dev.parameters = { docs: { disable: true } };

src/Forms/__stories__/__snapshots__/Checkbox.stories.storyshot

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ exports[`Storyshots Component API/Forms/Checkbox Compact 1`] = `
88
className="fd-form-item"
99
>
1010
<input
11-
aria-checked="false"
1211
className="fd-checkbox fd-checkbox--compact"
1312
id="fd-mocked-short-id"
1413
onChange={[Function]}
@@ -24,6 +23,54 @@ exports[`Storyshots Component API/Forms/Checkbox Compact 1`] = `
2423
</div>
2524
`;
2625

26+
exports[`Storyshots Component API/Forms/Checkbox Controlled Checked 1`] = `
27+
<div
28+
dir="ltr"
29+
>
30+
<div
31+
className="fd-form-item"
32+
>
33+
<input
34+
checked={false}
35+
className="fd-checkbox"
36+
id="fd-mocked-short-id"
37+
onChange={[Function]}
38+
type="checkbox"
39+
/>
40+
<label
41+
className="fd-form-label fd-checkbox__label"
42+
htmlFor="fd-mocked-short-id"
43+
>
44+
Text Option
45+
</label>
46+
</div>
47+
</div>
48+
`;
49+
50+
exports[`Storyshots Component API/Forms/Checkbox Default Checked 1`] = `
51+
<div
52+
dir="ltr"
53+
>
54+
<div
55+
className="fd-form-item"
56+
>
57+
<input
58+
className="fd-checkbox"
59+
defaultChecked={true}
60+
id="fd-mocked-short-id"
61+
onChange={[Function]}
62+
type="checkbox"
63+
/>
64+
<label
65+
className="fd-form-label fd-checkbox__label"
66+
htmlFor="fd-mocked-short-id"
67+
>
68+
Text Option
69+
</label>
70+
</div>
71+
</div>
72+
`;
73+
2774
exports[`Storyshots Component API/Forms/Checkbox Dev 1`] = `
2875
<div
2976
dir="ltr"
@@ -33,7 +80,6 @@ exports[`Storyshots Component API/Forms/Checkbox Dev 1`] = `
3380
disabled={false}
3481
>
3582
<input
36-
aria-checked="false"
3783
className="fd-checkbox"
3884
disabled={false}
3985
id="fd-mocked-short-id"
@@ -59,7 +105,6 @@ exports[`Storyshots Component API/Forms/Checkbox Disabled 1`] = `
59105
disabled={true}
60106
>
61107
<input
62-
aria-checked="false"
63108
className="fd-checkbox"
64109
disabled={true}
65110
id="fd-mocked-short-id"
@@ -84,7 +129,6 @@ exports[`Storyshots Component API/Forms/Checkbox Indeterminate 1`] = `
84129
className="fd-form-item"
85130
>
86131
<input
87-
aria-checked="mixed"
88132
className="fd-checkbox"
89133
id="fd-mocked-short-id"
90134
onChange={[Function]}
@@ -108,7 +152,6 @@ exports[`Storyshots Component API/Forms/Checkbox Primary 1`] = `
108152
className="fd-form-item"
109153
>
110154
<input
111-
aria-checked="false"
112155
className="fd-checkbox"
113156
id="fd-mocked-short-id"
114157
onChange={[Function]}
@@ -135,7 +178,6 @@ exports[`Storyshots Component API/Forms/Checkbox Validation State 1`] = `
135178
className="fd-form-item"
136179
>
137180
<input
138-
aria-checked="false"
139181
className="fd-checkbox is-error"
140182
id="fd-mocked-short-id"
141183
onChange={[Function]}
@@ -152,7 +194,6 @@ exports[`Storyshots Component API/Forms/Checkbox Validation State 1`] = `
152194
className="fd-form-item"
153195
>
154196
<input
155-
aria-checked="false"
156197
className="fd-checkbox is-warning"
157198
id="fd-mocked-short-id"
158199
onChange={[Function]}
@@ -169,7 +210,6 @@ exports[`Storyshots Component API/Forms/Checkbox Validation State 1`] = `
169210
className="fd-form-item"
170211
>
171212
<input
172-
aria-checked="false"
173213
className="fd-checkbox is-success"
174214
id="fd-mocked-short-id"
175215
onChange={[Function]}
@@ -186,7 +226,6 @@ exports[`Storyshots Component API/Forms/Checkbox Validation State 1`] = `
186226
className="fd-form-item"
187227
>
188228
<input
189-
aria-checked="false"
190229
className="fd-checkbox is-information"
191230
id="fd-mocked-short-id"
192231
onChange={[Function]}

src/List/__stories__/__snapshots__/List.stories.storyshot

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,6 @@ exports[`Storyshots Component API/List Selection 1`] = `
713713
className="fd-form-item fd-form-item--inline"
714714
>
715715
<input
716-
aria-checked="false"
717716
className="fd-checkbox"
718717
id="fd-mocked-short-id"
719718
onChange={[Function]}
@@ -743,7 +742,6 @@ exports[`Storyshots Component API/List Selection 1`] = `
743742
className="fd-form-item fd-form-item--inline"
744743
>
745744
<input
746-
aria-checked="false"
747745
className="fd-checkbox"
748746
id="fd-mocked-short-id"
749747
onChange={[Function]}
@@ -773,7 +771,6 @@ exports[`Storyshots Component API/List Selection 1`] = `
773771
className="fd-form-item fd-form-item--inline"
774772
>
775773
<input
776-
aria-checked="false"
777774
className="fd-checkbox"
778775
id="fd-mocked-short-id"
779776
onChange={[Function]}
@@ -803,7 +800,6 @@ exports[`Storyshots Component API/List Selection 1`] = `
803800
className="fd-form-item fd-form-item--inline"
804801
>
805802
<input
806-
aria-checked="false"
807803
className="fd-checkbox"
808804
id="fd-mocked-short-id"
809805
onChange={[Function]}

src/MultiInput/MultiInput.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ class MultiInput extends Component {
2525
bShowList: false,
2626
tags: []
2727
};
28+
29+
this.multiInputId = shortid.generate();
2830
}
2931

3032
// create tags to display in dropdown list
@@ -35,8 +37,8 @@ class MultiInput extends Component {
3537
checked={this.isChecked(item)}
3638
className='fd-list__input'
3739
compact={this.props.compact}
38-
id={index + `_${shortid.generate()}`}
39-
labelClasses='fd-list__label'
40+
id={index + `_${this.multiInputId}`}
41+
labelClassName='fd-list__label'
4042
onChange={() => this.updateSelectedTags(item)}
4143
value={item}>
4244
<List.Text>{item}</List.Text>

src/Table/__stories__/__snapshots__/Table.stories.storyshot

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ exports[`Storyshots Component API/Table Rich Table 1`] = `
111111
className="fd-form-item"
112112
>
113113
<input
114-
aria-checked="false"
115114
className="fd-checkbox"
116115
id="fd-mocked-short-id"
117116
onChange={[Function]}
@@ -168,7 +167,6 @@ exports[`Storyshots Component API/Table Rich Table 1`] = `
168167
className="fd-form-item"
169168
>
170169
<input
171-
aria-checked="false"
172170
className="fd-checkbox"
173171
id="fd-mocked-short-id"
174172
onChange={[Function]}
@@ -258,7 +256,6 @@ exports[`Storyshots Component API/Table Rich Table 1`] = `
258256
className="fd-form-item"
259257
>
260258
<input
261-
aria-checked="false"
262259
className="fd-checkbox"
263260
id="fd-mocked-short-id"
264261
onChange={[Function]}
@@ -348,7 +345,6 @@ exports[`Storyshots Component API/Table Rich Table 1`] = `
348345
className="fd-form-item"
349346
>
350347
<input
351-
aria-checked="false"
352348
className="fd-checkbox"
353349
id="fd-mocked-short-id"
354350
onChange={[Function]}
@@ -438,7 +434,6 @@ exports[`Storyshots Component API/Table Rich Table 1`] = `
438434
className="fd-form-item"
439435
>
440436
<input
441-
aria-checked="false"
442437
className="fd-checkbox"
443438
id="fd-mocked-short-id"
444439
onChange={[Function]}

0 commit comments

Comments
 (0)