Skip to content

Commit 696e4a4

Browse files
authored
Dialog - Fix Unimplemented TransitionEnd Event (#2299)
* fire transition callbacks when not support event * add more callback tests * rename
1 parent ee12076 commit 696e4a4

File tree

2 files changed

+62
-36
lines changed

2 files changed

+62
-36
lines changed

src/components/dialog/Dialog.jsx

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import cx from 'classnames';
66
import {useBodyNoScroll} from './useBodyNoScroll';
77
import {useFocusTrap} from './useFocusTrap';
88

9+
// https://github.com/jsdom/jsdom/issues/1781
10+
const supportsTransitions = window.TransitionEvent !== undefined;
11+
912
export type DialogPropsType = $ReadOnly<{
1013
open: boolean,
1114
children: React.Node,
@@ -78,44 +81,53 @@ function BaseDialog({
7881
*/
7982
const [deferredOpen, setDeferredOpen] = React.useState<boolean>(false);
8083

84+
const fireTransitionEndCallbacks = React.useCallback(() => {
85+
if (open) {
86+
if (onEntryTransitionEnd) {
87+
onEntryTransitionEnd();
88+
}
89+
} else if (onExitTransitionEnd) {
90+
onExitTransitionEnd();
91+
}
92+
}, [open, onEntryTransitionEnd, onExitTransitionEnd]);
93+
8194
React.useEffect(() => {
8295
setDeferredOpen(open);
83-
}, [open]);
8496

85-
useBodyNoScroll();
86-
useFocusTrap({dialogRef: containerRef, overlayRef});
97+
if (!supportsTransitions) {
98+
fireTransitionEndCallbacks();
99+
}
100+
}, [open, fireTransitionEndCallbacks]);
87101

88-
const handleOverlayClick = React.useCallback(
89-
(event: SyntheticMouseEvent<HTMLDivElement>) => {
90-
if (onDismiss && event.target === event.currentTarget) {
91-
onDismiss();
92-
}
93-
},
94-
[onDismiss]
95-
);
102+
useBodyNoScroll();
103+
useFocusTrap({
104+
dialogRef: containerRef,
105+
overlayRef,
106+
});
96107

97108
const handleTransitionEnd = React.useCallback(
98109
(event: TransitionEvent) => {
99110
if (
100-
event.target !== event.currentTarget ||
101-
event.propertyName !== lastTransitionName
111+
event.target === event.currentTarget &&
112+
event.propertyName === lastTransitionName
102113
) {
103-
return;
114+
fireTransitionEndCallbacks();
104115
}
116+
},
117+
[fireTransitionEndCallbacks, lastTransitionName]
118+
);
105119

106-
if (open) {
107-
if (onEntryTransitionEnd) {
108-
onEntryTransitionEnd();
109-
}
110-
} else if (onExitTransitionEnd) {
111-
onExitTransitionEnd();
120+
const handleOverlayClick = React.useCallback(
121+
(event: SyntheticMouseEvent<HTMLDivElement>) => {
122+
if (onDismiss && event.target === event.currentTarget) {
123+
onDismiss();
112124
}
113125
},
114-
[open, lastTransitionName, onEntryTransitionEnd, onExitTransitionEnd]
126+
[onDismiss]
115127
);
116128

117129
const handleKeyUp = React.useCallback(
118-
event => {
130+
(event: SyntheticKeyboardEvent<HTMLDivElement>) => {
119131
if (onDismiss && event.key === 'Escape') {
120132
onDismiss();
121133
event.stopPropagation();
@@ -158,7 +170,7 @@ function BaseDialog({
158170
role="dialog"
159171
ref={containerRef}
160172
className={containerClass}
161-
onTransitionEnd={handleTransitionEnd}
173+
onTransitionEnd={supportsTransitions ? handleTransitionEnd : undefined}
162174
aria-modal="true"
163175
aria-labelledby={ariaLabelledBy}
164176
aria-label={ariaLabel}

src/components/dialog/Dialog.spec.jsx

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ describe('<Dialog>', () => {
4343

4444
it('fires onDismiss callback on Escape key', () => {
4545
const onDismiss = jest.fn();
46-
4746
const wrapper = mount(
4847
<Dialog onDismiss={onDismiss} open>
4948
content text
@@ -68,16 +67,13 @@ describe('<Dialog>', () => {
6867

6968
it('fires onEntryTransitionEnd callback on entry', () => {
7069
const onEntryTransitionEnd = jest.fn();
71-
const wrapper = mount(
70+
71+
mount(
7272
<Dialog onEntryTransitionEnd={onEntryTransitionEnd} open>
7373
content text
7474
</Dialog>
7575
);
7676

77-
wrapper.find('[role="dialog"]').simulate('transitionEnd', {
78-
propertyName: 'transform',
79-
});
80-
8177
expect(onEntryTransitionEnd).toHaveBeenCalledTimes(1);
8278
});
8379

@@ -90,22 +86,40 @@ describe('<Dialog>', () => {
9086
);
9187

9288
wrapper.setProps({open: false});
93-
wrapper.find('[role="dialog"]').simulate('transitionEnd', {
94-
propertyName: 'opacity',
95-
});
96-
9789
expect(onExitTransitionEnd).toHaveBeenCalledTimes(1);
9890
});
9991

92+
it('does not fire onEntryTransitionEnd callback before open', () => {
93+
const onEntryTransitionEnd = jest.fn();
94+
95+
mount(
96+
<Dialog onEntryTransitionEnd={onEntryTransitionEnd} open={false}>
97+
content text
98+
</Dialog>
99+
);
100+
101+
expect(onEntryTransitionEnd).toHaveBeenCalledTimes(0);
102+
});
103+
104+
it('does not fire onExitTransitionEnd callback before open', () => {
105+
const onExitTransitionEnd = jest.fn();
106+
107+
mount(
108+
<Dialog onExitTransitionEnd={onExitTransitionEnd} open={false}>
109+
content text
110+
</Dialog>
111+
);
112+
113+
expect(onExitTransitionEnd).toHaveBeenCalledTimes(0);
114+
});
115+
100116
it('returns null after exit transition', () => {
101117
const wrapper = mount(<Dialog open>content text</Dialog>);
102118

103119
expect(wrapper.isEmptyRender()).toBe(false);
104120

105121
wrapper.setProps({open: false});
106-
wrapper.find('[role="dialog"]').simulate('transitionEnd', {
107-
propertyName: 'opacity',
108-
});
122+
wrapper.update();
109123

110124
expect(wrapper.isEmptyRender()).toBe(true);
111125
});

0 commit comments

Comments
 (0)