Skip to content

Commit 6d26aa2

Browse files
lencionibeefancohen
lencioni
authored and
beefancohen
committed
Allow alt="" or role="presentation" (#22)
* Allow alt="" or role="presentation" As we discussed on #6, it is okay to use `alt=""` without `role="presentation"`. It is possible that we want to enforce no alt prop if `role="presentation"` but I'll leave that for another time and possibly a different rule. * Slightly simplify img-uses-alt error tests We don't really get much value out of having two ways to check for errors. Let's just use the one function everywhere.
1 parent dbc9773 commit 6d26aa2

File tree

4 files changed

+59
-64
lines changed

4 files changed

+59
-64
lines changed

‎docs/rules/img-uses-alt.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,16 @@ To tell this plugin to also check your `Image` element, specify this in your `.e
4141
}
4242
```
4343

44-
Note that passing props as spread attribute without `alt` explicitly defined will cause this rule to fail. Explicitly pass down `alt` prop for rule to pass. The prop must have an actual value to pass. Use `Image` component above as a reference for destructuring and applying the prop. **It is a good thing to explicitly pass props that you expect to be passed for self-documentation.**
44+
Note that passing props as spread attribute without `alt` explicitly defined will cause this rule to fail. Explicitly pass down `alt` prop or use `role="presentation"` for rule to pass. Use `Image` component above as a reference for destructuring and applying the prop. **It is a good thing to explicitly pass props that you expect to be passed for self-documentation.**
4545

4646
### Succeed
4747
```jsx
4848
<img src="foo" alt="Foo eating a sandwich." />
4949
<img src="foo" alt={"Foo eating a sandwich."} />
5050
<img src="foo" alt={altText} />
5151
<img src="foo" alt={`${person} smiling`} />
52-
<img src="foo" alt="" role="presentation" /> <!-- Alt text can be an empty string if `role="presentation"` -->
52+
<img src="foo" alt="" />
53+
<img src="foo" role="presentation" />
5354
```
5455

5556
### Fail

‎src/rules/img-uses-alt.js‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,39 +22,38 @@ module.exports = context => ({
2222
return;
2323
}
2424

25+
const hasRoleProp = hasAttribute(node.attributes, 'role');
26+
const roleValue = getAttributeValue(hasRoleProp);
27+
const isPresentation = hasRoleProp && roleValue.toLowerCase() === 'presentation';
28+
29+
if (isPresentation) {
30+
return;
31+
}
32+
2533
const hasAltProp = hasAttribute(node.attributes, 'alt');
2634

2735
// Missing alt prop error.
2836
if (!hasAltProp) {
2937
context.report({
3038
node,
31-
message: `${nodeType} elements must have an alt prop.`
39+
message: `${nodeType} elements must have an alt prop or use role="presentation".`
3240
});
3341
return;
3442
}
3543

3644
// Check if alt prop is undefined.
37-
const altProp = getAttributeValue(hasAltProp);
38-
39-
// Check if alt prop is ""
40-
const emptyAlt = hasAltProp && hasAltProp.value
41-
&& hasAltProp.value.type === 'Literal'
42-
&& hasAltProp.value.value === "";
43-
44-
const hasRoleProp = hasAttribute(node.attributes, 'role');
45-
const roleProp = getAttributeValue(hasRoleProp);
45+
const altValue = getAttributeValue(hasAltProp);
4646

47-
// Allow altProp to be "" if `role="presentation"` is present.
48-
const isValid = altProp || (emptyAlt && hasRoleProp && roleProp.toUpperCase() === 'PRESENTATION');
49-
50-
// Undefined alt prop error.
51-
if (!isValid) {
52-
context.report({
53-
node,
54-
message: `${nodeType} alt prop must have a value. You can set alt="" if role="presentation" is applied.`
55-
});
47+
if (altValue || altValue === '') {
5648
return;
5749
}
50+
51+
// Undefined alt prop error.
52+
context.report({
53+
node,
54+
message:
55+
`Invalid alt value for ${nodeType}. Use alt="" or role="presentation" for presentational images.`
56+
});
5857
}
5958
});
6059

‎src/util/getAttributeValue.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import buildTemplateLiteral from './buildTemplateLiteral';
44

