Better solution for applying focus

reactjs · Nov 11, 2014 · 2ac5290 · 2ac5290
1 parent c04e083
commit 2ac5290
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 20 deletions.
diff --git a/lib/components/ModalPortal.js b/lib/components/ModalPortal.js
@@ -33,14 +33,33 @@ var ModalPortal = module.exports = React.createClass({
   },
 
   componentDidMount: function() {
+    // Focus needs to be set when mounting and already open
+    if (this.props.isOpen) {
+      this.setFocusAfterRender(true);
+    }
     this.handleProps(this.props);
-    this.maybeFocus();
   },
 
   componentWillReceiveProps: function(newProps) {
+    // Focus only needs to be set once when the modal is being opened
+    if (!this.props.isOpen && newProps.isOpen) {
+      this.setFocusAfterRender(true);
+    }
+
     this.handleProps(newProps);
   },
 
+  componentDidUpdate: function () {
+    if (this.focusAfterRender) {
+      this.focusContent();
+      this.setFocusAfterRender(false);
+    }
+  },
+
+  setFocusAfterRender: function (focus) {
+    this.focusAfterRender = focus;
+  },
+
   handleProps: function(props) {
     if (props.isOpen === true)
       this.open();
@@ -65,17 +84,6 @@ var ModalPortal = module.exports = React.createClass({
       this.closeWithoutTimeout();
   },
 
-  componentDidUpdate: function() {
-    this.maybeFocus();
-  },
-
-  maybeFocus: function() {
-    if (this.props.isOpen &&
-      !this.refs.content.getDOMNode().contains(document.activeElement)) {
-      this.focusContent();
-    }
-  },
-
   focusContent: function() {
     this.refs.content.getDOMNode().focus();
   },

diff --git a/specs/Modal.spec.js b/specs/Modal.spec.js
@@ -92,9 +92,10 @@ describe('Modal', function () {
   });
 
   it('focuses the modal content', function() {
-    var modal = renderModal({isOpen: true});
-    strictEqual(document.activeElement, modal.portal.refs.content.getDOMNode());
-    unmountModal();
+    renderModal({isOpen: true}, null, function () {
+      strictEqual(document.activeElement, this.portal.refs.content.getDOMNode());
+      unmountModal();
+    });
   });
 
   it('adds --after-open for animations', function() {
@@ -129,4 +130,3 @@ describe('Modal', function () {
     //unmountModal();
   //});
 });
-
diff --git a/specs/helper.js b/specs/helper.js
@@ -10,16 +10,15 @@ throws = assert.throws;
 
 var _currentDiv = null;
 
-renderModal = function(def) {
-  def.ariaHideApp = false;
+renderModal = function(props, children, callback) {
+  props.ariaHideApp = false;
   _currentDiv = document.createElement('div');
   document.body.appendChild(_currentDiv);
-  return React.renderComponent(Modal.apply(Modal, arguments), _currentDiv);
+  return React.renderComponent(Modal(props, children), _currentDiv, callback);
 };
 
 unmountModal = function() {
   React.unmountComponentAtNode(_currentDiv);
   document.body.removeChild(_currentDiv);
   _currentDiv = null;
 };
-