fix(Modal): add/remove dimmer classes in raf (#2010)

* fix(Modal): add/remove dimmer classes in raf * test(Modal): abstract assertBodyContains * fix(Modal): add/remove dimmer classes properly
Semantic-Org · Sep 30, 2017 · 9c97ed9 · 9c97ed9
1 parent 5a37f28
commit 9c97ed9
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 43 deletions.
diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js
@@ -199,21 +199,8 @@ class Modal extends Component {
 
   handlePortalMount = (e) => {
     debug('handlePortalMount()')
-    const { dimmer } = this.props
-    const mountNode = this.getMountNode()
-
-    if (dimmer) {
-      debug('adding dimmer')
-      mountNode.classList.add('dimmable')
-      mountNode.classList.add('dimmed')
 
-      if (dimmer === 'blurring') {
-        debug('adding blurred dimmer')
-        mountNode.classList.add('blurring')
-      }
-    }
-
-    this.setPosition()
+    this.setPositionAndClassNames()
 
     const { onMount } = this.props
     if (onMount) onMount(e, this.props)
@@ -241,9 +228,20 @@ class Modal extends Component {
 
   handleRef = c => (this.ref = c)
 
-  setPosition = () => {
+  setPositionAndClassNames = () => {
+    const { dimmer } = this.props
+    const mountNode = this.getMountNode()
+
+    if (dimmer) {
+      mountNode.classList.add('dimmable')
+      mountNode.classList.add('dimmed')
+
+      if (dimmer === 'blurring') {
+        mountNode.classList.add('blurring')
+      }
+    }
+
     if (this.ref) {
-      const mountNode = this.getMountNode()
       const { height } = this.ref.getBoundingClientRect()
 
       const marginTop = -Math.round(height / 2)
@@ -268,7 +266,7 @@ class Modal extends Component {
       if (Object.keys(newState).length > 0) this.setState(newState)
     }
 
-    this.animationRequestId = requestAnimationFrame(this.setPosition)
+    this.animationRequestId = requestAnimationFrame(this.setPositionAndClassNames)
   }
 
   renderContent = (rest) => {

diff --git a/test/specs/modules/Modal/Modal-test.js b/test/specs/modules/Modal/Modal-test.js
@@ -7,7 +7,7 @@ import ModalActions from 'src/modules/Modal/ModalActions'
 import ModalDescription from 'src/modules/Modal/ModalDescription'
 import Portal from 'src/addons/Portal/Portal'
 
-import { assertNodeContains, assertBodyContains, domEvent, sandbox } from 'test/utils'
+import { assertNodeContains, assertBodyClasses, assertBodyContains, domEvent, sandbox } from 'test/utils'
 import * as common from 'test/specs/commonTests'
 
 // ----------------------------------------
@@ -20,20 +20,6 @@ let wrapper
 const wrapperMount = (...args) => (wrapper = mount(...args))
 const wrapperShallow = (...args) => (wrapper = shallow(...args))
 
-const assertBodyClasses = (...rest) => {
-  const hasClasses = typeof rest[rest.length - 1] === 'boolean' ? rest.pop() : true
-
-  rest.forEach((className) => {
-    const didFind = document.body.classList.contains(className)
-    const message = [
-      `document.body ${didFind ? 'has' : 'does not have'} class "${className}".`,
-      `It has class="${document.body.classList}"`,
-    ].join(' ')
-
-    didFind.should.equal(hasClasses, message)
-  })
-}
-
 describe('Modal', () => {
   beforeEach(() => {
     wrapper = undefined
@@ -242,13 +228,13 @@ describe('Modal', () => {
 
     describe('true', () => {
       it('adds/removes body classes "dimmable dimmed" on mount/unmount', () => {
-        assertBodyClasses('dimmable', 'dimmed', false)
+        assertBodyClasses('dimmable dimmed', false)
 
         wrapperMount(<Modal open dimmer />)
-        assertBodyClasses('dimmable', 'dimmed')
+        assertBodyClasses('dimmable dimmed')
 
         wrapper.unmount()
-        assertBodyClasses('dimmable', 'dimmed', false)
+        assertBodyClasses('dimmable dimmed', false)
       })
 
       it('adds a dimmer to the body', () => {
@@ -260,24 +246,24 @@ describe('Modal', () => {
     describe('false', () => {
       it('does not render a dimmer', () => {
         wrapperMount(<Modal open dimmer={false} />)
-        assertBodyClasses('dimmable', 'dimmed', 'blurring', false)
+        assertBodyClasses('dimmable dimmed blurring', false)
       })
 
       it('does not add any dimmer classes to the body', () => {
         wrapperMount(<Modal open dimmer={false} />)
-        assertBodyClasses('dimmable', 'dimmed', 'blurring', false)
+        assertBodyClasses('dimmable dimmed blurring', false)
       })
     })
 
     describe('blurring', () => {
       it('adds/removes body classes "dimmable dimmed blurring" on mount/unmount', () => {
-        assertBodyClasses('dimmable', 'dimmed', 'blurring', false)
+        assertBodyClasses('dimmable dimmed blurring', false)
 
         wrapperMount(<Modal open dimmer='blurring' />)
-        assertBodyClasses('dimmable', 'dimmed', 'blurring')
+        assertBodyClasses('dimmable dimmed blurring')
 
         wrapper.unmount()
-        assertBodyClasses('dimmable', 'dimmed', 'blurring', false)
+        assertBodyClasses('dimmable dimmed blurring', false)
       })
 
       it('adds a dimmer to the body', () => {
@@ -288,13 +274,13 @@ describe('Modal', () => {
 
     describe('inverted', () => {
       it('adds/removes body classes "dimmable dimmed" on mount/unmount', () => {
-        assertBodyClasses('dimmable', 'dimmed', false)
+        assertBodyClasses('dimmable dimmed', false)
 
         wrapperMount(<Modal open dimmer />)
-        assertBodyClasses('dimmable', 'dimmed')
+        assertBodyClasses('dimmable dimmed')
 
         wrapper.unmount()
-        assertBodyClasses('dimmable', 'dimmed', false)
+        assertBodyClasses('dimmable dimmed', false)
       })
 
       it('adds an inverted dimmer to the body', () => {

diff --git a/test/utils/assertBodyClasses.js b/test/utils/assertBodyClasses.js
@@ -0,0 +1,22 @@
+/**
+ * Assert whether or not the document.body has the specified HTML classes.
+ *
+ * @param {string|string[]} classes An array of strings string of HTML classes
+ * @param {boolean} [hasClasses=true] Indicating whether to assert "has" or "does not have" classes
+ */
+const assertBodyClasses = (classes, hasClasses = true) => {
+  const classesArr = [].concat(classes).join(' ').split(' ')
+
+  classesArr.forEach((className) => {
+    const didFind = document.body.classList.contains(className)
+
+    const message = [
+      `document.body should ${hasClasses ? 'have' : 'not have'} class "${className}",`,
+      `but it ${didFind ? 'has' : 'does not have'} class="${document.body.classList}"`,
+    ].join(' ')
+
+    didFind.should.equal(hasClasses, message)
+  })
+}
+
+export default assertBodyClasses
diff --git a/test/utils/index.js b/test/utils/index.js
@@ -1,3 +1,4 @@
+export { default as assertBodyClasses } from './assertBodyClasses'
 export * from './assertNodeContains'
 export { default as consoleUtil } from './consoleUtil'
 export { default as domEvent } from './domEvent'