[HOLD for payment 2024-12-03] [$250] Composer - Emoji picker does not open when opening context menu and clicking emoji picker

[IuliiaHerets]

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: 9.0.51-1 Reproducible in staging?: Y Reproducible in production?: N Issue was found when executing this PR: https://github.com/Expensify/App/pull/50974 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Open report.
3. Send a message.
4. Right click on the message.
5. Click on the emoji picker in the composer.
6. Right click on the message.
7. Click on the emoji picker in the composer.

Expected Result:

Emoji picker will open.

Actual Result:

Emoji picker does not open.

Workaround:

Unknown

Platforms:

• [ ] Android: Standalone
• [ ] Android: HybridApp
• [ ] Android: mWeb Chrome
• [ ] iOS: Standalone
• [ ] iOS: HybridApp
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/5310a60b-024f-4d96-8979-ff4b57c9eea6

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021848230166570821208
• Upwork Job ID: 1848230166570821208
• Last Price Increase: 2024-10-28
• Automatic offers:
• shubham1206agra | Reviewer | 104727454

Issue Owner

Current Issue Owner: @MitchExpensify

[melvin-bot[bot]]

Triggered auto assignment to @MonilBhavsar (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot[bot]]

💬 A slack conversation has been started in #expensify-open-source

[melvin-bot[bot]]

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[github-actions[bot]]

:wave: 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.

[bernhardoj]

Proposal

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

Emoji picker doesn't show when we open emoji picker first, and then context menu, and then emoji picker button again.

What is the root cause of that problem?

When we open the emoji picker and then context menu, you can notice that the emoji picker button tooltip won't show. It's because isPopoverRelatedToTooltipOpen is not updated properly. https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/Tooltip/PopoverAnchorTooltip.tsx#L11-L32

isPopoverRelatedToTooltipOpen tells whether the tooltip element contains/is the same as the visible/active popover anchor. It's to prevent the tooltip of a button that shows the popover from showing while the popover is active. For example, if we open the emoji picker, the emoji picker button tooltip won't show.

So, currently, isPopoverRelatedToTooltipOpen for the emoji picker button is true. When we press the emoji picker button again, isPopoverRelatedToTooltipOpen will update from true to false (because the context menu is closed) which will re-render the emoji picker button, so the press/click event is never received. https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/Tooltip/PopoverAnchorTooltip.tsx#L27-L38

isPopoverRelatedToTooltipOpen doesn't update properly because it depends on a (active popover) ref (other than the isOpen state). https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/Tooltip/PopoverAnchorTooltip.tsx#L25

When we open an emoji picker and then the context menu, the isOpen state of the popover stays true, so the memo is not recalculated.

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

We need to create a new state to store the active popover to re-trigger the isPopoverRelatedToTooltipOpen memo. Because we only want to get the anchor ref, we can store the anchor ref only to the state instead of the whole popover element. https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/Tooltip/PopoverAnchorTooltip.tsx#L19

First, in PopoverProvider, create the new state. https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/PopoverProvider/index.tsx#L21-L24

const [activePopoverAnchor, setActivePopoverAnchor] = useState(null);

Then, set the popover anchor state when a popover is shown. https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/PopoverProvider/index.tsx#L105-L111

setActivePopoverAnchor(popoverParams.anchorRef.current);

Clear it here too. https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/PopoverProvider/index.tsx#L25-L32

Then, pass the state to the context value, https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/PopoverProvider/index.tsx#L116-L124

const contextValue = useMemo(
    () => ({
        onOpen,
        close: closePopover,
        popover: activePopoverRef.current,
        popoverAnchor: activePopoverAnchor,
        isOpen,
    }),
    [onOpen, closePopover, isOpen, activePopoverAnchor],
);

Last, replace all popover.anchorRef.current to popoverAnchor. https://github.com/Expensify/App/blob/66cf824036a51183d73dab2cc621901e43fe3196/src/components/Tooltip/PopoverAnchorTooltip.tsx#L8-L25

const {isOpen, popoverAnchor} = useContext(PopoverContext);

const isPopoverRelatedToTooltipOpen = useMemo(() => {
    // eslint-disable-next-line @typescript-eslint/dot-notation
    const tooltipNode = (tooltipRef.current?.['_childNode'] as Node | undefined) ?? null;

    if (
        isOpen &&
        popoverAnchor &&
        tooltipNode &&
        ((popoverAnchor instanceof Node && tooltipNode.contains(popoverAnchor)) || tooltipNode === popoverAnchor)
    ) {
        return true;
    }

    return false;
}, [isOpen, popoverAnchor]);

[bernhardoj]

This doesn't happen in prod because, in prod, you can't open the chat context menu while an emoji picker is visible. But this issue happens in prod if you open the LHN context menu. So, I posted my proposal above because it's an existing issue.

https://github.com/user-attachments/assets/fe44ecac-9691-44b4-9c9b-c928de040d0f

[MonilBhavsar]

@bernhardoj looks like it is coming from regression from this PR https://github.com/Expensify/App/pull/50974?

[bernhardoj]

It happens after https://github.com/Expensify/App/pull/50974, yes, but it's an existing issue which I explained in my comment above.

[MonilBhavsar]

Thanks for the context! So this is not really a blocker

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@MonilBhavsar, @MitchExpensify, @shubham1206agra Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MitchExpensify]

@shubham1206agra, how does @bernhardoj 's proposal look to you?

[shubham1206agra]

So, currently, isPopoverRelatedToTooltipOpen for the emoji picker button is true.

@bernhardoj Can you tell me why is it true?

[bernhardoj]

isPopoverRelatedToTooltipOpen tells whether the tooltip element contains/is the same as the visible/active popover anchor. It's to prevent the tooltip of a button that shows the popover from showing while the popover is active. For example, if we open the emoji picker, the emoji picker button tooltip won't show.

Because the emoji picker is visible.

[shubham1206agra]

@bernhardoj I am not sure storing refs in state is a good idea.

[bernhardoj]

We are not really storing the ref, but the ref value, that is the HTML element. isPopoverRelatedToTooltipOpen memo affects the rendering, so we need to re-calculate it when the anchor element is updated.

[shubham1206agra]

@bernhardoj Can you show me an example where we used state to store HTMLElement?

[bernhardoj]

Here is a few examples

https://github.com/Expensify/App/blob/186c1a976acbc65faf8b739e34bb0c7936fbd1f2/src/pages/NewChatSelectorPage.tsx#L19-L21 https://github.com/Expensify/App/blob/186c1a976acbc65faf8b739e34bb0c7936fbd1f2/src/pages/iou/request/IOURequestStartPage.tsx#L95-L97

[melvin-bot[bot]]

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

[MitchExpensify]

Bump on checking out @bernhardoj 's response here, @shubham1206agra

[shubham1206agra]

@bernhardoj Can you give me a test branch before I approve this?

[bernhardoj]

Here is the test branch: https://github.com/bernhardoj/Expensify/tree/fix/51114-emoji-picker-won't-open

[shubham1206agra]

Lets assign @bernhardoj here.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

Current assignee @MonilBhavsar is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[MitchExpensify]

Lets assign @bernhardoj here.

Sound good to you @MonilBhavsar ?

[melvin-bot[bot]]

@MonilBhavsar @MitchExpensify @shubham1206agra this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MonilBhavsar]

Looks good to me. Thanks for detailed proposal!

[melvin-bot[bot]]

📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[bernhardoj]

PR is ready

cc: @shubham1206agra