fix(ui): bitwarden auto complete not working for local login #4088
Conversation
Zebebles
requested review from
sct,
TheCatLady,
danshilm and
OwsleyJr
as code owners
|
@s0up4200 FYI |
Looks clean! Thanks for fixing my mess 😄 |
| {...(allowAutoComplete | ||
| ? {} | ||
| : { | ||
| autoComplete: 'off', | ||
| 'data-1pignore': 'true', | ||
| 'data-lpignore': 'true', | ||
| 'data-bwignore': 'true', | ||
| })} |
There was a problem hiding this comment.
| {...(allowAutoComplete | |
| ? {} | |
| : { | |
| autoComplete: 'off', | |
| 'data-1pignore': 'true', | |
| 'data-lpignore': 'true', | |
| 'data-bwignore': 'true', | |
| })} | |
| autoComplete={!allowAutoComplete ? 'off' : undefined} | |
| data-1pignore={!allowAutoComplete || undefined} | |
| data-lpignore={!allowAutoComplete || undefined} | |
| data-bwignore={!allowAutoComplete || undefined} |
The original syntax is a bit ugly and hard to read, also the "data-1pignore" was duplicated Actually, no, it was lpignore - lowercase L instead of , added it back in
There was a problem hiding this comment.
I can see why the object spreading / ternary mix could be considered ugly and hard to read if someone's coming across it for the first time.
I still stand by it, as I think that the final value of the data-props is clearer at a glance than the boolean expressions.
It also keeps all of the effects of the allowAutoComplete prop organised and contained.
Maybe an alternative change to improve readability would be to keep the values in a const instead?
Let me know what you think. Happy to change it if others agree with you
const PREVENT_AUTOCOMPLETE_PROPS = { // At top of file with type defs
autoComplete: 'off',
'data-1pignore': 'true',
'data-lpignore': 'true',
'data-bwignore': 'true'
};
<Component
{...(allowAutoComplete ? {} : PREVENT_AUTOCOMPLETE_PROPS)}
{...componentProps}
className={`rounded-l-only ${componentProps.className ?? ''}`}
/>
There was a problem hiding this comment.
I don't think you need to change this. I like the object spread! Looks cleaner to me.
|
Related to fallenbagel/jellyseerr#1487 |
Zebebles
force-pushed
the
fix/bitwarden-doesnt-autofill
branch
from
aa4f3dd to
b0e4a9b
Compare
Description
As described in the linked issue, Bitwarden just looks for the existence of the
data-bwignoreattribute rather than what its value is. Therefore, thedata-bwignore="false"inLocalLogin.tsxresults in Bitwarden ignoring the field.This PR addresses that by replacing the
data-xxignoreattributes with a singleallowAutoCompleteprop that conditionally includes these attributes when false or undefined.I also enabled auto complete on 'new password' fields across the ui as most password managers have a
generate passwordpopup for these.To-Dos
yarn buildyarn i18n:extractIssues Fixed or Closed