-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HOLD for payment 2024-08-07] [$250] App crashes when opening a emoji picker #45354
Comments
This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989 |
Triggered auto assignment to @puneetlath ( |
Problem Statement: The Expensify app crashes when a user attempts to open the emoji picker, resulting in a poor user experience and potential loss of data. Root Cause: The root cause of the problem is likely due to a compatibility issue between the emoji picker library and the Expensify app's tech stack, or a memory leak caused by the emoji picker's implementation. Changes to Solve the Problem: To solve the problem, I propose the following changes: Update Emoji Picker Library: Update the emoji picker library to the latest version, which may include bug fixes and performance improvements. // Update emoji picker library to latest version // Optimize emoji picker implementation const EmojiComponent = () => { const handleEmojiSelect = (emoji) => { return ( javascript // Implement error handling Use a Different Emoji Picker Library: Explore alternative emoji picker libraries, such as react-emoji-picker, to see if they provide better compatibility and performance. |
Unable to reproduce it on the latest main |
ProposalPlease re-state the problem that we are trying to solve in this issue.App crashes when opening a emoji picker. What is the root cause of that problem?I get the same issue on one of my accounts. This happens when there's a frequently used emoji which we pick from Onyx, but it has no corresponding mapping in Because of which Lines 37 to 45 in eb83ae1
This leads to emoji being undefined in the below line: Line 203 in eb83ae1
This can also happen when there's a specific unicode of an emoji which is not available in our mapping. What changes do you think we should make in order to solve the problem?Add a check after this:
We can also consider adding an undefined and null check in this logic (and at similar places in EmojiUtils). We might also need to expand the emoji mapping table. |
Assuming @rayane-djouah is going to be the C+ here, can you check my proposal? |
Still able to reproduce. @puneetlath - can we make it Screen.Recording.2024-07-15.at.10.13.45.AM.mov |
Test branch to check the fix @rayane-djouah : https://github.com/ShridharGoel/ExpensifyApp/tree/test-emoji |
Hey @rayane-djouah i am not able to reproduce it on any account, is there any other steps that I should make to encounter the issue? |
Unable to reproduce on staging and new.expensify.com today, ¯_(ツ)_/¯ (was able to last week). |
Reproduction steps:
I think that this is reproducible with more special emojis, not just the spade emoji |
Job added to Upwork: https://www.upwork.com/jobs/~01bf8f915a1c60a468 |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
@ShridharGoel's proposal looks good to me. 🎀👀🎀 C+ reviewed |
📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @ShridharGoel 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
The emoji should be inserted from the device keyboard or by copy/paste, not from the app's emoji picker/suggestions. |
@ShridharGoel - Friendly reminder. Thanks! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.14-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-08-07. 🎊 For reference, here are some details about the assignees on this issue:
|
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:
|
@ShridharGoel has been paid. @rayane-djouah bump on the checklist for you. |
Regression Test Proposal
Do we agree 👍 or 👎 |
Regression test issue: https://github.com/Expensify/Expensify/issues/418908 All paid. Thanks everyone! |
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:
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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: @rayane-djouah
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1720566364883959
Action Performed:
Expected Result:
App does not crash
Actual Result:
App crash
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
logs-2024-07-12 19_15_44.066.txt
2024-07-12_12-15-09.mp4
Screen.Recording.2024-07-09.at.11.41.05.PM.mov
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @Issue Owner
Current Issue Owner: @puneetlathThe text was updated successfully, but these errors were encountered: