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(module:form): form.reset() will make new editContext #1035

Merged
merged 4 commits into from
Jan 24, 2021

Conversation

anddrzejb
Copy link
Member

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • Bundle size optimization
  • Performance optimization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

#947

💡 Background and solution

Microsoft.Aspnetcore.Componets.Forms.Editcontext does not really have any way of resetting the validation messages (source). So in form.Reset() method I assign a new EditContext(model) method and that actually resets validation messages.

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Changelog is provided or not needed

@codecov-io
Copy link

codecov-io commented Jan 23, 2021

Codecov Report

Merging #1035 (fe3a0d6) into master (9fb9bff) will decrease coverage by 0.14%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1035      +/-   ##
=========================================
- Coverage    5.48%   5.34%   -0.15%     
=========================================
  Files         426     426              
  Lines       23170   23181      +11     
=========================================
- Hits         1270    1238      -32     
- Misses      21900   21943      +43     
Impacted Files Coverage Δ
components/form/Form.razor.cs 0.00% <0.00%> (ø)
components/tabs/Tabs.razor.cs 44.48% <0.00%> (-11.03%) ⬇️
components/core/Base/AntComponentBase.cs 37.17% <0.00%> (-5.13%) ⬇️
components/tabs/TabPane.razor.cs 90.47% <0.00%> (-1.59%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fb9bff...fe3a0d6. Read the comment docs.

@ElderJames
Copy link
Member

Thank you.
Does it fix the other form issues?

@anddrzejb
Copy link
Member Author

anddrzejb commented Jan 24, 2021

Not yet. I was trying to address milestone issue. You can wait with merge, I will try to have a look at the other form validation issue.

@anddrzejb
Copy link
Member Author

anddrzejb commented Jan 24, 2021

@ElderJames Commit 61918dc fixes issues #851 & #982 (I imagine it also fixes #991 as it was marked as duplicate of #982).
Edit: it also seems to be fixing #469.

@anddrzejb
Copy link
Member Author

anddrzejb commented Jan 24, 2021

@ElderJames In commit fe3a0d6 I added single extra method ValidationReset(). This is the result of my works on #516. In all honesty, I do not think we can make anything in the project to comply with #516 request. The problem is that the request is not to validate when textbox is disabled, but ValidationAttribute does not have access to bounded text box (and actually I do not think it should). So when validation is running, it has only access to the model it validates. The only other solution is to add a bool Disabled to the model and the use a custom validation attribute. This is when ValidationReset() comes in as a necessary method (and also Reset() cannot be used).
Anyway, if this is accepted, I have a ready sample for #516 to explain how the request can be achieved.

Also - test are failing on tabs again. I did not change anything in tabs....I will look into that...

Edit: I had a look into the tests - in my dev machine it failed on the same test. Then I rerun it and it was ok. I have run the tests since then 4 times, with 100% success rate... I don't know, seems to me it was a fluke...How can I reissue the checks in github?

@ElderJames
Copy link
Member

@anddrzejb Thanks for fixing so many issues. The check is pass while I re-run it.

@anddrzejb
Copy link
Member Author

Got lucky here - a lot of the issues were simple fix & same fix applied to most of them.
I will also try to fix #1036 so please wait with the merge.

@anddrzejb
Copy link
Member Author

@ElderJames Apparently there is nothing to be fixed in #1036 so I think you can merge this PR if you are fine with it.

Copy link
Member

@ElderJames ElderJames 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!

@ElderJames ElderJames merged commit 318317c into ant-design-blazor:master Jan 24, 2021
@anddrzejb anddrzejb deleted the formValidationReset branch January 24, 2021 16:21
@ElderJames ElderJames linked an issue Jan 26, 2021 that may be closed by this pull request
ElderJames pushed a commit that referenced this pull request Apr 23, 2022
* fix(module:form): form.reset() will make new editContext

fixes 947

* fix(module:form): validation is reset when model changes

fixes 851
fixes 982
fixes 991

* feat(module:form): validation reset method
ElderJames pushed a commit that referenced this pull request Apr 30, 2022
* fix(module:form): form.reset() will make new editContext

fixes 947

* fix(module:form): validation is reset when model changes

fixes 851
fixes 982
fixes 991

* feat(module:form): validation reset method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment