[$250] Android - 2-FA – Use two-factor authentication code and Use recovery codes blinks

[lanitochka17]

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

---

Version Number: 9.0.30-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): applausetester+gm103086@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

User have 2-FA established

1. Open New Dot app
2. On login page enter your email address and magic code
3. Verify 2-FA page is opened
4. Switch between "Use two-factor authentication code" and "Use recovery codes"

Expected Result:

"Use two-factor authentication code" and "Use recovery codes" options should not blink

Actual Result:

Options "Use two-factor authentication code" and "Use recovery codes" blinks

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6595150_1725614013433.Recording__3889.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021834200655136685716
• Upwork Job ID: 1834200655136685716
• Last Price Increase: 2024-09-19
• Automatic offers:
• brunovjk | Reviewer | 104097398

Issue Owner

Current Issue Owner: @brunovjk

[melvin-bot]

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@kevinksullivan FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The use recovery code/use 2fa code blinks when pressing it.

What is the root cause of that problem?

The text button for use recovery code/2fa code use the same Pressable. We just conditionally render the text depends on whether we want to use recovery/2fa code.

App/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.tsx

Lines 347 to 357 in 9affb13

	<PressableWithFeedback
	style={[styles.mt2]}
	onPress={switchBetween2faAndRecoveryCode}
	hoverDimmingValue={1}
	pressDimmingValue={0.2}
	disabled={isValidateCodeFormSubmitting}
	role={CONST.ROLE.BUTTON}
	accessibilityLabel={isUsingRecoveryCode ? translate('recoveryCodeForm.use2fa') : translate('recoveryCodeForm.useRecoveryCode')}
	>
	<Text style={[styles.link]}>{isUsingRecoveryCode ? translate('recoveryCodeForm.use2fa') : translate('recoveryCodeForm.useRecoveryCode')}</Text>
	</PressableWithFeedback>

So, when we press the text button, the press dim is applied, which reduces the opacity to 0.2 and because it uses the same Pressable, the dim affects the 2nd text too. For example, if the text is initially Use recovery code, when we press it, the text is switched to Use 2FA code and the dim is applied briefly to Use 2FA code too, so it looks like the text is blinking. The 0.2 opacity makes the "blink" obvious.

What changes do you think we should make in order to solve the problem?

We can remove the 0.2 dim value and just let it use the default value (0.8), just like the Go back text button but I don't think it solves the root cause.

So, to solve the root cause, we can set a key to the pressable so a new instance of pressable is created every time we switch between codes.

We can use the boolean value (and convert it to string) as the key or use the text itself or any other value.

key={isUsingRecoveryCode}
key={isUsingRecoveryCode ? translate('recoveryCodeForm.use2fa') : translate('recoveryCodeForm.useRecoveryCode')}

[github-actions]

@bernhardoj Your proposal will be dismissed because you did not follow the proposal template.

[melvin-bot]

@kevinksullivan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@kevinksullivan Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021834200655136685716

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk (External)

[kevinksullivan]

Looping in another BZ as I am going OOO

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[brunovjk]

I will review the proposals once I am done with another PR.

[brunovjk]

Reviewing.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[brunovjk]

@lanitochka17 Is this issue only reproducible on Android? I couldn’t reproduce it on any platform. @bernhardoj Were you able to reproduce this without any issues? Thanks!

[bernhardoj]

Yes, you can see that when the text switches, the text dims.

android.mp4

[melvin-bot]

@kevinksullivan @VictoriaExpensify @brunovjk this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[brunovjk]

@bernhardoj is this blinking?

Screen.Recording.2024-09-20.at.15.01.17.mov

Judging by the archive history, it doesn't seem like a regression to me, it's been like that for a long time.
I believe that here we can just remove pressDimmingValue={0.2} and leave as Go back, mentioned in your proposal.

Screen.Recording.2024-09-20.at.15.05.32.mov

What do you mean by "I don't think it solves the root cause."? Thank you :D

[bernhardoj]

is this blinking?

not really.

I think it's reproducible when the app is a bit lagging. When you press the text, the dims effect is applied to the switched text, so it looks like its blinking.

Use recovery code
Press
Switched to Use 2FA code
Dims applied to Use 2FA code

So, reducing the dims doesn't solve the issue. We need to prevent the dim effect to be applied to the switched text.

[brunovjk]

Hey @bernhardoj, I tested the changes and noticed only a small improvement with the key addition, maybe because my emulator isn’t lagging now.

Without key:

Screen.Recording.2024-09-21.at.09.47.09.mov

With key:

Screen.Recording.2024-09-21.at.09.48.31.mov

Before proceeding, I’ll ask an internal to confirm whether we should remove pressDimmingValue={0.2} here for consistency.

With key and without pressDimmingValue={0.2}:

Screen.Recording.2024-09-21.at.09.54.23.mov

Once confirmed, we can move forward with your proposal and test thoroughly. Thanks for your work!

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[brunovjk]

Hey @NikkiWines, could you please check my previous comment and confirm if we should remove pressDimmingValue={0.2} here for consistency? Let me know if anything needs clarification!

Thanks!

[NikkiWines]

Removing the dimming value sounds right to me 👍

[brunovjk]

Removing the dimming value sounds right to me 👍

Great, so we can go with @bernhardoj's proposal :D

[NikkiWines]

Yep! @bernhardoj proposal looks good 👍

[melvin-bot]

📣 @brunovjk 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[bernhardoj]

PR is ready

cc: @brunovjk

[brunovjk]

The PR went into production on Sep 30.

cc: @VictoriaExpensify

[brunovjk]

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: I haven't found any PR that could have introduced this bug, as it's an edge case and it seems like it's always been there.
• [@brunovjk] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@brunovjk] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@brunovjk] Determine if we should create a regression test for this bug. Yes
• [@brunovjk] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal:

Prerequisite: 2FA is enabled; Plataform: Android native.

• Log out
• Enter your login and magic code
• Press the Use two-factor authentication code/Use recovery codes multiple times to switch between those two text
• Verify the text is not blinking.

Do we agree 👍 or 👎?

cc: @VictoriaExpensify

[VictoriaExpensify]

Payment summary:
Contributor: @bernhardoj owed $250 via NewDot
Contributor+: @brunovjk paid $250 via Upwork

Thanks for your work on this!

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj