[HOLD for payment 2023-08-01] [$1000] Web – 2FA - The copy icon placed in "copy code" button is clicked, the icon remains in a hovered state.

[kbecciv]

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

---

Action Performed:

1. Go to the "Settings" section.
2. Click on the "Security" option.
3. Select the "Two-factor authentication" feature.
4. Notice that when the copy icon placed in "copy code" button is clicked, the icon remains in a hovered position until the cursor is placed over it and then moved away.

Expected Result:

The copy icon should not remain in a hovered position when the "copy code" button is clicked.

Actual Result:

The copy icon should not remain in a hovered position when the "copy code" button is clicked until the cursor is placed over it and then moved away.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.31-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Bug.18.mp4

Desktop.2023.06.26.-.17.03.34.02.mp4

Expensify/Expensify Issue URL:
Issue reported by: @usmantariq96
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687498320311389

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01827743d082c5b93a
• Upwork Job ID: 1674404614868873216
• Last Price Increase: 2023-07-06

[melvin-bot]

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[Ollyws]

Proposal

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

If the user hovers out when the copied! message is displayed, the button remains in a hovered state.

What is the root cause of that problem?

In PressableWithDelayToggle, the hover state does not update because the pressable hit-box moves from underneath the mouse when the text changes length (to Copied!).

This is because the browser updates the hover state on mousemove, if the component itself moves it will not be triggered.

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

Move the Text to be inside the Pressable component and modify the styles so the icon changes when the user hovers the button itself, then it will respond properly when hovering regardless of the content position. This is the expected behaviour for buttons, whereas at the moment only the icon responds to hover.

What alternative solutions did you explore? (Optional)

None

[CortneyOfstad]

Was able to recreate this as well, so getting eyes on the proposal 👍

[melvin-bot]

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

[melvin-bot]

Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[CortneyOfstad]

@abdulrahuman5196 Any thoughts on the proposal above?

[abdulrahuman5196]

@Ollyws I am not sure if i am understanding the proposal correctly.

In PressableWithDelayToggle, the text is rendered outside of the Pressable component which causes it to block the hoverout event when it is long enough.

How is text blocking the hover updates on the different child inside PressableWithoutFeedback ?

[Ollyws]

@abdulrahuman5196
Actually that was perhaps not a good explanation.
Essentially the pressable component is only wrapping the icon, which means it will only highlight when we hover over the icon specifically (contrary to the behaviour of other buttons in our app and generally, where the icon will be highlighted on hover of the button itself).

This means when the icon changes position when the text changes the hoverout is never triggered.

Essentially my proposal is to put all of the content, text and icon inside the pressable component so firstly, the icon is highlighted when we hover the button itself and also that the hoverout will always respond regardless of the position of the icon. Making the hitbox for the hover effect the entire button essentially, as is the case with most buttons.

(this will also require a couple of minor css changes to deal with padding)

The result would look like this:

button_expl.mp4

[abdulrahuman5196]

Essentially the pressable component is only wrapping the icon, which means it will only highlight when we hover over the icon specifically

@Ollyws I agree on this. it weird that while only hovering the icon it goes to hover state, it should be in hover state even if we go over the text. @CortneyOfstad Kindly correct me if wrong, if the expectation is to show the hover state when only we go over the icon not the icon text.

But regardless, the current issue is that the hover state itself is not updated correctly leading to the copy icon being in hovered state. Could you mention why do you think both are related? Why does the hover provide wrong results when hovered over the icon(which is the root cause actually).

[CortneyOfstad]

@abdulrahuman5196 I believe you are correct that the hover state should be when we go over all text 👍

[Ollyws]

@abdulrahuman5196 As far as I can see the hover state does not update because the pressable hit-box moves from underneath the mouse when the text changes length (to Copied!).
If you change the text so it's the same length so the pressable doesn't move then the hover works fine.

I think this is because the browser updates the hover state on mousemove, if the component itself moves it will not be triggered.

[CortneyOfstad]

Heading OoO for the week (back July 10) so re-assigning this in the meantime 👍

[melvin-bot]

Triggered auto assignment to @maddylewis (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

@maddylewis, @abdulrahuman5196 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[maddylewis]

this was assigned to me while i was out of office - reviewing today.

[melvin-bot]

@arosiclair, @Ollyws, @maddylewis, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[maddylewis]

To-dos:

• hire @abdulrahuman5196 in Upwork
• hire @Ollyws in Upwork
• hire @usmantariq96 as reporter in Upwork
• Pay $1000 to @abdulrahuman5196
• Pay $1000 to @Ollyws
• Pay $250 to @usmantariq96

[maddylewis]

@abdulrahuman5196 - will you provide an update on this one? are we waiting for the fix to be deployed? #22430 (comment)

once the fix is live, do we need to wait for the 7-day regression period before issuing payment? lmk - thanks!

[abdulrahuman5196]

@maddylewis

will you provide an update on this one? are we waiting for the fix to be deployed?

The fix was merged yesterday. And waiting for prod deployment

once the fix is live, do we need to wait for the 7-day regression period before issuing payment?

Yes

[usmantariq96]

@maddylewis offer accepted with thanks.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.44-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix for: Web – 2FA - The copy icon placed in "copy code" button is clicked, the icon remains in a hovered state. #22430

If no regressions arise, payment will be issued on 2023-08-01. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @Ollyws requires payment
• @abdulrahuman5196 requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
• [@abdulrahuman5196] 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:
• [@abdulrahuman5196] 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:
• [@abdulrahuman5196] Determine if we should create a regression test for this bug.
• [@abdulrahuman5196] 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.
• [@maddylewis] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[maddylewis]

moving to daily since payment date is coming up.

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
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:
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:

This is not a regression. The component has been this way with PressableWithDelayToggle.js

Determine if we should create a regression test for this bug.
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.

This is corner case. So regression test wouldn't be benificial here.

@maddylewis

[maddylewis]

Payments

Outlined here - #21673 (comment)

• Contributor assigned on July 6
• PR merged on July 21
• Requested refund of $500 from both C+ and Contributor

---

• @abdulrahuman5196 (C+ $500)
• @Ollyws (Contrbutor $500)

[Ollyws]

@maddylewis refunded

[abdulrahuman5196]

@maddylewis Done

[maddylewis]

thanks!

[maddylewis]

thanks!