From f4c489553fa4df52d2d54f93ba9dad3d0d4aae7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=81=8F=E5=8F=B3?= Date: Sat, 18 Apr 2020 01:09:14 +0800 Subject: [PATCH] fix: Modal.info onOk execute times (#23360) * refactor: modal confirm onOk code * fix: Modal.xxx onOk execute time when has close argument close #23358 * Add test case * fix lint --- components/modal/ActionButton.tsx | 74 ++++++++++++---------- components/modal/__tests__/confirm.test.js | 42 +++++++++++- components/modal/index.en-US.md | 4 +- components/modal/index.zh-CN.md | 4 +- 4 files changed, 83 insertions(+), 41 deletions(-) diff --git a/components/modal/ActionButton.tsx b/components/modal/ActionButton.tsx index cdf6ec3f3d..cc8f0aa5d2 100644 --- a/components/modal/ActionButton.tsx +++ b/components/modal/ActionButton.tsx @@ -35,55 +35,59 @@ export default class ActionButton extends React.Component) { + 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 ( - ); diff --git a/components/modal/__tests__/confirm.test.js b/components/modal/__tests__/confirm.test.js index 478f5c50ea..30ba10bf00 100644 --- a/components/modal/__tests__/confirm.test.js +++ b/components/modal/__tests__/confirm.test.js @@ -9,6 +9,7 @@ describe('Modal.confirm triggers callbacks correctly', () => { afterEach(() => { errorSpy.mockReset(); document.body.innerHTML = ''; + Modal.destroyAll(); }); afterAll(() => { @@ -74,12 +75,19 @@ describe('Modal.confirm triggers callbacks correctly', () => { expect(errorSpy).not.toHaveBeenCalled(); }); + it('should not hide confirm when onOk return Promise.resolve', () => { + open({ + onOk: () => Promise.resolve(''), + }); + $$('.ant-btn-primary')[0].click(); + expect($$('.ant-modal-confirm')).toHaveLength(1); + }); + it('should emit error when onOk return Promise.reject', () => { const error = new Error('something wrong'); open({ onOk: () => Promise.reject(error), }); - // Fifth Modal $$('.ant-btn-primary')[0].click(); // wait promise return Promise.resolve().then(() => { @@ -125,6 +133,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 => { @@ -235,9 +259,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); + }); }); diff --git a/components/modal/index.en-US.md b/components/modal/index.en-US.md index 7009d08f63..6557731701 100644 --- a/components/modal/index.en-US.md +++ b/components/modal/index.en-US.md @@ -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. diff --git a/components/modal/index.zh-CN.md b/components/modal/index.zh-CN.md index 87d1dbc5fd..6a7d4fca06 100644 --- a/components/modal/index.zh-CN.md +++ b/components/modal/index.zh-CN.md @@ -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) | - | 以上函数调用后，会返回一个引用，可以通过该引用更新和关闭弹窗。