Skip to content

Commit

Permalink
fix(Modal): support IE11, fix scrolling glitches (#3679)
Browse files Browse the repository at this point in the history
* fix(Modal): support IE11, fix scrolling glitches

* fix UT

* add missing UT for StepContent

* add missing UT for NodeRegistry

* add missing UT for GridColumn

* add missing UT for PortalInner

* add missing UT for getLegacyStyles

* add missing UT for canFit
  • Loading branch information
layershifter authored Jul 7, 2019
1 parent 087593c commit c884fa9
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 25 deletions.
43 changes: 21 additions & 22 deletions src/modules/Modal/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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')

Expand Down Expand Up @@ -151,6 +153,7 @@ class Modal extends Component {
static Description = ModalDescription
static Actions = ModalActions

legacy = isBrowser() && isLegacy()
ref = createRef()
dimmerRef = createRef()
latestDocumentMouseDownEvent = null
Expand Down Expand Up @@ -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)

Expand All @@ -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,
Expand All @@ -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}
Expand Down
54 changes: 54 additions & 0 deletions src/modules/Modal/utils/index.js
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions test/specs/addons/MountNode/lib/NodeRegistry-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
19 changes: 19 additions & 0 deletions test/specs/addons/Portal/PortalInner-test.js
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -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()
Expand Down
1 change: 1 addition & 0 deletions test/specs/collections/Grid/GridColumn-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ describe('GridColumn', () => {
rendersContent: false,
})

common.implementsCreateMethod(GridColumn)
common.implementsMultipleProp(GridColumn, 'only', SUI.VISIBILITY)
common.implementsTextAlignProp(GridColumn)
common.implementsVerticalAlignProp(GridColumn)
Expand Down
5 changes: 3 additions & 2 deletions test/specs/elements/Step/StepContent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
})
})
4 changes: 3 additions & 1 deletion test/specs/modules/Modal/Modal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
)
Expand Down
25 changes: 25 additions & 0 deletions test/specs/modules/Modal/utils/canFit-test.js
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
18 changes: 18 additions & 0 deletions test/specs/modules/Modal/utils/getLegacyStyles-test.js
Original file line number Diff line number Diff line change
@@ -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)
})
})
})

0 comments on commit c884fa9

Please sign in to comment.