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.

Sign up for GitHub

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

Jump to bottom

TextField: Form.Reset clears masked textfield #3935

Merged
merged 4 commits into from
Feb 16, 2022
Merged

TextField: Form.Reset clears masked textfield #3935

merged 4 commits into from
Feb 16, 2022

Conversation

Mr-Technician
Copy link
Member

Description

Calling Reset on a form will not reset a masked MudTextField, as shown in this TryMudBlazor. Type a phone number in the form, click reset, and the number will remain. If you fill any of the other fields they will reset as expected.

How Has This Been Tested?

I have added a test that tests resetting a Masked MudTextField within a MudForm. It also works within the unit tests viewer.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@henon
Copy link
Collaborator

henon commented Feb 13, 2022

The test seems to fail on the CI server? I'll re-run it.

@Mr-Technician
Copy link
Member Author

The test seems to fail on the CI server? I'll re-run it.

Looks like it failed again. :(

@henon
Copy link
Collaborator

henon commented Feb 14, 2022

I'll look at it today. I have a lot of experience with that kind of thing

@henon
Copy link
Collaborator

henon commented Feb 15, 2022 •
edited
Loading

Ok, I looked at it. It appears that adding the Mask parameter to the existing test component broke another test. You need to know that by setting the Mask parameter the internal MudInput of MudTextField is swapped out against a MudMask. This is probably the cause of the problem.

So I advise you to create a separate test component for testing reset with a masked text field and revert the change to the existing ResetTest test component.

@Mr-Technician
Copy link
Member Author

Mr-Technician commented Feb 15, 2022 •
edited
Loading

@henon This was my conclusion today as well. I opened a discussion on this topic: #3960

@codecov
Copy link

codecov bot commented Feb 15, 2022 •
edited
Loading

Codecov Report

Merging #3935 (8380750) into dev (7b360dd) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3935      +/-   ##
==========================================
+ Coverage   91.04%   91.12%   +0.07%     
==========================================
  Files         350      351       +1     
  Lines       11462    11605     +143     
==========================================
+ Hits        10436    10575     +139     
- Misses       1026     1030       +4
Impacted Files Coverage Δ
...dBlazor/Components/TextField/MudTextField.razor.cs 75.00% <100.00%> (+1.31%) ⬆️
src/MudBlazor/Components/Picker/MudPicker.razor 86.95% <0.00%> (ø)
.../MudBlazor/Utilities/MaskAlgorithms/PatternMask.cs 100.00% <0.00%> (ø)
src/MudBlazor/Utilities/MaskAlgorithms/DateMask.cs 92.24% <0.00%> (ø)
src/MudBlazor/Components/Picker/MudPicker.razor.cs 97.67% <0.00%> (+0.07%) ⬆️
src/MudBlazor/Components/Mask/MudMask.razor.cs 93.43% <0.00%> (+3.22%) ⬆️

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 7b360dd...8380750. Read the comment docs.

@Mr-Technician
Copy link
Member Author

Ok, I think we're all good now. Are you able to review this change @henon?

@henon henon changed the title MudMask: Fix bug where Form.Reset wouldn't clear masked textfield children. TextField: Form.Reset clears masked textfield Feb 16, 2022
@henon henon merged commit 79af7e9 into MudBlazor:dev Feb 16, 2022
@henon
Copy link
Collaborator

henon commented Feb 16, 2022

Nice and clean. Thank you!

@henon henon added this to the 6.0.7 milestone Feb 16, 2022
@Mr-Technician Mr-Technician deleted the mask-form-reset branch February 16, 2022 16:06
@Garderoben Garderoben added the bug Something does not work as intended/expected label Feb 25, 2022
jammerware pushed a commit to jammerware/MudBlazor that referenced this pull request Sep 20, 2022
@Mr-Technician @jammerware
TextField: Form.Reset clears masked textfield (MudBlazor#3935)
c345f2a 
* Fix bug where Form.Reset wouldn't clear masked textfield children.

* Add tests for masked TextField form reset.

* Revert "Add tests for masked TextField form reset."

This reverts commit e35d250.

* Add Form reset Mask test.
jammerware pushed a commit to jammerware/MudBlazor that referenced this pull request Sep 20, 2022
@Mr-Technician @jammerware
TextField: Form.Reset clears masked textfield (MudBlazor#3935)
41bcf8b 
* Fix bug where Form.Reset wouldn't clear masked textfield children.

* Add tests for masked TextField form reset.

* Revert "Add tests for masked TextField form reset."

This reverts commit e35d250.

* Add Form reset Mask test.
jammerware pushed a commit to jammerware/MudBlazor that referenced this pull request Sep 20, 2022
@Mr-Technician @jammerware
TextField: Form.Reset clears masked textfield (MudBlazor#3935)
df74f89 
* Fix bug where Form.Reset wouldn't clear masked textfield children.

* Add tests for masked TextField form reset.

* Revert "Add tests for masked TextField form reset."

This reverts commit e35d250.

* Add Form reset Mask test.
jammerware pushed a commit to jammerware/MudBlazor that referenced this pull request Sep 20, 2022
@Mr-Technician @jammerware
TextField: Form.Reset clears masked textfield (MudBlazor#3935)
00db660 
* Fix bug where Form.Reset wouldn't clear masked textfield children.

* Add tests for masked TextField form reset.

* Revert "Add tests for masked TextField form reset."

This reverts commit e35d250.

* Add Form reset Mask test.
3dots pushed a commit to 3dots/MudBlazor that referenced this pull request Mar 23, 2023
@Mr-Technician @3dots
TextField: Form.Reset clears masked textfield (MudBlazor#3935)
fe46965 
* Fix bug where Form.Reset wouldn't clear masked textfield children.

* Add tests for masked TextField form reset.

* Revert "Add tests for masked TextField form reset."

This reverts commit e35d250.

* Add Form reset Mask test.
ferraridavide pushed a commit to ferraridavide/MudBlazor that referenced this pull request May 30, 2023
@Mr-Technician @ferraridavide
TextField: Form.Reset clears masked textfield (MudBlazor#3935)
59eb2d1 
* Fix bug where Form.Reset wouldn't clear masked textfield children.

* Add tests for masked TextField form reset.

* Revert "Add tests for masked TextField form reset."

This reverts commit e35d250.

* Add Form reset Mask test.
ilovepilav pushed a commit to ilovepilav/MudBlazor that referenced this pull request Nov 25, 2023
@Mr-Technician @ilovepilav
TextField: Form.Reset clears masked textfield (MudBlazor#3935)
653308d 
* Fix bug where Form.Reset wouldn't clear masked textfield children.

* Add tests for masked TextField form reset.

* Revert "Add tests for masked TextField form reset."

This reverts commit e35d250.

* Add Form reset Mask test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something does not work as intended/expected
Projects
None yet
Milestone
6.0.7
Development

Successfully merging this pull request may close these issues.
3 participants
@Mr-Technician @henon @Garderoben