Skip to content

Commit

Permalink
Fix/scroll lock (#1597)
Browse files Browse the repository at this point in the history
* fix: Scroll Lock Issue

first: Opening an ActionMenu was not locking the scroll

second: We had an issue with body-scroll-lock and MUI's Modal
See mui/material-ui#5750 (comment)
for more informations. To resume: MUI adds overflow:hidden
on body when opening a Modal, body-scroll-lock too (throught Overlay
-> ActionMenu).

So when opening a Dialog from an ActionMenu, it's result on a conflict
between MUI and body-scroll-lock : We can't scroll anymore after
closing the dialog.

By using React-Remove-Scroll we're avoiding this issue.

* docs: Open modal from ActionMenu

* refactor: Don't do lot of work when inlining

returns as quick as possible

* feat: Block Scroll when BottomDrawer is opened

* feat: Block Scroll when Dialog is opened

* fix: Don't block if the content is smaller

We use Overlay when opening an ActionMenu. We
don't want to block the scroll on Desktop if the
ActionMenu is displayed.

* fix: Snapshots

* test: Make Dialog sreenshots per example
  • Loading branch information
Crash-- authored Oct 22, 2020
1 parent 9166356 commit a2eaf3a
Show file tree
Hide file tree
Showing 10 changed files with 338 additions and 93 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@
"dependencies": {
"@babel/runtime": "^7.3.4",
"@popperjs/core": "^2.4.4",
"body-scroll-lock": "^2.5.8",
"classnames": "^2.2.5",
"cozy-interapp": "0.5.4",
"date-fns": "^1.28.5",
Expand All @@ -147,6 +146,7 @@
"react-markdown": "^4.0.8",
"react-pdf": "^4.0.5",
"react-popper": "^2.2.3",
"react-remove-scroll": "^2.4.0",
"react-select": "2.2.0",
"react-swipeable-views": "0.13.3"
},
Expand Down
86 changes: 82 additions & 4 deletions react/ActionMenu/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Use an ActionMenu to show a list of actions. ActionMenus automatically switch th

### Classic

```
```jsx
import ActionMenu, { ActionMenuItem, ActionMenuRadio } from 'cozy-ui/transpiled/react/ActionMenu';
import DropdownButton from 'cozy-ui/transpiled/react/DropdownButton';
import Icon from 'cozy-ui/transpiled/react/Icon';
Expand Down Expand Up @@ -39,7 +39,7 @@ desktop, we display a popper and not a BottomDrawer, context for the
user is not lost, so the ActionMenuHeader would be redundant. This is
why it is not rendered unless we are on mobile.

```
```jsx
import ActionMenu, { ActionMenuItem, ActionMenuHeader } from './index';
import DropdownButton from 'cozy-ui/transpiled/react/DropdownButton';
import Icon from '../Icon';
Expand Down Expand Up @@ -67,7 +67,7 @@ const hideMenu = () => setState({ menuDisplayed: false });

### With dedicated click handler

```
```jsx
import ActionMenu, { ActionMenuItem, ActionMenuHeader } from './index';
import DropdownButton from 'cozy-ui/transpiled/react/DropdownButton';
import Icon from '../Icon';
Expand Down Expand Up @@ -96,7 +96,7 @@ const hideMenu = () => setState({ menuDisplayed: false });
You can pass a reference to a custom DOM element through the `anchorElRef` prop to attach the menu to that element.
We use [popper.js](https://popper.js.org/docs/v2/) under the hood. You can use the `popperOptions` prop to pass options to the popper.js instance. This lets you control things like placement relative to the anchor, positioning strategies and more — refer to the popper doc for all the details.

```
```jsx
import ActionMenu, { ActionMenuItem } from 'cozy-ui/transpiled/react/ActionMenu';
import DropdownButton from 'cozy-ui/transpiled/react/DropdownButton';
import Icon from 'cozy-ui/transpiled/react/Icon';
Expand All @@ -121,3 +121,81 @@ const anchorRef = React.createRef();
</ActionMenu>}
</div>
```

### Open Dialog from ActionMenu

```jsx
import ActionMenu, { ActionMenuItem } from 'cozy-ui/transpiled/react/ActionMenu';
import DropdownButton from 'cozy-ui/transpiled/react/DropdownButton';
import Icon from 'cozy-ui/transpiled/react/Icon';
import Dialog, {
DialogTitle,
DialogActions,
DialogContent,
DialogContentText,
DialogCloseButton,
} from 'cozy-ui/transpiled/react/Dialog';
import {
BreakpointsProvider
} from 'cozy-ui/transpiled/react/hooks/useBreakpoints'
import MuiCozyTheme from 'cozy-ui/transpiled/react/MuiCozyTheme/'
import Button from 'cozy-ui/transpiled/react/Button'

const testRef = React.createRef();

initialState = { menuDisplayed: isTesting(), modalOpened: isTesting() };

const showMenu = () => setState({ menuDisplayed: true });
const hideMenu = () => setState({ menuDisplayed: false });

const anchorRef = React.createRef();
const onClose = () => setState({ modalOpened: !state.modalOpened });

<div>
<BreakpointsProvider>
<MuiCozyTheme>
<>
<DropdownButton onClick={showMenu} ref={anchorRef}>Show action menu</DropdownButton>
{state.menuDisplayed &&
<ActionMenu
anchorElRef={anchorRef}
popperOptions={{ placement: 'bottom-end'}}
onClose={hideMenu}>
<ActionMenuItem
left={<Icon icon='file' />}
onClick={() => setState({ modalOpened: !state.modalOpened })}>
Item 1
</ActionMenuItem>
</ActionMenu>}
<Dialog open={state.modalOpened} onClose={() => onClose()}>
<DialogCloseButton onClick={() => onClose()} />
<DialogTitle>Ada Lovelace</DialogTitle>
<DialogContent>
Augusta Ada King-Noel, Countess of Lovelace (née Byron; 10 December 1815
27 November 1852) was an English mathematician and writer, chiefly
known for her work on Charles Babbage's proposed mechanical
general-purpose computer, the Analytical Engine. She was the first to
recognise that the machine had applications beyond pure calculation, and
published the first algorithm intended to be carried out by such a
machine. As a result, she is often regarded as the first to recognise
the full potential of a "computing machine" and the first computer
programmer.
</DialogContent>
<DialogActions layout="row">
<Button
theme="secondary"
onClick={() => onClose()}
label={'Close Modal'}
/>
<Button
theme="primary"
label={'Touch Me'}
onClick={() => alert('click')}
/>
</DialogActions>
</Dialog>
</>
</MuiCozyTheme>
</BreakpointsProvider>
</div>
```
28 changes: 23 additions & 5 deletions react/ActionMenu/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,28 @@ const ActionMenuWrapper = ({
placement,
preventOverflow,
children
}) => {
if (!inline) return <BottomDrawer onClose={onClose}>{children}</BottomDrawer>
return (
<NotInlineWrapper
anchorElRef={anchorElRef}
popperOptions={popperOptions}
placement={placement}
preventOverflow={preventOverflow}
onClose={onClose}
>
{children}
</NotInlineWrapper>
)
}

const NotInlineWrapper = ({
anchorElRef,
popperOptions,
placement,
preventOverflow,
onClose,
children
}) => {
const [popperElement, setPopperElement] = React.useState(null)
const referenceElement = anchorElRef ? anchorElRef.current : null
Expand All @@ -44,8 +66,7 @@ const ActionMenuWrapper = ({
popperElement,
options
)

return inline ? (
return (
<div
ref={setPopperElement}
style={{
Expand All @@ -56,11 +77,8 @@ const ActionMenuWrapper = ({
>
<ClickAwayListener onClickAway={onClose}>{children}</ClickAwayListener>
</div>
) : (
<BottomDrawer onClose={onClose}>{children}</BottomDrawer>
)
}

const logDepecratedPlacement = createDepreciationLogger()
const logDepecratedOverflow = createDepreciationLogger()
const logDepecratedContainer = createDepreciationLogger()
Expand Down
26 changes: 15 additions & 11 deletions react/BottomDrawer/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import React, { Component } from 'react'
import PropTypes from 'prop-types'
import Hammer from 'hammerjs'
import once from 'lodash/once'
import { RemoveScroll } from 'react-remove-scroll'

import styles from './styles.styl'
import Overlay from '../Overlay'

Expand Down Expand Up @@ -128,17 +130,19 @@ class BottomDrawer extends Component {
this.menuNode = React.createRef()
this.wrapperNode = React.createRef()
return (
<div ref={this.wrapperNode}>
<Overlay
style={{ opacity: closing ? 0 : 1 }}
onClick={this.animateClose}
onEscape={this.animateClose}
>
<div ref={this.menuNode} className={styles['BottomDrawer-content']}>
{children}
</div>
</Overlay>
</div>
<RemoveScroll>
<div ref={this.wrapperNode}>
<Overlay
style={{ opacity: closing ? 0 : 1 }}
onClick={this.animateClose}
onEscape={this.animateClose}
>
<div ref={this.menuNode} className={styles['BottomDrawer-content']}>
{children}
</div>
</Overlay>
</div>
</RemoveScroll>
)
}
}
Expand Down
52 changes: 52 additions & 0 deletions react/Dialog/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,55 @@ initialState = { modalOpened: isTesting() }
</BreakpointsProvider>
</>
```
### With long content
```jsx
import Dialog, {
DialogTitle,
DialogActions,
DialogContent,
DialogContentText,
DialogCloseButton,
} from 'cozy-ui/transpiled/react/Dialog';

import {
BreakpointsProvider
} from 'cozy-ui/transpiled/react/hooks/useBreakpoints'

import MuiCozyTheme from 'cozy-ui/transpiled/react/MuiCozyTheme/'
import Button from 'cozy-ui/transpiled/react/Button'
const onClose = () => setState({ modalOpened: !state.modalOpened })

initialState = { modalOpened: isTesting() }
;<>
<button onClick={() => setState({ modalOpened: !state.modalOpened })}>
Toggle modal
</button>
<BreakpointsProvider>
<MuiCozyTheme>
<Dialog open={state.modalOpened} onClose={() => onClose()}>
<DialogCloseButton onClick={() => onClose()} />
<DialogTitle>Ada Lovelace</DialogTitle>
<DialogContent>
{content.ada.long}
</DialogContent>
<DialogActions layout="row">
<Button
theme="secondary"
onClick={() => onClose()}
label={'Close Modal'}
/>
<Button
theme="primary"
label={'Touch Me'}
onClick={() => alert('click')}
/>
</DialogActions>
</Dialog>
</MuiCozyTheme>
</BreakpointsProvider>
</>
```
24 changes: 14 additions & 10 deletions react/Dialog/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,24 @@ import React from 'react'
import MUIDialog from '@material-ui/core/Dialog'
import withMobileDialog from '@material-ui/core/withMobileDialog'
import PropTypes from 'prop-types'
import { RemoveScroll } from 'react-remove-scroll'

export const Dialog = props => {
const { onClose, fullScreen, children, open, scroll, ...otherProps } = props
const Wrapper = open ? RemoveScroll : React.Fragment
return (
<MUIDialog
open={open}
onClose={onClose}
fullScreen={fullScreen}
aria-labelledby="dialog-title"
scroll={scroll}
{...otherProps}
>
{children}
</MUIDialog>
<Wrapper>
<MUIDialog
open={open}
onClose={onClose}
fullScreen={fullScreen}
aria-labelledby="dialog-title"
scroll={scroll}
{...otherProps}
>
{children}
</MUIDialog>
</Wrapper>
)
}
Dialog.defaultProps = {
Expand Down
17 changes: 8 additions & 9 deletions react/Overlay/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import styles from './styles.styl'
import cx from 'classnames'
import omit from 'lodash/omit'
import PropTypes from 'prop-types'
import { disableBodyScroll, clearAllBodyScrollLocks } from 'body-scroll-lock'
import { RemoveScroll } from 'react-remove-scroll'

const ESC_KEYCODE = 27

Expand All @@ -21,10 +21,6 @@ const bodyTallerThanWindow = () => {
*/
class Overlay extends Component {
componentDidMount() {
if (bodyTallerThanWindow()) {
disableBodyScroll(document.body)
disableBodyScroll(document.body.parentNode)
}
if (this.props.onEscape) {
document.addEventListener('keydown', this.handleKeydown)
}
Expand All @@ -37,9 +33,6 @@ class Overlay extends Component {
}

componentWillUnmount() {
// restauration function can be undefined if there was
// an error during mounting/rendering
clearAllBodyScrollLocks()
if (this.props.onEscape) {
document.removeEventListener('keydown', this.handleKeydown)
}
Expand All @@ -54,13 +47,19 @@ class Overlay extends Component {
render() {
const { children, className } = this.props
const domProps = omit(this.props, nonDOMProps)
// We use Overlay when opening an ActionMenu.
// We don't want to block the scroll on Desktop if the ActionMenu
// is displayed. So no RemoveScroll if the content is too small
// @todo Overlay should not RemoveScroll by itself. It should
// be done by lower component (like ActionMenu / Dialog / Modal...)
const Wrapper = bodyTallerThanWindow() ? React.Fragment : RemoveScroll
return (
<div
onClick={this.handleClick}
className={cx(styles['c-overlay'], className)}
{...domProps}
>
{children}
<Wrapper>{children}</Wrapper>
</div>
)
}
Expand Down
Loading

0 comments on commit a2eaf3a

Please sign in to comment.