Improving autofill background appearance on password inputs #7099

anthony-hull · 2022-01-09T18:49:44Z

Details

When auto filling any normal field we should see the whole input styled by autofill stylings
When autofilling a password input we should see part of the input styled with a strait border and a space before the visibility toggle button.

Fixed Issues

$ #7016

Tests

At login screen auto fill email box
see the background fills the whole box
autofill the password box
see that the box is filled like the screenshots

Verify that no errors appear in the JS console

QA Steps

Verify that no errors appear in the JS console

Tested On

Screenshots

Web

Hasn't regressed on other non secure inputs

Mobile Web

Desktop

There isn't any autofill on this

iOS

iOS doesn't colour backrounds on autofill. N/A

Android

Fix border underling for inputs on android:

anthony-hull · 2022-01-10T22:05:13Z

Hey team, I'm struggling to test on iOS. The autofill isn't triggering on the login. I would install 1Password but I don't have a paid iOS dev account to build for physical devices, so I'm stuck on the simulator. Do you have an idea?
Also and ideas on how to test autofill on the desktop client?

rushatgabhane · 2022-01-10T23:04:30Z

Hey team, I'm struggling to test on iOS. The autofill isn't triggering on the login. I would install 1Password but I don't have a paid iOS dev account to build for physical devices, so I'm stuck on the simulator. Do you have an idea?
Also and ideas on how to test autofill on the desktop client?

On it! Just need some time

anthony-hull · 2022-01-11T12:50:57Z

@Luke9389 do you know why that unit test is failing?

rushatgabhane · 2022-01-11T12:52:41Z

do you know why that unit test is failing?

It's an issue with OSBotify's GPG key. Not related to the PR

rushatgabhane · 2022-01-11T12:58:00Z

@anthony-hull
I tried all major password managers and couldn't get them to autofill on desktop. Password managers don't support it yet I think.

On iOS, I can autofill but the color of TextInput stays the same so nothing to show here

anthony-hull · 2022-01-11T13:05:40Z

Thank you very much for the help!

Does it behave the same on the prod app from the app store?

rushatgabhane · 2022-01-11T13:08:16Z

Does it behave the same on the prod app from the app store

It goes yellow for me. Hmm, weird

src/components/TextInput/BaseTextInput.js

rushatgabhane

Good work! Some suggestions

rushatgabhane · 2022-01-11T13:25:19Z

@anthony-hull
There's a very small white gap at bottom of TextInput when you fill with a password manager (Lastpass) on Android. The gap size changes when you hide/show password.

Is this fixable?

anthony-hull · 2022-01-11T15:03:05Z

I will

@anthony-hull There's a very small white gap at bottom of TextInput when you fill with a password manager (Lastpass) on Android. The gap size changes when you hide/show password.

Is this fixable?

I'll have a look! Should be able to today. If not tomorrow :)

anthony-hull · 2022-01-12T13:22:05Z

I think this happens when you auto fill into the empty field when it's not using the secure input prop. That is when the password is toggled to visible and then you autofill.

Autofilling the field in its default state I couldn't reproduce this.

rushatgabhane · 2022-01-12T19:41:34Z

I think this happens when you auto fill into the empty field when it's not using the secure input prop. That is when the password is toggled to visible and then you autofill.

Oh true, have you found a solution for this?

anthony-hull · 2022-01-12T20:26:30Z

I've not been able to so far. I've not done much native visual debugging before now.
I can't see anything different about the properties of the native element between the two states in Flipper.

anthony-hull · 2022-01-12T21:10:45Z

do you have any ideas? or advice on how to debug this?

rushatgabhane · 2022-01-13T09:58:54Z

Hmm you could experiment with padding. Maybe 1px more towards the bottom.

anthony-hull · 2022-01-14T00:05:30Z

I tried padding. Didn't seem to make a difference. I'll have another go tomorrow morning!

rushatgabhane · 2022-01-14T04:20:33Z

Alright, I took a look into this and like @anthony-hull said, this only happens if you autofill when password is visible.

On second thoughts, to notice this you gotta zoom and pixel peep.
I think this is good to merge.

cc: @Luke9389 lmk if you think this should be fixed

Luke9389 · 2022-01-14T20:15:45Z

Hmm, let me see if I can crack it. It seems like we're off by the width of the border.
@shawnborton what do you think about this? We've got a line of white pixels at the bottom of this input (I see it in a few other screenshots too).

rushatgabhane · 2022-01-15T14:11:13Z

I was wrong. It also happens when you use google autofill. And it's very noticeable

