Autocomplete: Allow overriding autocomplete via UserAttributes#9337
Conversation
This removes the need for the new property and thus essentially reverts MudBlazor#9330.
github-actions
bot
added
enhancement
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #9337 +/- ##
==========================================
+ Coverage 89.82% 90.74% +0.91%
==========================================
Files 412 403 -9
Lines 11878 12664 +786
Branches 2364 2448 +84
==========================================
+ Hits 10670 11492 +822
+ Misses 681 619 -62
- Partials 527 553 +26 ☔ View full report in Codecov by Sentry. |
| return userAutocomplete; | ||
| } | ||
|
|
||
| return $"mud-disable-{Guid.NewGuid()}"; |
There was a problem hiding this comment.
Don't we need to cache the Guid.NewGuid() to a private field so it doesn't change on every render?
There was a problem hiding this comment.
This might be a special case where you really want a new ID on every render to counter Chrome's nasty autocompletion. But of course @Aaron2404 claims that this is not reliable any more.
There was a problem hiding this comment.
Yeah, atleast not on chrome. But I'd say it's a good shout to keep the behaviour the same until changing the default method gets looked into properly.
There was a problem hiding this comment.
Do I understand correctly that this line basically disables the autocomplete assistant (aka type of information: first name, last name, etc.)? Why can't autocomplete="off" be used?
There was a problem hiding this comment.
Yes, atleast on chrome simply using "off" works. I'll try to test on other browsers and PR a default method which works for them. However it's kind of annoying to test, because it takes quite a while for the suggestions to get generated.
There was a problem hiding this comment.
Why can't
autocomplete="off"be used?
I just had a dejavue. We were already exactly at this point in Feb 22, 2021: #959 (comment)
There was a problem hiding this comment.
That's honestly awesome to see haha, I can imagine the dejavu. The issue back in 2021 seemed to be that "off" didn't work on chrome, which is promising as it does now. So I'm hoping that I'll find it works everywhere when I test it, could prevent a lot of headaches.
There was a problem hiding this comment.
I just had a dejavue. We were already exactly at this point in Feb 22, 2021: #959 (comment)
Hmm, I see. Though it was 2-3 years ago, so it could be worth checking again, and Aaron confirmed it works for him. Also, if it's in a form, you might need to put autocomplete="off" on the form itself. I think the problem is that Chrome tries to be smart and ignore developers and use the id and label names to enable auto-filling.
I found this discussion as well https://gist.github.com/niksumeiko/360164708c3b326bd1c8
There was a problem hiding this comment.
Don't you hate it when dumb software tries to be smarter than its users. Anyway, no matter what the result of @Aaron2404's investigation, this PR should be merged in the meantime I think. Even if we set it to "off" I think it might be useful to allow overriding the value. So you can merge, if you agree @ScarletKuro
There was a problem hiding this comment.
I agree, it's definitely useful to allow overriding, regardless of if we find a better default method.
4 participants
Description
This removes the need for the new property and thus essentially reverts #9330.
How Has This Been Tested?
unit
Type of Changes
Checklist
dev).