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

Select: Required now also works when MultiSelection="true" (#4328, #4346) #4346

Merged
merged 3 commits into from
Apr 6, 2022

Conversation

gismofx
Copy link
Contributor

@gismofx gismofx commented Apr 3, 2022

Description

Fixes #4328
MudSelect When Required=true and MultiSelection=true Required Value check now works. Fixed by overriding HasValue method in MudSelect.

How Has This Been Tested?

Test Added To SelectTests
MultiSelectWithRequiredValue

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.

@gismofx
Copy link
Contributor Author

gismofx commented Apr 3, 2022

I ran all tests locally:
image

@codecov
Copy link

codecov bot commented Apr 3, 2022

Codecov Report

Merging #4346 (17e330e) into dev (4d9a839) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #4346      +/-   ##
==========================================
- Coverage   91.22%   91.17%   -0.05%     
==========================================
  Files         361      361              
  Lines       12450    12461      +11     
==========================================
+ Hits        11357    11361       +4     
- Misses       1093     1100       +7     
Impacted Files Coverage Δ
src/MudBlazor/Components/Select/MudSelect.razor.cs 96.72% <100.00%> (+0.02%) ⬆️
...r/Components/Autocomplete/MudAutocomplete.razor.cs 91.16% <0.00%> (-2.99%) ⬇️
...c/MudBlazor/Components/Popover/MudPopover.razor.cs 84.21% <0.00%> (-0.79%) ⬇️

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 4d9a839...17e330e. Read the comment docs.

@gismofx
Copy link
Contributor Author

gismofx commented Apr 4, 2022

Please wait on merge. I have fixed a typo in the test and need add two more checks to the test to show that the required error is not present when MultiSelect has value(s). Standby by please

@gismofx
Copy link
Contributor Author

gismofx commented Apr 4, 2022

Ready for review :)

@mckaragoz
Copy link
Member

Looking good for me

@henon henon changed the title Fixes #4328 - MudSelect Required value check now works when MultiSelection is true Select: Required now also works when MultiSelection="true" (#4328, #4346) Apr 6, 2022
@henon henon added bug Something does not work as intended/expected bug-functional labels Apr 6, 2022
@henon henon added this to the 6.0.10 milestone Apr 6, 2022
@henon henon merged commit aa2060c into MudBlazor:dev Apr 6, 2022
@henon
Copy link
Collaborator

henon commented Apr 6, 2022

Perfect! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MudSelect on MultiSelection prints the message Required even if a selection has been made.
3 participants