App icon indicating copy to clipboard operation
App copied to clipboard

[$500] mWeb- Blank page is displayed when tapped on Concierge or Send a message options

Open kbecciv opened this issue 1 year ago β€’ 12 comments

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


Issue found when executing PR https://github.com/Expensify/App/pull/11857

Action Performed:

  1. Open the site https://help.expensify.com/main
  2. Click on Concierge button and verify it opens the Concierge chat in NewDot if the user is logged in.
  3. Click on the Send message button and verify it opens the Concierge chat in NewDot too.

Expected Result:

By clicking on the Send message button and verify it opens the Concierge chat in NewDot too.

Actual Result:

Blank page is displayed when tapped on Concierge or Send a message options

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.2.16.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

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

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/196014717-570a6458-0e7c-45e9-9e9a-693a550cec66.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

kbecciv avatar Oct 16 '22 02:10 kbecciv

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

melvin-bot[bot] avatar Oct 16 '22 02:10 melvin-bot[bot]

@kbecciv can you confirm this only happens on Mobile Web? I have tested on Desktop and also on Android with the native app and it works.

cc @marcochavezf as you are the author of https://github.com/Expensify/App/pull/11857. Can this be external? πŸ€”

flodnv avatar Oct 17 '22 13:10 flodnv

@flodnv Correct, it happens only on mobile Web. Windows/Chrome is Pass for QA team.

https://user-images.githubusercontent.com/93399543/196203915-ad9af389-2887-4c23-92e2-45ffadbbb672.mp4

kbecciv avatar Oct 17 '22 14:10 kbecciv

Oh interesting it only happens in mWeb (no native app installed). Yeah it can be external. FWIW this issue is not related to the help.expensify.com site, the problem is that for some reason https://new.expensify.com/concierge is not opening the Concierge report correctly only on mWeb (only if the user is logged in).

marcochavezf avatar Oct 18 '22 16:10 marcochavezf

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

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

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

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

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

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

Posted job.

https://www.upwork.com/ab/applicants/1582465027628163072/job-details https://www.upwork.com/jobs/~0175ba72296e865638

lschurr avatar Oct 18 '22 20:10 lschurr

@lschurr time to double? We don't have any proposals yet

rushatgabhane avatar Oct 25 '22 17:10 rushatgabhane

Yep! Doubled - https://www.upwork.com/jobs/~0175ba72296e865638

lschurr avatar Oct 25 '22 20:10 lschurr

Proposal

The root cause of this bug is due to the recent addition of react-freeze, specifically in SidebarLinks. This causes the LHN to freeze on small screens if the drawer is closed, which (when navigating directly to a chat) means App.setSidebarLoaded in the onLayout is never called and ONYXKEYS.IS_SIDEBAR_LOADED is never set to true.

This causes a problem when navigating directly to a chat on mobile because ReportScreen will not render unless ONYXKEYS.IS_SIDEBAR_LOADED is true, leaving the user with a blank screen.

One solution to this is to subscribe to ONYXKEYS.IS_SIDEBAR_LOADED in SidebarLinks and only enable Freeze after it has been set to true, however this causes an unnecessary re-render.

A better solution is to assign a variable in onLayout after App.setSideBarLoaded and only enable Freeze when it is true:

https://github.com/Expensify/App/blob/8a6bc17ecd01375527541829f5b6f79151c0da64/src/pages/home/sidebar/SidebarLinks.js#L140-L158

-               <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 Oct 29 '22 11:10 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 Oct 29 '22 11:10 melvin-bot[bot]

Hi @rushatgabhane - could you review the added proposal?

lschurr avatar Oct 31 '22 15:10 lschurr

Issue not reproducible during KI retests. (First week)

mvtglobally avatar Nov 03 '22 03:11 mvtglobally

Still reproducible for me:

https://user-images.githubusercontent.com/11609254/199677636-09a083fe-f091-4595-b9cd-180c403c5531.mp4

Ollyws avatar Nov 03 '22 08:11 Ollyws

I can reproduce this if I use mobile web and go to new.expensify.com/concierge. Should that link work to find your conversation with Concierge?

@flodnv are you able to reproduce?

@rushatgabhane can you review the proposal?

lschurr avatar Nov 03 '22 15:11 lschurr

which (when navigating directly to a chat) means App.setSidebarLoaded in the onLayout is never called and ONYXKEYS.IS_SIDEBAR_LOADED is never set to true

makes sense!

rushatgabhane avatar Nov 04 '22 14:11 rushatgabhane

@flodnv I believe we're fixing the root cause with @Ollyws' proposal.

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed

rushatgabhane avatar Nov 04 '22 14:11 rushatgabhane

@Ollyws your proposal looks good to me, but,

One solution to this is to subscribe to ONYXKEYS.IS_SIDEBAR_LOADED in SidebarLinks and only enable Freeze after it has been set to true, however this causes an unnecessary re-render.

For my own understanding can you clarify why "this causes an unnecessary re-render"?

flodnv avatar Nov 07 '22 11:11 flodnv

@flodnv Using withOnyx, it will update whatever prop we assign it causing a re-render when we set ONYXKEYS.IS_SIDEBAR_LOADED to true in App.setSidebarLoaded().

Ollyws avatar Nov 07 '22 11:11 Ollyws

Thanks, πŸ‘ to your proposal!

flodnv avatar Nov 07 '22 11:11 flodnv

Awesome. @Ollyws will you please apply for the job in Upwork? Additionally, when do you expect to have a PR ready for review?

lschurr avatar Nov 07 '22 20:11 lschurr

@lschurr Applied. I'll aim to have the PR submitted the same day I'm assigned to the job.

Ollyws avatar Nov 08 '22 09:11 Ollyws

Offer sent in Upwork.

lschurr avatar Nov 08 '22 17:11 lschurr

@Ollyws let's merge the PR as soon as we can!

rushatgabhane avatar Nov 08 '22 17:11 rushatgabhane

@rushatgabhane Agreed, want that bonus.

Ollyws avatar Nov 08 '22 17:11 Ollyws

@lschurr Offer accepted.

Ollyws avatar Nov 08 '22 18:11 Ollyws

Issue not reproducible during KI retests. (Second week)

mvtglobally avatar Nov 09 '22 03:11 mvtglobally

@lschurr @rushatgabhane Can I submit the PR now or do I need to wait until I'm assigned to this?

Ollyws avatar Nov 09 '22 10:11 Ollyws

πŸ“£ @Ollyws You have been assigned to this job by @flodnv! Please apply to this job in Upwork 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 πŸ“–

melvin-bot[bot] avatar Nov 09 '22 10:11 melvin-bot[bot]