diff --git a/src/ListGroupItem.js b/src/ListGroupItem.js index dae133d54f..ac79d8bd37 100644 --- a/src/ListGroupItem.js +++ b/src/ListGroupItem.js @@ -1,7 +1,7 @@ import React, { cloneElement } from 'react'; import BootstrapMixin from './BootstrapMixin'; import classNames from 'classnames'; - +import SafeAnchor from './SafeAnchor'; const ListGroupItem = React.createClass({ mixins: [BootstrapMixin], @@ -51,12 +51,12 @@ const ListGroupItem = React.createClass({ renderAnchor(classes) { return ( - {this.props.header ? this.renderStructuredContent() : this.props.children} - + ); }, diff --git a/src/MenuItem.js b/src/MenuItem.js index d5bdce7a89..bf63602cfd 100644 --- a/src/MenuItem.js +++ b/src/MenuItem.js @@ -1,5 +1,6 @@ import React from 'react'; import classNames from 'classnames'; +import SafeAnchor from './SafeAnchor'; const MenuItem = React.createClass({ propTypes: { @@ -15,7 +16,6 @@ const MenuItem = React.createClass({ getDefaultProps() { return { - href: '#', active: false }; }, @@ -29,9 +29,9 @@ const MenuItem = React.createClass({ renderAnchor() { return ( - + {this.props.children} - + ); }, diff --git a/src/NavItem.js b/src/NavItem.js index 9bc768df48..8613635567 100644 --- a/src/NavItem.js +++ b/src/NavItem.js @@ -1,6 +1,7 @@ import React from 'react'; import classNames from 'classnames'; import BootstrapMixin from './BootstrapMixin'; +import SafeAnchor from './SafeAnchor'; const NavItem = React.createClass({ mixins: [BootstrapMixin], @@ -15,12 +16,6 @@ const NavItem = React.createClass({ target: React.PropTypes.string }, - getDefaultProps() { - return { - href: '#' - }; - }, - render() { let { disabled, @@ -38,8 +33,7 @@ const NavItem = React.createClass({ href, title, target, - onClick: this.handleClick, - ref: 'anchor' + onClick: this.handleClick }; if (href === '#') { @@ -48,9 +42,9 @@ const NavItem = React.createClass({ return (
  • - + { children } - +
  • ); }, diff --git a/src/PageItem.js b/src/PageItem.js index 5a7c22d7f9..44ef5e419c 100644 --- a/src/PageItem.js +++ b/src/PageItem.js @@ -1,5 +1,6 @@ import React from 'react'; import classNames from 'classnames'; +import SafeAnchor from './SafeAnchor'; const PageItem = React.createClass({ @@ -14,12 +15,6 @@ const PageItem = React.createClass({ eventKey: React.PropTypes.any }, - getDefaultProps() { - return { - href: '#' - }; - }, - render() { let classes = { 'disabled': this.props.disabled, @@ -31,14 +26,13 @@ const PageItem = React.createClass({
  • - + onClick={this.handleSelect}> {this.props.children} - +
  • ); }, diff --git a/src/SafeAnchor.js b/src/SafeAnchor.js new file mode 100644 index 0000000000..8316bbe634 --- /dev/null +++ b/src/SafeAnchor.js @@ -0,0 +1,38 @@ +import React from 'react'; + +/** + * Note: This is intended as a stop-gap for accessibility concerns that the + * Bootstrap CSS does not address as they have styled anchors and not buttons + * in many cases. + */ +export default class SafeAnchor extends React.Component { + constructor(props) { + super(props); + + this.handleClick = this.handleClick.bind(this); + } + + handleClick(event) { + if (this.props.href === undefined) { + event.preventDefault(); + } + + if (this.props.onClick) { + this.props.onClick(event); + } + } + + render() { + return ( + + ); + } +} + +SafeAnchor.propTypes = { + href: React.PropTypes.string, + onClick: React.PropTypes.func +}; diff --git a/src/SubNav.js b/src/SubNav.js index e9487c1ada..814604cc76 100644 --- a/src/SubNav.js +++ b/src/SubNav.js @@ -4,6 +4,7 @@ import classNames from 'classnames'; import ValidComponentChildren from './utils/ValidComponentChildren'; import createChainedFunction from './utils/createChainedFunction'; import BootstrapMixin from './BootstrapMixin'; +import SafeAnchor from './SafeAnchor'; const SubNav = React.createClass({ mixins: [BootstrapMixin], @@ -99,14 +100,13 @@ const SubNav = React.createClass({ return (
  • - + onClick={this.handleClick}> {this.props.text} - + diff --git a/src/Thumbnail.js b/src/Thumbnail.js index ea05cf85d2..8085e63fea 100644 --- a/src/Thumbnail.js +++ b/src/Thumbnail.js @@ -1,6 +1,7 @@ import React from 'react'; import classSet from 'classnames'; import BootstrapMixin from './BootstrapMixin'; +import SafeAnchor from './SafeAnchor'; const Thumbnail = React.createClass({ mixins: [BootstrapMixin], @@ -16,9 +17,9 @@ const Thumbnail = React.createClass({ if(this.props.href) { return ( - + {this.props.alt} - + ); } else { diff --git a/src/index.js b/src/index.js index 0f05aed32f..ed0d61b513 100644 --- a/src/index.js +++ b/src/index.js @@ -42,6 +42,7 @@ import Pager from './Pager'; import Popover from './Popover'; import ProgressBar from './ProgressBar'; import Row from './Row'; +import SafeAnchor from './SafeAnchor'; import SplitButton from './SplitButton'; import SubNav from './SubNav'; import TabbedArea from './TabbedArea'; @@ -97,6 +98,7 @@ export default { Popover, ProgressBar, Row, + SafeAnchor, SplitButton, SubNav, TabbedArea, diff --git a/test/NavSpec.js b/test/NavSpec.js index 81a8d09fe4..1061edb738 100644 --- a/test/NavSpec.js +++ b/test/NavSpec.js @@ -83,9 +83,9 @@ describe('Nav', function () { ); - let items = ReactTestUtils.scryRenderedComponentsWithType(instance, NavItem); + let items = ReactTestUtils.scryRenderedDOMComponentsWithTag(instance, 'A'); - ReactTestUtils.Simulate.click(items[1].refs.anchor); + ReactTestUtils.Simulate.click(items[1]); }); it('Should set the correct item active by href', function () { diff --git a/test/PageItemSpec.js b/test/PageItemSpec.js index 99e3d0b10d..7f079f4be2 100644 --- a/test/PageItemSpec.js +++ b/test/PageItemSpec.js @@ -36,7 +36,7 @@ describe('PageItem', function () { it('Should call "onSelect" when item is clicked', function (done) { function handleSelect(key, href) { assert.equal(key, 1); - assert.equal(href, '#'); + assert.equal(href, undefined); done(); } let instance = ReactTestUtils.renderIntoDocument( diff --git a/test/SafeAnchorSpec.js b/test/SafeAnchorSpec.js new file mode 100644 index 0000000000..75b9d0a8de --- /dev/null +++ b/test/SafeAnchorSpec.js @@ -0,0 +1,93 @@ +import React from 'react'; +import ReactTestUtils from 'react/lib/ReactTestUtils'; +import SafeAnchor from '../src/SafeAnchor'; + +describe('SafeAnchor', function() { + it('renders an anchor tag', function() { + const instance = ReactTestUtils.renderIntoDocument(); + const node = React.findDOMNode(instance); + + node.tagName.should.equal('A'); + }); + + it('forwards arbitrary props to the anchor', function() { + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + anchor.props.herpa.should.equal('derpa'); + }); + + it('forwards provided href', function() { + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + anchor.props.href.should.equal('http://google.com'); + }); + + it('ensures that an href is provided', function() { + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + anchor.props.href.should.equal(''); + }); + + it('forwards onClick handler', function(done) { + const handleClick = (event) => { + done(); + }; + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + ReactTestUtils.Simulate.click(anchor); + }); + + it('prevents default when no href is provided', function(done) { + const handleClick = (event) => { + event.defaultPrevented.should.be.true; + done(); + }; + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + ReactTestUtils.Simulate.click(anchor); + }); + + it('does not prevent default when href is provided', function(done) { + const handleClick = (event) => { + expect(event.defaultPrevented).to.not.be.ok; + done(); + }; + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + ReactTestUtils.Simulate.click(anchor); + }); + + it('forwards provided role', function () { + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + anchor.props.role.should.equal('test'); + }); + + it('forwards provided role with href', function () { + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + anchor.props.role.should.equal('test'); + }); + + it('set role=button with no provided href', function () { + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + anchor.props.role.should.equal('button'); + }); + + it('sets no role with provided href', function () { + const instance = ReactTestUtils.renderIntoDocument(); + const anchor = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'A'); + + expect(anchor.props.role).to.be.undefined; + }); +});