[HOLD for payment 2024-01-26] [$500] Emoji - When resizing the window, the open emoji picker moves to the LHN side

[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.3.97-0
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: Applause - Internal Team
Slack conversation:

Action Performed:

1. Open https://staging.new.expensify.com/
2. Log in with any account
3. Open any report
4. In Compose Box, open the emoji picker
5. Reduce the width of the open browser window

Expected Result:

Emoji picker should be fixed above the Compose Box when resizing the window and should remain open when going into mobile mode

Actual Result:

When resizing the window, the open emoji picker moves to the LHN side

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

Bug6270312_1699554384569.Recording__692.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b9e60337c2ef461a
• Upwork Job ID: 1722692276888514560
• Last Price Increase: 2023-11-30

[melvin-bot]

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

[melvin-bot]

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

[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

[melvin-bot]

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

[jeet-dhandha]

Proposal

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

• Emoji picker sticking to left side of screen when we resize the window horizontally.

What is the root cause of that problem?

• In below code snippet the value comes as {horizontal: 0, vertical: 0} when we resize the window horizontally.

App/src/components/EmojiPicker/EmojiPicker.js

Line 132 in e2c1c03

calculateAnchorPosition(emojiPopoverAnchor.current, emojiPopoverAnchorOrigin).then((value) => {

• This happens because when we copy reference of anchor element and then measure it's position using measureInWindow the response of callback comes as (x, y, width, height) => (0, 0, 0, 0) when we resize the window horizontally.

• And if we check the original anchor element's position using measure it's position is correct.

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

• First we create a useDimensionChange hook which will take a callback function and will call it whenever the dimension of the element changes.
• then we move the calculate logic to a function onAnchorDimensionChange which takes the original ref and pass it to useImperativeHandle.

App/src/components/EmojiPicker/EmojiPicker.js

Lines 124 to 135 in e2c1c03

	const emojiPopoverDimensionListener = Dimensions.addEventListener('change', () => {
	if (!emojiPopoverAnchor.current) {
	// In small screen width, the window size change might be due to keyboard open/hide, we should avoid hide EmojiPicker in those cases
	if (isEmojiPickerVisible && !isSmallScreenWidth) {
	hideEmojiPicker();
	}
	return;
	}
	calculateAnchorPosition(emojiPopoverAnchor.current, emojiPopoverAnchorOrigin).then((value) => {
	setEmojiPopoverAnchorPosition(value);
	});
	});

• then we create a onAnchorDimensionChange function in EmojiPickerAction.ts and call it in useDimensionChange at each place where we use the showEmojiPicker function.

Below example is for EmojiPickerButton.js but we have to do the same for other places where we use showEmojiPicker function.

// `emojiPopoverAnchor.current` is the original ref of the anchor element from `EmojiPickerButton.js`
useDimensionChange(() => EmojiPickerAction.onAnchorDimensionChange(emojiPopoverAnchor.current));

Video for EmojiPickerButton.js

video

c8046c9e-f613-4550-b13f-b300ef68a079.mp4

What alternative solutions did you explore? (Optional)

• N/A

[tienifr]

Proposal

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

When resizing the window, the open emoji picker moves to the LHN side

What is the root cause of that problem?

When we resize the window, the ref of the emoji button here will change, however, since we only passed the value (.current) of that ref here as the anchorRef, that anchorRef does not change and points to a non-existent component, leading to the dimensions become all 0 and the emoji picker moves to LHN side.

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

The way we're passing refs as params to other components are not ideal, that way the original ref and the ref that we pass can get out of sync, causing issues like this one.

So the pattern that we use is usually:

• Create and update the ref inside the anchor component (in this case, inside EmojiPickerButton)
• Pass that ref to another component that displays the actual popover (in this case, the EmojiPicker)

1. We need to make sure they are always in sync, or ideally, always 1 single ref. We should create a new hook useSyncedRef that will return 1 ref for both components.

import React from 'react';

const refs = {}

const useSyncedRef = (refKey, initialValue = undefined) => {
  if (!refs[refKey]) {
    refs[refKey] = React.createRef(initialValue);
  }

  const refCallback = (value) => {
    refs[refKey].current = value;
  }

  return { ref: refs[refKey], refCallback };
}

export default useSyncedRef;

2. In EmojiPickerButton here

const {refCallback} = useSyncedRef('EmojiPickerAnchor');

And replace this
with

ref={refCallback}

3. In EmojiPicker here, replace with

const { ref: emojiPopoverAnchor } = useSyncedRef('EmojiPickerAnchor');

And we don't need to pass the ref from the EmojiPickerButton to EmojiPicker via showEmojiPicker any more.

What alternative solutions did you explore? (Optional)

We can just add another method updateAnchorRef inside EmojiPicker and call it whenever the ref is updated in EmojiPickerButton

Any place that has this issue can be fixed the same way by the main or alternative approach, but the main approach is global.

[bernhardoj]

Proposal

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

Resizing the browser window moves the emoji picker to the left side of the screen.

What is the root cause of that problem?

In EmojiPicker, we have a dimension change listener to calculate a new position.

App/src/components/EmojiPicker/EmojiPicker.js

Lines 124 to 135 in d4a97bc

	const emojiPopoverDimensionListener = Dimensions.addEventListener('change', () => {
	if (!emojiPopoverAnchor.current) {
	// In small screen width, the window size change might be due to keyboard open/hide, we should avoid hide EmojiPicker in those cases
	if (isEmojiPickerVisible && !isSmallScreenWidth) {
	hideEmojiPicker();
	}
	return;
	}
	calculateAnchorPosition(emojiPopoverAnchor.current, emojiPopoverAnchorOrigin).then((value) => {
	setEmojiPopoverAnchorPosition(value);
	});
	});

The problem is, that the x, y, w, and h returned from calculateAnchorPosition is 0.

This is because the element we want to measure (emojiPopoverAnchor.current) is not available on the dom anymore. emojiPopoverAnchor in this case is the emoji picker button.

The element becomes not available as soon as the emoji picker menu shows. Why?

The emoji picker button is wrapped with a PopoverAnchorTooltip.

App/src/components/EmojiPicker/EmojiPickerButton.js

Lines 38 to 40 in d4a97bc

	<Tooltip text={props.translate('reportActionCompose.emoji')}>
	<PressableWithoutFeedback
	ref={emojiPopoverAnchor}

PopoverAnchorTooltip will conditionally render the element with/without a tooltip based on the popover visibility.

App/src/components/Tooltip/PopoverAnchorTooltip.js

Lines 40 to 52 in d4a97bc

	if (!shouldRender || isPopoverRelatedToTooltipOpen) {
	return children;
	}
	return (
	<BaseTooltip
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...props}
	tooltipRef={tooltipRef}
	>
	{children}
	</BaseTooltip>
	);

When we first open the web, the emoji picker button is rendered with a tooltip, but when the emoji picker popover shows, we render the emoji picker button without a tooltip. This becomes a problem because if we render it with a tooltip, we clone the children (in this case the emoji picker button). So, both elements are different instances and the element that we pass to the EmojiPicker is the element of the with a tooltip.

App/src/components/EmojiPicker/EmojiPickerButton.js

Line 45 in 358e4b1

EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.emojiPickerID);

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

Instead of passing the element as the emojiPopoverAnchorValue to EmojiPicker, we should pass the ref object so we can always get the latest value from the ref object.

-EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.emojiPickerID);
+EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor, undefined, () => {}, props.emojiPickerID);

