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

@@ -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>
     );

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);
+  });
 });

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.
 

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) | - |
 
 以上函数调用后，会返回一个引用，可以通过该引用更新和关闭弹窗。