From 05b2f2efda2d3584ca82c93fd1341f87f565f3d7 Mon Sep 17 00:00:00 2001 From: Bartosz Lorek Date: Sat, 12 Feb 2022 15:17:50 +0100 Subject: [PATCH 1/3] fire transition callbacks when not support event --- src/components/dialog/Dialog.jsx | 58 ++++++++++++++++----------- src/components/dialog/Dialog.spec.jsx | 16 ++------ 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/components/dialog/Dialog.jsx b/src/components/dialog/Dialog.jsx index 0835ac5f4..a8fe02673 100644 --- a/src/components/dialog/Dialog.jsx +++ b/src/components/dialog/Dialog.jsx @@ -6,6 +6,9 @@ import cx from 'classnames'; import {useBodyNoScroll} from './useBodyNoScroll'; import {useFocusTrap} from './useFocusTrap'; +// https://github.com/jsdom/jsdom/issues/1781 +const supportsTransitions = window.TransitionEvent !== undefined; + export type DialogPropsType = $ReadOnly<{ open: boolean, children: React.Node, @@ -78,44 +81,53 @@ function BaseDialog({ */ const [deferredOpen, setDeferredOpen] = React.useState(false); + const fireTransitionEndCallbacks = React.useCallback(() => { + if (open) { + if (onEntryTransitionEnd) { + onEntryTransitionEnd(); + } + } else if (onExitTransitionEnd) { + onExitTransitionEnd(); + } + }, [open, onEntryTransitionEnd, onExitTransitionEnd]); + React.useEffect(() => { setDeferredOpen(open); - }, [open]); - useBodyNoScroll(); - useFocusTrap({dialogRef: containerRef, overlayRef}); + if (!supportsTransitions) { + fireTransitionEndCallbacks(); + } + }, [open, fireTransitionEndCallbacks]); - const handleOverlayClick = React.useCallback( - (event: SyntheticMouseEvent) => { - if (onDismiss && event.target === event.currentTarget) { - onDismiss(); - } - }, - [onDismiss] - ); + useBodyNoScroll(); + useFocusTrap({ + dialogRef: containerRef, + overlayRef, + }); const handleTransitionEnd = React.useCallback( (event: TransitionEvent) => { if ( - event.target !== event.currentTarget || - event.propertyName !== lastTransitionName + event.target === event.currentTarget && + event.propertyName === lastTransitionName ) { - return; + fireTransitionEndCallbacks(); } + }, + [fireTransitionEndCallbacks, lastTransitionName] + ); - if (open) { - if (onEntryTransitionEnd) { - onEntryTransitionEnd(); - } - } else if (onExitTransitionEnd) { - onExitTransitionEnd(); + const handleOverlayClick = React.useCallback( + (event: SyntheticMouseEvent) => { + if (onDismiss && event.target === event.currentTarget) { + onDismiss(); } }, - [open, lastTransitionName, onEntryTransitionEnd, onExitTransitionEnd] + [onDismiss] ); const handleKeyUp = React.useCallback( - event => { + (event: SyntheticKeyboardEvent) => { if (onDismiss && event.key === 'Escape') { onDismiss(); event.stopPropagation(); @@ -158,7 +170,7 @@ function BaseDialog({ role="dialog" ref={containerRef} className={containerClass} - onTransitionEnd={handleTransitionEnd} + onTransitionEnd={supportsTransitions ? handleTransitionEnd : undefined} aria-modal="true" aria-labelledby={ariaLabelledBy} aria-label={ariaLabel} diff --git a/src/components/dialog/Dialog.spec.jsx b/src/components/dialog/Dialog.spec.jsx index b9dc28f87..737a98b0a 100644 --- a/src/components/dialog/Dialog.spec.jsx +++ b/src/components/dialog/Dialog.spec.jsx @@ -43,7 +43,6 @@ describe('', () => { it('fires onDismiss callback on Escape key', () => { const onDismiss = jest.fn(); - const wrapper = mount( content text @@ -68,16 +67,13 @@ describe('', () => { it('fires onEntryTransitionEnd callback on entry', () => { const onEntryTransitionEnd = jest.fn(); - const wrapper = mount( + + mount( content text ); - wrapper.find('[role="dialog"]').simulate('transitionEnd', { - propertyName: 'transform', - }); - expect(onEntryTransitionEnd).toHaveBeenCalledTimes(1); }); @@ -90,10 +86,6 @@ describe('', () => { ); wrapper.setProps({open: false}); - wrapper.find('[role="dialog"]').simulate('transitionEnd', { - propertyName: 'opacity', - }); - expect(onExitTransitionEnd).toHaveBeenCalledTimes(1); }); @@ -103,9 +95,7 @@ describe('', () => { expect(wrapper.isEmptyRender()).toBe(false); wrapper.setProps({open: false}); - wrapper.find('[role="dialog"]').simulate('transitionEnd', { - propertyName: 'opacity', - }); + wrapper.update(); expect(wrapper.isEmptyRender()).toBe(true); }); From 77f1f28bd9288cf7dab675f585d4e80490ac71d3 Mon Sep 17 00:00:00 2001 From: Bartosz Lorek Date: Sat, 12 Feb 2022 15:34:16 +0100 Subject: [PATCH 2/3] add more callback tests --- src/components/dialog/Dialog.spec.jsx | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/components/dialog/Dialog.spec.jsx b/src/components/dialog/Dialog.spec.jsx index 737a98b0a..f314c0fa9 100644 --- a/src/components/dialog/Dialog.spec.jsx +++ b/src/components/dialog/Dialog.spec.jsx @@ -89,6 +89,30 @@ describe('', () => { expect(onExitTransitionEnd).toHaveBeenCalledTimes(1); }); + it('does not fire onEntryTransitionEnd callback before mount', () => { + const onEntryTransitionEnd = jest.fn(); + + mount( + + content text + + ); + + expect(onEntryTransitionEnd).toHaveBeenCalledTimes(0); + }); + + it('does not fire onExitTransitionEnd callback before mount', () => { + const onExitTransitionEnd = jest.fn(); + + mount( + + content text + + ); + + expect(onExitTransitionEnd).toHaveBeenCalledTimes(0); + }); + it('returns null after exit transition', () => { const wrapper = mount(content text); From 64be9a9ba15d6c966a33b0c4edcef969544fadd1 Mon Sep 17 00:00:00 2001 From: Bartosz Lorek Date: Sat, 12 Feb 2022 17:44:44 +0100 Subject: [PATCH 3/3] rename --- src/components/dialog/Dialog.spec.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/dialog/Dialog.spec.jsx b/src/components/dialog/Dialog.spec.jsx index f314c0fa9..cfd731316 100644 --- a/src/components/dialog/Dialog.spec.jsx +++ b/src/components/dialog/Dialog.spec.jsx @@ -89,7 +89,7 @@ describe('', () => { expect(onExitTransitionEnd).toHaveBeenCalledTimes(1); }); - it('does not fire onEntryTransitionEnd callback before mount', () => { + it('does not fire onEntryTransitionEnd callback before open', () => { const onEntryTransitionEnd = jest.fn(); mount( @@ -101,7 +101,7 @@ describe('', () => { expect(onEntryTransitionEnd).toHaveBeenCalledTimes(0); }); - it('does not fire onExitTransitionEnd callback before mount', () => { + it('does not fire onExitTransitionEnd callback before open', () => { const onExitTransitionEnd = jest.fn(); mount(