Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui-modal): make Modal's header non-sticky with small window height #1854

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/ui-modal/src/Modal/ModalHeader/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type ModalHeaderOwnProps = {
children?: React.ReactNode
variant?: 'default' | 'inverse'
spacing?: 'default' | 'compact'
smallPortView?: boolean
}

type ModalHeaderStyleProps = {
Expand All @@ -55,7 +56,8 @@ type ModalHeaderStyle = ComponentStyle<'modalHeader'>
const propTypes: PropValidators<PropKeys> = {
children: PropTypes.node,
variant: PropTypes.oneOf(['default', 'inverse']),
spacing: PropTypes.oneOf(['default', 'compact'])
spacing: PropTypes.oneOf(['default', 'compact']),
smallPortView: PropTypes.bool
}

const allowedProps: AllowedPropKeys = ['children', 'variant', 'spacing']
Expand Down
3 changes: 2 additions & 1 deletion packages/ui-modal/src/Modal/ModalHeader/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const generateStyle = (
props: ModalHeaderProps,
state: ModalHeaderStyleProps
): ModalHeaderStyle => {
const { variant, spacing } = props
const { variant, spacing, smallPortView } = props
const { withCloseButton } = state

const sizeVariants = {
Expand Down Expand Up @@ -83,6 +83,7 @@ const generateStyle = (
borderBottomWidth: '0.0625rem',
borderBottomStyle: 'solid',
borderBottomColor: componentTheme.borderColor,
...(smallPortView ? { position: 'relative' } : {}),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to add this because otherwise the close button starts scrolling with the text instead of staying in the Modal.Header

...sizeVariants[spacing!],
...inverseStyle
}
Expand Down
88 changes: 81 additions & 7 deletions packages/ui-modal/src/Modal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { Children, Component, isValidElement, ReactElement } from 'react'
import { passthroughProps, safeCloneElement } from '@instructure/ui-react-utils'
import { createChainedFunction } from '@instructure/ui-utils'
import { testable } from '@instructure/ui-testable'
import { canUseDOM } from '@instructure/ui-dom-utils'

import { Transition } from '@instructure/ui-motion'
import { Portal } from '@instructure/ui-portal'
Expand Down Expand Up @@ -86,7 +87,8 @@ class Modal extends Component<ModalProps, ModalState> {

this.state = {
transitioning: false,
open: props.open ?? false
open: props.open ?? false,
windowHeight: 99999
}
}

Expand All @@ -101,6 +103,7 @@ class Modal extends Component<ModalProps, ModalState> {

componentDidMount() {
this.props.makeStyles?.()
window.addEventListener('resize', this.updateHeight)
}

componentDidUpdate(prevProps: ModalProps) {
Expand All @@ -110,6 +113,14 @@ class Modal extends Component<ModalProps, ModalState> {
this.props.makeStyles?.()
}

componentWillUnmount() {
window.removeEventListener('resize', this.updateHeight)
}

updateHeight = () => {
this.setState({ windowHeight: window.innerHeight })
}

get defaultFocusElement() {
return this.props.defaultFocusElement
}
Expand Down Expand Up @@ -146,21 +157,84 @@ class Modal extends Component<ModalProps, ModalState> {
}
}

getWindowHeightInRem = (): number => {
if (!canUseDOM) {
return Infinity
}
const rootFontSize = parseFloat(
getComputedStyle(document.documentElement)?.fontSize || '16'
)
if (isNaN(rootFontSize) || rootFontSize <= 0) {
return Infinity
}
return window.innerHeight / rootFontSize
}

renderChildren() {
const { children, variant, overflow } = this.props

// header should be non-sticky for small viewport height (ca. 320px)
if (this.getWindowHeightInRem() <= 20) {
return this.renderForSmallViewportHeight()
}

return Children.map(children as ReactElement, (child) => {
if (!child) return // ignore null, falsy children
return this.cloneChildWithProps(child, variant, overflow)
})
}

renderForSmallViewportHeight() {
const { children, variant, overflow, styles } = this.props

const headerAndBody: React.ReactNode[] = []

const childrenArray = Children.toArray(children)

// Separate header and body elements
const filteredChildren = childrenArray.filter((child) => {
if (isValidElement(child)) {
return safeCloneElement(child, {
variant: variant,
overflow: (child?.props as { overflow: string })?.overflow || overflow
})
} else {
return child
Comment on lines -156 to -161
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this to the cloneChildWithProps function

if (child.type === Modal.Header || child.type === Modal.Body) {
if (child.type === Modal.Header) {
const headerWithProp = safeCloneElement(child, {
smallPortView: true
})
headerAndBody.push(headerWithProp)
} else {
headerAndBody.push(child)
}
return false
}
}
return true
})

// Adds the <div> to the beginning of the filteredChildren array
if (headerAndBody.length > 0) {
Comment on lines +212 to +213
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to put the header and the body into the same element in order to be able scroll them together, so it results in some minor DOM change.

filteredChildren.unshift(
<div css={styles?.joinedHeaderAndBody}>{headerAndBody}</div>
)
}

return Children.map(filteredChildren as ReactElement[], (child) => {
if (!child) return // ignore null, falsy children
return this.cloneChildWithProps(child, variant, overflow)
})
}

cloneChildWithProps(
child: React.ReactNode,
variant: string | undefined,
overflow: string | undefined
) {
if (isValidElement(child)) {
return safeCloneElement(child, {
variant: variant,
overflow: (child?.props as { overflow: string })?.overflow || overflow
})
} else {
return child
}
}

renderDialog(
Expand Down
5 changes: 4 additions & 1 deletion packages/ui-modal/src/Modal/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,14 @@ type ModalProps = ModalOwnProps &
WithStyleProps<ModalTheme, ModalStyle> &
OtherHTMLAttributes<ModalOwnProps>

type ModalStyle = ComponentStyle<'modal' | 'constrainContext'>
type ModalStyle = ComponentStyle<
'modal' | 'constrainContext' | 'joinedHeaderAndBody'
>

type ModalState = {
transitioning: boolean
open: boolean
windowHeight: number
}

const propTypes: PropValidators<PropKeys> = {
Expand Down
4 changes: 4 additions & 0 deletions packages/ui-modal/src/Modal/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ const generateStyle = (
position: 'relative',
width: '100%',
height: '100%'
},
joinedHeaderAndBody: {
borderRadius: componentTheme.borderRadius,
overflowY: 'scroll'
}
}
}
Expand Down
Loading