Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix!: fix rendering issues in Safari, Firefox by changing the structure
  • Loading branch information
pradel committed Nov 15, 2020
1 parent b016ec4 commit 5727913
Show file tree
Hide file tree
Showing 7 changed files with 201 additions and 112 deletions.
30 changes: 15 additions & 15 deletions react-responsive-modal/__tests__/index.test.tsx
Expand Up @@ -12,7 +12,7 @@ describe('modal', () => {
</Modal>
);

fireEvent.click(getByTestId('overlay'));
fireEvent.click(getByTestId('modal-container'));
expect(onClose).toHaveBeenCalledTimes(1);
});

Expand All @@ -24,7 +24,7 @@ describe('modal', () => {
</Modal>
);

fireEvent.click(getByTestId('overlay'));
fireEvent.click(getByTestId('modal-container'));
expect(onClose).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -147,7 +147,7 @@ describe('modal', () => {
</Modal>
);
// Simulate the browser animation end
fireEvent.animationEnd(getByTestId('overlay'));
fireEvent.animationEnd(getByTestId('modal'));
await waitFor(
() => {
expect(queryByTestId('modal')).not.toBeInTheDocument();
Expand Down Expand Up @@ -195,7 +195,7 @@ describe('modal', () => {
</React.Fragment>
);

fireEvent.animationEnd(getAllByTestId('overlay')[1]);
fireEvent.animationEnd(getAllByTestId('modal')[1]);
await waitFor(
() => {
expect(queryByText(/second modal/)).not.toBeInTheDocument();
Expand All @@ -216,7 +216,7 @@ describe('modal', () => {
</React.Fragment>
);

fireEvent.animationEnd(getAllByTestId('overlay')[0]);
fireEvent.animationEnd(getAllByTestId('modal')[0]);
await waitFor(
() => {
expect(queryByText(/first modal/)).not.toBeInTheDocument();
Expand Down Expand Up @@ -251,7 +251,7 @@ describe('modal', () => {
</React.Fragment>
);

fireEvent.animationEnd(getAllByTestId('overlay')[1]);
fireEvent.animationEnd(getAllByTestId('modal')[1]);
await waitFor(
() => {
expect(queryByText(/second modal/)).not.toBeInTheDocument();
Expand Down Expand Up @@ -344,7 +344,7 @@ describe('modal', () => {
<div>modal content</div>
</Modal>
);
fireEvent.animationEnd(getByTestId('overlay'));
fireEvent.animationEnd(getByTestId('modal'));
expect(queryByTestId('modal')).toBeNull();
});
});
Expand All @@ -357,10 +357,10 @@ describe('modal', () => {
</Modal>
);

expect(getByTestId('modal').classList.length).toBe(1);
expect(getByTestId('modal-container').classList.length).toBe(1);
expect(
getByTestId('modal').classList.contains(
'react-responsive-modal-modalCenter'
getByTestId('modal-container').classList.contains(
'react-responsive-modal-containerCenter'
)
).toBeFalsy();
});
Expand All @@ -372,10 +372,10 @@ describe('modal', () => {
</Modal>
);

expect(getByTestId('modal').classList.length).toBe(2);
expect(getByTestId('modal-container').classList.length).toBe(2);
expect(
getByTestId('modal').classList.contains(
'react-responsive-modal-modalCenter'
getByTestId('modal-container').classList.contains(
'react-responsive-modal-containerCenter'
)
).toBeTruthy();
});
Expand Down Expand Up @@ -464,7 +464,7 @@ describe('modal', () => {
</Modal>
);

fireEvent.click(getByTestId('overlay'));
fireEvent.click(getByTestId('modal-container'));
expect(onOverlayClick).toHaveBeenCalledTimes(1);
});
});
Expand All @@ -478,7 +478,7 @@ describe('modal', () => {
</Modal>
);

fireEvent.animationEnd(getByTestId('overlay'));
fireEvent.animationEnd(getByTestId('modal'));
expect(onAnimationEnd).toHaveBeenCalledTimes(1);
});
});
Expand Down
6 changes: 3 additions & 3 deletions react-responsive-modal/src/CloseIcon.tsx
Expand Up @@ -15,7 +15,7 @@ interface CloseIconProps {
classes: {
closeButton?: string;
};
onClickCloseIcon: () => void;
onClick: () => void;
}

const CloseIcon = ({
Expand All @@ -24,13 +24,13 @@ const CloseIcon = ({
styles,
id,
closeIcon,
onClickCloseIcon,
onClick,
}: CloseIconProps) => (
<button
id={id}
className={cx(classes.closeButton, classNames?.closeButton)}
style={styles?.closeButton}
onClick={onClickCloseIcon}
onClick={onClick}
data-testid="close-button"
>
{closeIcon ? (
Expand Down
123 changes: 75 additions & 48 deletions react-responsive-modal/src/index.tsx
Expand Up @@ -7,12 +7,16 @@ import { modalManager, useModalManager } from './modalManager';
import { isBrowser, blockNoScroll, unblockNoScroll } from './utils';

const classes = {
root: 'react-responsive-modal-root',
overlay: 'react-responsive-modal-overlay',
overlayAnimationIn: 'react-responsive-modal-overlay-in',
overlayAnimationOut: 'react-responsive-modal-overlay-out',
modalContainer: 'react-responsive-modal-container',
modalContainerCenter: 'react-responsive-modal-containerCenter',
modal: 'react-responsive-modal-modal',
modalCenter: 'react-responsive-modal-modalCenter',
modalAnimationIn: 'react-responsive-modal-modal-in',
modalAnimationOut: 'react-responsive-modal-modal-out',
closeButton: 'react-responsive-modal-closeButton',
animationIn: 'react-responsive-modal-fadeIn',
animationOut: 'react-responsive-modal-fadeOut',
};

export interface ModalProps {
Expand Down Expand Up @@ -74,18 +78,24 @@ export interface ModalProps {
* An object containing classNames to style the modal.
*/
classNames?: {
root?: string;
overlay?: string;
overlayAnimationIn?: string;
overlayAnimationOut?: string;
modalContainer?: string;
modal?: string;
modalAnimationIn?: string;
modalAnimationOut?: string;
closeButton?: string;
closeIcon?: string;
animationIn?: string;
animationOut?: string;
};
/**
* An object containing the styles objects to style the modal.
*/
styles?: {
root?: React.CSSProperties;
overlay?: React.CSSProperties;
modalContainer?: React.CSSProperties;
modal?: React.CSSProperties;
closeButton?: React.CSSProperties;
closeIcon?: React.CSSProperties;
Expand Down Expand Up @@ -146,7 +156,7 @@ export const Modal = ({
closeIconId,
closeIcon,
focusTrapped = true,
animationDuration = 500,
animationDuration = 300,
classNames,
styles,
role = 'dialog',
Expand Down Expand Up @@ -179,6 +189,7 @@ export const Modal = ({
if (blockScroll) {
blockNoScroll();
}

if (
refContainer.current &&
!container &&
Expand All @@ -198,6 +209,7 @@ export const Modal = ({
) {
unblockNoScroll();
}

if (
refContainer.current &&
!container &&
Expand Down Expand Up @@ -264,10 +276,6 @@ export const Modal = ({
refShouldClose.current = false;
};

const handleClickCloseIcon = () => {
onClose();
};

const handleAnimationEnd = () => {
if (!open) {
setShowPortal(false);
Expand All @@ -278,52 +286,71 @@ export const Modal = ({

const containerModal = container || refContainer.current;

const overlayAnimation = open
? classNames?.overlayAnimationIn ?? classes.overlayAnimationIn
: classNames?.overlayAnimationOut ?? classes.overlayAnimationOut;

const modalAnimation = open
? classNames?.modalAnimationIn ?? classes.modalAnimationIn
: classNames?.modalAnimationOut ?? classes.modalAnimationOut;

return showPortal && containerModal
? ReactDom.createPortal(
<div
style={{
animation: `${
open
? classNames?.animationIn ?? classes.animationIn
: classNames?.animationOut ?? classes.animationOut
} ${animationDuration}ms`,
...styles?.overlay,
}}
className={cx(classes.overlay, classNames?.overlay)}
onClick={handleClickOverlay}
onAnimationEnd={handleAnimationEnd}
data-testid="overlay"
className={cx(classes.root, classNames?.root)}
style={styles?.root}
data-testid="root"
>
<div
ref={refModal}
className={cx(classes.overlay, classNames?.overlay)}
data-testid="overlay"
aria-hidden={true}
style={{
animation: `${overlayAnimation} ${animationDuration}ms`,
...styles?.overlay,
}}
/>
<div
className={cx(
classes.modal,
center && classes.modalCenter,
classNames?.modal
classes.modalContainer,
center && classes.modalContainerCenter,
classNames?.modalContainer
)}
style={styles?.modal}
onMouseDown={handleModalEvent}
onMouseUp={handleModalEvent}
onClick={handleModalEvent}
id={modalId}
role={role}
aria-modal="true"
aria-labelledby={ariaLabelledby}
aria-describedby={ariaDescribedby}
data-testid="modal"
style={styles?.modalContainer}
data-testid="modal-container"
onClick={handleClickOverlay}
>
{focusTrapped && <FocusTrap container={refModal} />}
{children}
{showCloseIcon && (
<CloseIcon
classes={classes}
classNames={classNames}
styles={styles}
closeIcon={closeIcon}
onClickCloseIcon={handleClickCloseIcon}
id={closeIconId}
/>
)}
<div
ref={refModal}
className={cx(classes.modal, classNames?.modal)}
style={{
animation: `${modalAnimation} ${animationDuration}ms`,
...styles?.modal,
}}
onMouseDown={handleModalEvent}
onMouseUp={handleModalEvent}
onClick={handleModalEvent}
onAnimationEnd={handleAnimationEnd}
id={modalId}
role={role}
aria-modal="true"
aria-labelledby={ariaLabelledby}
aria-describedby={ariaDescribedby}
data-testid="modal"
>
{focusTrapped && <FocusTrap container={refModal} />}
{children}
{showCloseIcon && (
<CloseIcon
classes={classes}
classNames={classNames}
styles={styles}
closeIcon={closeIcon}
onClick={onClose}
id={closeIconId}
/>
)}
</div>
</div>
</div>,
containerModal
Expand Down

0 comments on commit 5727913

Please sign in to comment.