search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.49k stars 2.84k forks source link

[$250] Web - Emoji - Emoji picker is moved to the top when changing the size of the window #51200

Open lanitochka17 opened 10 hours ago

lanitochka17 commented 10 hours ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.51-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): dave0123seife@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

1, Navigate to any chat

  1. Open emoji picker
  2. Reduce the size of the window

Expected Result:

Emoji picker stays in its position

Actual Result:

Reducing the size of the windows cause the Emoji picker to be moved to the top

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/c1f0ebb0-e5c5-4820-ab2b-e07c70324a7d

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021848576183089723860
  • Upwork Job ID: 1848576183089723860
  • Last Price Increase: 2024-10-22
Issue OwnerCurrent Issue Owner: @abdulrahuman5196
melvin-bot[bot] commented 10 hours ago

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

lanitochka17 commented 10 hours ago

@jliexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

jliexpensify commented 3 hours ago

@Expensify/design hello, is this a bug? I feel like it might be, as we're automatically moving it - but it could also be intentional, to get the picker out of the way as the window is now reduced. Keen for your thoughts, cheers!

dubielzyk-expensify commented 2 hours ago

Yeah I would expect this to keep it's positioned anchored to the button that triggered it

melvin-bot[bot] commented 2 hours ago

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

melvin-bot[bot] commented 2 hours ago

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

bernhardoj commented 2 hours ago

Edited by proposal-police: This proposal was edited at 2024-10-22 04:18:12 UTC.

Proposal

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

The emoji picker is moved to the top left when resizing window.

What is the root cause of that problem?

In the emoji picker component, we already have a dimension listener to recalculate the position. https://github.com/Expensify/App/blob/ac57d30444f5ed2bf0be3bc72705e0da64faa03e/src/components/EmojiPicker/EmojiPicker.tsx#L164-L173

However, the value returned is always 0 (-14 for top for me). The reason is, everytime we resize the window, the emoji picker button (which is the anchor where we calculate the position based on) is always re-rendered. https://github.com/Expensify/App/blob/ac57d30444f5ed2bf0be3bc72705e0da64faa03e/src/components/EmojiPicker/EmojiPickerButton.tsx#L33-L45

I verified this by appending a random number to the ID and it keeps changing when resizing. This causes the PopoverAnchorTooltip to also re-rendered and isPopoverRelatedToTooltipOpen is recalculated on every render (react-compiler wrongly optimize it) which will return false because tooltipNode is undefined which will remount the button whenever isPopoverRelatedToTooltipOpen is changed. https://github.com/Expensify/App/blob/7111c19f641bade5330116336e09f923be0fa80d/src/components/Tooltip/PopoverAnchorTooltip.tsx#L7-L35

So, when we are trying to recalculate the new position, the previous ref is not "valid" anymore. The emoji picker button keeps getting re-rendered because we pass inline function to the props. https://github.com/Expensify/App/blob/ac57d30444f5ed2bf0be3bc72705e0da64faa03e/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx#L518-L528

So, when the ReportActionCompose is re-rendered because of the window size, the emoji picker button is also re-rendered.

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

Wrap both function to a useCallback so it only re-renders when it's really need to. https://github.com/Expensify/App/blob/ac57d30444f5ed2bf0be3bc72705e0da64faa03e/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx#L518-L528

And the focus function here too because onModalHide depends on it. https://github.com/Expensify/App/blob/ac57d30444f5ed2bf0be3bc72705e0da64faa03e/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx#L203-L209

We need to apply this to other usage of emoji picker too, for example, in edit composer.