screen-20220115-170514.mp4

shawnborton · 2022-01-17T08:08:43Z

I think we should try to fix the 1px gap issue before merging this.

anthony-hull · 2022-01-18T20:06:40Z

That is true.
I just tried the app on my iPhone installed from the App Store and auto fill worked with 1Password and no highlight showed. Looks to me there isn't a background for auto fill on iOS

rushatgabhane · 2022-01-18T20:09:28Z

Bitwarden, Lastpass and iOS password manager don't highlight. Okay, probably safe to assume that there is no highlighting on iOS 😅

anthony-hull · 2022-01-18T21:23:23Z

great news! I'll update the PR tomorrow morning with test screenshots and the fix for this second issue too!
Thanks for the help :)

rushatgabhane · 2022-01-19T18:46:52Z

src/components/TextInput/index.android.js

+        autoCompleteType={props.autoCompleteType === 'new-password' ? 'password' : props.autoCompleteType}
+        innerRef={ref}
+
+        // style added here to fix autofill background not filling to the bottom of the input


Start the comment with capital letter

@anthony-hull LGTM
approving after this is done 👍

@anthony-hull waiting on this

this file doesn't exist anymore. I reverted the commit.

rushatgabhane

@Luke9389 looks like the 1px gap issue is for TextInput component too.
It's reproducible on main for web.

It's on inputs for pages like DebitCardPage, but not on phone/email input even though both use the same component.

This is unrelated to the current PR and issue. What do you suggest is the best course of action?
Anthony's fix for Android doesn't work here because only some TextInputs have the 1px gap issue.

anthony-hull · 2022-01-19T23:02:09Z

Feels too much of a hack to do this to all inputs.
I think we need another issue and to have a root cause rather than a hack to hide it

rushatgabhane · 2022-01-19T23:03:08Z

I agree @anthony-hull

Luke9389 · 2022-01-19T23:48:17Z

Yup, I agree that we should open a new issue and solve the gap separately.

Luke9389 · 2022-01-19T23:49:42Z

I think @rushatgabhane should be the one to get the bug report credit for that since he mentioned it first, here. Are you ok with that @anthony-hull?

anthony-hull · 2022-01-20T00:02:48Z

Yeah that makes sense to me!
Thanks for asking

This reverts commit 5c4da64.

rushatgabhane

@Luke9389 LGTM! 🎉 I've put the white line issue on slack so that it can be created.

Luke9389 · 2022-01-21T00:21:13Z

Just wanted to let y'all know that I'll review this tomorrow. Super stacked with stuff to do today 😅

rushatgabhane · 2022-01-21T00:35:33Z

Nw, take your time 😄

anthony-hull · 2022-01-21T12:58:13Z

Cool :)
Hoping no changes. I'm on holiday until Tuesday with no access to a computer

anthony-hull · 2022-01-25T09:53:54Z

I'm back today :)

rushatgabhane · 2022-01-25T09:55:40Z

@Luke9389 bump :)

Luke9389 · 2022-01-26T00:30:07Z

Thanks, @rushatgabhane. Sorry, I've been swamped recently :D

Luke9389

Thanks @rushatgabhane and @anthony-hull!

OSBotify · 2022-01-26T00:32:07Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-02-01T18:48:07Z

🚀 Deployed to staging by @Luke9389 in version: 1.1.33-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2022-02-02T18:52:31Z

🚀 Deployed to production by @sketchydroide in version: 1.1.34-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

added style changes

24a442c

anthony-hull marked this pull request as ready for review January 10, 2022 22:03

anthony-hull requested a review from a team as a code owner January 10, 2022 22:03

botify requested review from Luke9389 and rushatgabhane January 10, 2022 22:03

MelvinBot removed the request for review from a team January 10, 2022 22:03

rushatgabhane reviewed Jan 11, 2022

View reviewed changes

src/components/TextInput/BaseTextInput.js Outdated Show resolved Hide resolved

rushatgabhane suggested changes Jan 11, 2022

View reviewed changes

anthony-hull added 2 commits January 11, 2022 14:57

Merge branch 'main' into anthony--password-autofill-background

5c6fb33

multi line styles

96ed982

Enragedsaturday mentioned this pull request Jan 16, 2022

Remove didMount to stop auto focus on PasswordForm #7090

Merged

anthony-hull added 2 commits January 19, 2022 15:30

Merge branch 'main' into anthony--password-autofill-background

e8dc273

add margin fix for autofill bg on android

5c4da64

anthony-hull requested a review from rushatgabhane January 19, 2022 17:34

