[HOLD for payment 2023-12-13] [$1000] Deeplink - FAB menu briefly shown when navigate to concierge deeplink with new account

[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: 1.3.87-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Log out of the ND app
2. Go to https://staging.new.expensify.com/concierge
3. Log in with a new account

Expected Result:

Concierge chat is opened, and the FAB menu is not displayed

Actual Result:

The FAB menu is briefly displayed when navigating to the concierge chat. The FAB icon is displayed as "x" instead of "+"

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

Android: Native

Bug6243487_1697744044675.video_2023-10-19_15-29-54.mp4

0-02-01-a546fbcbcd25efb0d31ad3d4733437f49ef76d416931d3aded2e1ec08630f9a5_c0f77a4a57c59d14.1.mp4

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01277cf9c667bd137d
• Upwork Job ID: 1715096447474085888
• Last Price Increase: 2023-11-20
• Automatic offers:
• jjcoffee | Reviewer | 27768526
• dukenv0307 | Contributor | 27768530

Issue Owner

Current Issue Owner: @lakchote

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @kadiealexander (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

[melvin-bot]

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

[abzokhattab]

not able to reproduce on ios-mweb

[jjcoffee]

@abzokhattab The FAB is displaying open in your screenshot, which it shouldn't be.

[ishpaul777]

Proposal

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

FAB menu shown when navigate to concierge deeplink with new account

What is the root cause of that problem?

this Happens because we are explictly opening Popover when the route name is "home" or "CentralPaneNavigator".

https://github.com/ishpaul777/App/blob/05ed9f2a8812c81e19deed1e01b7eb79ae88bc44/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js#L160

we are not returning early incase of full width report view (mobile screen width), hence the popover menu opens

why the FAB only shows briefly.

because of this useEffect when navigation to conceirge trigerred the screen becomes inactive and popover hides, but this isn't the case for ios native there must a race condition in screen getting focussed and fab menu becoming visible because fab option stays open ios native and don't hide when chat is open.

Also why is this only happening for the concierge chat (if that's actually the case)?

Fab Menu auto. opens for new users only, as a new user there is no chat a user a can deeplink to so this doesn't happen for other chats but for concierge only.

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

1. Check for mobile screen width using the useWindowDimensions hook.

2. If the route name is "CentralPaneNavigator" and the screen is in mobile view, we should return early to prevent the Popover from opening.

What alternative solutions did you explore? (Optional)

N/a

[dukenv0307]

My main proposal is so that the FAB will reopen to onboard users after coming back from Concierge, while the alternate proposal is in case we decide the FAB should not open.

Proposal

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

The FAB menu is briefly displayed when navigating to the concierge chat. The FAB icon is displayed as "x" instead of "+"

What is the root cause of that problem?

We're allowing the Create menu to open here as long as the currentRoute is either NAVIGATORS.CENTRAL_PANE_NAVIGATOR or SCREENS.HOME, so even though the SidebarScreen that has the FAB button is not focused, it's still showing the Create Menu on the Concierge page as we can see above, because we'll land on the HOME screen first before navigating to Concierge page. This happens not only to Concierge page, but to any other report page (can be reproduced by starting chat with a new user, then deeplink to that report, then login by that new user).

The Create menu opens briefly because in here, we have the logic to hide the create menu if the screen is unfocused. So the Create menu only shows for a bit, then is closed. Sometimes we'll see that the Create menu opens and stay there (it does disappear), because there's a race condition here, where the screen unfocusing and the setIsCreateMenuActive(true) happens at the same time, so when this line is triggered, the isCreateMenuActive is not set to true yet, so the setIsCreateMenuActive(false) does not run.

This is only happening for small screen because in large screen, the LHN along with the floating button will show side-by-side with the opened report, so it makes sense for the FAB menu to show upon concierge deeplink navigation. Meanwhile for small screen, the floating button and opened report are on 2 different screens so it Concierge shows, the FAB menu should not show.

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

On small screen width device, we should show the Create menu only if the SidebarScreen is having focus. We can update this to

isVisible={isCreateMenuActive && (!isSmallScreenWidth || props.isFocused)}

We can consider setting the state of the FAB button based on the same condition as well, although if the screen is not focused, we won't be seeing the FAB button anyway.

Although not strictly required for this issue, if we want to fix the race condition mentioned in the RCA, we can move this line to before this one so we'll always set setIsCreateMenuActive to false when calling hideCreateMenu, even in the case isCreateMenuActive is not yet updated to the latest value. Or can add a isCreateMenuActiveRef and use it instead of the state.

What alternative solutions did you explore? (Optional)

If we don't want to reopen the FAB after going back from Concierge, we can make sure we use a isFocusedRef rather than props.isFocused here. This is so that the showContextMenu always use the latest value of the isFocused, and it will not use the stale prop which causes the context menu to be opened even though the screen has already lost focus.

[melvin-bot]

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

[kadiealexander]

@jjcoffee any thoughts on the proposals above?

[jjcoffee]

Neither of the proposals explain why the FAB only shows briefly. Also why is this only happening for the concierge chat (if that's actually the case)?

[ishpaul777]

Neither of the proposals explain why the FAB only shows briefly.

because of this useEffect when naivgation to conceirge trigerred the screen becomes inactive and popover hides, but this isn't the case for ios native there must a race condition in screen getting focussed and fab menu becoming visible because fab option stays open ios native and don't hide when chat is open.

Also why is this only happening for the concierge chat (if that's actually the case)?

i dont think this only happens for concierge chat will confirm once i get my hand on my phone and add a video.

Fab Menu auto. opens for new users only, as a new user there is no chats a user a can deeplink to so this doesn't happen for other chats but for concierfe only.

[melvin-bot]

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

[melvin-bot]

Upwork job price has been updated to $1000

[kadiealexander]

Not overdue, get your proposals in!

[dukenv0307]

Neither of the proposals explain why the FAB only shows briefly. Also why is this only happening for the concierge chat (if that's actually the case)?

I'll provide a detailed update shortly on this

[jjcoffee]

Haha well, looks like we're still on a 50:50 split on Slack about this, but I think we can just go ahead with @dukenv0307's proposal here. The RCA is correct and sufficiently detailed, and the solution is nice and straight forward. In comparison @rojiphil's proposal feels a bit over-engineered when we can have a much simpler fix.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[jjcoffee]

@lakchote We were trying to settle on the right behaviour once we navigate back from the concierge chat, i.e. should the create menu show as open, or does it make sense to keep it hidden since we've already navigated directly to a chat. I asked on Slack but we're pretty much at a 50:50 split 😄

Let me know if you have any thoughts, otherwise I think we can just go with what @dukenv0307 has in their proposal (open once navigating back from the chat).

[melvin-bot]

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

[lakchote]

@jjcoffee In my opinion, we shoud show the FAB but only once?

It's important to clearly communicate the value of the app, and I think by showing it once to the user is a good idea to convert them to paid down the line.

If we did it more than once, it could be frustrating.

@dukenv0307's proposal LGTM 👍

[melvin-bot]

📣 @jjcoffee 🎉 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

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[dukenv0307]

@jjcoffee The PR is ready for review.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[jjcoffee]

Deployed to production Dec 6, not sure what happened to the automation! @kadiealexander Can you update the title and add the Awaiting Payment label?

[sonialiap]

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:

• [@jjcoffee] The PR that introduced the bug has been identified. Link to the PR: [HOLD for payment 2023-12-13] [$1000] Deeplink - FAB menu briefly shown when navigate to concierge deeplink with new account #30021 (comment)
• [@jjcoffee] 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: [HOLD for payment 2023-12-13] [$1000] Deeplink - FAB menu briefly shown when navigate to concierge deeplink with new account #30021 (comment)
• [@jjcoffee] 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: [HOLD for payment 2023-12-13] [$1000] Deeplink - FAB menu briefly shown when navigate to concierge deeplink with new account #30021 (comment)
• [@jjcoffee] Determine if we should create a regression test for this bug.
• [@jjcoffee] 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.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/350223

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - it looks like this behaviour was probably always present
• 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
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• 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

For small screen devices only:

1. Log out
2. Navigate to https://staging.new.expensify.com/concierge
3. Sign in with a completely new account
4. After successful sign in, verify that the create menu does not open
5. Click on the back button
6. Verify that the create menu appears

Do we agree 👍 or 👎

[melvin-bot]

Issue is ready for payment but no BZ is assigned. @stephanieelliott you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

[kadiealexander]

Everyone is paid!