[HOLD for payment 2024-04-15] [Pay meow][$500] Typed messages disappear for a moment when typing in the compose box

[m-natarajan]

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

---

Version Number: 1.4.56-2
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: @mallenexpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1711132608014349

Notes

There are two versions of this bug that have the same outcome, text disappears. As viewed in the video below, this can occur immediately after switching to a new chat. This has also starting happening more this week with random, one-off characters disappearing or showing out of order.

Action Performed:

1. Navigate to a chat
2. Immediately start typing text

Expected Result:

The text to show in the box

Actual Result:

Text gets lost after typing a handful of characters.

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

Recording.2898.mp4

2024-03-22_11-32-40.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0167dbea954a52c8eb
• Upwork Job ID: 1775277314298826752
• Last Price Increase: 2024-04-02

[melvin-bot]

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

[bernhardoj]

Proposal

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

The text disappears when we type into the composer immediately after navigating to it.

What is the root cause of that problem?

In SilentCommentUpdater, we have a logic to update the composer text with the draft.

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.tsx

Lines 19 to 32 in d8361ee

	useEffect(() => {
	/**
	* Schedules the callback to run when the main thread is idle.
	*/
	if ('requestIdleCallback' in window) {
	const callbackID = requestIdleCallback(() => {
	updateComment(comment ?? '');
	});
	return () => cancelIdleCallback(callbackID);
	}
	updateComment(comment ?? '');
	// eslint-disable-next-line react-hooks/exhaustive-deps -- We need to run this on mount
	}, []);

After the chat switch improvement PR, we delay the update comment call. So, when you open a chat then type immediately, it will be replaced with the above effect logic.

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

Remove the requestIdleCallback delay.

What alternative solutions did you explore? (Optional)

The purpose of that effect logic is to replace any emoji text in Spanish when we switch our language to Spanish (for example, if we type :rosa: then switch to Spanish, we want to replace it with 🌹, just like Slack did). It was introduced in this PR and specifically added for the small screens because, for large screens, the useEffect below already handles it by comparing the preferred locale while the report screen is still mounted.

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.tsx

Lines 34 to 46 in d8361ee

	useEffect(() => {
	// Value state does not have the same value as comment props when the comment gets changed from another tab.
	// In this case, we should synchronize the value between tabs.
	const shouldSyncComment = prevCommentProp !== comment && value !== comment;
	// As the report IDs change, make sure to update the composer comment as we need to make sure
	// we do not show incorrect data in there (ie. draft of message from other report).
	if (preferredLocale === prevPreferredLocale && reportID === prevReportId && !shouldSyncComment) {
	return;
	}
	updateComment(comment ?? '');
	}, [prevCommentProp, prevPreferredLocale, prevReportId, comment, preferredLocale, reportID, updateComment, value, commentRef]);

If we don't want that feature anymore, we can remove it, but I'm sure we want to keep it, so the other alternative is to replace the emoji text in the useState initialization. If we do it in the useState initialization, we can reduce lots of unnecessary code.

So, first, we remove this effect code:

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.tsx

Lines 19 to 32 in d8361ee

	useEffect(() => {
	/**
	* Schedules the callback to run when the main thread is idle.
	*/
	if ('requestIdleCallback' in window) {
	const callbackID = requestIdleCallback(() => {
	updateComment(comment ?? '');
	});
	return () => cancelIdleCallback(callbackID);
	}
	updateComment(comment ?? '');
	// eslint-disable-next-line react-hooks/exhaustive-deps -- We need to run this on mount
	}, []);

Then do the emoji replacement in here:

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.tsx

Lines 265 to 270 in d8361ee

	const [value, setValue] = useState(() => {
	if (draftComment) {
	emojisPresentBefore.current = EmojiUtils.extractEmojis(draftComment);
	}
	return draftComment;
	});

