Skip to content

Commit

Permalink
fix(base-modal): modal scroll (#820)
Browse files Browse the repository at this point in the history
* fix(base-modal): fix scroll

* fix(base-modal): container styles restore
  • Loading branch information
reme3d2y committed Sep 10, 2021
1 parent 0d5b2a3 commit 1b2d94a
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 75 deletions.
12 changes: 6 additions & 6 deletions packages/base-modal/src/Component.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,26 +79,26 @@ describe('BaseModal', () => {
it('should attach a handler to the backdrop that fires onClose', () => {
const onClose = jest.fn();
const { getByRole } = render(modal({ onClose }));
const backdrop = getByRole('dialog').firstChild as HTMLElement;
fireEvent.click(backdrop);

fireEvent.click(getByRole('dialog'));

expect(onClose.mock.calls.length).toBe(1);
});

it('should let the user disable backdrop click triggering onClose', () => {
const onClose = jest.fn();
const { getByRole } = render(modal({ onClose, disableBackdropClick: true }));
const backdrop = getByRole('dialog').firstChild as HTMLElement;
fireEvent.click(backdrop);

fireEvent.click(getByRole('dialog'));

expect(onClose.mock.calls.length).toBe(0);
});

it('should call through to the user specified onBackdropClick callback', () => {
const onBackdropClick = jest.fn();
const { getByRole } = render(modal({ onBackdropClick }));
const backdrop = getByRole('dialog').firstChild as HTMLElement;
fireEvent.click(backdrop);

fireEvent.click(getByRole('dialog'));

expect(onBackdropClick.mock.calls.length).toBe(1);
});
Expand Down
60 changes: 37 additions & 23 deletions packages/base-modal/src/Component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ import { Portal, PortalProps } from '@alfalab/core-components-portal';
import { Backdrop as DefaultBackdrop, BackdropProps } from '@alfalab/core-components-backdrop';
import { Stack, stackingOrder } from '@alfalab/core-components-stack';

import { handleContainer, hasScrollbar, isScrolledToBottom, isScrolledToTop } from './utils';
import {
handleContainer,
hasScrollbar,
isScrolledToBottom,
isScrolledToTop,
restoreContainerStyles,
} from './utils';

import styles from './index.module.css';

Expand Down Expand Up @@ -222,7 +228,7 @@ export const BaseModal = forwardRef<HTMLDivElement, BaseModalProps>(
const wrapperRef = useRef<HTMLDivElement>(null);
const scrollableNodeRef = useRef<HTMLDivElement | null>(null);
const contentNodeRef = useRef<HTMLDivElement | null>(null);
const restoreContainerStyles = useRef<null | Function>(null);
const restoreContainerStylesRef = useRef<null | Function>(null);

const checkToHasScrollBar = () => {
if (scrollableNodeRef.current) {
Expand All @@ -234,6 +240,10 @@ export const BaseModal = forwardRef<HTMLDivElement, BaseModalProps>(

const shouldRender = keepMounted || open || !exited;

const getContainer = useCallback(() => {
return (container ? container() : document.body) as HTMLElement;
}, [container]);

const resizeObserver = useMemo(() => new ResizeObserver(checkToHasScrollBar), []);

const addResizeHandle = useCallback(() => {
Expand Down Expand Up @@ -294,7 +304,7 @@ export const BaseModal = forwardRef<HTMLDivElement, BaseModalProps>(
);

const handleBackdropClick = (event: MouseEvent<HTMLElement>) => {
if (!disableBackdropClick) {
if (!disableBackdropClick && event.target === wrapperRef.current) {
handleClose(event, 'backdropClick');
}
};
Expand Down Expand Up @@ -361,34 +371,35 @@ export const BaseModal = forwardRef<HTMLDivElement, BaseModalProps>(

if (onUnmount) onUnmount();

if (restoreContainerStyles.current) {
restoreContainerStyles.current();
restoreContainerStyles.current = null;
if (restoreContainerStylesRef.current) {
restoreContainerStylesRef.current();
}
},
[handleScroll, onUnmount, removeResizeHandle, transitionProps],
);

useEffect(() => {
if (open) {
restoreContainerStyles.current = handleContainer(
(container ? container() : document.body) as HTMLElement,
);
handleContainer(getContainer());

restoreContainerStylesRef.current = () => {
restoreContainerStylesRef.current = null;
restoreContainerStyles(getContainer());
};
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [open]);
}, [getContainer, open]);

useEffect(() => {
if (open) setExited(false);
}, [open]);

useEffect(() => {
return () => {
resizeObserver.disconnect();

if (restoreContainerStyles.current) {
restoreContainerStyles.current();
if (restoreContainerStylesRef.current) {
restoreContainerStylesRef.current();
}

resizeObserver.disconnect();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down Expand Up @@ -428,27 +439,30 @@ export const BaseModal = forwardRef<HTMLDivElement, BaseModalProps>(
disabled={disableFocusLock || !open}
returnFocus={!disableRestoreFocus}
>
{Backdrop && (
<Backdrop
{...backdropProps}
className={cn(backdropProps.className, styles.backdrop)}
open={open}
style={{
zIndex: computedZIndex,
}}
/>
)}
<div
role='dialog'
className={cn(styles.wrapper, wrapperClassName, {
[styles.hidden]: !open && exited,
})}
ref={mergeRefs([ref, wrapperRef])}
onKeyDown={handleKeyDown}
onClick={handleBackdropClick}
tabIndex={-1}
data-test-id={dataTestId}
style={{
zIndex: computedZIndex,
}}
>
{Backdrop && (
<Backdrop
{...backdropProps}
className={cn(backdropProps.className, styles.backdrop)}
open={open}
onClick={handleBackdropClick}
/>
)}
<CSSTransition
appear={true}
timeout={200}
Expand Down
2 changes: 1 addition & 1 deletion packages/base-modal/src/index.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
}

.backdrop {
z-index: -1;
z-index: 0;
}

.appear,
Expand Down
69 changes: 52 additions & 17 deletions packages/base-modal/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,56 @@ const getPaddingRight = (node: Element) => {
return parseInt(window.getComputedStyle(node).paddingRight, 10) || 0;
};

export const handleContainer = (container: HTMLElement) => {
const restoreStyle: Array<{
value: string;
key: string;
el: HTMLElement;
}> = [];
type SavedStyle = {
value: string;
key: string;
el: HTMLElement;
};

const restoreStylesStore: Array<{
container: HTMLElement;
modals: number;
styles: SavedStyle[];
}> = [];

export const restoreContainerStyles = (container: HTMLElement) => {
const index = restoreStylesStore.findIndex(s => s.container === container);
const existingStyles = restoreStylesStore[index];

if (!existingStyles) return;

existingStyles.modals -= 1;

if (existingStyles.modals <= 0) {
restoreStylesStore.splice(index, 1);

existingStyles.styles.forEach(({ value, el, key }) => {
if (value) {
el.style.setProperty(key, value);
} else {
el.style.removeProperty(key);
}
});
}
};

export const handleContainer = (container?: HTMLElement) => {
if (!container) return;

const existingStyles = restoreStylesStore.find(s => s.container === container);

if (existingStyles) {
existingStyles.modals += 1;
return;
}

const containerStyles: SavedStyle[] = [];

if (isOverflowing(container)) {
// Вычисляет размер до применения `overflow hidden` для избежания скачков
const scrollbarSize = getScrollbarSize();

restoreStyle.push({
containerStyles.push({
value: container.style.paddingRight,
key: 'padding-right',
el: container,
Expand All @@ -69,21 +107,18 @@ export const handleContainer = (container: HTMLElement) => {

// Блокируем скролл даже если отсутствует скроллбар
if (scrollContainer.style.overflow !== 'hidden') {
restoreStyle.push({
containerStyles.push({
value: scrollContainer.style.overflow,
key: 'overflow',
el: scrollContainer,
});
}

scrollContainer.style.overflow = 'hidden';

return () => {
restoreStyle.forEach(({ value, el, key }) => {
if (value) {
el.style.setProperty(key, value);
} else {
el.style.removeProperty(key);
}
});
};
restoreStylesStore.push({
container,
modals: 1,
styles: containerStyles,
});
};
32 changes: 16 additions & 16 deletions packages/bottom-sheet/src/__snapshots__/component.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,20 @@ exports[`Bottom sheet Snapshots tests should match snapshot 1`] = `
<div
data-focus-lock-disabled="false"
>
<div
style="opacity: 1; transition: opacity 300ms ease-in-out; position: relative; z-index: 100;"
>
<div
aria-hidden="true"
class="backdrop appear appearActive"
/>
</div>
<div
class="wrapper"
role="dialog"
style="z-index: 100;"
tabindex="-1"
>
<div
style="opacity: 1; transition: opacity 300ms ease-in-out;"
>
<div
aria-hidden="true"
class="backdrop appear appearActive"
/>
</div>
<div
class="component modal appear appearActive"
>
Expand Down Expand Up @@ -97,20 +97,20 @@ exports[`Bottom sheet Snapshots tests should match snapshot with action button 1
<div
data-focus-lock-disabled="false"
>
<div
style="opacity: 1; transition: opacity 300ms ease-in-out; position: relative; z-index: 100;"
>
<div
aria-hidden="true"
class="backdrop appear appearActive"
/>
</div>
<div
class="wrapper"
role="dialog"
style="z-index: 100;"
tabindex="-1"
>
<div
style="opacity: 1; transition: opacity 300ms ease-in-out;"
>
<div
aria-hidden="true"
class="backdrop appear appearActive"
/>
</div>
<div
class="component modal appear appearActive"
>
Expand Down
26 changes: 18 additions & 8 deletions packages/bottom-sheet/src/component.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,22 +166,32 @@ describe('Bottom sheet', () => {
});

describe('Interactions tests', () => {
it('should close if on backdrop click', async () => {
it('should close on dialog click', async () => {
const { getByTestId, queryByTestId } = render(
<BottomSheetWrapper dataTestId={dataTestId} />,
);

const backdrop = document.querySelector('.backdrop');

if (backdrop) {
fireEvent.click(backdrop);
}
fireEvent.click(getByTestId(dataTestId));

await waitForElementToBeRemoved(() => getByTestId(dataTestId));

const component = await queryByTestId(dataTestId);
expect(queryByTestId(dataTestId)).not.toBeInTheDocument();
});

expect(component).not.toBeInTheDocument();
it('should not close on dialog content click', async () => {
const { getByTestId, queryByTestId } = render(
<BottomSheetWrapper dataTestId={dataTestId} />,
);

const content = getByTestId(dataTestId).firstElementChild as HTMLElement;

if (content) {
fireEvent.click(content);
}

await new Promise(res => setTimeout(res, 1000));

expect(queryByTestId(dataTestId)).toBeInTheDocument();
});

it('should swiping on touchmove', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ export const SwipeableBackdrop: FC<SwipeableBackdropProps> = ({
opacity,
handlers,
opacityTimeout,
style,
...backdropProps
}) => (
<div
{...handlers}
style={{
opacity,
transition: opacity === 1 ? `opacity ${opacityTimeout}ms ease-in-out` : '',
position: 'relative',
...style,
}}
>
<Backdrop {...backdropProps} />
Expand Down
4 changes: 0 additions & 4 deletions packages/drawer/src/__snapshots__/Component.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ exports[`Drawer should match snapshot 1`] = `
style="z-index: 100;"
tabindex="-1"
>
<div
aria-hidden="true"
class="backdrop backdropEnter backdropEnterActive"
/>
<div
class="component enter enterActive"
>
Expand Down

0 comments on commit 1b2d94a

Please sign in to comment.