Merge fe6a63b into 843f63c

reactstrap · Jun 12, 2016 · b86f5ad · b86f5ad
2 parents 843f63c + fe6a63b
commit b86f5ad
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 17 deletions.
diff --git a/package.json b/package.json
@@ -69,7 +69,7 @@
     "coveralls": "^2.11.9",
     "css-loader": "^0.23.1",
     "ejs": "^2.4.1",
-    "enzyme": "^2.2.0",
+    "enzyme": "^2.3.0",
     "eslint": "^2.9.0",
     "eslint-config-airbnb": "^9.0.0",
     "eslint-plugin-import": "^1.7.0",

diff --git a/src/Fade.jsx b/src/Fade.jsx
@@ -34,6 +34,9 @@ class Fade extends React.Component {
     this.state = {
       mounted: !props.transitionAppear,
     };
+
+    this.onLeave = this.onLeave.bind(this);
+    this.onEnter = this.onEnter.bind(this);
   }
 
   onEnter(cb) {

diff --git a/src/Modal.jsx b/src/Modal.jsx
@@ -20,7 +20,7 @@ class Modal extends React.Component {
   constructor(props) {
     super(props);
 
-    this.handleProps = this.handleProps.bind(this);
+    this.togglePortal = this.togglePortal.bind(this);
     this.handleBackdropClick = this.handleBackdropClick.bind(this);
     this.handleEscape = this.handleEscape.bind(this);
     this.destroy = this.destroy.bind(this);
@@ -36,7 +36,11 @@ class Modal extends React.Component {
 
   componentDidUpdate(prevProps) {
     if (this.props.isOpen !== prevProps.isOpen) {
-      this.handleProps();
+      // handle portal events/dom updates
+      this.togglePortal();
+    } else if (this._element) {
+      // rerender portal
+      this.renderIntoSubtree();
     }
   }
 
@@ -71,16 +75,17 @@ class Modal extends React.Component {
     }
   }
 
-  handleProps() {
+  togglePortal() {
     if (this.props.isOpen) {
       this.show();
+      this._focus = true;
     } else {
       this.hide();
     }
   }
 
   destroy() {
-    let classes = document.body.className.replace('modal-open', '');
+    const classes = document.body.className.replace('modal-open', '');
     this.removeEvents();
 
     if (this._element) {
@@ -117,7 +122,6 @@ class Modal extends React.Component {
     );
 
     this.renderIntoSubtree();
-    this._element.focus();
   }
 
   renderIntoSubtree() {
@@ -126,36 +130,44 @@ class Modal extends React.Component {
       this.renderChildren(),
       this._element
     );
+
+    // check if modal should receive focus
+    if (this._focus) {
+      this._element.focus();
+      this._focus = false;
+    }
   }
 
   renderChildren() {
     return (
       <TransitionGroup component="div">
-        { this.props.isOpen && (
+        {this.props.isOpen && (
           <Fade
-            key={Math.random()}
+            key="modal-dialog"
             onEnter={this.onEnter}
             onLeave={this.onExit}
             transitionAppearTimeout={300}
             transitionEnterTimeout={300}
             transitionLeaveTimeout={300}
             className="modal"
             style={{ display: 'block' }}
-            tabIndex="-1">
+            tabIndex="-1"
+          >
             <div className="modal-dialog" role="document" ref={(c) => this._dialog = c }>
               <div className="modal-content">
-                { this.props.children }
+                {this.props.children}
               </div>
             </div>
           </Fade>
         )}
-        { this.props.isOpen && (
+        {this.props.isOpen && (
           <Fade
-            key={Math.random()}
+            key="modal-backdrop"
             transitionAppearTimeout={150}
             transitionEnterTimeout={150}
             transitionLeaveTimeout={150}
-            className="modal-backdrop"/>
+            className="modal-backdrop"
+          />
         )}
       </TransitionGroup>
     );

diff --git a/test/Modal.spec.js b/test/Modal.spec.js
@@ -116,8 +116,8 @@ describe('Modal', () => {
     wrapper.unmount();
   });
 
-  it('should not call handleProps when isOpen does not change', () => {
-    spyOn(Modal.prototype, 'handleProps').and.callThrough();
+  it('should not call togglePortal when isOpen does not change', () => {
+    spyOn(Modal.prototype, 'togglePortal').and.callThrough();
     spyOn(Modal.prototype, 'componentDidUpdate').and.callThrough();
     const wrapper = mount(
       <Modal isOpen={isOpen} toggle={toggle}>
@@ -127,7 +127,7 @@ describe('Modal', () => {
 
     jasmine.clock().tick(300);
     expect(isOpen).toBe(false);
-    expect(Modal.prototype.handleProps).not.toHaveBeenCalled();
+    expect(Modal.prototype.togglePortal).not.toHaveBeenCalled();
     expect(Modal.prototype.componentDidUpdate).not.toHaveBeenCalled();
 
     wrapper.setProps({
@@ -136,12 +136,39 @@ describe('Modal', () => {
     jasmine.clock().tick(300);
 
     expect(isOpen).toBe(false);
-    expect(Modal.prototype.handleProps).not.toHaveBeenCalled();
+    expect(Modal.prototype.togglePortal).not.toHaveBeenCalled();
     expect(Modal.prototype.componentDidUpdate).toHaveBeenCalled();
 
     wrapper.unmount();
   });
 
+  it('should renderIntoSubtree when props updated', () => {
+    isOpen = true;
+    spyOn(Modal.prototype, 'togglePortal').and.callThrough();
+    spyOn(Modal.prototype, 'renderIntoSubtree').and.callThrough();
+    const wrapper = mount(
+      <Modal isOpen={isOpen} toggle={toggle}>
+        Yo!
+      </Modal>
+    );
+
+    jasmine.clock().tick(300);
+    expect(isOpen).toBe(true);
+    expect(Modal.prototype.togglePortal.calls.count()).toEqual(0);
+    expect(Modal.prototype.renderIntoSubtree.calls.count()).toEqual(1);
+
+    wrapper.setProps({
+      isOpen: isOpen
+    });
+    jasmine.clock().tick(300);
+
+    expect(isOpen).toBe(true);
+    expect(Modal.prototype.togglePortal.calls.count()).toEqual(0);
+    expect(Modal.prototype.renderIntoSubtree.calls.count()).toEqual(2);
+
+    wrapper.unmount();
+  });
+
   it('should close modal when escape key pressed', () => {
     isOpen = true;
     const wrapper = mount(