[HOLD for payment 2021-08-24] Desktop - Error message for password mismatch not dismissed

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue is failing #4409 for destop

Action Performed:

1. Launch the app and login
2. Click Settings > Change password
3. Enter different passwords for the newPassword field and confirmPassword field.
4. Confirm error is shown
5. Enter correct password in both field and click away from the text box so focus is lost

Expected Result:

Error should automatically go and the button should be enabled in case the passwords match

Actual Result:

Save button enabled and error message not disappear

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platform:

Where is this issue occurring?

• Desktop App

Version Number: 1.0.85-6

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:
On Web, the error message is dismissed and the PR is a pass.

Bug5193944_Recording__1009.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @marcaaron (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[OSBotify]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[parasharrajat]

cc @mananjadhav .

[mananjadhav]

This PR #4607 has the fix for it. Waiting on this to get it merged.

[anthony-hull]

in my PR I'm intending to remove the second password field because the user will be able to unhide their password, removing the purpose of password repetition.
#2734

[mananjadhav]

@anthony-hull I would still recommend keeping "Confirm Password" field. It ensures that the user has double checked the password by typing it twice.

With one field we can't guarantee that the user has clicked on visibility toggle to ensure he has typed it correctly. What do you think?

[anthony-hull]

Very good point, I agree that they might not check. But in that case the user could just reset their password.

My proposed way you reduce the frustration of typing the password out twice for all users who follow the flow.
Keeping the two boxes you help only those who don't bother toggling AND make a mistake. And I think the chance of mistake is doubled when you have to type twice, further making it a frustrating journey as you might have got it right first time, but got it wrong the second time!

I'm also intending to have the password show by default on mobile devices as it's trivial to make sure someone doesn't see your screen when using a mobile. It's not like a desktop where it's on a big monitor for all to see. So many mobile users won't have the toggle issue.

There are trade offs for both options. I personally hate typing my password out twice.

I did a little research and:
https://web.dev/sign-in-form-best-practices/#no-double-inputs

[mananjadhav]

Good thought. Lets get more eyes to see what they think

[AndrewGable]

Is this a duplicate of #4606 ?

[AndrewGable]

@puneetlath, @shawnborton, and @deetergp - What are your on this comment? It does seem like there might be two parallel conversations and ideas for what the best UX will be for the change password field. I thought we'd just match expensify.com UX, but let's discuss so we aren't duplicating work.

[mananjadhav]

@AndrewGable I would say, one case was left out and merged in the same issue's PR #4607. If you can review then this will get resolved.

[puneetlath]

I agree with @anthony-hull. I think the ability to show your password and not have to type it twice is the better UX.

[marcaaron]

Not sure what there is left to triage here seems like @AndrewGable is on it. 🎉
Since this is a deploy blocker perhaps we should just fix this regression for now and discuss the ideal UX elsewhere?

[AndrewGable]

Just wrapped discussing this up with @puneetlath - I think our suggestion is to fix the deploy blocker ASAP (merge #4607), then we can implement the proposed UX in #2734

[AndrewGable]

@isagoico - Can you verify this is fixed by #4607 then close? Thanks!

[isagoico]

Retested and it was a pass 💃 🎉

Recording.101.mp4