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

Add <Form disableInvalidFormNotification> to allow disabling notifications when the form is invalid #9353

Merged
merged 4 commits into from
Nov 10, 2023

Conversation

tim-pankoke
Copy link
Contributor

This should close #9350

@tim-pankoke tim-pankoke changed the title add disabling invalid form notification add disabling invalid form notification [RFR] Oct 12, 2023
@tim-pankoke tim-pankoke changed the title add disabling invalid form notification [RFR] [RFR] add disabling invalid form notification Oct 12, 2023
@fzaninotto
Copy link
Member

Could you please add documentation and unit tests?

@tim-pankoke tim-pankoke changed the title [RFR] add disabling invalid form notification [WIP] add disabling invalid form notification Oct 13, 2023
@tim-pankoke tim-pankoke changed the title [WIP] add disabling invalid form notification [RFR] add disabling invalid form notification Nov 3, 2023
@tim-pankoke
Copy link
Contributor Author

Could you please add documentation and unit tests?

added docs and a test to verify my changes  ✌️

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.

Otherwise it should be good. I'll just request @fzaninotto 's review to validate the new prop's name.
In any case, thanks!

Comment on lines 191 to 194
await waitFor(() => {
const notification = screen.queryByText('ra.message.invalid_form');
expect(notification).toBeNull();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This test passes even without disableInvalidFormNotification.

It is not always easy to check for the absence of something with Jest/RTL. You need to find an event to await for, that proves the form submission is complete, before attempting to check if the notification is there or not. You can try providing a jest.fn() as onSuccess to see if it helps, or try rendering the record and have RTL look for the changed value in the record.

Copy link
Contributor Author

@tim-pankoke tim-pankoke Nov 6, 2023

Choose a reason for hiding this comment

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

Thanks for the feedback! 🙏
I updated the unit test so that I'm waiting for the validation message to be shown before checking the absence of the notification.

@slax57 slax57 requested a review from fzaninotto November 6, 2023 13:39
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.

LGTM, thanks!

@fzaninotto fzaninotto merged commit d347bf0 into marmelab:next Nov 10, 2023
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto added this to the 4.16.0 milestone Nov 10, 2023
@fzaninotto fzaninotto changed the title [RFR] add disabling invalid form notification Add <Form disableInvalidFormNotification> to allow disabling notifications when the form is invalid Nov 10, 2023
@tim-pankoke tim-pankoke deleted the next branch November 12, 2023 19:38
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