Make sure that closed Modals are always removed

This commit is contained in:
Fedor Indutny 2022-12-13 17:56:32 -08:00 committed by GitHub
parent 94875efaf6
commit 6cd16cf117
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 8 deletions

View file

@ -2,7 +2,7 @@
// SPDX-License-Identifier: AGPL-3.0-only // SPDX-License-Identifier: AGPL-3.0-only
import type { ReactElement, ReactNode } from 'react'; import type { ReactElement, ReactNode } from 'react';
import React, { useRef, useState } from 'react'; import React, { useEffect, useRef, useState } from 'react';
import type { ContentRect, MeasuredComponentProps } from 'react-measure'; import type { ContentRect, MeasuredComponentProps } from 'react-measure';
import Measure from 'react-measure'; import Measure from 'react-measure';
import classNames from 'classnames'; import classNames from 'classnames';
@ -12,10 +12,12 @@ import { animated } from '@react-spring/web';
import type { LocalizerType } from '../types/Util'; import type { LocalizerType } from '../types/Util';
import { ModalHost } from './ModalHost'; import { ModalHost } from './ModalHost';
import type { Theme } from '../util/theme'; import type { Theme } from '../util/theme';
import { assertDev } from '../util/assert';
import { getClassNamesFor } from '../util/getClassNamesFor'; import { getClassNamesFor } from '../util/getClassNamesFor';
import { useAnimated } from '../hooks/useAnimated'; import { useAnimated } from '../hooks/useAnimated';
import { useHasWrapped } from '../hooks/useHasWrapped'; import { useHasWrapped } from '../hooks/useHasWrapped';
import { useRefMerger } from '../hooks/useRefMerger'; import { useRefMerger } from '../hooks/useRefMerger';
import * as log from '../logging/log';
type PropsType = { type PropsType = {
children: ReactNode; children: ReactNode;
@ -56,8 +58,8 @@ export function Modal({
hasHeaderDivider = false, hasHeaderDivider = false,
hasFooterDivider = false, hasFooterDivider = false,
padded = true, padded = true,
}: Readonly<ModalPropsType>): ReactElement { }: Readonly<ModalPropsType>): JSX.Element | null {
const { close, modalStyles, overlayStyles } = useAnimated(onClose, { const { close, isClosed, modalStyles, overlayStyles } = useAnimated(onClose, {
getFrom: () => ({ opacity: 0, transform: 'translateY(48px)' }), getFrom: () => ({ opacity: 0, transform: 'translateY(48px)' }),
getTo: isOpen => getTo: isOpen =>
isOpen isOpen
@ -65,6 +67,20 @@ export function Modal({
: { opacity: 0, transform: 'translateY(48px)' }, : { opacity: 0, transform: 'translateY(48px)' },
}); });
useEffect(() => {
if (!isClosed) {
return noop;
}
const timer = setTimeout(() => {
log.error(`Modal ${modalName} is closed, but still visible`);
assertDev(false, `Invisible modal ${modalName}`);
}, 0);
return () => {
clearTimeout(timer);
};
}, [modalName, isClosed]);
return ( return (
<ModalHost <ModalHost
modalName={modalName} modalName={modalName}
@ -286,8 +302,8 @@ export function PagedModal({
onClose = noop, onClose = noop,
theme, theme,
useFocusTrap, useFocusTrap,
}: PagedModalProps): ReactElement { }: PagedModalProps): JSX.Element | null {
const { close, modalStyles, overlayStyles } = useAnimated(onClose, { const { close, isClosed, modalStyles, overlayStyles } = useAnimated(onClose, {
getFrom: () => ({ opacity: 0, transform: 'translateY(48px)' }), getFrom: () => ({ opacity: 0, transform: 'translateY(48px)' }),
getTo: isOpen => getTo: isOpen =>
isOpen isOpen
@ -295,6 +311,24 @@ export function PagedModal({
: { opacity: 0, transform: 'translateY(48px)' }, : { opacity: 0, transform: 'translateY(48px)' },
}); });
useEffect(() => {
if (!isClosed) {
return noop;
}
const timer = setTimeout(() => {
log.error(`PagedModal ${modalName} is closed, but still visible`);
assertDev(false, `Invisible paged modal ${modalName}`);
}, 0);
return () => {
clearTimeout(timer);
};
}, [modalName, isClosed]);
if (isClosed) {
return null;
}
return ( return (
<ModalHost <ModalHost
modalName={modalName} modalName={modalName}

View file

@ -10,6 +10,12 @@ export type ModalConfigType = {
transform?: string; transform?: string;
}; };
enum ModalState {
Open = 'Open',
Closing = 'Closing',
Closed = 'Closed',
}
export function useAnimated( export function useAnimated(
onClose: () => unknown, onClose: () => unknown,
{ {
@ -21,10 +27,13 @@ export function useAnimated(
} }
): { ): {
close: () => unknown; close: () => unknown;
isClosed: boolean;
modalStyles: SpringValues<ModalConfigType>; modalStyles: SpringValues<ModalConfigType>;
overlayStyles: SpringValues<ModalConfigType>; overlayStyles: SpringValues<ModalConfigType>;
} { } {
const [isOpen, setIsOpen] = useState(true); const [state, setState] = useState(ModalState.Open);
const isOpen = state === ModalState.Open;
const isClosed = state === ModalState.Closed;
const modalRef = useSpringRef(); const modalRef = useSpringRef();
@ -32,7 +41,8 @@ export function useAnimated(
from: getFrom(isOpen), from: getFrom(isOpen),
to: getTo(isOpen), to: getTo(isOpen),
onRest: () => { onRest: () => {
if (!isOpen) { if (state === ModalState.Closing) {
setState(ModalState.Closed);
onClose(); onClose();
} }
}, },
@ -59,10 +69,18 @@ export function useAnimated(
}); });
useChain(isOpen ? [overlayRef, modalRef] : [modalRef, overlayRef]); useChain(isOpen ? [overlayRef, modalRef] : [modalRef, overlayRef]);
const close = useCallback(() => setIsOpen(false), []); const close = useCallback(() => {
setState(currentState => {
if (currentState === ModalState.Open) {
return ModalState.Closing;
}
return currentState;
});
}, []);
return { return {
close, close,
isClosed,
overlayStyles, overlayStyles,
modalStyles, modalStyles,
}; };