Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Modal.info onOk execute times #23360

Merged
merged 4 commits into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 39 additions & 35 deletions components/modal/ActionButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,55 +35,59 @@ export default class ActionButton extends React.Component<ActionButtonProps, Act
clearTimeout(this.timeoutId);
}

handlePromiseOnOk(returnValueOfOnOk?: PromiseLike<any>) {
const { closeModal } = this.props;
if (!returnValueOfOnOk || !returnValueOfOnOk.then) {
return;
}
this.setState({ loading: true });
returnValueOfOnOk.then(
(...args: any[]) => {
// It's unnecessary to set loading=false, for the Modal will be unmounted after close.
// this.setState({ loading: false });
closeModal(...args);
},
(e: Error) => {
// Emit error when catch promise reject
// eslint-disable-next-line no-console
console.error(e);
// See: https://github.com/ant-design/ant-design/issues/6183
this.setState({ loading: false });
this.clicked = false;
},
);
}

onClick = () => {
const { actionFn, closeModal } = this.props;
if (this.clicked) {
return;
}
this.clicked = true;
if (actionFn) {
let ret;
if (actionFn.length) {
ret = actionFn(closeModal);
} else {
ret = actionFn();
if (!ret) {
closeModal();
}
}
if (ret && ret.then) {
this.setState({ loading: true });
ret.then(
(...args: any[]) => {
// It's unnecessary to set loading=false, for the Modal will be unmounted after close.
// this.setState({ loading: false });
closeModal(...args);
},
(e: Error) => {
// Emit error when catch promise reject
// eslint-disable-next-line no-console
console.error(e);
// See: https://github.com/ant-design/ant-design/issues/6183
this.setState({ loading: false });
this.clicked = false;
},
);
}
} else {
if (!actionFn) {
closeModal();
return;
}
let returnValueOfOnOk;
if (actionFn.length) {
returnValueOfOnOk = actionFn(closeModal);
// https://github.com/ant-design/ant-design/issues/23358
this.clicked = false;
} else {
returnValueOfOnOk = actionFn();
if (!returnValueOfOnOk) {
closeModal();
return;
}
}
this.handlePromiseOnOk(returnValueOfOnOk);
};

render() {
const { type, children, buttonProps } = this.props;
const { loading } = this.state;
return (
<Button
type={type}
onClick={this.onClick}
loading={loading}
{...buttonProps}
>
<Button type={type} onClick={this.onClick} loading={loading} {...buttonProps}>
{children}
</Button>
);
Expand Down
33 changes: 32 additions & 1 deletion components/modal/__tests__/confirm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ describe('Modal.confirm triggers callbacks correctly', () => {
afterEach(() => {
errorSpy.mockReset();
document.body.innerHTML = '';
Modal.destroyAll();
});

afterAll(() => {
Expand Down Expand Up @@ -125,6 +126,22 @@ describe('Modal.confirm triggers callbacks correctly', () => {
jest.useRealTimers();
});

it('should not close modals when click confirm button when onOk has argument', () => {
jest.useFakeTimers();
['info', 'success', 'warning', 'error'].forEach(type => {
Modal[type]({
title: 'title',
content: 'content',
onOk: close => null, // eslint-disable-line no-unused-vars
});
expect($$(`.ant-modal-confirm-${type}`)).toHaveLength(1);
$$('.ant-btn')[0].click();
jest.runAllTimers();
expect($$(`.ant-modal-confirm-${type}`)).toHaveLength(1);
});
jest.useRealTimers();
});

it('could be update', () => {
jest.useFakeTimers();
['info', 'success', 'warning', 'error'].forEach(type => {
Expand Down Expand Up @@ -235,9 +252,23 @@ describe('Modal.confirm triggers callbacks correctly', () => {
it('ok button should trigger onOk once when click it many times quickly', () => {
const onOk = jest.fn();
open({ onOk });
// Fifth Modal
$$('.ant-btn-primary')[0].click();
$$('.ant-btn-primary')[0].click();
expect(onOk).toHaveBeenCalledTimes(1);
});

// https://github.com/ant-design/ant-design/issues/23358
it('ok button should trigger onOk multiple times when onOk has close argument', () => {
const onOk = jest.fn();
open({
onOk: close => {
onOk();
(() => {})(close); // do nothing
},
});
$$('.ant-btn-primary')[0].click();
$$('.ant-btn-primary')[0].click();
$$('.ant-btn-primary')[0].click();
expect(onOk).toHaveBeenCalledTimes(3);
});
});
4 changes: 2 additions & 2 deletions components/modal/index.en-US.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ The items listed above are all functions, expecting a settings object as paramet
| title | Title | string\|ReactNode | - |
| width | Width of the modal dialog | string\|number | 416 |
| zIndex | The `z-index` of the Modal | Number | 1000 |
| onCancel | Specify a function that will be called when the user clicks the Cancel button. The parameter of this function is a function whose execution should include closing the dialog. You can also just return a promise and when the promise is resolved, the modal dialog will also be closed | function | - |
| onOk | Specify a function that will be called when the user clicks the OK button. The parameter of this function is a function whose execution should include closing the dialog. You can also just return a promise and when the promise is resolved, the modal dialog will also be closed | function | - |
| onCancel | Specify a function that will be called when the user clicks the Cancel button. The parameter of this function is a function whose execution should include closing the dialog. You can also just return a promise and when the promise is resolved, the modal dialog will also be closed | function(close) | - |
| onOk | Specify a function that will be called when the user clicks the OK button. The parameter of this function is a function whose execution should include closing the dialog. You can also just return a promise and when the promise is resolved, the modal dialog will also be closed | function(close) | - |

All the `Modal.method`s will return a reference, and then we can update and close the modal dialog by the reference.

Expand Down
4 changes: 2 additions & 2 deletions components/modal/index.zh-CN.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ title: Modal
| title | 标题 | string\|ReactNode | - |
| width | 宽度 | string\|number | 416 |
| zIndex | 设置 Modal 的 `z-index` | Number | 1000 |
| onCancel | 取消回调,参数为关闭函数,返回 promise 时 resolve 后自动关闭 | function | - |
| onOk | 点击确定回调,参数为关闭函数,返回 promise 时 resolve 后自动关闭 | function | - |
| onCancel | 取消回调,参数为关闭函数,返回 promise 时 resolve 后自动关闭 | function(close) | - |
| onOk | 点击确定回调,参数为关闭函数,返回 promise 时 resolve 后自动关闭 | function(close) | - |

以上函数调用后,会返回一个引用,可以通过该引用更新和关闭弹窗。

Expand Down