From 9c09e2a5068e437c5767c4c5299bba16d9bb1aa8 Mon Sep 17 00:00:00 2001 From: Matthew Smith Date: Fri, 29 May 2015 09:07:02 -0600 Subject: [PATCH] [fixed] Keyboard accessibility for anchors serving as buttons Bootstrap uses a lot of styling that is specifically targeting anchor tags that may also serve in the capacity as a button. Unfortunately since Bootstrap does not style said buttons we have to use an anchor tag instead. But in order to maintain keyboard functionality for accessibility concerns even those anchor tags must provide an href. The solution is to add an internal `SafeAnchor` component which ensures that something exists for the href attribute, and calls `event.preventDefault()` when the anchor is clicked. It will then continue to invoke any additional `onClick` handler provided. --- src/ListGroupItem.js | 6 +-- src/MenuItem.js | 6 +-- src/NavItem.js | 14 ++----- src/PageItem.js | 14 ++----- src/SafeAnchor.js | 38 +++++++++++++++++ src/SubNav.js | 8 ++-- src/Thumbnail.js | 5 ++- src/index.js | 2 + test/NavSpec.js | 4 +- test/PageItemSpec.js | 2 +- test/SafeAnchorSpec.js | 93 ++++++++++++++++++++++++++++++++++++++++++ 11 files changed, 157 insertions(+), 35 deletions(-) create mode 100644 src/SafeAnchor.js create mode 100644 test/SafeAnchorSpec.js 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; + }); +});