const [value, setValue] = useState(() => {
    if (draftComment) {
        emojisPresentBefore.current = EmojiUtils.extractEmojis(draftComment);
        // Replace any emoji text with emoji code to replace any unprocessed emoji text because of the locale difference.
        const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis(draftComment, preferredSkinTone, preferredLocale);
        if (emojis.length) {
            // If there is a text replaced to emoji, add it to the frequently used emojis, save the new draft, and returns it.
            const newEmojis = EmojiUtils.getAddedEmojis(emojis, emojisPresentBefore.current);
            if (newEmojis.length) {
                insertedEmojisRef.current = newEmojis;
                debouncedUpdateFrequentlyUsedEmojis();
            }
            const newCommentConverted = convertToLTRForComposer(newComment);

            emojisPresentBefore.current = emojis;
    
            Report.saveReportComment(reportID, newCommentConverted || '');
    
            return newCommentConverted;
        }
    }

    return draftComment;
});

We eliminate lots of code from calling updateComment and one of the most important things is, that we don't broadcast the typing event when mounting.

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.tsx

Lines 448 to 450 in d8361ee

	if (newCommentConverted) {
	debouncedBroadcastUserIsTyping(reportID);
	}

[mallenexpensify]

Checking on in the issue @bernhardoj linked above

• [HOLD for payment 2024-03-29] [Performance] Chat Switch Improvement Part 1 #32793 (comment)

[jacobkim9881]

Proposal

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

On Desktop app, typing as soon as opening a report makes comments on compose box flick.

What is the root cause of that problem?

<SilentCommentUpdater> updates the comment of a report under certain conditions. One of conditions is when you types on different tabs of a browser. The comment is synchronized by the component. It works well on browsers but it doesn't on electron app.

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.tsx

Lines 8 to 12 in 0509f8d

	/**
	* This component doesn't render anything. It runs a side effect to update the comment of a report under certain conditions.
	* It is connected to the actual draft comment in onyx. The comment in onyx might updates multiple times, and we want to avoid
	* re-rendering a UI component for that. That's why the side effect was moved down to a separate component.
	*/

Let me explain detail what's happening. On every typing motion(key down, key up), acts and it acts many times. Every comment is shown but empty value too. So to keep this from acting useless, there are conditions.

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.tsx

Lines 34 to 46 in 0509f8d

	useEffect(() => {
	// Value state does not have the same value as comment props when the comment gets changed from another tab.
	// In this case, we should synchronize the value between tabs.
	const shouldSyncComment = prevCommentProp !== comment && value !== comment;
	// As the report IDs change, make sure to update the composer comment as we need to make sure
	// we do not show incorrect data in there (ie. draft of message from other report).
	if (preferredLocale === prevPreferredLocale && reportID === prevReportId && !shouldSyncComment) {
	return;
	}
	updateComment(comment ?? '');
	}, [prevCommentProp, prevPreferredLocale, prevReportId, comment, preferredLocale, reportID, updateComment, value, commentRef]);

When <SilentCommentUpdater> acts for each time, some conditions are checked on the same time, acting to keep it from acting useless. As it is &&, one of conditions are missed then other conditions are chained as all acts once.

2024-03-27.11.31.11.mov

Let me explain how <SilentCommentUpdater> acts on certain conditions. If all two conditions are true then the comment on the compose box isn't changed.

const shouldSyncComment = prevCommentProp !== comment && value !== comment;

First, any value is typed. And then shown comment is saved in Onyx. So when switching reports, comment can be loaded on to the compose box. At last saved comment is saved into prevCommentProp by usePrevious().

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.tsx

Line 14 in 0509f8d

const prevCommentProp = usePrevious(comment);

If typed value, saved comment and previous comment aren't same then the comment on the compose box is changed.

prevCommentProp !== comment && value !== comment

If any of three is different the compose box is changed too. It means typing isn't finished.

prevCommentProp !== comment or value !== comment

When typing is finished then all three is same.

prevCommentProp == value == comment

Let me explain how empty value makes flickering. Every comment, even empty value go through too. As explained above, if any of three (typed value, saved comment and previous comment) is different the compose box is changed. Typed value is empty and then empty value is tossed to be saved into Onyx and at last previous comment is saved as empty value.

