App icon indicating copy to clipboard operation
App copied to clipboard

[$250] Login - After creating a new account the user is not brought to Concierge conversation

Open kbecciv opened this issue 3 years ago • 14 comments

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Navigate to https://staging.new.expensify.com/
  2. In the email field, enter a gmail email address for a new account
  3. Press continue
  4. Set a password that follows the password requirements
  5. After setting the password, verify user is brought to Concierge conversation

Expected Result:

After creating a new account the user is brought to Concierge conversation

Actual Result:

After creating a new account the user is not brought to Concierge conversation

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • iOS
  • Android
  • Mobile Web

Version Number: 1.2,17.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/196317411-f4fbc586-07eb-4057-8565-036c6d358e0b.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

kbecciv avatar Oct 18 '22 01:10 kbecciv

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

melvin-bot[bot] avatar Oct 18 '22 01:10 melvin-bot[bot]

I was able to reproduce this on staging when when the width of the viewport is at most 800*915. This is an external fix

Justicea83 avatar Oct 18 '22 13:10 Justicea83

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

melvin-bot[bot] avatar Oct 18 '22 13:10 melvin-bot[bot]

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

melvin-bot[bot] avatar Oct 18 '22 13:10 melvin-bot[bot]

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

melvin-bot[bot] avatar Oct 18 '22 13:10 melvin-bot[bot]

This needs a RCA right?

rushatgabhane avatar Oct 19 '22 23:10 rushatgabhane

Upwork: https://www.upwork.com/jobs/~010696c2ee4726b141

dylanexpensify avatar Oct 20 '22 14:10 dylanexpensify

@eVoloshchak, @Gonals, @dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] avatar Oct 24 '22 07:10 melvin-bot[bot]

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

melvin-bot[bot] avatar Oct 24 '22 07:10 melvin-bot[bot]

Reviewing today!

dylanexpensify avatar Oct 24 '22 09:10 dylanexpensify

@rushatgabhane what do you mean by RCA? 😂

dylanexpensify avatar Oct 27 '22 09:10 dylanexpensify

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

melvin-bot[bot] avatar Oct 31 '22 07:10 melvin-bot[bot]

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

melvin-bot[bot] avatar Oct 31 '22 08:10 melvin-bot[bot]

reviewing today!

dylanexpensify avatar Oct 31 '22 09:10 dylanexpensify

doubled

dylanexpensify avatar Nov 02 '22 15:11 dylanexpensify

not overdue, will put some more $$$ to it if no updates by Monday

dylanexpensify avatar Nov 04 '22 11:11 dylanexpensify

doubling

dylanexpensify avatar Nov 07 '22 08:11 dylanexpensify

I have a doubt about this issue, we have created an issue in the past that for a new user we should open the FAB menu,

so how can we handle this scenario in mobile view for a new user who should open the FAB menu and also navigate to Concierge conversation?

cc: @eVoloshchak @dylanexpensify

jayeshmangwani avatar Nov 07 '22 14:11 jayeshmangwani

I'd say we should prioritize navigating to the Concierge conversation over opening the Fab menu.

Gonals avatar Nov 07 '22 15:11 Gonals

Proposal

The root cause of this issue is that on smaller screens, despite being on the Concierge route the drawer is open leaving the user on the sidebar screen. This is the default behaviour for report routes as you can see by navigating to any report url on web using a small screen.

In order to display the concierge chat on small screens the drawer needs to be closed. We can close the drawer on the first user login in Welcome.js, or if we are not on a small screen show the FAB menu.

We will first need to pass the necessary props to Welcome.show(): https://github.com/Expensify/App/blob/2820c728c938d94b007aa747d4ba594547edc7af/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js#L57-L63

- Welcome.show({routes, showCreateMenu: this.showCreateMenu});
+ Welcome.show({isSmallScreenWidth: this.props.isSmallScreenWidth, routes, showCreateMenu: this.showCreateMenu});

Then close the drawer in Welcome.js: https://github.com/Expensify/App/blob/2820c728c938d94b007aa747d4ba594547edc7af/src/libs/actions/Welcome.js#L121-L125

        if (!Policy.isAdminOfFreePolicy(allPolicies) && !isDisplayingWorkspaceRoute) {
+            if (isSmallScreenWidth) {
+                Navigation.closeDrawer();
+            } else {
                showCreateMenu();
+            }
        }

However, the above changes will not work until these changes are merged as the user will just be navigated to a blank screen. But you can test them easily enough by changing SidebarLinks:

