From 2ac529071858aada6534a50ca22efb78aa4fabfa Mon Sep 17 00:00:00 2001
From: mzabriskie <mzabriskie@gmail.com>
Date: Tue, 11 Nov 2014 13:29:29 -0700
Subject: [PATCH] Better solution for applying focus

---
 lib/components/ModalPortal.js | 32 ++++++++++++++++++++------------
 specs/Modal.spec.js           |  8 ++++----
 specs/helper.js               |  7 +++----
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lib/components/ModalPortal.js b/lib/components/ModalPortal.js
index 54ad7c65..854e0692 100644
--- 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
index 39cf993e..504d43a0 100644
--- 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
index 7eb4b260..738caaf4 100644
--- a/specs/helper.js
+++ b/specs/helper.js
@@ -10,11 +10,11 @@ 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() {
@@ -22,4 +22,3 @@ unmountModal = function() {
   document.body.removeChild(_currentDiv);
   _currentDiv = null;
 };
-