value : empty -> comment : empty -> prevCommentProp: empty

So empty value is saved on compose box too. All three value are same so to stop synchronizing. But after empty value is finished, the same typed previous value is saved into Onyx again. So it is synchronized on the compose box too. If empty value is stoped to be checked then flickering is stoped too.

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

If it is electron app, then we should check the value is empty or not. If the value is empty then we let it stop from synchronizing. We import getPlatform from '@libs/getPlatform'; and add if(!value) return; to keep updateComment(comment ?? ''); in .

import getPlatform from '@libs/getPlatform';

...

useEffect(() => { 
     // Value state does not have the same value as comment props when the comment gets changed from another tab. 
     // In this case, we should synchronize the value between tabs. 
     const shouldSyncComment = prevCommentProp !== comment && value !== comment; 
  
     if(!value && getPlatform() === 'desktop') return;

     // As the report IDs change, make sure to update the composer comment as we need to make sure 
     // we do not show incorrect data in there (ie. draft of message from other report). 
     if (preferredLocale === prevPreferredLocale && reportID === prevReportId && !shouldSyncComment) { 
         return; 
     } 
  
     updateComment(comment ?? ''); 

What alternative solutions did you explore? (Optional)

N/A

[mallenexpensify]

Waiting for an update here

• [HOLD for payment 2024-03-29] [Performance] Chat Switch Improvement Part 1 #32793 (comment)

[mountiny]

Asked in Slack the PR authors too https://expensify.slack.com/archives/C03UK30EA1Z/p1711752149732359

[mallenexpensify]

Reported again this morn https://expensify.slack.com/archives/C049HHMV9SM/p1711975160869279

[hurali97]

Hey guys, I can't seem to repro this issue. See the attached video, is there something I am missing in the repro steps? As mentioned in PR description, I tested it on Desktop.

issue-repro.mp4

Apart from this, @bernhardoj's proposal here looks good in terms of removing the delay to call updateComment but I was wondering, whether we should add an if-statement to check if comment is not empty and only then update it? Even if we want to remove the delay, can we consider adding this check before calling updateComment? So that we are not updating comment when there's nothing to update.

[bernhardoj]

can we consider adding this check before calling updateComment

We can definitely add the check. There won't be any issue with adding the check.

I think you need to type it faster. I also sometimes can't reproduce the issue when I was milliseconds late.

Screen.Recording.2024-04-02.at.16.34.30.mov

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0167dbea954a52c8eb

[melvin-bot]

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

[mountiny]

We have used @bernhardoj solution so we should pay them $125 for help

[mallenexpensify]

If we used their solution/code, we should compensate $250. 50% is our standard amount. I updated the internal SO recently.

Partial payments and edge cases
If the Help Wanted and External labels are applied to an issue and a contributor posts a solution before anyone else, where the final PR includes a significant amount of the code to fix the bug, the contributor might be due compensation. Discuss in the GH issue, #bug-zero or #contributor-plus if needed. The standard amount is 50% when the contributor didn't author the PR nor test it.

I'll take over as BZ to see this one through since I've been reporting and monitoring these issues.

[mountiny]

Sounds good to me

[mallenexpensify]

@bernhardoj can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~0167dbea954a52c8eb

[bernhardoj]

@mallenexpensify accepted

[mallenexpensify]

Contributor: @bernhardoj paid $250 via Upwork

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.60-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] Remove the requestIdleCallback from silentComposer #39535

If no regressions arise, payment will be issued on 2024-04-15. 🎊

For reference, here are some details about the assignees on this issue:

• @alitoshmatov requires payment (Needs manual offer from BZ)

[melvin-bot]

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:

• [@alitoshmatov] The PR that introduced the bug has been identified. Link to the PR:
• [@alitoshmatov] 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:
• [@alitoshmatov] 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:
• [@alitoshmatov] Determine if we should create a regression test for this bug.
• [@alitoshmatov] 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.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: