Address review comments

reactjs · Jun 25, 2017 · 480e6dd · 480e6dd
1 parent 7634245
commit 480e6dd
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 12 deletions.
diff --git a/specs/Modal.spec.js b/specs/Modal.spec.js
@@ -166,7 +166,6 @@ describe('State', () => {
     expect(
       moverlay(modal).className.includes('myOverlayClass')
     ).toBeTruthy();
-    unmountModal();
   });
 
   it('overrides content classes with custom object className', () => {
@@ -183,7 +182,6 @@ describe('State', () => {
     ).toEqual(
       'myClass myClass_after-open'
     );
-    unmountModal();
   });
 
   it('overrides overlay classes with custom object overlayClassName', () => {
@@ -240,22 +238,22 @@ describe('State', () => {
     unmountModal();
     expect(isBodyWithReactModalOpenClass()).toBeTruthy();
     unmountModal();
-    expect(isBodyWithReactModalOpenClass()).toBeFalsy();
+    expect(!isBodyWithReactModalOpenClass()).toBeTruthy();
   });
 
   it('should not add classes to document.body for unopened modals', () => {
     renderModal({ isOpen: true });
     expect(isBodyWithReactModalOpenClass()).toBeTruthy();
     renderModal({ isOpen: false, bodyOpenClassName: 'testBodyClass' });
-    expect(isBodyWithReactModalOpenClass('testBodyClass')).toBeFalsy();
+    expect(!isBodyWithReactModalOpenClass('testBodyClass')).toBeTruthy()
   });
 
   it('should not remove classes from document.body when rendering unopened modal', () => {
     renderModal({ isOpen: true });
     expect(isBodyWithReactModalOpenClass()).toBeTruthy();
     renderModal({ isOpen: false, bodyOpenClassName: 'testBodyClass' });
     renderModal({ isOpen: false });
-    expect(isBodyWithReactModalOpenClass('testBodyClass')).toBeFalsy();
+    expect(!isBodyWithReactModalOpenClass('testBodyClass')).toBeTruthy()
     expect(isBodyWithReactModalOpenClass()).toBeTruthy();
     renderModal({ isOpen: false });
     renderModal({ isOpen: false });

diff --git a/src/components/ModalPortal.js b/src/components/ModalPortal.js
@@ -69,27 +69,25 @@ export default class ModalPortal extends Component {
     // Focus needs to be set when mounting and already open
     if (this.props.isOpen) {
       this.setFocusAfterRender(true);
-      this.addModalInstance();
       this.open();
     }
   }
 
   componentWillReceiveProps(newProps) {
     if (process.env.NODE_ENV !== "production") {
       if (newProps.bodyOpenClassName !== this.props.bodyOpenClassName) {
+        // eslint-disable-next-line no-console
         console.warn(
-          'React-Modal: "bodyOpenClassName" prop has been modified. ' + 
+          'React-Modal: "bodyOpenClassName" prop has been modified. ' +
           'This may cause unexpected behavior when multiple modals are open.'
         );
       }
     }
     // Focus only needs to be set once when the modal is being opened
     if (!this.props.isOpen && newProps.isOpen) {
       this.setFocusAfterRender(true);
-      this.addModalInstance();
       this.open();
     } else if (this.props.isOpen && !newProps.isOpen) {
-      this.removeModalInstance();
       this.close();
     }
   }
@@ -148,6 +146,7 @@ export default class ModalPortal extends Component {
   }
 
   open = () => {
+    this.addModalInstance();
     if (this.state.afterOpen && this.state.beforeClose) {
       clearTimeout(this.closeTimer);
       this.setState({ beforeClose: false });
@@ -165,6 +164,7 @@ export default class ModalPortal extends Component {
   }
 
   close = () => {
+    this.removeModalInstance();
     if (this.props.closeTimeoutMS > 0) {
       this.closeWithTimeout();
     } else {

diff --git a/src/helpers/refCount.js b/src/helpers/refCount.js
@@ -19,7 +19,5 @@ export function count(bodyClass) {
 }
 
 export function totalCount() {
-  const count = Object.keys(modals)
-    .reduce((acc, curr) => acc + modals[curr], 0);
-  return count;
+  return Object.keys(modals).reduce((acc, curr) => acc + modals[curr], 0);
 }