fix(Modal): support IE11, fix scrolling glitches

src/modules/Modal/Modal.js

@@ -2,6 +2,7 @@ import cx from 'classnames'
 import _ from 'lodash'
 import PropTypes from 'prop-types'
 import React, { createRef, Fragment, isValidElement } from 'react'
+import shallowEqual from 'shallowequal'
 
 import {
   AutoControlledComponent as Component,
@@ -23,6 +24,7 @@ import ModalContent from './ModalContent'
 import ModalActions from './ModalActions'
 import ModalDescription from './ModalDescription'
 import Ref from '../../addons/Ref'
+import { canFit, getLegacyStyles, isLegacy } from './utils'
 
 const debug = makeDebugger('modal')
 
@@ -151,6 +153,7 @@ class Modal extends Component {
   static Description = ModalDescription
   static Actions = ModalActions
 
+  legacy = isBrowser() && isLegacy()
   ref = createRef()
   dimmerRef = createRef()
   latestDocumentMouseDownEvent = null
@@ -258,39 +261,34 @@ class Modal extends Component {
   }
 
   setPositionAndClassNames = () => {
-    const { dimmer } = this.props
-    let classes
-
-    if (dimmer) {
-      classes = 'dimmable dimmed'
-
-      if (dimmer === 'blurring') {
-        classes += ' blurring'
-      }
-    }
+    const { centered, dimmer } = this.props
 
+    let scrolling
     const newState = {}
 
     if (this.ref.current) {
-      const { height } = this.ref.current.getBoundingClientRect()
+      const rect = this.ref.current.getBoundingClientRect()
+      const isFitted = canFit(rect)
 
-      // Leaving the old calculation here since we may need it as an older browser fallback
-      // SEE: https://github.com/Semantic-Org/Semantic-UI/issues/6185#issuecomment-376725956
-      // const marginTop = -Math.round(height / 2)
-      const marginTop = null
-      const scrolling = height > window.innerHeight
+      scrolling = !isFitted
+      // Styles should be computed for IE11
+      const legacyStyles = this.legacy ? getLegacyStyles(isFitted, centered, rect) : {}
 
-      if (this.state.marginTop !== marginTop) {
-        newState.marginTop = marginTop
+      if (!shallowEqual(this.state.legacyStyles, legacyStyles)) {
+        newState.legacyStyles = legacyStyles
       }
 
       if (this.state.scrolling !== scrolling) {
         newState.scrolling = scrolling
       }
-
-      if (scrolling) classes += ' scrolling'
     }
 
+    const classes = cx(
+      useKeyOnly(dimmer, 'dimmable dimmed'),
+      useKeyOnly(dimmer === 'blurring', ' blurring'),
+      useKeyOnly(scrolling, ' scrolling'),
+    )
+
     if (this.state.mountClasses !== classes) newState.mountClasses = classes
     if (!_.isEmpty(newState)) this.setState(newState)
 
@@ -312,12 +310,13 @@ class Modal extends Component {
       size,
       style,
     } = this.props
-    const { marginTop, mountClasses, scrolling } = this.state
+    const { legacyStyles, mountClasses, scrolling } = this.state
 
     const classes = cx(
       'ui',
       size,
       useKeyOnly(basic, 'basic'),
+      useKeyOnly(this.legacy, 'legacy'),
       useKeyOnly(scrolling, 'scrolling'),
       'modal transition visible active',
       className,
@@ -329,7 +328,7 @@ class Modal extends Component {
 
     return (
       <Ref innerRef={this.ref}>
-        <ElementType {...rest} className={classes} style={{ marginTop, ...style }}>
+        <ElementType {...rest} className={classes} style={{ ...legacyStyles, ...style }}>
           <MountNode className={mountClasses} node={mountNode} />
 
           {closeIconJSX}

src/modules/Modal/utils/index.js

@@ -0,0 +1,54 @@
+// https://github.com/Semantic-Org/Semantic-UI/blob/2.4.1/src/definitions/modules/modal.js#L956
+const OFFSET = 0
+// https://github.com/Semantic-Org/Semantic-UI/blob/2.4.1/src/definitions/modules/modal.js#L990
+const PADDING = 50
+
+/**
+ * Ensures that modal can fit viewport without scroll.
+ *
+ * @param modalRect {DOMRect}
+ *
+ * @see https://github.com/Semantic-Org/Semantic-UI/blob/2.4.1/src/definitions/modules/modal.js#L608
+ */
+export const canFit = (modalRect) => {
+  // original: scrollHeight = $module.prop('scrollHeight'),
+  // is replaced by .height because scrollHeight provides integer which produces glitches
+  // https://github.com/Semantic-Org/Semantic-UI-React/issues/2221
+  const scrollHeight = modalRect.height + OFFSET
+  // $module.outerHeight() + settings.offset
+  const height = modalRect.height + OFFSET
+
+  // original: $(window).height()
+  const contextHeight = window.innerHeight
+  const verticalCenter = contextHeight / 2
+  const topOffset = -(height / 2)
+
+  // padding with edge of page
+  const paddingHeight = PADDING
+  const startPosition = verticalCenter + topOffset // 0
+
+  // original: scrollHeight > height
+  //     ? startPosition + scrollHeight + paddingHeight < contextHeight
+  //     : height + paddingHeight * 2 < contextHeight
+  return startPosition + scrollHeight + paddingHeight < contextHeight
+}
+
+/**
+ * Creates legacy styles for IE11.
+ *
+ * @param isFitted {Boolean}
+ * @param centered {Boolean}
+ * @param modalRect {DOMRect}
+ *
+ * @see https://github.com/Semantic-Org/Semantic-UI/blob/2.4.1/src/definitions/modules/modal.js#L718
+ */
+export const getLegacyStyles = (isFitted, centered, modalRect) => {
+  const marginTop = centered && isFitted ? -(modalRect.height / 2) : 0
+  const marginLeft = -(modalRect.width / 2)
+
+  return { marginLeft, marginTop }
+}
+
+// https://github.com/Semantic-Org/Semantic-UI/blob/2.4.1/src/definitions/modules/modal.js#L631
+/* istanbul ignore next */
+export const isLegacy = () => !window.ActiveXObject && 'ActiveXObject' in window

test/specs/addons/MountNode/lib/NodeRegistry-test.js

@@ -66,5 +66,16 @@ describe('NodeRegistry', () => {
       handler.should.have.been.calledOnce()
       handler.should.have.been.calledWithMatch('foo', undefined)
     })
+
+    it('passes when unexisting nodeRef is passed', () => {
+      const handler = sandbox.spy()
+      const registry = new NodeRegistry()
+
+      registry.del('foo', 'FooComponent')
+      registry.emit('foo', handler)
+
+      handler.should.have.been.calledOnce()
+      handler.should.have.been.calledWithMatch('foo', undefined)
+    })
   })
 })

test/specs/addons/Portal/PortalInner-test.js

@@ -1,6 +1,7 @@
 import React, { createRef } from 'react'
 
 import PortalInner from 'src/addons/Portal/PortalInner'
+import { isBrowser } from 'src/lib'
 import * as common from 'test/specs/commonTests'
 import { sandbox } from 'test/utils'
 
@@ -10,6 +11,24 @@ describe('PortalInner', () => {
     requiredProps: { children: <p /> },
   })
 
+  describe('children', () => {
+    before(() => {
+      isBrowser.override = false
+    })
+
+    after(() => {
+      isBrowser.override = null
+    })
+
+    it('renders `null` when is SSR', () => {
+      mount(
+        <PortalInner>
+          <p />
+        </PortalInner>,
+      ).should.be.blank()
+    })
+  })
+
   describe('innerRef', () => {
     it('returns ref', () => {
       const innerRef = createRef()

test/specs/collections/Grid/GridColumn-test.js

@@ -8,6 +8,7 @@ describe('GridColumn', () => {
     rendersContent: false,
   })
 
+  common.implementsCreateMethod(GridColumn)
   common.implementsMultipleProp(GridColumn, 'only', SUI.VISIBILITY)
   common.implementsTextAlignProp(GridColumn)
   common.implementsVerticalAlignProp(GridColumn)

test/specs/elements/Step/StepContent-test.js

@@ -7,16 +7,17 @@ describe('StepContent', () => {
   common.isConformant(StepContent)
   common.rendersChildren(StepContent)
 
+  common.implementsCreateMethod(StepContent)
   common.implementsShorthandProp(StepContent, {
     autoGenerateKey: false,
     propKey: 'title',
     ShorthandComponent: StepTitle,
-    mapValueToProps: content => ({ content }),
+    mapValueToProps: (content) => ({ content }),
   })
   common.implementsShorthandProp(StepContent, {
     autoGenerateKey: false,
     propKey: 'description',
     ShorthandComponent: StepDescription,
-    mapValueToProps: content => ({ content }),
+    mapValueToProps: (content) => ({ content }),
   })
 })

test/specs/modules/Modal/Modal-test.js

@@ -502,8 +502,10 @@ describe('Modal', () => {
     })
 
     it('does not add the scrolling class to the body when equal to the window height', (done) => {
+      /* 101 is `padding * 2 + 1, see Modal/utils */
+      const height = window.innerHeight - 101
       wrapperMount(
-        <Modal open style={{ height: window.innerHeight }}>
+        <Modal open style={{ height }}>
           foo
         </Modal>,
       )

test/specs/modules/Modal/utils/canFit-test.js

@@ -0,0 +1,25 @@
+import { canFit } from 'src/modules/Modal/utils'
+
+describe('canFit', () => {
+  const innerHeight = window.innerHeight
+
+  before(() => {
+    window.innerHeight = 1000
+  })
+
+  after(() => {
+    window.innerHeight = innerHeight
+  })
+
+  it('computes proper result', () => {
+    ;[
+      // { rect: { height: 1000 }, fit: false },
+      // { rect: { height: 950 }, fit: false },
+      { rect: { height: 900 }, fit: false },
+      { rect: { height: 850 }, fit: true },
+      { rect: { height: 800 }, fit: true },
+    ].forEach((check) => {
+      canFit(check.rect).should.be.equal(check.fit)
+    })
+  })
+})

test/specs/modules/Modal/utils/getLegacyStyles-test.js

@@ -0,0 +1,18 @@
+import { getLegacyStyles } from 'src/modules/Modal/utils'
+
+const rectMock = {
+  height: 200,
+  width: 100,
+}
+
+describe('getLegacyStyles', () => {
+  it('computes proper result', () => {
+    ;[
+      { centered: true, fitted: false, result: { marginTop: 0, marginLeft: -50 } },
+      { centered: false, fitted: true, result: { marginTop: 0, marginLeft: -50 } },
+      { centered: true, fitted: true, result: { marginTop: -100, marginLeft: -50 } },
+    ].forEach(({ centered, fitted, result }) => {
+      getLegacyStyles(centered, fitted, rectMock).should.be.deep.equal(result)
+    })
+  })
+})