Skip to content

Commit

Permalink
fix: afterClose unexpected call when mount dialog (#244)
Browse files Browse the repository at this point in the history
  • Loading branch information
afc163 authored Mar 5, 2021
1 parent cb7c94f commit 119d525
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 21 deletions.
9 changes: 6 additions & 3 deletions src/Dialog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export type IDialogChildProps = {
// TODO: refactor to remove this.
getOpenCount: () => number;
scrollLocker?: ScollLocker;
} & IDialogPropTypes
} & IDialogPropTypes;

export default function Dialog(props: IDialogChildProps) {
const {
Expand Down Expand Up @@ -81,7 +81,10 @@ export default function Dialog(props: IDialogChildProps) {
lastOutSideActiveElementRef.current = null;
}

afterClose?.();
// Trigger afterClose only when change visible from true to false
if (animatedVisible) {
afterClose?.();
}
}
}

Expand All @@ -91,7 +94,7 @@ export default function Dialog(props: IDialogChildProps) {

// >>> Content
const contentClickRef = useRef(false);
const contentTimeoutRef = useRef<number>();
const contentTimeoutRef = useRef<NodeJS.Timeout>();

// We need record content click incase content popup out of dialog
const onContentMouseDown: React.MouseEventHandler = () => {
Expand Down
61 changes: 43 additions & 18 deletions tests/index.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/* eslint-disable react/no-render-return-value, max-classes-per-file, func-names, no-console */
import React, { cloneElement } from 'react';
import { act } from 'react-dom/test-utils';
import { mount, ReactWrapper } from 'enzyme';
import type { ReactWrapper } from 'enzyme';
import { mount } from 'enzyme';
import Portal from 'rc-util/lib/Portal';
import KeyCode from 'rc-util/lib/KeyCode';
import Dialog, { DialogProps } from '../src';
import type { DialogProps } from '../src';
import Dialog from '../src';

describe('dialog', () => {
beforeEach(() => {
Expand Down Expand Up @@ -250,10 +252,7 @@ describe('dialog', () => {
});

expect(
(wrapper
.find('.rc-dialog')
.at(0)
.getDOMNode() as HTMLDivElement).style['transform-origin'],
(wrapper.find('.rc-dialog').at(0).getDOMNode() as HTMLDivElement).style['transform-origin'],
).toBeTruthy();
});

Expand Down Expand Up @@ -295,18 +294,6 @@ describe('dialog', () => {
expect(onClose).not.toHaveBeenCalled();
});

it('afterClose', () => {
const afterClose = jest.fn();

const wrapper = mount(<Dialog afterClose={afterClose} visible />);
jest.runAllTimers();

wrapper.setProps({ visible: false });
jest.runAllTimers();

expect(afterClose).toHaveBeenCalledTimes(1);
});

it('zIndex', () => {
const wrapper = mount(<Dialog visible zIndex={903} />);
expect(wrapper.find('.rc-dialog-wrap').props().style.zIndex).toBe(903);
Expand Down Expand Up @@ -397,4 +384,42 @@ describe('dialog', () => {
expect(getRenderTimes()).toEqual(2);
});
});

describe('afterClose', () => {
it('should trigger afterClose when set visible to false', () => {
const afterClose = jest.fn();

const wrapper = mount(<Dialog afterClose={afterClose} visible />);
jest.runAllTimers();

wrapper.setProps({ visible: false });
jest.runAllTimers();

expect(afterClose).toHaveBeenCalledTimes(1);
});

it('should not trigger afterClose when mount dialog of getContainer={false}', () => {
const afterClose = jest.fn();

const wrapper = mount(<Dialog afterClose={afterClose} getContainer={false} />);
jest.runAllTimers();

wrapper.setProps({ visible: false });
jest.runAllTimers();

expect(afterClose).toHaveBeenCalledTimes(0);
});

it('should not trigger afterClose when mount dialog of forceRender={true}', () => {
const afterClose = jest.fn();

const wrapper = mount(<Dialog afterClose={afterClose} forceRender />);
jest.runAllTimers();

wrapper.setProps({ visible: false });
jest.runAllTimers();

expect(afterClose).toHaveBeenCalledTimes(0);
});
});
});

1 comment on commit 119d525

@vercel
Copy link

@vercel vercel bot commented on 119d525 Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.