Skip to content

Commit

Permalink
[fixed] bug where placeholder links required a tabindex and an ARIA r…
Browse files Browse the repository at this point in the history 
…ole (closes #62)

[fixed] bug where elements with role=none required a label (closes #63)
[fixed] bug where elements with aria-hidden required a label (closes #64)
[added] test to ensure interactive elements hidden using aria-hidden are removed from the tab flow
  • Loading branch information
Todd Kloots committed Jun 11, 2015
1 parent 9b90134 commit 4d1f885
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 22 deletions.
86 changes: 84 additions & 2 deletions lib/__tests__/index-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ describe('props', () => {
<div onClick={k} role="button"/>;
});
});

it('does not warn with no role and `aria-hidden="true"`', () => {
doNotExpectWarning(assertions.props.onClick.NO_ROLE.msg, () => {
<a aria-hidden="true" onClick={k}/>;
});
});
});

describe('tabIndex', () => {
Expand Down Expand Up @@ -98,6 +104,48 @@ describe('props', () => {
});
});
});

describe('aria-hidden', () => {
describe('when set to `true`', () => {
it('warns when applied to an interactive element without `tabIndex="-1"`', () => {
expectWarning(assertions.props['aria-hidden'].TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN.msg, () => {
<a aria-hidden="true" href="/foo"/>;
});
});

it('warns when applied to an interactive element with `tabIndex="0"`', () => {
expectWarning(assertions.props['aria-hidden'].TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN.msg, () => {
<a aria-hidden="true" tabIndex="0"/>;
});
});

it('does not warn when applied to a placeholder link', () => {
expectWarning(assertions.props['aria-hidden'].TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN.msg, () => {
<a aria-hidden="true"/>;
});
});

it('does not warn when applied to an interactive element with `tabIndex="-1"`', () => {
doNotExpectWarning(assertions.props['aria-hidden'].TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN.msg, () => {
<a aria-hidden="true" tabIndex="-1"/>;
});
});

it('does not warn when applied to a non-interactive element', () => {
doNotExpectWarning(assertions.props['aria-hidden'].TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN.msg, () => {
<div aria-hidden="true"/>;
});
});
});

describe('when set to `false`', () =>{
it('does not warn when applied to an interactive element with `tabIndex="-1"`', () => {
doNotExpectWarning(assertions.props['aria-hidden'].TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN.msg, () => {
<a aria-hidden="false" tabIndex="-1"/>;
});
});
});
});
});

describe('tags', () => {
Expand Down Expand Up @@ -132,6 +180,22 @@ describe('tags', () => {
});

describe('a', () => {
describe('placeholder links without href', () => {
it('does not warn', () => {
doNotExpectWarning(assertions.tags.a.HASH_HREF_NEEDS_BUTTON.msg, () => {
<a class="foo" />;
});
});
});

describe('placeholder links without tabindex', () => {
it('does not warn', () => {
doNotExpectWarning(assertions.tags.a.TABINDEX_NEEDS_BUTTON.msg, () => {
<a class="foo" />;
});
});
});

describe('with [href="#"]', () => {
it('warns', () => {
expectWarning(assertions.tags.a.HASH_HREF_NEEDS_BUTTON.msg, () => {
Expand Down Expand Up @@ -194,9 +258,27 @@ describe('labels', () => {
});
});

it('does not warn when the ARIA role is presentation', () => {
it('does not warn when `role="presentation"`', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<span role="presentation" />;
<img role="presentation" />;
});
});

it('does not warn when `role="none"`', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<img role="none" />;
});
});

it('does not warn when `aria-hidden="true"`', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<button aria-hidden="true" />;
});
});

it('warns when `aria-hidden="false"`', () => {
expectWarning(assertions.render.NO_LABEL.msg, () => {
<button aria-hidden="false" />;
});
});

Expand Down
70 changes: 57 additions & 13 deletions lib/assertions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ var INTERACTIVE = {
}
};

const presentationRoles = new Set(['presentation', 'none']);

var isHiddenFromAT = (props) => {
return props['aria-hidden'] == 'true';
};

var hasAlt = (props) => {
return typeof props.alt === 'string';
};
Expand Down Expand Up @@ -105,18 +111,31 @@ var hasChildTextNode = (props, children, failureCB) => {
return hasText;
};

exports.mobileExclusions = [
'NO_TABINDEX',
'BUTTON_ROLE_SPACE',
'BUTTON_ROLE_ENTER',
'TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN'
];

