Skip to content

Commit

Permalink
[fixed] Label test should require a label for placeholder anchors (re…
Browse files Browse the repository at this point in the history 
…solves #68)

[fixed] Label assertion errors if label is a number (resolves #67)
  • Loading branch information
Todd Kloots committed Jun 12, 2015
1 parent 9e8b71a commit 598c8f8
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 27 deletions.
58 changes: 34 additions & 24 deletions lib/__tests__/index-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,39 +183,39 @@ describe('tags', () => {
describe('placeholder links without href', () => {
it('does not warn', () => {
doNotExpectWarning(assertions.tags.a.HASH_HREF_NEEDS_BUTTON.msg, () => {
<a class="foo" />;
<a class="foo"/>;
});
});
});

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

describe('with [href="#"]', () => {
it('warns', () => {
expectWarning(assertions.tags.a.HASH_HREF_NEEDS_BUTTON.msg, () => {
<a onClick={k} href="#" />;
<a onClick={k} href="#"/>;
});
});
});

describe('with [tabIndex="0"] and no href', () => {
it('warns', () => {
expectWarning(assertions.tags.a.TABINDEX_NEEDS_BUTTON.msg, () => {
<a onClick={k} tabIndex="0" />;
<a onClick={k} tabIndex="0"/>;
});
});
});

describe('with a real href', () => {
it('does not warn', () => {
doNotExpectWarning(assertions.tags.a.HASH_HREF_NEEDS_BUTTON.msg, () => {
<a onClick={k} href="/foo/bar" />;
<a onClick={k} href="/foo/bar"/>;
});
});
});
Expand Down Expand Up @@ -248,55 +248,67 @@ describe('labels', () => {

it('warns if there is no label on an interactive element', () => {
expectWarning(assertions.render.NO_LABEL.msg, () => {
<button />;
<button/>;
});
});

it('warns if there is no label on a placeholder link', () => {
expectWarning(assertions.render.NO_LABEL.msg, () => {
<a/>;
});
});

it('does not warn when a placeholder link has a label', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<a>foo</a>;
});
});

it('warns if there is no label on an element with an ARIA role', () => {
expectWarning(assertions.render.NO_LABEL.msg, () => {
<span role="button" />;
<span role="button"/>;
});
});

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

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

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

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

it('does not warn if the element is not interactive', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<div />;
<div/>;
});
});

it('does not warn if there is an aria-label', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<button aria-label="foo" />;
<button aria-label="foo"/>;
});
});

it('does not warn if there is an aria-labelled-by', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<button aria-labelled-by="foo" />;
<button aria-labelled-by="foo"/>;
});
});

Expand Down Expand Up @@ -355,12 +367,6 @@ describe('labels', () => {
});
});

it('does not warn if an anchor has no href', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<a/>;
});
});

it('warns if an anchor has a tabIndex but no href', () => {
expectWarning(assertions.render.NO_LABEL.msg, () => {
<a tabIndex="0"/>;
Expand Down Expand Up @@ -438,7 +444,7 @@ describe('labels', () => {
render: function() {
return (
<div className="foo">
<span><span><img /></span></span>
<span><span><img/></span></span>
</div>
);
}
Expand All @@ -454,7 +460,7 @@ describe('labels', () => {
render: function() {
return (
<div className="foo">
<span><span>Foo <img /></span></span>
<span><span>Foo <img/></span></span>
</div>
);
}
Expand Down Expand Up @@ -521,11 +527,10 @@ describe('labels', () => {
});

expectWarning(assertions.render.NO_LABEL.msg, () => {
React.render(<div role="button"><Bar/><div /><Foo/></div>, fixture);
React.render(<div role="button"><Bar/><div/><Foo/></div>, fixture);
});
});


it('does not error when the component has a componentDidMount callback', () => {
var Bar = React.createClass({
_privateProp: 'bar',
Expand All @@ -545,6 +550,11 @@ describe('labels', () => {
});
});

it('does not warn when the label is a number', () => {
doNotExpectWarning(assertions.render.NO_LABEL.msg, () => {
<a>{1111}</a>;
});
});
});

describe('filterFn', () => {
Expand Down
11 changes: 8 additions & 3 deletions lib/assertions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ var hasChildTextNode = (props, children, failureCB) => {
return;
else if (typeof child === 'undefined')
return;
else if (typeof child === 'string')
else if (typeof child === 'string' || typeof child === 'number')
hasText = true;
else if (child.type === 'img' && child.props.alt)
hasText = true;
else if (child.props.children)
else if (child.props && child.props.children)
hasText = hasChildTextNode(child.props, child.props.children, failureCB);
else if (typeof child.type === 'function') {
// There can be false negatives if one of the children is a Component,
Expand Down Expand Up @@ -231,7 +231,12 @@ exports.render = {
if (isHiddenFromAT(props) || presentationRoles.has(props.role))
return;

if (!(isInteractive(tagName, props) || props.role))
if (!(
isInteractive(tagName, props) ||
(tagName == 'a' && !props.href) ||
props.role
)
)
return;

var failed = !(
Expand Down

0 comments on commit 598c8f8

Please sign in to comment.