We should make the above changes to every usage of EmojiPickerAction.showEmojiPicker

Then, we need to modify EmojiPicker so it works with the ref, that is by accessing the element by using double .current.

App/src/components/EmojiPicker/EmojiPicker.js

Lines 51 to 58 in 358e4b1

	if (emojiPopoverAnchor.current && emojiPopoverAnchor.current.blur) {
	// Drop focus to avoid blue focus ring.
	emojiPopoverAnchor.current.blur();
	}
	const anchorOriginValue = anchorOrigin || DEFAULT_ANCHOR_ORIGIN;
	calculateAnchorPosition(emojiPopoverAnchor.current, anchorOriginValue).then((value) => {

App/src/components/EmojiPicker/EmojiPicker.js

Lines 125 to 132 in 358e4b1

	if (!emojiPopoverAnchor.current) {
	// In small screen width, the window size change might be due to keyboard open/hide, we should avoid hide EmojiPicker in those cases
	if (isEmojiPickerVisible && !isSmallScreenWidth) {
	hideEmojiPicker();
	}
	return;
	}
	calculateAnchorPosition(emojiPopoverAnchor.current, emojiPopoverAnchorOrigin).then((value) => {

emojiPopoverAnchor.current.current

the first .current is to access the emojiPopoverAnchor ref object of EmojiPickerButton, and the second .current is to access the emojiPopoverAnchor element of EmojiPickerButton (the Pressable)

Last, we need to modify below code so it pass the correct ref to the popover

App/src/components/EmojiPicker/EmojiPicker.js

Line 157 in 358e4b1

anchorRef={emojiPopoverAnchor}

anchorRef={emojiPopoverAnchor.current ?? {}}

[melvin-bot]

@lschurr, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[aimane-chnaif]

reviewing proposals

[dragnoir]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/rachidlajmi

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[dragnoir]

Proposal

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

When resizing the window on the web browser, the open emoji picker moves to the LHN side.

What is the root cause of that problem?

The issue is exclusively observed in web browsers when users interact using a mouse. I conducted tests on a touchscreen device and encountered no problems. Further testing in Google Chrome's DevTools using the 'toggle device' option and responsive layout modes also did not reproduce the issue.

The specific problem arises when a user clicks the emoji button. This action triggers the addition of a background style to the anchor element in the DOM. Our calculateAnchorPosition function retrieves the emojiPopoverAnchor.current element, which includes this background style. However, by the time measureInWindow initiates its search for this element, the DOM has already updated to a version where the anchor no longer has the background style. This discrepancy is likely causing the issue we're observing.

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

To address the issue, we can implement a detection mechanism to ascertain if the user is interacting via a mouse. In scenarios where the window is being resized, we can strategically hide the Emoji Picker. This approach maintains the existing user experience on touchscreen devices, while effectively resolving the issue on desktops and laptops. By hiding the Emoji Picker during window resizing and only displaying it again upon user click, we ensure a seamless experience across different devices.

// state for user using a mouse
const [isUsingMouse, setIsUsingMouse] =  useState(false);

useEffect(() => {
	const  handleMouseMove  = () => {
		setIsUsingMouse(true);
		// Remove the event listener after detecting mouse movement
		window.removeEventListener('mousemove', handleMouseMove);
	};

	if (!isUsingMouse) {
		window.addEventListener('mousemove', handleMouseMove);
	}

	const  emojiPopoverDimensionListener  =  Dimensions.addEventListener('change', () => {
		if (isUsingMouse) {
			hideEmojiPicker();
		}
		// rest of the code ..
		return () => {
			emojiPopoverDimensionListener.remove();
			window.removeEventListener('mousemove', handleMouseMove);
	};
}, [isEmojiPickerVisible, isSmallScreenWidth, emojiPopoverAnchorOrigin]);

What alternative solutions did you explore? (Optional)

• N/A

[melvin-bot]

📣 @dragnoir! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[dragnoir]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/rachidlajmi

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[dragnoir]

Proposal

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

When resizing the window on the web browser, the open emoji picker moves to the LHN side.

What is the root cause of that problem?

I was reviewing my last proposal and with more testing, it seems that I am wrong, it's not about the style on the DOM. I checked more about the element position and the best practice to it and I found that EmojiPickerButton is not structured the way it's recommanded for measureInWindow.
There's some rendering issues that prevent the function from getting the right anchor details.

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

The return of EmojiPickerButton.js should be embedded inside a view where the refs should be.

import {View} from  'react-native';

return (
	<View
		ref={emojiPopoverAnchor}
	>
		<Tooltip  text={props.translate('reportActionCompose.emoji')}>
			<PressableWithoutFeedback
	// rest of the code
			</PressableWithoutFeedback>
		</Tooltip>
	</View>
);

so moving ref={emojiPopoverAnchor} from PressableWithoutFeedback to the new View will solve the issue.

Video after the code update

browser_resize.mp4

What alternative solutions did you explore? (Optional)

• N/A

[lschurr]

@aimane-chnaif can you review the proposals here?

[melvin-bot]

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

[aimane-chnaif]

reviewing

[lschurr]

Hi @aimane-chnaif @bernhardoj @neil-marcellini - Could someone take a look here and let me know how to proceed? Has the regression been fixed?

[aimane-chnaif]

not fixed yet

[lschurr]

Any update @aimane-chnaif?

[aimane-chnaif]

PR is in review. Not yet in mergeable state

[neil-marcellini]

Yep, under review.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fix tooltip doesn't show after closing the emoji picker #33690

If no regressions arise, payment will be issued on 2024-01-26. 🎊

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

• @bernhardoj requires payment (Needs manual offer from BZ)
• @aimane-chnaif 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:

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

[lschurr]

@aimane-chnaif - Can you work through the checklist on this one?

[aimane-chnaif]

• The PR that introduced the bug has been identified. Link to the PR: Add popover anchor tooltip #30150
• 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: https://github.com/Expensify/App/pull/30150/files#r1439823954
• 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: N/A
• Determine if we should create a regression test for this bug. This is kind of edge case and caught by QA team so no need regression test
• 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.

[lschurr]

It looks like the automation didn't work here. Moving to Daily and will handle payments shortly.

[lschurr]

Payment summary:

• Bug reporter: Internal (no payment)
• Contributor: @bernhardoj $250
• Contributor+: @aimane-chnaif $250

[lschurr]

Offers sent in Upwork.

[aimane-chnaif]

Offer accepted already - #31155 (comment)

[lschurr]

Ah, sorry about that! I'll use the previous offers.

[lschurr]

Payments issued. Closing this one out.