rushatgabhane reviewed Jan 19, 2022

View reviewed changes

Revert "add margin fix for autofill bg on android"

8783e2b

This reverts commit 5c4da64.

anthony-hull requested a review from rushatgabhane January 20, 2022 10:41

rushatgabhane approved these changes Jan 20, 2022

View reviewed changes

Luke9389 approved these changes Jan 26, 2022

View reviewed changes

Luke9389 merged commit d6af3c1 into Expensify:main Jan 26, 2022

mvtglobally mentioned this pull request Jan 31, 2022

[HOLD for payment 2022-09-16] [$4000] There's a very small white gap at bottom for TextInput when using autofill - @rushatgabhane #7470

Closed

OSBotify mentioned this pull request Feb 1, 2022

Deploy Checklist: New Expensify 2022-02-01 #7504

Closed

46 tasks

botify mentioned this pull request Feb 2, 2022

[HOLD for payment 2022-02-09] The password input element doesn't get fully highlighted when using auto-fill - Reported by @thesahindia #7016

Closed

Improving autofill background appearance on password inputs #7099

Improving autofill background appearance on password inputs #7099

Conversation

anthony-hull commented Jan 9, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

anthony-hull commented Jan 10, 2022 • edited Loading

rushatgabhane commented Jan 10, 2022

anthony-hull commented Jan 11, 2022

rushatgabhane commented Jan 11, 2022 • edited Loading

rushatgabhane commented Jan 11, 2022 • edited Loading

anthony-hull commented Jan 11, 2022

rushatgabhane commented Jan 11, 2022 • edited Loading

rushatgabhane left a comment

Choose a reason for hiding this comment

rushatgabhane commented Jan 11, 2022 • edited Loading

anthony-hull commented Jan 11, 2022

anthony-hull commented Jan 12, 2022

rushatgabhane commented Jan 12, 2022 • edited Loading

anthony-hull commented Jan 12, 2022

anthony-hull commented Jan 12, 2022

rushatgabhane commented Jan 13, 2022

anthony-hull commented Jan 14, 2022

rushatgabhane commented Jan 14, 2022 • edited Loading

Luke9389 commented Jan 14, 2022

rushatgabhane commented Jan 15, 2022 • edited Loading

shawnborton commented Jan 17, 2022

anthony-hull commented Jan 18, 2022

rushatgabhane commented Jan 18, 2022 • edited Loading

anthony-hull commented Jan 18, 2022 • edited Loading

rushatgabhane Jan 19, 2022

Choose a reason for hiding this comment

rushatgabhane Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

rushatgabhane Jan 20, 2022

Choose a reason for hiding this comment

anthony-hull Jan 20, 2022

Choose a reason for hiding this comment

rushatgabhane left a comment • edited Loading

Choose a reason for hiding this comment

anthony-hull commented Jan 19, 2022

rushatgabhane commented Jan 19, 2022

Luke9389 commented Jan 19, 2022

Luke9389 commented Jan 19, 2022

anthony-hull commented Jan 20, 2022

rushatgabhane left a comment • edited Loading

Choose a reason for hiding this comment

Luke9389 commented Jan 21, 2022

rushatgabhane commented Jan 21, 2022 • edited Loading

anthony-hull commented Jan 21, 2022

anthony-hull commented Jan 25, 2022

rushatgabhane commented Jan 25, 2022

Luke9389 commented Jan 26, 2022

Luke9389 left a comment

Choose a reason for hiding this comment

OSBotify commented Jan 26, 2022

OSBotify commented Feb 1, 2022

OSBotify commented Feb 2, 2022

anthony-hull commented Jan 9, 2022 •

edited

Loading

anthony-hull commented Jan 10, 2022 •

edited

Loading

rushatgabhane commented Jan 11, 2022 •

edited

Loading

rushatgabhane commented Jan 11, 2022 •

edited

Loading

rushatgabhane commented Jan 11, 2022 •

edited

Loading

rushatgabhane commented Jan 11, 2022 •

edited

Loading

rushatgabhane commented Jan 12, 2022 •

edited

Loading

rushatgabhane commented Jan 14, 2022 •

edited

Loading

rushatgabhane commented Jan 15, 2022 •

edited

Loading

rushatgabhane commented Jan 18, 2022 •

edited

Loading

anthony-hull commented Jan 18, 2022 •

edited

Loading

rushatgabhane Jan 19, 2022 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

rushatgabhane left a comment •

edited

Loading

rushatgabhane commented Jan 21, 2022 •

edited

Loading