MudBaseInput: The validation is only triggered if the field is dirty…
Description
The problem we had in our project is that the validation should only be triggered when the input value of a form input element has been changed by the user (it is dirty). Currently, the validation is triggered when the user clicks into the input element and leaves it without any modification. This Pull Request resolves the issue described in #2879.
I introduced the parameter OnlyValidateIfDirty on the MudBaseInput class and a private field _isDirty. The flag (_isDirty) is set to true when the value of the input element has been changed by the user (SetValueAsync method). With this change, the on blur event only calls validate and set touched to true when the parameter OnlyValidateIfDirty is false or the is dirty flag was set to true.
How Has This Been Tested?
I added a new unit test for the TextField component which verifies the new and old behavior.
We included the new DLL into our project and verifed the old and new behavior by manually testing the feature.
The functionality should not change the current behavior when the OnlyValidateIfDirty parameter is set to false, which is the default.
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
Checklist:
- [x] The PR is submitted to the correct branch (
dev). - [x] My code follows the code style of this project.
- [x] I've added relevant tests.
Codecov Report
Merging #3726 (63ff54c) into dev (d55e31c) will increase coverage by
0.00%. The diff coverage is100.00%.
@@ Coverage Diff @@
## dev #3726 +/- ##
=======================================
Coverage 90.03% 90.04%
=======================================
Files 371 371
Lines 13382 13385 +3
=======================================
+ Hits 12049 12052 +3
Misses 1333 1333
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/MudBlazor/Base/MudBaseInput.cs | 91.66% <100.00%> (+0.26%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
We are currently having the exact same issue in our project with "not dirty" textfields that still get validated. Will this PR make it into the next MudBlazor update or should we look into a workaround?
I've lately had a very similar problem. only trigger validation if not empty or whitespace, but in fact I prefere the concept of only validating dirty fields.
Good work, thanks @Schaeri, I hope this gets integrated soon.
Are there any plans to merge back this pull request?
If this change would not fit into the mudblazor design concept, then a feedback would be helpful. That way I could modify the code to suit all project requirements. Or if such a feature has no chance of getting into mudblazor, this information would be helpful too. In order to decide what to do next in our current project.
We waiting for feedback. Thanks a lot.
Hello @just-the-benno
Thanks for the feedback and for testing the new feature. I was aware of the inconsistency, better said we also have no requirement in our project to change the validation behavior dynamically. The flag OnlyValidateIfDirty is for us simply a static configuration that is set once in the page design. In our use case always True.
It is now a requirement question how the UI element in MudBlazor should behave for all projects. If you find, the flag must also be able to be set dynamically, at any time, then I will try to fulfill this requirement. And revise the implementation and tests again. Then the question is how the UI element should behave. Just a reset of the validation? Or resetting the UI element to the initial state. So also deleting the input?
Depending on the requirement I will then also try to map the feature as suggested in the documentation.
If the implementation is left as is I would include two elements in one form and also describe the restrictions. If this is ok for you in a separate chapter in the form validation page of the documentation.
If I revise the implementation again, then I will include the feature as suggested directly in the Simple Form Validation example.
Hi @Schaeri
Thanks for your thoughts. I wanted to bring it to your attention. I think in most applicat,ions this value is not dynamic so it is - in my view - acceptable to not cater for changes OnlyValidateIfDirty during the lifetime of the component. If the need arises in the future, our "future self" can worry about it. :)
To merge your PR finally into the code base, please provide a simple (static) example, and we are good to go.
Hi @just-the-benno I have created the example.
Looking at the code I see that _isDirty is never ever set to false except at initialization. Shouldn't a Reset() reset it, or am I missing something?
If a user decides to reset a form, I'd argue that they have still interacted with a component before (or not), so the reset itself wouldn't change the _isDirty state.
If I understand the PR correctly, the intention here is to prevent showing validation messages for fields when a user hasn't interacted with them previously. And hence, you don't see _isDirty = false anywhere because this would mean to undo the interaction.
What are your thoughts, @Schaeri, regarding reset?
Hello @just-the-benno, Hello @henon
As @just-the-benno explained this execution is correct for me. The flag stores if a user made a manipulation on the input element or not. If this was not the case no validation is called in case of OnlyValidateIfDirty="true".
The question if Reset resets this state or leaves it is again a requirement question for me. I also don't know in which situations the reset is called. Is this just something manual that doesn't happen in the framework code? Or does the framework also call a reset in certain situations?
In our use case we never call a reset. Thus, unfortunately, we have not considered the case. If the requirement is that a reset resets the control to its original state, i.e. sets the initial default value and also resets the validation then one can also argue that the user manipulation should be reset.
As long as our requirement is not violated I can adjust the reset behavior to your wishes. And test if there are no side effects for us.
Please give me feedback if I should change the reset behavior.
Reset will only be called manually. It allows to reset the form into its initial untouched state. This is of course most useful for our documentation examples but I am sure there are also sensible use-cases of it in the wild.
So I think reset should also reset the flag.
Hi @Schaeri
Based on @henon suggestions, I think this is the last and final change before merging.
Can you please implement the logic that set the value of _isDirty to false in case a reset is requested? You can either test it with a new test or include this in your current "flow" test.
@just-the-benno: Thanks for the help. I implemented the rest and adapted the test.
Thanks a lot!
I'm glad that your changes could finally make it into the project. And I'm sorry that we didn't start earlier to work together.