Feature Suggestion: show password toggle on password fields

[anthony-hull]

View password feature on password fields

Motivation

Improve accessibility for users.

Implementation

I think it would be good to replicate the UX of Edge browser where when you type it shows an icon button inside the field to toggle show password.
You should also be able to tab to the icon on desktop to maintain keyboard nav.

Edge cases:
Hiding native view password icon in Edge
Password managers tamper with the DOM in this location

Upwork job: https://www.upwork.com/jobs/~01f52f49fe3707fca2

[mallenexpensify]

Holding until #1517 is closed then I'll start a review.
@anthony-hull I'm going to assign to you for now, can you kick it back to me once 1517 is closed and I'll take it from there?

[rdjuric]

Was about to suggest this feature and found an open issue for it 😃

cc'ing @mallenexpensify since the PR you mentioned is closed now.

[anthony-hull]

Ahh yes. I forgot this one was open :) Thank you Rafael

[mallenexpensify]

@anthony-hull can you provide more detail on what you're recommending, and possibly include a screenshot or vid? Any chance this has been discussed in Slack, #expensify-open-source yet? If so, can you drop in a link? Thanks

[anthony-hull]

Yes we had a discussion on slack
I will follow up with a video on Monday. I'm busy the next 4 days.

[anthony-hull]

I think it's best to show the password by default on mobile devices. You can trivially move your screen in a way to obscure the password as you type. Additionally as you type there is a large on screen keyboard that people can watch you enter it on. I'm not sure there is much of a security gain from it vs the loss of UX.

On desktop, mask passwords by default, but add a “Show” option so that people can remember the password and check their work.
On mobile, unmask passwords by default, but add a “Hide” option. It’s especially hard to type a complex, secure password on mobile.
Unmasking passwords on mobile is not really hurting security very much. Mobile keyboards indicate exactly which letters were typed, anyway.

Ironically now I think of it, I just added the confirm password field in the set password form. If you allow the user to check their work or show by default you can remove the need for repetition. We could have just one entry box and make it easier.

I think behaviour of showing and hiding that mailchip implemented is a nice way to go:
https://login.mailchimp.com/signup/

Or how Edge does it with the icon inside the input:

We could hide the password again when the box loses focus.

[mallenexpensify]

Thanks @anthony-hull , I'm double checking to make sure this isn't something that's already being worked on internally. I'll followup tomorrow

[mallenexpensify]

Thanks for holding, nobody mentioned this is something that's currently be worked on. I agree this is feature that would be useful. First, can you take a peek at this issue to see if any work being done there would help on mobile for this issue? #3108 (I have no idea, just searched 'show password' in the repo to ensure something wasn't already being worked on).

Then, can you submit a proposal for how you want to implement this and I'll assign an engineer to review?
Thanks

[anthony-hull]

It would make sense to use the new input component in that issue. I'm happy to hold until it's merged.
When secureTextEntry is passed into the component I would change the behaviour to:

Implement the show/hide icon as a background image svg with onClick handlers to change the visibility of the text input.
The visibility can be controlled by changing the props passed to the wrapped text input field.

[MelvinBot]

Triggered auto assignment to @deetergp (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[MelvinBot]

Triggered auto assignment to @puneetlath (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[mallenexpensify]

I used Exported to assign an engineer for review. @deetergp , can you review @anthony-hull 's proposal?

@puneetlath , I unassigned myself then added External because I'm OOO next week. Also, I removed Exported label because it hasn't been posted to Upwork yet, it makes more sense to me to get Deeter to agree on the solution before posting.

[deetergp]

The feature @anthony-hull proposes sounds good, but we're going to need a more detailed description about how he is going to do it before moving forward.

[anthony-hull]

The exact UX isn't agreed.
https://github.com/Expensify/Expensify.cash/pull/3414/files

The following props are set for TextInputs are used for passwords:

secureTextEntry
autoCompleteType="password"
textContentType="password"

When textContentType is set to password in the props of ExpensiTextInput:
src/components/ExpensiTextInput/index.js I will add an SVG of an eye or whatever icon I'm given to the input. Above or inside the input depending on the designs.
I will hold state in the ExpensiTextInput component for show password or not. When you click on the icon it will toggle that state.
The state will control the Icon shown and if the secureTextEntry is true or false when passed into the wrapped TextInput component:

<Icon name={this.state.iconType} onPress={() => this.toggleShow />
<Input secureTextEntry={this.state.show}  />

[deetergp]

Your proposal makes sense and sounds good, @anthony-hull. Thanks!

@puneetlath I am unsure of the next step, since this has not been posted on Upwork.

[puneetlath]

Created the Upwork job and invited @anthony-hull to it: https://www.upwork.com/jobs/~01f52f49fe3707fca2

[anthony-hull]

I accepted the job on UW. I can start work with what I have. But in parallel:
Ideally I would like an eye icon of two states provided by your designers.
I see you have:
in Expensify.cash\assets\images

But I think we need a toggled state with the eye closed to show the different hidden/shown states.

[puneetlath]

@Expensify/design anyone able to help with icons for this?

[deetergp]

There's some discussion going on in the draft PR, but this is continuing to move forward.

[puneetlath]

Hi @anthony-hull. Just checking in again to see how this is going 😄.

[anthony-hull]

Yeah sorry I'm on holiday until Monday :) I have the code ready just still need to test on Android and Desktop

[deetergp]

Heya, @anthony-hull, have you made any progress with this issue?

[anthony-hull]

Yes I'm waiting for a PR review.
I messaged the assigned staff member yesterday to let them know it's ready.

[johnmlee101]

Do we think that this should also be on hold due to N6?

[puneetlath]

Yes, I think we should hold merging it until n6 is released. We can still approve it, but I think we should hold merging. And we'll provide @anthony-hull with the bonus as compensation for the delay.

[kadiealexander]

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

[MelvinBot]

This issue has not been updated in over 15 days. @deetergp, @puneetlath, @anthony-hull eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[deetergp]

Keep your shirt on, Melvin — it's still a weekly.

[parasharrajat]

I will be reviewing the PR and I will let you know @puneetlath when this is ready for final review and possibly for merge. See More

[MelvinBot]

This issue has not been updated in over 15 days. @deetergp, @puneetlath, @anthony-hull eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[puneetlath]

@anthony-hull @johnmlee101 how's this one going?

[anthony-hull]

Sorry notification got missed by me. PR is ready hopefully for final review. Just need to test on all platforms again

[puneetlath]

Paid with n6 bonus. Thanks!