[changed] focus target of the modal to its content

react-bootstrap · Aug 11, 2015 · 4f7823e · 4f7823e
1 parent 281b546
commit 4f7823e
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 14 deletions.
diff --git a/package.json b/package.json
@@ -92,6 +92,7 @@
   "dependencies": {
     "classnames": "^2.1.3",
     "dom-helpers": "^2.2.4",
-    "react-prop-types": "^0.2.1"
+    "react-prop-types": "^0.2.1",
+    "warning": "^2.0.0"
   }
 }
diff --git a/src/Modal.js b/src/Modal.js
@@ -1,5 +1,6 @@
 /*eslint-disable react/prop-types */
 import React, { cloneElement } from 'react';
+import warning from 'warning';
 import elementType from 'react-prop-types/lib/elementType';
 
 import Portal from './Portal';
@@ -154,11 +155,16 @@ const Modal = React.createClass({
       return null;
     }
 
-    if (dialog.props.role === undefined) {
-      dialog = cloneElement(dialog, { role: 'document' });
+    let { role, tabIndex } = dialog.props;
+
+    if (role === undefined || tabIndex === undefined) {
+      dialog = cloneElement(dialog, {
+        role: role === undefined ? 'document' : role,
+        tabIndex: tabIndex == null ? '-1' : tabIndex
+      });
     }
 
-    if ( Transition ) {
+    if (Transition) {
       dialog = (
         <Transition
           transitionAppear
@@ -180,7 +186,6 @@ const Modal = React.createClass({
     return (
       <Portal container={props.container}>
         <div
-          tabIndex='-1'
           ref={'modal'}
           role={props.role || 'dialog'}
           style={props.style}
@@ -318,20 +323,28 @@ const Modal = React.createClass({
     }
   },
 
-  checkForFocus(){
+  checkForFocus() {
     if (canUseDom) {
       this.lastFocus = activeElement();
     }
   },
 
   focus() {
     let autoFocus = this.props.autoFocus;
-    let modalContent = React.findDOMNode(this.refs.modal);
+    let modalContent = this.getDialogElement();
     let current = activeElement(ownerDocument(this));
     let focusInModal = current && contains(modalContent, current);
 
     if (modalContent && autoFocus && !focusInModal) {
       this.lastFocus = current;
+
+      if (!modalContent.hasAttribute('tabIndex')){
+        modalContent.setAttribute('tabIndex', -1);
+        warning(false,
+          'The modal content node does not accept focus. ' +
+          'For the benefit of assistive technologies, the tabIndex of the node is being set to "-1".');
+      }
+
       modalContent.focus();
     }
   },
@@ -352,9 +365,9 @@ const Modal = React.createClass({
     }
 
     let active = activeElement(ownerDocument(this));
-    let modal = React.findDOMNode(this.refs.modal);
+    let modal = this.getDialogElement();
 
-    if ( modal && modal !== active && !contains(modal, active)){
+    if (modal && modal !== active && !contains(modal, active)) {
       modal.focus();
     }
   },

diff --git a/test/ModalSpec.js b/test/ModalSpec.js
@@ -282,8 +282,8 @@ describe('Modal', function () {
       document.activeElement.should.equal(focusableContainer);
 
       let instance = render(
-        <Modal show className='modal'>
-          <strong>Message</strong>
+        <Modal show>
+          <strong className='modal'>Message</strong>
         </Modal>
         , focusableContainer);
 
@@ -311,7 +311,9 @@ describe('Modal', function () {
 
       render(
         <Modal show>
-          <input autoFocus />
+          <div className='modal'>
+            <input autoFocus/>
+          </div>
         </Modal>
         , focusableContainer);
 
@@ -325,14 +327,31 @@ describe('Modal', function () {
       document.activeElement.should.equal(focusableContainer);
 
       render(
-        <Modal show className='modal'>
-          <input autoFocus/>
+        <Modal show>
+          <div className='modal'>
+            <input autoFocus/>
+          </div>
         </Modal>
         , focusableContainer);
 
       focusableContainer.focus();
       document.activeElement.className.should.contain('modal');
     });
+
+    it('Should warn if the modal content is not focusable', function () {
+      let Dialog = ()=> ({ render(){ return <div/>; } });
+
+      sinon.stub(console, 'error');
+
+      render(
+        <Modal show>
+          <Dialog />
+        </Modal>
+        , focusableContainer);
+
+      expect(console.error).to.have.been.calledOnce;
+      console.error.restore();
+    });
   });
 
 });