-               <Freeze freeze={this.props.isSmallScreenWidth && !this.props.isDrawerOpen}>
+               <Freeze freeze={this.props.isSmallScreenWidth && !this.props.isDrawerOpen && this.isSidebarLoaded}>
                    <LHNOptionsList
                        contentContainerStyles={[
                            styles.sidebarListContainer,
                            {paddingBottom: StyleUtils.getSafeAreaMargins(this.props.insets).marginBottom},
                        ]}
                        data={optionListItems}
                        focusedIndex={_.findIndex(optionListItems, (
                            option => option.toString() === this.props.reportIDFromRoute
                        ))}
                        onSelectRow={(option) => {
                            Navigation.navigate(ROUTES.getReportRoute(option.reportID));
                            this.props.onLinkClick();
                        }}
                        shouldDisableFocusOptions={this.props.isSmallScreenWidth}
                        optionMode={this.props.priorityMode === CONST.PRIORITY_MODE.GSD ? 'compact' : 'default'}
-                       onLayout={App.setSidebarLoaded}
+                       onLayout={() => {
+                           App.setSidebarLoaded();
+                           this.isSidebarLoaded = true;
+                       }}
                    />
                </Freeze>

Ollyws avatar Nov 08 '22 13:11 Ollyws

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

melvin-bot[bot] avatar Nov 08 '22 13:11 melvin-bot[bot]

Proposal

as we have to navigate to Concierge for mobile devices as web is working fine, we have to make 2 changes here

  1. we have to check if a mobile device we have to pass isSmallScreenWidth here, for checking mobile device in Welcome actions

https://github.com/Expensify/App/blob/952e5c39e4be989bd4aaea251ce5d3c4c64faa7a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js#L62

-          Welcome.show({routes, showCreateMenu: this.showCreateMenu});
+          Welcome.show({routes, showCreateMenu: this.showCreateMenu,isSmallScreenWidth: this.props.isSmallScreenWidth });

  1. then navigate to the Concierge chat

    1. in welcome page we have to import Report actions for accessing navigateToConciergeChat function
    +import * as Report from './Report';
    
    1. pass one more param in show function for accessing isSmallScreenWidth https://github.com/Expensify/App/blob/952e5c39e4be989bd4aaea251ce5d3c4c64faa7a/src/libs/actions/Welcome.js#L98
    -function show({routes, showCreateMenu}) {
    +function show({routes, showCreateMenu, isSmallScreenWidth}) {
    
    
    1. at last when we are showing create a menu, rather than need to check if isSmallScreenWidth then navigateToConciergeChat https://github.com/Expensify/App/blob/952e5c39e4be989bd4aaea251ce5d3c4c64faa7a/src/libs/actions/Welcome.js#L123-L125
    -            showCreateMenu();
    +            if(isSmallScreenWidth) {
    +               Report.navigateToConciergeChat(); 
    +            } else {
    +               showCreateMenu();
    +           } 
    
    

Video After Fix

https://user-images.githubusercontent.com/35371050/200774718-1f2f0578-186d-4a1b-8dd0-bd7ce131630e.mov

jayeshmangwani avatar Nov 09 '22 08:11 jayeshmangwani

@jayeshmangwani, thanks for the proposal! It is based on @Ollyws's proposal with the exception of using Report.navigateToConciergeChat(), which in turn also calls DrawerActions.closeDrawer(), so the proposals are identical

eVoloshchak avatar Nov 09 '22 14:11 eVoloshchak

@Ollyws, thanks, you've correctly identified the root cause of the issue

The root cause of this issue is that on smaller screens, despite being on the Concierge route the drawer is open leaving the user on the sidebar screen. This is the default behaviour for report routes as you can see by navigating to any report url on web using a small screen.

I think we should fix that. Your fix will work, but only for this specific case (first time logging in).

eVoloshchak avatar Nov 09 '22 14:11 eVoloshchak

@eVoloshchak If you open expensify web you'll notice it navigates you to a report route by default, if we were to make the drawer close on every report route the user would be presented with a chat instead of the sidebar every time they open expensify web on small screens.

Ollyws avatar Nov 09 '22 14:11 Ollyws

I'm talking about this case

https://user-images.githubusercontent.com/9059945/200853314-5a1dd839-5e82-4979-b4ce-ba4deb58dee8.mov

if we were to make the drawer close on every report route the user would be presented with a chat instead of the sidebar every time they open expensify web on small screens.

Seems like expected behavior, currently it just navigates to the main screen ignoring reportID in the URL

@Gonals, could you please take a look at this? I'm not entirely sure what is the expected behavior here

eVoloshchak avatar Nov 09 '22 14:11 eVoloshchak

Hmmm. It makes sense to me that, if we are providing a reportID, we should direct the user to the chat, even in small screens, rather than the main menu.

@marcaaron, do you have an opinion on this?

Gonals avatar Nov 09 '22 14:11 Gonals

We are discussing deprecating the drawer navigator so this should not be a problem in the future. I'd suggest we do nothing for now and close this issue or put it on hold and add it to the long list of bugs related to the drawer navigator.

marcaaron avatar Nov 11 '22 17:11 marcaaron

@marcaaron can we also add HOLD to the title ?

madmax330 avatar Nov 14 '22 07:11 madmax330

added to title, @madmax330 !

dylanexpensify avatar Nov 14 '22 08:11 dylanexpensify