exports.props = {
onClick: {
NO_ROLE: {
msg: 'You have a click handler on a non-interactive element but no `role` DOM property. It will be unclear what this element is supposed to do to a screen-reader user. http://www.w3.org/TR/wai-aria/roles#role_definitions',
test (tagName, props, children) {
if (isHiddenFromAT(props))
return true;

return !(!isInteractive(tagName, props) && !props.role);
}
},

NO_TABINDEX: {
msg: 'You have a click handler on a non-interactive element but no `tabIndex` DOM property. The element will not be navigable or interactive by keyboard users. http://www.w3.org/TR/wai-aria-practices/#focus_tabindex',
test (tagName, props, children) {
if (isHiddenFromAT(props))
return true;

return !(
!isInteractive(tagName, props) &&
props.tabIndex == null // tabIndex={0} is valid
Expand All @@ -127,17 +146,35 @@ exports.props = {
BUTTON_ROLE_SPACE: {
msg: 'You have `role="button"` but did not define an `onKeyDown` handler. Add it, and have the "Space" key do the same thing as an `onClick` handler.',
test (tagName, props, children) {
if (isHiddenFromAT(props))
return true;

return !(props.role === 'button' && !props.onKeyDown);
}
},

BUTTON_ROLE_ENTER: {
msg: 'You have `role="button"` but did not define an `onKeyDown` handler. Add it, and have the "Enter" key do the same thing as an `onClick` handler.',
test (tagName, props, children) {
if (isHiddenFromAT(props))
return true;

return !(props.role === 'button' && !props.onKeyDown);
}
}
},

'aria-hidden': {
'TABINDEX_REQUIRED_WHEN_ARIA_HIDDEN': {
msg: 'You have `aria-hidden="true"` applied to an interactive element but have not removed it from the tab flow. This could result in a hidden tab stop for users of screen readers.',
test (tagName, props, children) {
return !(
(isInteractive(tagName, props) || (tagName == 'a' && !props.href)) &&
props['aria-hidden'] == 'true' &&
props.tabIndex != '-1'
);
}
}
}
};

Expand All @@ -146,13 +183,19 @@ exports.tags = {
HASH_HREF_NEEDS_BUTTON: {
msg: 'You have an anchor with `href="#"` and no `role` DOM property. Add `role="button"` or better yet, use a `<button/>`.',
test (tagName, props, children) {
if (isHiddenFromAT(props))
return true;

return !(!props.role && props.href === '#');
}
},
TABINDEX_NEEDS_BUTTON: {
msg: 'You have an anchor with a tabIndex, no `href` and no `role` DOM property. Add `role="button"` or better yet, use a `<button/>`.',
test (tagName, props, children) {
return !(!props.role && props.tabIndex !== null && !props.href);
if (isHiddenFromAT(props))
return true;

return !(!props.role && props.tabIndex != null && !props.href);
}
}
},
Expand All @@ -161,6 +204,9 @@ exports.tags = {
MISSING_ALT: {
msg: 'You forgot an `alt` DOM property on an image. Screen-reader users will not know what it is.',
test (tagName, props, children) {
if (isHiddenFromAT(props))
return true;

return hasAlt(props);
}
},
Expand All @@ -169,6 +215,9 @@ exports.tags = {
// TODO: have some way to set localization strings to match against
msg: 'Screen-readers already announce `img` tags as an image, you don\'t need to use the word "image" in the description',
test (tagName, props, children) {
if (isHiddenFromAT(props))
return true;

return !(hasAlt(props) && props.alt.match('image'));
}
}
Expand All @@ -179,22 +228,17 @@ exports.render = {
NO_LABEL: {
msg: 'You have an unlabled element or control. Add `aria-label` or `aria-labelled-by` attribute, or put some text in the element.',
test (tagName, props, children, failureCB) {
var labelRequired = (
isInteractive(tagName, props) ||
props.role && props.role != 'presentation'
);
if (isHiddenFromAT(props) || presentationRoles.has(props.role))
return;

if (!labelRequired)
if (!(isInteractive(tagName, props) || props.role))
return;

var failed = !(
labelRequired &&
(
props['aria-label'] ||
props['aria-labelled-by'] ||
(tagName === 'img' && props.alt) ||
hasChildTextNode(props, children, failureCB)
)
props['aria-label'] ||
props['aria-labelled-by'] ||
(tagName === 'img' && props.alt) ||
hasChildTextNode(props, children, failureCB)
);

if (failed)
Expand Down
8 changes: 1 addition & 7 deletions lib/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
var assertions = require('./assertions');
var after = require('./after');

const mobileExclusions = [
'NO_TABINDEX',
'BUTTON_ROLE_SPACE',
'BUTTON_ROLE_ENTER'
];

var shouldRunTest = (testName, options) => {
var exclude = options.exclude || [];

if (options.device == 'mobile') {
exclude = new Set(exclude.concat(mobileExclusions));
exclude = new Set(exclude.concat(assertions.mobileExclusions));
exclude = [...exclude];
}

Expand Down

0 comments on commit 4d1f885

Please sign in to comment.