Skip to content

Commit a473127

Browse files
committed
fix(ui-modal): save modal from unnecessary rerender
INSTUI-4575
1 parent 23be361 commit a473127

File tree

5 files changed

+34
-60
lines changed

5 files changed

+34
-60
lines changed

packages/ui-modal/src/Modal/ModalBody/styles.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,13 @@ const generateStyle = (
5353
label: 'modalBody',
5454
boxSizing: 'border-box',
5555
flex: '1 1 auto',
56-
overflowY: 'auto',
5756
'&:focus': {
5857
outline: 'none'
5958
},
59+
// ModalBody is set to scrollable above 20rem height
60+
'@media (min-height: 20rem)': {
61+
overflowY: 'auto'
62+
},
6063
...backgroundStyle
6164
}
6265
}

packages/ui-modal/src/Modal/ModalHeader/props.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ type ModalHeaderOwnProps = {
3636
children?: React.ReactNode
3737
variant?: 'default' | 'inverse'
3838
spacing?: 'default' | 'compact'
39-
smallViewPort?: boolean
4039
}
4140

4241
type ModalHeaderStyleProps = {
@@ -56,8 +55,7 @@ type ModalHeaderStyle = ComponentStyle<'modalHeader'>
5655
const propTypes: PropValidators<PropKeys> = {
5756
children: PropTypes.node,
5857
variant: PropTypes.oneOf(['default', 'inverse']),
59-
spacing: PropTypes.oneOf(['default', 'compact']),
60-
smallViewPort: PropTypes.bool
58+
spacing: PropTypes.oneOf(['default', 'compact'])
6159
}
6260

6361
const allowedProps: AllowedPropKeys = ['children', 'variant', 'spacing']

packages/ui-modal/src/Modal/ModalHeader/styles.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const generateStyle = (
4444
props: ModalHeaderProps,
4545
state: ModalHeaderStyleProps
4646
): ModalHeaderStyle => {
47-
const { variant, spacing, smallViewPort } = props
47+
const { variant, spacing } = props
4848
const { withCloseButton } = state
4949

5050
const sizeVariants = {
@@ -83,9 +83,13 @@ const generateStyle = (
8383
borderBottomWidth: '0.0625rem',
8484
borderBottomStyle: 'solid',
8585
borderBottomColor: componentTheme.borderColor,
86-
...(smallViewPort ? { position: 'relative' } : {}),
8786
...sizeVariants[spacing!],
88-
...inverseStyle
87+
...inverseStyle,
88+
89+
// Position is set to relative for small viewports to ensure proper close button positioning during scrolling
90+
'@media (max-height: 20rem)': {
91+
position: 'relative'
92+
}
8993
}
9094
}
9195
}

packages/ui-modal/src/Modal/index.tsx

Lines changed: 13 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,11 @@
2222
* SOFTWARE.
2323
*/
2424

25-
import { Children, Component, isValidElement, ReactElement } from 'react'
25+
import { Children, Component, isValidElement } from 'react'
2626

2727
import { passthroughProps, safeCloneElement } from '@instructure/ui-react-utils'
2828
import { createChainedFunction } from '@instructure/ui-utils'
2929
import { testable } from '@instructure/ui-testable'
30-
import { canUseDOM } from '@instructure/ui-dom-utils'
3130

3231
import { Transition } from '@instructure/ui-motion'
3332
import { Portal } from '@instructure/ui-portal'
@@ -156,69 +155,31 @@ class Modal extends Component<ModalProps, ModalState> {
156155
}
157156
}
158157

159-
getWindowHeightInRem = (): number => {
160-
if (!canUseDOM) {
161-
return Infinity
162-
}
163-
const rootFontSize = parseFloat(
164-
getComputedStyle(document.documentElement)?.fontSize || '16'
165-
)
166-
if (isNaN(rootFontSize) || rootFontSize <= 0) {
167-
return Infinity
168-
}
169-
return window.innerHeight / rootFontSize
170-
}
171-
172158
renderChildren() {
173-
const { children, variant, overflow } = this.props
174-
175-
// header should be non-sticky for small viewport height (ca. 320px)
176-
if (this.getWindowHeightInRem() <= 20) {
177-
return this.renderForSmallViewportHeight()
178-
}
179-
180-
return Children.map(children as ReactElement, (child) => {
181-
if (!child) return // ignore null, falsy children
182-
return this.cloneChildWithProps(child, variant, overflow)
183-
})
184-
}
185-
186-
renderForSmallViewportHeight() {
187159
const { children, variant, overflow, styles } = this.props
160+
const childrenArray = Children.toArray(children)
188161

189162
const headerAndBody: React.ReactNode[] = []
163+
const others: React.ReactNode[] = []
190164

191-
const childrenArray = Children.toArray(children)
192-
193-
// Separate header and body elements
194-
const filteredChildren = childrenArray.filter((child) => {
165+
childrenArray.forEach((child) => {
195166
if (isValidElement(child)) {
196167
if (child.type === Modal.Header || child.type === Modal.Body) {
197-
if (child.type === Modal.Header) {
198-
const headerWithProp = safeCloneElement(child, {
199-
smallViewPort: true
200-
})
201-
headerAndBody.push(headerWithProp)
202-
} else {
203-
headerAndBody.push(child)
204-
}
205-
return false
168+
headerAndBody.push(this.cloneChildWithProps(child, variant, overflow))
169+
} else {
170+
others.push(this.cloneChildWithProps(child, variant, overflow))
206171
}
207172
}
208-
return true
209173
})
210174

211-
// Adds the <div> to the beginning of the filteredChildren array
212-
if (headerAndBody.length > 0) {
213-
filteredChildren.unshift(
175+
// Putting ModalHeader and ModalBody into the same container, so they can be styled together;
176+
// this is needed to apply a media query breakpoint
177+
const wrappedHeaderAndBody =
178+
headerAndBody.length > 0 ? (
214179
<div css={styles?.joinedHeaderAndBody}>{headerAndBody}</div>
215-
)
216-
}
180+
) : null
217181

218-
return Children.map(filteredChildren as ReactElement[], (child) => {
219-
if (!child) return // ignore null, falsy children
220-
return this.cloneChildWithProps(child, variant, overflow)
221-
})
182+
return [...(wrappedHeaderAndBody ? [wrappedHeaderAndBody] : []), ...others]
222183
}
223184

224185
cloneChildWithProps(

packages/ui-modal/src/Modal/styles.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,15 @@ const generateStyle = (
111111
},
112112
joinedHeaderAndBody: {
113113
borderRadius: componentTheme.borderRadius,
114-
overflowY: 'scroll'
114+
display: 'flex',
115+
flexDirection: 'column',
116+
overflow: 'hidden',
117+
118+
// ModalHeader and ModalBody is set to scrollable above 20rem height instead of just the ModalBody
119+
'@media (max-height: 20rem)': {
120+
overflowY: 'auto',
121+
maxHeight: '20rem'
122+
}
115123
}
116124
}
117125
}

0 commit comments

Comments
 (0)