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

UpdateButton Confirm Dialogue Close Cannot Be Triggered with a Custom onSuccess Mutation Option (no redirect) #9194

Closed
maxschridde1494 opened this issue Aug 17, 2023 · 7 comments · Fixed by #9196
Assignees
Labels

Comments

@maxschridde1494
Copy link
Contributor

What you were expecting:
I expect that I would be able to close the update button confirm dialogue even with a custom onSuccess mutation option.

What happened instead:
onSuccess method is run (i.e. custom notification, page refresh, etc.) but the confirm dialogue remains open without a page redirect.

Steps to reproduce:
Pass a custom onSuccess mutation option to a confirmable UpdateButton that does not trigger a redirect.

Related code:

<UpdateButton
          label='Send Password Reset'
          mutationMode="pessimistic"
          confirmTitle="Password Reset"
          confirmContent="Are you sure you want to send a password reset email?"
          data={{ ... }}
          mutationOptions={{
            onSuccess: () => {
              notify('Password reset email sent.', { type: 'success' })
              refresh()
            }
          }} 
        />

Other information:
Another idea, although somewhat unrelated, would be providing the ability to override the onSuccess / onError notification messages without having to pass custom mutation options. That would be a workaround for my case, but doesn't solve the underlying issue.

Environment

  • React-admin version: 4.13.0
  • React version: 18.2.0
  • Browser: Chrome
@fzaninotto
Copy link
Member

Confirmed, thanks for the report.

@maxschridde1494
Copy link
Contributor Author

Thanks for the quick response and fix! Do you know when we can expect a new release that will include the fix?

@fzaninotto
Copy link
Member

I've just released v4.13.1, which contains the fix.

In the future, check https://github.com/marmelab/react-admin/milestones: it shows the rough schedule for upcoming releases. In general, we release a bug fix version every week, and new features every month.

@fzaninotto
Copy link
Member

Reopened as #9196 will be reverted (it was a wrong fix)

@fzaninotto fzaninotto reopened this Aug 21, 2023
@slax57
Copy link
Contributor

slax57 commented Aug 21, 2023

Reopened as #9196 will be reverted (it was a wrong fix)

My bad! Reviewing PR #9208 made me realize that setOpen(false) used to be both in onSuccess and onError, so there is no BC!

However the rest of my comment is still valid I believe. Wdyt? Shoud we keep the fix or work on a better one?

@fzaninotto
Copy link
Member

You're right! It's not a breaking change.

The rest of your comment is valid: onSuccess now works, but onSettled doesn't. I think the cases where uses may want to pass a custom onSettled are rare, so we can decide not to support it.

So the original problem IS, indeed, fixed.

@slax57
Copy link
Contributor

slax57 commented Aug 21, 2023

Agreed. Thanks for your reply, and sorry for the noise 😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants