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 input validation and dirty state after re-enabling disabled fields #10163

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

michel-paiva
Copy link

@michel-paiva michel-paiva commented Aug 26, 2024

Problem

  • Disabled and re-enabled fields did not update their dirty state, causing validation to run after they have been re-enabled.

Related issues:
Closes #10156
Closes #9870

Solution

  • This fix addresses the issue remove the workaround and making sure that a boolean value is passed to react-hook-form controller.

How To Test

  • Verify that input fields with validation are correctly marked as dirty and trigger validation when they are re-enabled after being disabled.

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Boss. 🫡

@slax57 slax57 added this to the 5.1.3 milestone Aug 27, 2024
@slax57 slax57 changed the title Fix input validation and dirty state after re-enabling disabled fields, by updating react-hook-form and removing workaround Fix input validation and dirty state after re-enabling disabled fields Aug 27, 2024
@slax57
Copy link
Contributor

slax57 commented Aug 27, 2024

It looks like one of the unit tests is failing, and this seems to be consequent to the react-hook-form version upgrade. This doesn't seem to be related to the code changed by this PR.
I'm currently investigating it...

@slax57 slax57 removed this from the 5.1.3 milestone Aug 27, 2024
@fzaninotto
Copy link
Member

IMHO, this PR should be against the next branch as it upgrades a dependency to a minor version. There is a possibility that it breaks existing apps, which would be unexpected in a minor and even more in a bugfix release.

@slax57
Copy link
Contributor

slax57 commented Aug 27, 2024

Okay, I managed to track down the issue, and filed a new issue on react-hook-form's repo: https://github.com/react-hook-form/react-hook-form/issues/12217

Depending on their answer, we may need to apply the workaround I mentioned to the useFormGroup.ts file.

Let's wait for their reply before taking actions.

@michel-paiva
Copy link
Author

@slax57 seems like they tried to optimize renders by only setting isValidating for inputs that has async validation
https://github.com/react-hook-form/react-hook-form/pull/12192/files

@slax57
Copy link
Contributor

slax57 commented Aug 28, 2024

@michel-paiva Okay, so this will probably not get fixed on their side.
That's why we opened #10168 to fix this issue on our side on master.

Now, can you please rebase your branch and PR to target the next branch?
You may also need to cherry-pick the fix I made to have your PR pass the CI checks.

Thank you.

…s, by updating react-hook-form and removing workaround
@michel-paiva michel-paiva changed the base branch from master to next August 28, 2024 09:46
@michel-paiva
Copy link
Author

@slax57 done as you said, let's see!

@slax57 slax57 added the RFR Ready For Review label Aug 30, 2024
@slax57 slax57 added this to the 5.2.0 milestone Aug 30, 2024
@slax57 slax57 merged commit a370a12 into marmelab:next Aug 30, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
3 participants