55
const getValue = value => {
66
if (value.type === 'Literal') {
7-
return value.value === "" ? undefined : value.value;
7+
return value.value;
88
} else if (value.type === 'Identifier') {
99
return value.name === "" ? undefined : value.name;
1010
} else if (value.type === 'JSXElement') {
@@ -17,7 +17,7 @@ const getValue = value => {
1717

1818
switch (type) {
1919
case 'Literal':
20-
return obj.value === "" ? undefined : obj.value;
20+
return obj.value;
2121
case 'TemplateLiteral':
2222
return buildTemplateLiteral(obj);
2323
case 'Identifier':

‎tests/src/rules/img-uses-alt.js‎

Lines changed: 36 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,16 @@ const parserOptions = {
2525

2626
const ruleTester = new RuleTester();
2727

28-
const customMissingPropError = type => ({
29-
message: `${type} elements must have an alt prop.`,
28+
const missingPropError = type => ({
29+
message: `${type} elements must have an alt prop or use role="presentation".`,
3030
type: 'JSXOpeningElement'
3131
});
3232

33-
const customAltValueError = type => ({
34-
message: `${type} alt prop must have a value. You can set alt="" if role="presentation" is applied.`,
33+
const altValueError = type => ({
34+
message: `Invalid alt value for ${type}. Use alt="" or role="presentation" for presentational images.`,
3535
type: 'JSXOpeningElement'
3636
});
3737

38-
const expectedMissingPropError = customMissingPropError('img');
39-
const expectedAltValueError = customAltValueError('img');
40-
4138
const string = [ 'Avatar' ];
4239
const array = [ [ 'Thumbnail', 'Image' ] ];
4340

@@ -65,11 +62,15 @@ ruleTester.run('img-uses-alt', rule, {
6562
{ code: '<img alt={foo.bar || ""} />', parserOptions },
6663
{ code: '<img alt={bar() || ""} />', parserOptions },
6764
{ code: '<img alt={foo.bar() || ""} />', parserOptions },
68-
{ code: '<img alt="" role="presentation" />', parserOptions }, // Allow alt to be undefined if role="presentation"
65+
{ code: '<img alt="" />', parserOptions },
66+
{ code: '<img alt=" " />', parserOptions },
67+
{ code: '<img alt="" role="presentation" />', parserOptions },
6968
{ code: '<img alt="" role={`presentation`} />', parserOptions },
7069
{ code: '<img alt="" role={"presentation"} />', parserOptions },
70+
{ code: '<img role="presentation" />;', parserOptions },
71+
{ code: '<img alt={undefined} role="presentation" />;', parserOptions },
72+
{ code: '<img alt role="presentation" />;', parserOptions },
7173
{ code: '<img alt="this is lit..." role="presentation" />', parserOptions },
72-
{ code: '<img alt=" " />', parserOptions }, // For decorative images.
7374

7475
// CUSTOM ELEMENT TESTS FOR STRING OPTION
7576
{ code: '<Avatar alt="foo" />;', options: string, parserOptions },
@@ -86,6 +87,7 @@ ruleTester.run('img-uses-alt', rule, {
8687
{ code: '<Avatar alt={() => void 0} />', options: string, parserOptions },
8788
{ code: '<AVATAR />', options: string, parserOptions },
8889
{ code: '<Avatar alt={alt || "foo" } />', options: string, parserOptions },
90+
{ code: '<Avatar alt="" />', options: string, parserOptions },
8991

9092
// CUSTOM ELEMENT TESTS FOR ARRAY OPTION TESTS
9193
{ code: '<Thumbnail alt="foo" />;', options: array, parserOptions },
@@ -119,66 +121,59 @@ ruleTester.run('img-uses-alt', rule, {
119121
],
120122
invalid: [
121123
// DEFAULT ELEMENT 'img' TESTS
122-
{ code: '<img />;', errors: [ expectedMissingPropError ], parserOptions },
123-
{ code: '<img alt />;', errors: [ expectedAltValueError ], parserOptions },
124-
{ code: '<img alt={undefined} />;', errors: [ expectedAltValueError ], parserOptions },
125-
{ code: '<img alt={`${undefined}`} />;', errors: [ expectedAltValueError ], parserOptions },
126-
{ code: '<img alt="" />;', errors: [ expectedAltValueError ], parserOptions },
127-
{ code: '<img role="presentation" />;', errors: [ expectedMissingPropError ], parserOptions },
128-
{ code: '<img alt={undefined} role="presentation" />;', errors: [ expectedAltValueError ], parserOptions },
129-
{ code: '<img alt role="presentation" />;', errors: [ expectedAltValueError ], parserOptions },
130-
{ code: '<img src="xyz" />', errors: [ expectedMissingPropError ], parserOptions },
131-
{ code: '<img {...this.props} />', errors: [ expectedMissingPropError ], parserOptions },
132-
{ code: '<img alt={false || false} />', errors: [ expectedAltValueError ], parserOptions },
124+
{ code: '<img />;', errors: [ missingPropError('img') ], parserOptions },
125+
{ code: '<img alt />;', errors: [ altValueError('img') ], parserOptions },
126+
{ code: '<img alt={undefined} />;', errors: [ altValueError('img') ], parserOptions },
127+
{ code: '<img alt={`${undefined}`} />;', errors: [ altValueError('img') ], parserOptions },
128+
{ code: '<img src="xyz" />', errors: [ missingPropError('img') ], parserOptions },
129+
{ code: '<img {...this.props} />', errors: [ missingPropError('img') ], parserOptions },
130+
{ code: '<img alt={false || false} />', errors: [ altValueError('img') ], parserOptions },
133131

134132
// CUSTOM ELEMENT TESTS FOR STRING OPTION
135133
{
136134
code: '<Avatar />;',
137-
errors: [ customMissingPropError('Avatar') ],
135+
errors: [ missingPropError('Avatar') ],
138136
options: string,
139137
parserOptions
140138
},
141-
{ code: '<Avatar alt />;', errors: [ customAltValueError('Avatar') ], options: string, parserOptions },
142-
{ code: '<Avatar alt={undefined} />;', errors: [ customAltValueError('Avatar') ], options: string, parserOptions },
139+
{ code: '<Avatar alt />;', errors: [ altValueError('Avatar') ], options: string, parserOptions },
140+
{ code: '<Avatar alt={undefined} />;', errors: [ altValueError('Avatar') ], options: string, parserOptions },
143141
{
144142
code: '<Avatar alt={`${undefined}`} />;',
145-
errors: [ customAltValueError('Avatar') ],
143+
errors: [ altValueError('Avatar') ],
146144
options: string,
147145
parserOptions
148146
},
149-
{ code: '<Avatar alt="" />;', errors: [ customAltValueError('Avatar') ], options: string, parserOptions },
150-
{ code: '<Avatar src="xyz" />', errors: [ customMissingPropError('Avatar') ], options: string, parserOptions },
151-
{ code: '<Avatar {...this.props} />', errors: [ customMissingPropError('Avatar') ], options: string, parserOptions },
147+
{ code: '<Avatar src="xyz" />', errors: [ missingPropError('Avatar') ], options: string, parserOptions },
148+
{ code: '<Avatar {...this.props} />', errors: [ missingPropError('Avatar') ], options: string, parserOptions },
152149

153150
// CUSTOM ELEMENT TESTS FOR ARRAY OPTION TESTS
154-
{ code: '<Thumbnail />;', errors: [ customMissingPropError('Thumbnail') ], options: array, parserOptions },
155-
{ code: '<Thumbnail alt />;', errors: [ customAltValueError('Thumbnail') ], options: array, parserOptions },
151+
{ code: '<Thumbnail />;', errors: [ missingPropError('Thumbnail') ], options: array, parserOptions },
152+
{ code: '<Thumbnail alt />;', errors: [ altValueError('Thumbnail') ], options: array, parserOptions },
156153
{
157154
code: '<Thumbnail alt={undefined} />;',
158-
errors: [ customAltValueError('Thumbnail') ],
155+
errors: [ altValueError('Thumbnail') ],
159156
options: array,
160157
parserOptions
161158
},
162159
{
163160
code: '<Thumbnail alt={`${undefined}`} />;',
164-
errors: [ customAltValueError('Thumbnail') ],
161+
errors: [ altValueError('Thumbnail') ],
165162
options: array,
166163
parserOptions
167164
},
168-
{ code: '<Thumbnail alt="" />;', errors: [ customAltValueError('Thumbnail') ], options: array, parserOptions },
169-
{ code: '<Thumbnail src="xyz" />', errors: [ customMissingPropError('Thumbnail') ], options: array, parserOptions },
165+
{ code: '<Thumbnail src="xyz" />', errors: [ missingPropError('Thumbnail') ], options: array, parserOptions },
170166
{
171167
code: '<Thumbnail {...this.props} />',
172-
errors: [ customMissingPropError('Thumbnail') ],
168+
errors: [ missingPropError('Thumbnail') ],
173169
options: array,
174170
parserOptions
175171
},
176-
{ code: '<Image />;', errors: [ customMissingPropError('Image') ], options: array, parserOptions },
177-
{ code: '<Image alt />;', errors: [ customAltValueError('Image') ], options: array, parserOptions },
178-
{ code: '<Image alt={undefined} />;', errors: [ customAltValueError('Image') ], options: array, parserOptions },
179-
{ code: '<Image alt={`${undefined}`} />;', errors: [ customAltValueError('Image') ], options: array, parserOptions },
180-
{ code: '<Image alt="" />;', errors: [ customAltValueError('Image') ], options: array, parserOptions },
181-
{ code: '<Image src="xyz" />', errors: [ customMissingPropError('Image') ], options: array, parserOptions },
182-
{ code: '<Image {...this.props} />', errors: [ customMissingPropError('Image') ], options: array, parserOptions }
172+
{ code: '<Image />;', errors: [ missingPropError('Image') ], options: array, parserOptions },
173+
{ code: '<Image alt />;', errors: [ altValueError('Image') ], options: array, parserOptions },
174+
{ code: '<Image alt={undefined} />;', errors: [ altValueError('Image') ], options: array, parserOptions },
175+
{ code: '<Image alt={`${undefined}`} />;', errors: [ altValueError('Image') ], options: array, parserOptions },
176+
{ code: '<Image src="xyz" />', errors: [ missingPropError('Image') ], options: array, parserOptions },
177+
{ code: '<Image {...this.props} />', errors: [ missingPropError('Image') ], options: array, parserOptions }
183178
]
184179
});

0 commit comments

Comments
 (0)