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 custom input's onChange handlers should have access to updated context value #8910

Merged
merged 2 commits into from
Jul 3, 2023

Conversation

WiXSL
Copy link
Contributor

@WiXSL WiXSL commented May 15, 2023

I'm updating a project from ra-v3 to ra-v4 where the value of change inputs is taken from the Form state within the onChange / onBlur event handler (instead of its e.target.value argument), and it doesn't have the updated value.

I found out that :
In react-admin v3 inputs run custom event handlers after react-final-form ones.
In react-admin v4 inputs run custom event handlers before react-hook-form ones.

So, changing this order of execution to match ra-v3 allows useFormContext().getValues() to have the latest value inside the custom event handlers.

Supersedes #8903

@slax57
Copy link
Contributor

slax57 commented May 15, 2023

@djhi May I let you check this comment and then merge if you approve this PR? 🙂

@slax57 slax57 requested a review from djhi May 15, 2023 15:04
@djhi djhi deleted the branch marmelab:next May 25, 2023 08:19
@djhi djhi closed this May 25, 2023
@WiXSL
Copy link
Contributor Author

WiXSL commented May 25, 2023

@slax57, @djhi, you didn't resolve what to do about this PR. Could you re-open it?

@djhi djhi reopened this May 25, 2023
@djhi
Copy link
Collaborator

djhi commented May 25, 2023

Sorry, the branch was deleted when I closed the merge master to next PR. Thanks for reporting!

@djhi djhi merged commit dd0e500 into marmelab:next Jul 3, 2023
@WiXSL WiXSL deleted the fix-eh-exec-order branch July 3, 2023 13:29
@WiXSL
Copy link
Contributor Author

WiXSL commented Jul 3, 2023

@slax57, I believe this wasn't included in the latest milestone and mentioned in the latest ChangeLog.
Just a heads-up

@djhi djhi added this to the 4.12.0 milestone Jul 4, 2023
@slax57
Copy link
Contributor

slax57 commented Jul 5, 2023

Right, thanks for the heads-up @WiXSL 🙂

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

Successfully merging this pull request may close these issues.

3 participants