Skip to content

Commit

Permalink
Fix: Apply correct padding when two modals opened in succession (reac…
Browse files Browse the repository at this point in the history
  • Loading branch information
robertwramos committed Nov 15, 2023
1 parent 85cd68b commit dc8951a
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 5 deletions.
9 changes: 4 additions & 5 deletions src/Modal.js
Expand Up @@ -115,7 +115,6 @@ class Modal extends React.Component {
super(props);

this._element = null;
this._originalBodyPadding = null;
this.getFocusableChildren = this.getFocusableChildren.bind(this);
this.handleBackdropClick = this.handleBackdropClick.bind(this);
this.handleBackdropMouseDown = this.handleBackdropMouseDown.bind(this);
Expand Down Expand Up @@ -369,13 +368,13 @@ class Modal extends React.Component {
this._mountContainer.appendChild(this._element);
}

this._originalBodyPadding = getOriginalBodyPadding();
if (Modal.openCount < 1) {
Modal.originalBodyOverflow = window.getComputedStyle(
document.body,
).overflow;
Modal.originalBodyPadding = getOriginalBodyPadding();
conditionallyUpdateScrollbar();
}
conditionallyUpdateScrollbar();

if (Modal.openCount === 0) {
document.body.className = classNames(
Expand Down Expand Up @@ -421,11 +420,10 @@ class Modal extends React.Component {
.replace(modalOpenClassNameRegex, ' ')
.trim();
document.body.style.overflow = Modal.originalBodyOverflow;
setScrollbarWidth(Modal.originalBodyPadding);
}
this.manageFocusAfterClose();
Modal.openCount = Math.max(0, Modal.openCount - 1);

setScrollbarWidth(this._originalBodyPadding);
}

clearBackdropAnimationTimeout() {
Expand Down Expand Up @@ -574,5 +572,6 @@ Modal.propTypes = propTypes;
Modal.defaultProps = defaultProps;
Modal.openCount = 0;
Modal.originalBodyOverflow = null;
Modal.originalBodyPadding = null;

export default Modal;
62 changes: 62 additions & 0 deletions src/__tests__/Modal.spec.js
Expand Up @@ -4,6 +4,7 @@ import { render, screen } from '@testing-library/react';
import user from '@testing-library/user-event';
import '@testing-library/jest-dom';
import { Modal, ModalBody, ModalHeader, ModalFooter, Button } from '..';
import * as Utils from '../utils';

describe('Modal', () => {
const toggle = () => {};
Expand Down Expand Up @@ -1115,4 +1116,65 @@ describe('Modal', () => {
);
expect(spy).not.toHaveBeenCalled();
});

it('should restore the original body overflow and padding when multiple modals are opened and closed', () => {
const spyConditionallyUpdateScrollbar = jest.spyOn(
Utils,
'conditionallyUpdateScrollbar',
);
spyConditionallyUpdateScrollbar.mockImplementation(() => {
Utils.setScrollbarWidth(15);
});

const spySetScrollbarWidth = jest.spyOn(Utils, 'setScrollbarWidth');

const { rerender } = render(
<>
<Modal isOpen toggle={toggle}>
a
</Modal>
<Modal isOpen={false} toggle={toggle}>
b
</Modal>
</>,
);

expect(document.body).toHaveClass('modal-open');
expect(document.body.style.paddingRight).toBe('15px');
expect(document.body.style.overflow).toBe('hidden');
expect(spyConditionallyUpdateScrollbar).toHaveBeenCalled();

spyConditionallyUpdateScrollbar.mockClear();

rerender(
<>
<Modal isOpen={false} toggle={toggle}>
a
</Modal>
<Modal isOpen toggle={toggle}>
b
</Modal>
</>,
);
jest.advanceTimersByTime(300);
expect(document.body).toHaveClass('modal-open');
expect(document.body.style.paddingRight).toBe('15px');
expect(document.body.style.overflow).toBe('hidden');
expect(spyConditionallyUpdateScrollbar).not.toHaveBeenCalled();

rerender(
<>
<Modal isOpen={false} toggle={toggle}>
a
</Modal>
<Modal isOpen={false} toggle={toggle}>
b
</Modal>
</>,
);
jest.advanceTimersByTime(300);
expect(document.body).not.toHaveClass('modal-open');
expect(spySetScrollbarWidth).toHaveBeenCalledWith(0);
expect(document.body.style.overflow).toBe('');
});
});

0 comments on commit dc8951a

Please sign in to comment.