[fix] Modal: fix onRequestClose target if using animations

Close #1824
Fix #1817
This commit is contained in:
James Ward
2020-11-26 02:56:49 -05:00
committed by Nicolas Gallagher
parent 58a8bbe094
commit a660377050
2 changed files with 90 additions and 11 deletions

View File

@@ -40,18 +40,27 @@ function ModalAnimation(props: ModalAnimationProps) {
const isAnimated = animationType && animationType !== 'none';
const animationEndCallback = useCallback(() => {
if (visible) {
if (onShow) {
onShow();
const animationEndCallback = useCallback(
(e: any) => {
if (e && e.currentTarget !== e.target) {
// If the event was generated for something NOT this element we
// should ignore it as it's not relevant to us
return;
}
} else {
setIsRendering(false);
if (onDismiss) {
onDismiss();
if (visible) {
if (onShow) {
onShow();
}
} else {
setIsRendering(false);
if (onDismiss) {
onDismiss();
}
}
}
}, [onDismiss, onShow, visible]);
},
[onDismiss, onShow, visible]
);
useEffect(() => {
if (visible) {

View File

@@ -2,7 +2,7 @@
import Modal from '..';
import React from 'react';
import { render } from '@testing-library/react';
import { fireEvent, render } from '@testing-library/react';
describe('components/Modal', () => {
test('visible by default', () => {
@@ -545,4 +545,74 @@ describe('components/Modal', () => {
expect(spy).toHaveBeenNthCalledWith(1, 'ref');
expect(spy).toHaveBeenNthCalledWith(2, 'mount');
});
test('escape key fires onRequestClose', () => {
const spy = jest.fn();
render(<Modal onRequestClose={spy} visible={true} />);
fireEvent.keyUp(document, { key: 'Escape' });
expect(spy).toHaveBeenCalledTimes(1);
});
test('escape key fires onRequestClose for top modal only', () => {
const spyA = jest.fn();
const spyB = jest.fn();
render(
<>
<Modal onRequestClose={spyA} visible={true} />
<Modal onRequestClose={spyB} visible={true} />
</>
);
fireEvent.keyUp(document, { key: 'Escape' });
expect(spyA).toHaveBeenCalledTimes(0);
expect(spyB).toHaveBeenCalledTimes(1);
});
test('escape key fires onRequestClose for top modal only with animation', () => {
const spyA = jest.fn();
const spyB = jest.fn();
const { getByTestId, rerender } = render(
<>
<Modal animationType={'slide'} onRequestClose={spyA} visible={false}>
<a data-testid={'a'} />
<Modal animationType={'slide'} onRequestClose={spyB} visible={false}>
<a data-testid={'b'} />
</Modal>
</Modal>
</>
);
rerender(
<>
<Modal animationType={'slide'} onRequestClose={spyA} visible={true}>
<a data-testid={'a'} />
<Modal animationType={'slide'} onRequestClose={spyB} visible={true}>
<a data-testid={'b'} />
</Modal>
</Modal>
</>
);
// This is kind of ugly but I can't find a better way to target just the animation div
const animationAElement = getByTestId('a').parentElement.parentElement.parentElement
.parentElement;
const animationBElement = getByTestId('b').parentElement.parentElement.parentElement
.parentElement;
fireEvent.animationEnd(animationAElement);
fireEvent.animationEnd(animationBElement);
fireEvent.keyUp(document, { key: 'Escape' });
expect(spyA).toHaveBeenCalledTimes(0);
expect(spyB).toHaveBeenCalledTimes(1);
});
});