Skip to content

Commit d8f1b58

Browse files
bernhardojnecolas
authored andcommittedAug 24, 2023
[fix] Correct timing of <Modal> onDismiss callback
Close #2558
1 parent ef95fc0 commit d8f1b58

File tree

2 files changed

+35
-4
lines changed

2 files changed

+35
-4
lines changed
 

‎packages/react-native-web/src/exports/Modal/ModalAnimation.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ function ModalAnimation(props: ModalAnimationProps): React.Node {
3737

3838
const [isRendering, setIsRendering] = React.useState(false);
3939
const wasVisible = React.useRef(false);
40+
const wasRendering = React.useRef(false);
4041

4142
const isAnimated = animationType && animationType !== 'none';
4243

@@ -54,14 +55,18 @@ function ModalAnimation(props: ModalAnimationProps): React.Node {
5455
}
5556
} else {
5657
setIsRendering(false);
57-
if (onDismiss) {
58-
onDismiss();
59-
}
6058
}
6159
},
62-
[onDismiss, onShow, visible]
60+
[onShow, visible]
6361
);
6462

63+
React.useEffect(() => {
64+
if (wasRendering.current && !isRendering && onDismiss) {
65+
onDismiss();
66+
}
67+
wasRendering.current = isRendering;
68+
}, [isRendering, onDismiss]);
69+
6570
React.useEffect(() => {
6671
if (visible) {
6772
setIsRendering(true);

‎packages/react-native-web/src/exports/Modal/__tests__/index-test.js

+26
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,32 @@ describe('components/Modal', () => {
309309
expect(document.activeElement).toBe(insideElement);
310310
});
311311

312+
test('focus is not trapped after closing modal', () => {
313+
const { rerender } = render(
314+
<>
315+
<a data-testid={'outside'} href={'#outside'}>
316+
Outside
317+
</a>
318+
<Modal visible={true} />
319+
</>
320+
);
321+
322+
const outsideElement = document.querySelector('[data-testid="outside"]');
323+
const onDismissCallback = jest.fn(() => outsideElement.focus());
324+
325+
rerender(
326+
<>
327+
<a data-testid={'outside'} href={'#outside'}>
328+
Outside
329+
</a>
330+
<Modal onDismiss={onDismissCallback} visible={false} />
331+
</>
332+
);
333+
334+
expect(onDismissCallback).toBeCalledTimes(1);
335+
expect(document.activeElement).toBe(outsideElement);
336+
});
337+
312338
test('focus is brought back to the element that triggered modal after closing', () => {
313339
const { rerender } = render(
314340
<>

0 commit comments

Comments
 (0)
Please sign in to comment.