[HOLD for payment 2022-11-29] [$250] Android/iOS - Chat and the FAB menu options open on the chat page when tapping both in quick succession.

[kavimuru]

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. Launch App and login
2. From the LHN home screen, tap on any chat row and quickly tap on the green + icon (FAB)

Expected Result:

Only the chat should open as it was the first tap.

Actual Result:

Chat and FAB menu open at the same time

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Android
• iOS

Version Number: v1.2.5-0
Reproducible in staging?: Y
**Reproducible in production?:**Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Android:

https://user-images.githubusercontent.com/43996225/191874271-d3bcc36c-d4d0-4898-8a4c-61a254d43849.mp4

iOS:

Upwork URL: https://www.upwork.com/jobs/~0159458d877f5bc785
Issue reported by: Applause internal team
Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[Beamanator]

Looks like a good External issue, maybe we can prevent opening the FAB / prevent having it clickable if the route includes a report ID? Just an idea

[trjExpensify]

1. Launch App and login as HT account

What does HT stand for and will a contributor who gets this issue understand that abbreviation and be able to log-in to a "HT account"?

Only the FAB should be open as last tap or only Chat page should open

The expected results are ambiguous. It would need to be either of these things, and we should provide guidance on which one we want it to be. IMO, the last tap should be the action we take. So the chat page wouldn't open, only the global create menu would. @Beamanator, what are your thoughts there? I kinda' think this issue goes away as the app becomes more performant when navigating. As in, you won't have time to move your thumb and click the FAB.

Where is this issue occurring?

Android

Did we try reproducing on iOS? I can actually reproduce it there, so going to update the OP:

Looks like a good External issue, maybe we can prevent opening the FAB / prevent having it clickable if the route includes a report ID?

@Beamanator but on web/desktop it's expected that you can open the global create menu while currently on a chat report, will that proposal not cause problems on those platforms?

[Beamanator]

Sooo good point, HT is "High Traffic" which I think is a bit difficult to test as a contributor, but actually I'm even reproducing easily on Android w/ a non-HT account, so I'll remove that from the OP & title

IMO, the last tap should be the action we take. So the chat page wouldn't open, only the global create menu would. @Beamanator, what are your thoughts there?

Interesting, I was thinking the first action we take 🤔 Should we bring this to Slack for a wider discussion?

I kinda' think this issue goes away as the app becomes more performant when navigating. As in, you won't have time to move your thumb and click the FAB.

That's possible, but it would be pretty easy to tap a report & touch the FAB in VERY quick succession, so I still think it would be good to address this for now & later

And thanks for reproducing on iOS! It's fine to have proposals that will be for only 2 platforms, as long as we make sure there's no regressions on web / desktop :D

[jeet-dhandha]

Proposal

1. For this we can simply navigate user back to HOME screen if he accidentally presses on the FAB menu.

Also tested both scenarios:

1. Press on FAB and then press on a chat.
2. Press on chat and then press on FAB.

Code:

Fixed Video:

Screen.Recording.2022-09-26.at.6.20.46.AM.mov

[Beamanator]

Awaiting proposal review, not overdue

[trjExpensify]

Job on Upwork here: https://www.upwork.com/jobs/~019fb94172f17c76bc

[chauchausoup]

I just found that the issue is replicating in few other places too.

• search box + LHN
• settings avatar + LHN

[trjExpensify]

I'm not sure I understand fully, @chauchausoup. Can you post us some screenshots/vids? 🤔

[trjExpensify]

In the meantime, @parasharrajat @Beamanator are we waiting on something for feedback on this current proposal? #11229 (comment)

[parasharrajat]

Looking at it now and trying to understand the issue first 🎃 .

[parasharrajat]

I do not get the expected result. What should be done to fix this? @trjExpensify

Only the FAB should be open as the last tap or only the Chat page should open

Both statements are contradicting each other. IMO, the first trigger action should take priority which is the chat opening action in our case. (try any native app and try to click two actions simultaneously, the one that is clicked first takes priority).

But the proposal #11229 (comment) is not matching any of them.

[Beamanator]

Agree with @parasharrajat 👍 And yes, your spidey-senses were correct @trjExpensify 😉

[trjExpensify]

Cool, so the job is here and I've sent you the offer @aimane-chnaif, and @parasharrajat for C+.

Look forward to the PR! 🚀

[melvin-bot]

@Beamanator, @trjExpensify, @parasharrajat, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[trjExpensify]

Morning, Melv. PR is up, awaiting a review from @parasharrajat! 🎉

[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:

• @trjExpensify A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:
• @parasharrajat @aimane-chnaif @Beamanator The PR that introduced the bug has been identified. Link to the PR:
  • This wasn't a regression, the "issue" has always been present, we just now decided to "fix" it
• @trjExpensify 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:
  • N/A in this case
• @trjExpensify A discussion in #contributor-plus 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:
• @trjExpensify Payment has been made to the issue reporter (if applicable)
• @trjExpensify Payment has been made to the contributor that fixed the issue (if applicable)
• @trjExpensify Payment has been made to the contributor+ that helped on the issue (if applicable)

[trjExpensify]

Hit staging, awaiting a prod deploy to start the timer.

[aimane-chnaif]

@trjExpensify Am I eligible for more compensation here?
Reason:

• complexity (handle all cases of multiple triggers at the same time)
• Also handles keyboard shortcut case ([HOLD] [$250] BUG: Fab menu shows when sidebar is opened reported by @mdneyazahmad #11708)

[trjExpensify]

Yep, we'll combine the payment for the two. Here's the job to accept the offer on (@parasharrajat as well).

I'm going to close the other issue out.

[aimane-chnaif]

Thanks @trjExpensify
Hope PR timeline is eligible for bonus as well

[aimane-chnaif]

Cool, so the job is here and I've sent you the offer @aimane-chnaif, and @parasharrajat for C+.

Look forward to the PR! 🚀

@trjExpensify I just noticed I have already been hired and accepted for this job 11 days ago so I think this one can be used for 50% bonus if you agree

[trjExpensify]

Ah, I couldn't find that one! Cool, so we'll use:

• That job for the bonus ($250 for getting it merged in +3 days)
• This one for the fix ($500 for the issues combined).

[melvin-bot]

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

• prevent FAB menu and other page showing simultaneously #12655

If no regressions arise, payment will be issued on 2022-11-29. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

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

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:

• [@parasharrajat / @aimane-chnaif / @Beamanator] The PR that introduced the bug has been identified. Link to the PR:
• [@parasharrajat / @aimane-chnaif / @Beamanator] 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:
• [@parasharrajat / @aimane-chnaif / @Beamanator] 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:
• [@trjExpensify] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[Beamanator]

I think this "issue" has been around forever, no specific bug - so I think we can check off the first 3 points - we probably should add some regression test for this though

[trjExpensify]

Settled up these contracts, so we're all square here. 🎉

we probably should add some regression test for this though

I don't think it's worth adding one for an edge case that in practice is incredibly hard to reproduce. Unless the app is very, very slow, you don't get the chance to tap both a chat row and the FAB in quick succession unless you force it using two thumbs or something.

Going to close this out, but let me know if you disagree.

[parasharrajat]

@trjExpensify Based on #11229 (comment), I should be eligible for the bonus as well.

[trjExpensify]

You were hired and paid on both of those linked jobs above, no?

[parasharrajat]

Yeah, found it. Thanks.