[HOLD for payment 2023-10-13] [$500] Web - emoji picker hides on resize window

[kbecciv]

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

---

Action Performed:

1. Go to any chat
2. Open emoji picker
3. Resize window (<800)
4. Verify its still open
5. Resize the window to the wider view

Expected Result:

Emoji picker should be open

Actual Result:

Emoji picker hides

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.30-5
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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-06-22.at.10.39.33.AM.1.mov

Recording.5151.mp4

Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687410686033509

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01004df42b8a89b793
• Upwork Job ID: 1674401243450486784
• Last Price Increase: 2023-09-24
• Automatic offers:
• gadhiyamanan | Reporter | 26809113
• shubham1206agra | Contributor | 26982232
• abdulrahuman5196 | Contributor | 26995658

[melvin-bot]

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

[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

[s-alves10]

Proposal

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

Emoji picker disappears when window resizes

What is the root cause of that problem?

The root cause is as in here. Remounting is the reason.
#20522 (comment)

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

As in the above proposal, using onyx store to save emoji status(opened, anchor, search, ...) data is a solution

What alternative solutions did you explore? (Optional)

[jeet-dhandha]

Proposal

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

• When resizing the browser window, the emoji picker closes. When moving from small width to large width and vice versa.

What is the root cause of that problem?

• The cause is that the emoji picker unmounts when the window is resized, this happens due to the PopoverWithMeasuredContent used in EmojiPicker.
• The isContentMeasured state is set to false when the window is resized, which causes the emoji picker to unmount.

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

• We can add a local variable in EmojiPickerAction file like activeType and a function like setActiveType to set the active type and getActiveType to get the active type.
• The activeType could vary depending on use case like COMPOSE for emoji button from composer input and if from AddReactionBubble which is down at chat can have REACTION_BUBBLE_${reportActionId}. Now whenever we call showEmojiPicker from any of this places we pass actionType and set it in the file.
• Thus after resizing the window the Composer Emoji Button or The Reaction Bubble re-mounts and then we can check if getActiveType() is same then we call their own showEmojiPicker function.

Code Changes Explaination:

• We need to add a local variable in EmojiPickerAction file like activeType and a function like setActiveType to set the active type and getActiveType to get the active type.
• Now we set activeType in showEmojiPicker to provided type from params and also set it to null in hideEmojiPicker function.
• We also need to set activeType to null in EmojiPicker's hideEmojiPicker function.
• Now we need to add a useEffect in EmojiPickerButton to check if the activeType is 'COMPOSE' then call showEmojiPicker function. (Dependencies of useEffect will be empty as we only want to call it once)
• Now we need to add a useEffect in AddReactionBubble to check if the activeType is ADD_REACTION_BUBBLE_${props.reportAction.reportActionID} then call the showEmojiPicker function. (Dependencies of useEffect will be empty as we only want to call it once)

Note: I am still not able to fix the picker closing issue for MiniQuickEmojiReaction. (Will try to fix that.)

Fixed Video

Fixed-Resize-Emoji.Picker.mp4

What alternative solutions did you explore? (Optional)

• N/A

[CortneyOfstad]

Was able to recreate, so going to get eyes on the two proposals above

[melvin-bot]

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

[melvin-bot]

Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[CortneyOfstad]

@abdulrahuman5196 Any thoughts on the two proposals above?

[abdulrahuman5196]

@CortneyOfstad Are we going to be working on this resize window issue? I thought we are not prioritizing these kind of window resizing bugs? Could you kindly confirm if this issue is something we want to fix before reviewing the proposals?

[CortneyOfstad]

@abdulrahuman5196 To be honest, I have been out the last two months, so wasn't sure about this change. But no worries. Can you reference where the conversation was had about not prioritizing these issues, so I can take a look?

[abdulrahuman5196]

Sorry @CortneyOfstad I thought this as a different issue. Apologies. This is due to the recent navigation refractor changes and the effect of mounting i assume.

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[abdulrahuman5196]

@mountiny I assume this behaviour is caused due to the navigation refactors and the mounting/unmounting when we switch to smaller screens. Correct me if wrong.

What is the current expectation of popovers? Do we expect it to close when resize the window to smaller ones? (I think its fine to close)
Previous working was emoji popover wouldn't close and transform. I wanted to confirm the expectation on this before reviewing the proposals.

[mountiny]

I was ooo this week. We got couple of problems raised which are tied to the fact the navigators unmount and mount again when resizing.

In case of this one i think we should close this and do nothing in this issue. We will be discussing with SWM if there is a way to find an updated way to handle the navigators so we do not change between them which would be the root cause solution rather than solving all the symptoms of the updated architecture.

cc @adamgrzybowski @WoLewicki

[adamgrzybowski]

Currently working on the solution for resizing without remounting navigators.

[jeet-dhandha]

Same issue on this #22196

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[mountiny]

Assigned, thanks!

[abdulrahuman5196]

@mountiny Kindly assign me as well C+ here

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

🎯 ⚡️ Woah @abdulrahuman5196 / @shubham1206agra, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @shubham1206agra got assigned: 2023-10-02 15:47:25 Z
• when the PR got merged: 2023-10-04 20:34:13 UTC

On to the next one 🚀

[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.3.78-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: emoji picker hides on resize window #28688

If no regressions arise, payment will be issued on 2023-10-13. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @abdulrahuman5196 requires payment offer (Contributor)
• @shubham1206agra requires payment offer (Contributor)
• @gadhiyamanan requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

• [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
• [@abdulrahuman5196] 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:
• [@abdulrahuman5196] 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:
• [@abdulrahuman5196] Determine if we should create a regression test for this bug.
• [@abdulrahuman5196] 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:

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
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:
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:

IMO not a regression from a single change. This is something which wasn't working after navigation refractors.

Determine if we should create a regression test for this bug.

Yes.

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.

1. Go to any chat
2. Open emoji picker
3. Resize window (<800)
4. Resize the window to a wider view

@mallenexpensify

[mountiny]

Lets make that regression test

[mountiny]

Issue created here https://github.com/Expensify/Expensify/issues/326639

[mallenexpensify]

Thanks @mountiny !

Issue reporter: @gadhiyamanan paid $250 via Upwork
Contributor: @shubham1206agra paid $750 via Upwork, inc. urgency bonus.
Contributor+: @abdulrahuman5196 paid $750 via Upwork, inc. urgency bonus.

@gadhiyamanan, I needed to create a new job to pay you, can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~0105078349f662f2d1

[gadhiyamanan]

@mallenexpensify offer accepted, thanks!

[mallenexpensify]

Thanks @gadhiyamanan
I updated my comment above to denote you've been paid!