Chat - Focus lost after choosing emoji from EmojPicker

[lanitochka17]

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.22-0
Reproducible in staging?: Y
Reproducible in production?: N
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:

1. Go to URL https://staging.new.expensify.com/
2. Login with any account
3. Open any chat
4. Open the emoji picker
5. Choose any emoji

Expected Result:

Focus presents in the composer

Actual Result:

Focus lost in composer

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

Bug6332575_1704403879275.bandicam_2024-01-04_23-09-06-274.1.mp4

View all open jobs on GitHub

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

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

[lanitochka17]

Production:

Recording.873.mp4

[DylanDylann]

Proposal

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

• Chat - Focus lost after choosing emoji from EmojPicker

What is the root cause of that problem?

• It is a regression from
• We have the logic that do not focus on the input composer if we the isPopoverOpen is true:
  

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

  Lines 414 to 422 in 095161a

  	const focus = useCallback(
  	(shouldDelay = false) => {
  	if (isPopoverOpen) {
  	return;
  	}
  	focusComposerWithDelay(textInputRef.current)(shouldDelay);
  	},
  	[isPopoverOpen],
  	);

• When we select a emoji, it will call the above focus function but at that time, isPopoverOpen is still true, that leads to the bug.

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

• First, we need to remove the logic that leads to the bug byt updating the above to:

const focus = useCallback( 
    (shouldDelay = false) => { 
        focusComposerWithDelay(textInputRef.current)(shouldDelay); 
    }, 
    [], 
); 

• Then, create the global variable, named willContextMenuOpen in here to know if the context menu is opening or not.
• Then, once we open the context menu, set the ComposerFocusManager.willContextMenuOpen = true. Do it by updating:
  

  App/src/components/LHNOptionsList/OptionRowLHN.js

  Lines 190 to 191 in 2d455bf

  	onSecondaryInteraction={(e) => {
  	showPopover(e);

  
  to:

                        onSecondaryInteraction={(e) => {
                            showPopover(e);
+                           ComposerFocusManager.willContextMenuOpen = true

• Then, prevent the app from focusing on the composer if the willContextMenuOpen is true. Do it by updating:
  

  App/src/libs/focusComposerWithDelay.ts

  Lines 17 to 19 in 2d455bf

  	if (!textInput || EmojiPickerAction.isEmojiPickerVisible()) {
  	return;
  	}

  
  to:

+      if (!textInput || EmojiPickerAction.isEmojiPickerVisible() || ComposerFocusManager.isContextMenuAction) {
+          ComposerFocusManager.isContextMenuAction = false
            return;
        }

What alternative solutions did you explore? (Optional)

• NA

[Pujan92]

Proposal

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

Composer isn't focus after selecting an emoji

What is the root cause of that problem?

On selection of emoji we are hiding the emoji picker but do not execute the close method of the PopoverContext which eventually doesn't update the isOpen value, due to that isPopoverOpen remains true here and it won't focus the composer.

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

Lines 414 to 422 in 50f2010

	const focus = useCallback(
	(shouldDelay = false) => {
	if (isPopoverOpen) {
	return;
	}
	focusComposerWithDelay(textInputRef.current)(shouldDelay);
	},
	[isPopoverOpen],
	);

App/src/components/EmojiPicker/EmojiPicker.js

Lines 109 to 120 in 50f2010

	const selectEmoji = (emoji, emojiObject) => {
	// Prevent fast click / multiple emoji selection;
	// The first click will hide the emoji picker by calling the hideEmojiPicker() function
	if (!isEmojiPickerVisible) {
	return;
	}
	hideEmojiPicker(false);
	if (_.isFunction(onEmojiSelected.current)) {
	onEmojiSelected.current(emoji, emojiObject);
	}
	};

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

Use Popover context in the EmojiPicker for close method and call it inside selectEmoji function.

App/src/components/EmojiPicker/EmojiPicker.js

Line 116 in 50f2010

hideEmojiPicker(false);

[situchan]

@Pujan92 what's offending PR? #33483 was deployed to production

[Pujan92]

@Pujan92 what's offending PR? #33483 was deployed to production

Offending PR is #33483 only bt currently emoji picker by mistake in other PR is opening as Modal(instead of PopoverWithoutOverlay) so we are not able to reproduce at the moment(bcoz Modal onClose is calling close method).
I think it will be reproducible in production once this PR hits there.

[mvtglobally]

still repro on build 1.4.22-2

Recording.894.mp4

[thienlnam]

Situ is handling as C+

[situchan]

I agree with #32970 (comment).
As regression test is already done, revert is safer approach and original issue is not that critical.
I am concerned applying patch will cause another unexpected regression.
@thienlnam what do you think?

[thienlnam]

Yeah I agree, this is a worse bug than the one the PR intended to fix. I'll just get a revert up for that

[Pujan92]

@situchan Don't you think calling close method is safer approach on emoji selection as mentioned in proposal?

[melvin-bot]

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

[thienlnam]

This blocker was fixed in the revert - adding the bug label so we can get payment issued

[sophiepintoraetz]

@thienlnam - can you confirm it's just $500 to @situchan?

[thienlnam]

Yes that is correct!

[DylanDylann]

@thienlnam I proposed the proposal that point out the RCA and solution to revert the PR here, can you help check

[thienlnam]

It doesn't look like your comment mentioned the PR that caused the regression - also since we're just reverting code here we're going to have the problem resolved in that PR

[DylanDylann]

@thienlnam My proposal have 2 sections:

First, we need to remove the logic that leads to the bug byt updating the above to ...

This is revert the regression PR

And the rest is the solution that will fix the issue that the regression PR try to fix

[thienlnam]

Yeah but I don't see the regression PR linked in your comment anywhere.

Additionally, in these scenarios where we revert a recent PR we don't issue payment for just pointing out where it was caused. If we had implemented your other solution that would be a different situation, but it sounds like we decided to just go with a revert here due to other reasons as well listed here

[DylanDylann]

@thienlnam

revert here due to other reasons as well listed #32970 (comment)

In the comment you mentioned, the author mentioned my proposal "More info is explained on the #33984 (comment) there."

Can you help check

[mkhutornyi]

@DylanDylann as you're not assigned, I don't think you're eligible even if it's not straight revert.
And this GH was not made external so there's no guarantee of payment even if your solution is used.
Similar case: #33228 (comment) (I even helped review PR)

[sophiepintoraetz]

Hey @DylanDylann - we have plenty of opportunities to jump in if you're looking for monetary compensation but as Jack has said, this is mainly a revert - we have not implemented code. If we implement something, I'd keep an eye out on #32970 (comment).

@situchan - offer sent!

[DylanDylann]

Thanks @sophiepintoraetz , @thienlnam.