Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for #30021] [$500] Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest #32233

Closed
2 of 6 tasks
lanitochka17 opened this issue Nov 29, 2023 · 16 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 29, 2023

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.4.5-4
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Precondition: user should be signed in

  1. Tap on Profile icon and Sign Out
  2. Paste public room deeplink in any other app messenger https://staging.new.expensify.com/r/5408450846930023
  3. Tap on the deeplink in previous step
  4. Wait a few seconds

Expected Result:

Public conversation room should be displayed

Actual Result:

FAB menu opens when public room conversation displayed

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

Bug6295198_1701295395502.screen-20231129-205052.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~015980250456a04d6a
  • Upwork Job ID: 1729988693915959296
  • Last Price Increase: 2023-11-29
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 29, 2023
Copy link

melvin-bot bot commented Nov 29, 2023

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

@melvin-bot melvin-bot bot changed the title Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest [$500] Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest Nov 29, 2023
Copy link

melvin-bot bot commented Nov 29, 2023

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

Copy link

melvin-bot bot commented Nov 29, 2023

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 melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 29, 2023
Copy link

melvin-bot bot commented Nov 29, 2023

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

@ZhenjaHorbach
Copy link
Contributor

Proposal

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

Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest

What is the root cause of that problem?

The main problem is
THAT we do not have such a condition
What do we want so that our menu does not open when we navigate to public room conversation via deeplink as a guest

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

