[Hold for PR 24041] [$1000] Dev: Web - Composer disappears while switching chats in high traffic account

[kbecciv]

This is held on #24041

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. Switch between large chats in a high traffic account

Expected Result:

Composer remains visible

Actual Result:

Composer disappears, then later reappears

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: Dev 1.3.40-1
Reproducible in staging?: n/a
Reproducible in production?: n/a
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
Notes/Photos/Videos: Any additional supporting documentation

composer-disappearing.mov

Expensify/Expensify Issue URL:
Issue reported by: Amy Evans
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689360443816699

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d355d230175a068f
• Upwork Job ID: 1681049473425895424
• Last Price Increase: 2023-07-24

[melvin-bot]

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

[conorpendergrast]

@amyevans I wasn't able to see this on v1.3.41-2 (Staging), so outside of dev. Have you only found this on dev so far?

[amyevans]

I reproduced it on prod, albeit a lot less pronounced:

Screen.Recording.2023-07-17.at.10.31.24.AM.mov

Can you try joining the SF lounge room (SO) and see if you experience it after that?

[conorpendergrast]

Ok perfect, thanks Amy! I was able to reproduce this very mildly on [email protected] on Production using that Lounge room 👍

[conorpendergrast]

I'll default to external for this, but let me know if you think this should be internal

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01d355d230175a068f

[melvin-bot]

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

[melvin-bot]

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

[hoangzinh]

Is it possible to add my ht account to that lounge room? Thanks

[conorpendergrast]

@hoangzinh You should be able to join using this link: https://new.expensify.com/r/8292963527436014
It's a real, public channel with real customers though, so please don't post testing content in there!

[rojiphil]

Proposal

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

While switching chats in high-traffic accounts, the composer disappears for a moment and is displayed at a later time.

What is the root cause of that problem?

The root cause of the problem is the loading of draft comments using withOnyx as shown below which prevents the rendering of the composer for some time.

App/src/pages/home/report/ReportActionCompose.js

Lines 1300 to 1305 in cd851d0

	comment: {
	key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`,
	},
	numberOfLines: {
	key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT_NUMBER_OF_LINES}${reportID}`,
	},

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

Solution 1:
We can load the draft comments after the initial mounting of the composer. This will prevent the composer from disappearing for a short time.

1. Since withOnyx blocks rendering, we can use onyxSubscribe to load the draft comment details after the composer is mounted at this location. We can unsubscribe the Onyx fetch during unmount stage:

        // Initialize the contents of the composer from Onyx
        this.unsubscribeOnyxComment = onyxSubscribe({
            key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT + this.props.reportID,
            callback: (comment) => {
                this.initializeOnyxData(comment);
            },
        });
        this.unsubscribeOnyxNumOfLines = onyxSubscribe({
            key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT_NUMBER_OF_LINES + this.props.reportID,
            callback: (numberOfLines) => {
                this.setState({numberOfLines});
            },
        });

2. Once the draft comment details are fetched from Onyx, comment and numberOfLines can be managed as states. The existing state initialization from constructor here and update of comments here and here during component mount can now be performed after the onyxSubscribe returns with the draft comments as follows.

    initializeOnyxData(comment) {
        if(!comment)
            return;

        this.setState({
            comment,
            isCommentEmpty: comment.length === 0,
            selection: {
                start: isMobileSafari && !this.shouldAutoFocus ? 0 : comment.length,
                end: isMobileSafari && !this.shouldAutoFocus ? 0 : comment.length,
            },
            value: comment,
        });
        this.comment = comment;

        this.updateComment(comment);

        if (comment.length !== 0) {
            Report.setReportWithDraft(this.props.reportID, true);
        }
     }

3. Since we manage draft comment and numberOfLines as state variables, the references to props can be removed from here and, further, replaced with state variables during component updation here and here like this.

        const shouldSyncComment = prevStates.comment !== this.state.comment && this.state.value !== this.state.comment;
        this.updateComment(this.state.comment);

Solution 2:
The current behavior shows an empty space within the ReportFooter while the draft comments are being fetched by the composer. Solution 2 aims at avoiding the empty space occupied by footer by not showing the footer itself until the draft comments are loaded. This can be achieved by moving the blocking withOnyx from ReportActionCompose to ReportFooter. Specifically, this code here.

Please have a look at the test videos that are before and after the changes

Before Fix:

before_fix.mp4

Solution 1:

Solution_1_Fix.mp4

Solution 2:

after_fix.mp4

What alternative solutions did you explore? (Optional)

I also considered fetching the draft comments when the report screen is getting loaded. But, then, that will delay the overall loading of the report screen and it didn't seem to be a good user experience to me. Since the draft comment is specific to the composer, delaying the load of the footer seems to be a better solution than delaying the load of the report screen.

[rojiphil]

Proposal

Updated proposal. Added Solution 1 which is more promising to me. @rushatgabhane Kindly review.

[rushatgabhane]

@rojiphil sorry but code diffs aren't accepted as proposals. could you please explain your changes in simple words

[rojiphil]

@rushatgabhane Sorry for the extra details. I am still learning the tricks of writing proposals here. Anyways, I have updated the proposal with only relevant details. Hopefully, this time it is good enough. Kindly review.

[rojiphil]

Proposal

Updated

[rojiphil]

Proposal

Updated @rushatgabhane, have further updated the proposal with relevant links to code. Hopefully, this should further help in understanding the proposal. Kindly review. Thanks

[melvin-bot]

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

[aldo-expensify]

Same ^

[conorpendergrast]

Same same

[melvin-bot]

@conorpendergrast, @rushatgabhane, @aldo-expensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[conorpendergrast]

Still the same, waiting for #24041 to deploy

[melvin-bot]

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

[conorpendergrast]

@sophiepintoraetz I'm on parental leave; re-assigning! This has been triaged and we're waiting for #24041 to deploy before we re-test and confirm it's fixed

[sophiepintoraetz]

@rushatgabhane @aldo-expensify - are we still holding here?

[aldo-expensify]

From looking at #24041, it was merged, but I don't see our typical deploy comments by Melvin-bot so I'm a bit confused.

I think we should just:

1. Remove the HOLD
2. Confirm this problem still exists

[aldo-expensify]

I think this is not happening anymore:

Screen.Recording.2023-09-14.at.11.28.27.AM.mov

The composer remains there even if the chat is still loading

@amyevans since you are the reporter, does the video above look like it confirms that this is not longer a problem? I used network tool to make the connection slow

[amyevans]

It looks good to me! But I'd also like @shawnborton to confirm since he was the original reporter 😄

[shawnborton]

Cool, we can close and reopen if we see it start happening again.