Skip to content

Commit

Permalink
fix(modals): prevent html overflow
Browse files Browse the repository at this point in the history
  • Loading branch information
hzhu committed Oct 8, 2020
1 parent 94002e5 commit 7fef3f2
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 19 deletions.
16 changes: 8 additions & 8 deletions packages/modals/.size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
{
"index.cjs.js": {
"bundled": 52497,
"minified": 37987,
"gzipped": 7696
"bundled": 53203,
"minified": 38201,
"gzipped": 7763
},
"index.esm.js": {
"bundled": 48983,
"minified": 34897,
"gzipped": 7518,
"bundled": 49689,
"minified": 35111,
"gzipped": 7589,
"treeshaked": {
"rollup": {
"code": 27649,
"code": 27863,
"import_statements": 792
},
"webpack": {
"code": 30826
"code": 31040
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions packages/modals/src/elements/DrawerModal/DrawerModal.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('DrawerModal', () => {

it('only locks page scrolling when drawer modal is opened', async () => {
const { getByText, getByRole, queryByRole } = render(<Example />);
const htmlElement = document.querySelector('html');

expect(queryByRole('dialog')).not.toBeInTheDocument();
expect(document.body).not.toHaveAttribute('style', 'overflow: hidden;');
Expand All @@ -57,11 +58,13 @@ describe('DrawerModal', () => {

expect(getByRole('dialog')).toBeInTheDocument();
expect(document.body).toHaveAttribute('style', 'overflow: hidden;');
expect(htmlElement).toHaveAttribute('style', 'overflow-y: unset;');

userEvent.type(getByRole('dialog'), '{esc}');

await waitFor(() => expect(queryByRole('dialog')).not.toBeInTheDocument());
expect(document.body).not.toHaveAttribute('style', 'overflow: hidden;');
expect(htmlElement).not.toHaveAttribute('style', 'overflow-y: unset;');
});

it('applies backdropProps to Backdrop element', () => {
Expand Down
23 changes: 18 additions & 5 deletions packages/modals/src/elements/DrawerModal/DrawerModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,32 @@ export const DrawerModal = forwardRef<
return undefined;
}

const htmlElement = environment.querySelector('html');
const bodyElement = environment.querySelector('body');
let previousHtmlOverflowY: string;
let previousBodyOverflow: string;

if (bodyElement && isOpen) {
const previousBodyOverflow = bodyElement.style.overflow;
previousBodyOverflow = bodyElement.style.overflow;

bodyElement.style.overflow = 'hidden';
}

return () => {
bodyElement.style.overflow = previousBodyOverflow;
};
if (htmlElement && isOpen) {
previousHtmlOverflowY = htmlElement.style.overflowY;

htmlElement.style.overflowY = 'unset';
}

return undefined;
return () => {
if (bodyElement) {
bodyElement.style.overflow = previousBodyOverflow;
}

if (htmlElement) {
htmlElement.style.overflowY = previousHtmlOverflowY;
}
};
}, [environment, isOpen]);

const rootNode = useMemo(() => {
Expand Down
22 changes: 21 additions & 1 deletion packages/modals/src/elements/Modal.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ describe('Modal', () => {
});

describe('componentDidMount()', () => {
it('should disable overflow scrolling for the html element', () => {
render(<BasicExample />);

expect(document.querySelector('html')?.style.overflowY).toBe('unset');
});

it('should disable overflow scrolling for body element', () => {
const { baseElement } = render(<BasicExample />);

Expand All @@ -48,12 +54,26 @@ describe('Modal', () => {
});

describe('componentWillUnmount()', () => {
it('should apply previous html overflowY style', () => {
const htmlElement = document.querySelector('html');

if (htmlElement) {
htmlElement.style.overflowY = 'scroll';

const { unmount } = render(<BasicExample />);

expect(htmlElement.style.overflowY).toBe('unset');
unmount();

expect(htmlElement.style.overflowY).toBe('scroll');
}
});

it('should apply previous body overflow style', () => {
document.body.style.overflow = 'auto';
const { baseElement, unmount } = render(<BasicExample />);

expect(baseElement.style.overflow).toBe('hidden');

unmount();

expect(baseElement.style.overflow).toBe('auto');
Expand Down
23 changes: 18 additions & 5 deletions packages/modals/src/elements/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ export const Modal = React.forwardRef<HTMLDivElement, IModalProps>(
}

const bodyElement = environment.querySelector('body');
const htmlElement = environment.querySelector('html');
let previousBodyPaddingRight: string;
let previousBodyOverflow: string;
let previousHtmlOverflowY: string;

if (bodyElement) {
if (isOverflowing(bodyElement)) {
Expand All @@ -138,17 +141,27 @@ export const Modal = React.forwardRef<HTMLDivElement, IModalProps>(
bodyElement.style.paddingRight = `${bodyPaddingRight + scrollbarSize}px`;
}

const previousBodyOverflow = bodyElement.style.overflow;
previousBodyOverflow = bodyElement.style.overflow;

bodyElement.style.overflow = 'hidden';
}

if (htmlElement) {
previousHtmlOverflowY = htmlElement.style.overflowY;

htmlElement.style.overflowY = 'unset';
}

return () => {
return () => {
if (bodyElement) {
bodyElement.style.overflow = previousBodyOverflow;
bodyElement.style.paddingRight = previousBodyPaddingRight;
};
}
}

return undefined;
if (htmlElement) {
htmlElement.style.overflowY = previousHtmlOverflowY;
}
};
}, [environment]);

const rootNode = useMemo(() => {
Expand Down

0 comments on commit 7fef3f2

Please sign in to comment.