-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
MudAutocomplete, MudSelect: Fix Focus and Clear #10740
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
MudAutocomplete, MudSelect: Fix Focus and Clear #10740
Conversation
…Blazor#10643)" This reverts commit 10a60e0.
danielchalmers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small api tweak to match mudmask
|
Are there any other controls that have a clear button that are afflicted by this? |
from ValueTask to Task Co-authored-by: Daniel Chalmers <[email protected]>
none that I'm aware of or that has been reported |
|
Sorry, is AutocompleteRetainFocusTest the right example or is it for the other PR? I'm seeing a message box when I try to clear the field. Video3.mp4 |
|
@ScarletKuro Could you take a look at the property logic when you get a chance? |
No just the Docs autocomplete first example is what I used here. I did go ahead and add the logic in autocompleterefocustest to prevent popup when the value is empty |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #10740 +/- ##
=======================================
Coverage 91.83% 91.83%
=======================================
Files 427 427
Lines 13425 13428 +3
Branches 2589 2591 +2
=======================================
+ Hits 12329 12332 +3
Misses 523 523
Partials 573 573 ☔ View full report in Codecov by Sentry. |
danielchalmers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on my end 👍
|
I hope we've made sure that everything works with both mouse and touch devices for the MudSelect and MudAutocomplete, because I'm starting to get confused. In v8, we changed from I'm also slightly confused by this new method:
Wasn't the Clear functionality working before without any additional methods? I thought it was just a problem with stopPropagation, like it was here #10737 and here #10353I'm just worried this might mess up the validation. |
I don't think so tbh, the reason I added Value/Text/bound Value underneath each is because ClearAsync method doesn't actually clear. And it has commented workarounds in it so I just copied the functionality like Select does it and left that method alone. I honestly don't use autocomplete in my projects so I'm unsure if clear actually worked right. Without the added method hitting the clear button only cleared the text so you could type something else and search. It left the Value in so if you didn't type something new it just reverts because it never fires the ValueChanged event. |
I didn't think about touch input initially -
input events get weirdly complex. this one seems to be the solution and @versile2 has done a better job of testing it than me |
src/MudBlazor.UnitTests.Viewer/TestComponents/Autocomplete/AutocompleteRetainFocusTest.razor
Outdated
Show resolved
Hide resolved
|
ScarletKuro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I haven't tested personally so feel free to merge if ya confident



Description
Resolves #10733 by allowing the Clear button to actually clear the values
Resolves #10705 by reordering the focus logic
Resolves #10718 by removing additional click handler
Resolves #10739
How Has This Been Tested?
Unit tests passing after changing Click(s) to MouseDown(s), Visual tests done with video in comments.
Type of Changes
Checklist
dev).