Skip to content

Lots of bug fixes #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kloots merged 1 commit into from
Jun 11, 2015
Merged

Lots of bug fixes #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 84 additions & 2 deletions lib/__tests__/index-test.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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