We can update this useEffect like

    useEffect(() => {
        const navigationState = props.navigation.getState();
        const routes = lodashGet(navigationState, 'routes', []);
        const currentRoute = routes[navigationState.index];
        if (currentRoute && ![NAVIGATORS.CENTRAL_PANE_NAVIGATOR, SCREENS.HOME].includes(currentRoute.name)) {
            return;
        }
        if (lodashGet(props.demoInfo, 'money2020.isBeginningDemo', false)) {
            return;
        }

        if (ReportUtils.isPublicRoom(props.report) && isAnonymousUser) {
            return;
        }

        Welcome.show({routes, showCreateMenu});
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [props.isLoading]);

useEffect(() => {
const navigationState = props.navigation.getState();
const routes = lodashGet(navigationState, 'routes', []);
const currentRoute = routes[navigationState.index];
if (currentRoute && ![NAVIGATORS.CENTRAL_PANE_NAVIGATOR, SCREENS.HOME].includes(currentRoute.name)) {
return;
}
if (lodashGet(props.demoInfo, 'money2020.isBeginningDemo', false)) {
return;
}
Welcome.show({routes, showCreateMenu});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [props.isLoading]);

What alternative solutions did you explore? (Optional)

NA

@DylanDylann
Copy link
Contributor

DylanDylann commented Nov 30, 2023

Proposal

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

  • Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest

What is the root cause of that problem?

  • We have a useEffect that will run once the API such as OpenApp, ReconnectApp, ... are called:
    useEffect(() => {
    const navigationState = props.navigation.getState();
    const routes = lodashGet(navigationState, 'routes', []);
    const currentRoute = routes[navigationState.index];
    if (currentRoute && ![NAVIGATORS.CENTRAL_PANE_NAVIGATOR, SCREENS.HOME].includes(currentRoute.name)) {
    return;
    }
    if (lodashGet(props.demoInfo, 'money2020.isBeginningDemo', false)) {
    return;
    }
    Welcome.show({routes, showCreateMenu});
    // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [props.isLoading]);
  • As the above, the Welcome.show({routes, showCreateMenu}) is called once the useEffect callback calls.
  • It will run showCreateMenu():
    showCreateMenu();

    as long as the isFirstTimeNewExpensifyUser is true (And a few conditions but are not related to this bug)
  • With the anonymous user, the isFirstTimeNewExpensifyUser is always true, so that leads to the showCreateMenu() is always called.

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

  • We can prevent the FAB menu from being opened if the user is anonymous.
  • Do it by updating:
    const showCreateMenu = useCallback(
    () => {
    if (!props.isFocused && props.isSmallScreenWidth) {
    return;
    }
    setIsCreateMenuActive(true);
    props.onShowCreateMenu();
    },
    // eslint-disable-next-line react-hooks/exhaustive-deps
    [props.isFocused, props.isSmallScreenWidth],
    );

    to:
    const showCreateMenu = useCallback(
        () => {
+          if ((!props.isFocused && props.isSmallScreenWidth) || Session.isAnonymousUser()) {
                return;
            }
            setIsCreateMenuActive(true);
            props.onShowCreateMenu();
        },
        // eslint-disable-next-line react-hooks/exhaustive-deps
        [props.isFocused, props.isSmallScreenWidth],
    );

What alternative solutions did you explore? (Optional)

  • Also, we can still allow to display the FAB menu when navigating to public room via deeplink as a guest but just for the first time (as we have done with authenticated user) - Currrently, once we refresh the page as a guest, it also opens the FABs menu. Do it by updating:
    // Update isFirstTimeNewExpensifyUser so the Welcome logic doesn't run again
    isFirstTimeNewExpensifyUser = false;

    to:
        // Update isFirstTimeNewExpensifyUser so the Welcome logic doesn't run again
        isFirstTimeNewExpensifyUser = false;
+      if(Session.isAnonymousUser()){
+          Onyx.set(ONYXKEYS.HAS_OPENED_FAB, true);
+        }
    const showCreateMenu = useCallback(
        () => {
+          if (!props.isFocused && props.isSmallScreenWidth || (Session.isAnonymousUser() && hasOpenedFABs)) {
                return;
            }
            setIsCreateMenuActive(true);
            props.onShowCreateMenu();
        },
        // eslint-disable-next-line react-hooks/exhaustive-deps
        [props.isFocused, props.isSmallScreenWidth],
    );

@ishpaul777
Copy link
Contributor

ishpaul777 commented Nov 30, 2023

we can hold this for #30021, the fix implemented for #30021 may solve this issue too

@DylanDylann
Copy link
Contributor

@ishpaul777 I think The #30021 is not related to this one

@ishpaul777
Copy link
Contributor

ishpaul777 commented Nov 30, 2023

oh so my proposal from #30021 will stands here with a slight variation.

Proposal

Problem

Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest
Here the Problem is that we opens the Fab menu inside a report not that we open fab menu for anonymous user, showing the fab menu to anonymous user is intentional to show main features of app and motivate them sign in (clicking a item links user to sign in)

Root Cause

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

https://github.com/Expensify/App/blob/main/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js#L156-L158

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

Changes

we need to return early here incase the screen width is small (mobile device) and user is anonymous because we cant link to the LHN by deeplink (route name for LHN "Home") as it opens signin modal, showing the popover automatically for small screenwidth not make sense.

@melvin-bot melvin-bot bot added the Overdue label Dec 2, 2023
@sonialiap
Copy link
Contributor

This issue sounds practically the same as #30021. The PR for that issue is waiting to be deployed so we should see fairly soon if that issue also fixes this one. Putting this on hold

@melvin-bot melvin-bot bot removed the Overdue label Dec 4, 2023
@sonialiap sonialiap changed the title [$500] Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest [HOLD for #30021] [$500] Chat - FAB menu opens when navigate to public room conversation via deeplink as a guest Dec 4, 2023
@DylanDylann
Copy link
Contributor

@sonialiap The 30021 is deployed to staging and maybe we still can reproduce this bug

@melvin-bot melvin-bot bot added the Overdue label Dec 7, 2023
@sonialiap
Copy link
Contributor

sonialiap commented Dec 7, 2023

Looks like it's fixed 🎉

Here's the result of following the reproduction steps above

WhatsApp Image 2023-12-07 at 17 39 39

@melvin-bot melvin-bot bot removed the Overdue label Dec 7, 2023
@DylanDylann
Copy link
Contributor

@sonialiap The bug is marked as "Can reproduce in MacOS: Chrome / Safari" as well

@DylanDylann
Copy link
Contributor

Here is from my side:

Screencast.from.07-12-2023.23.45.09.webm

@DylanDylann
Copy link
Contributor

@sonialiap Can you help check this one again #32233 (comment) since I still can reproduce. Thanks

@ntdiary
Copy link
Contributor

ntdiary commented Jan 10, 2024

@sonialiap Can you help check this one again #32233 (comment) since I still can reproduce. Thanks

Eh, sorry, I forgot about this. I can also reproduce it on the web platform. 😅

32233.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

6 participants