Skip to content
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

Fix emoji search behavior when deleting all text #8828

Merged
merged 2 commits into from
Apr 29, 2022
Merged

Conversation

tgolen
Copy link
Contributor

@tgolen tgolen commented Apr 29, 2022

Details

When all the text is deleted from the search, the highlighted emoji index was being reset to 8 (number of columns) instead of -1 (no emoji selected). This behavior was causing bugs like duplicating emoji section headers as well as inserting the text frequentlyUsed into the action compose input.

Fixed Issues

$ #8824

Tests and QA

NOTE: If you find yourself with multiple emoji section headers, there is only one way to fix them:

  1. Log into that account on https://expensify.com (OldDot)
  2. Run NVP.set('expensify_frequentlyUsedEmojis', []); from the JS console

To test that this fix is working properly:

  1. Open a chat
  2. Click the emoji picker to open it
  3. Type any text into the search box
  4. Delete all the text in the search box
  5. Press the Enter key
  6. Verify that nothing happens (there is nothing inserted into the action compose input)
  7. Verify that you can open and close the emoji picker and the emoji sections are not duplicated
  8. Open the emoji picker again
  9. Select any emoji
  10. Verify the emoji popover closes, the emoji was added to the compose input
  11. Open the emoji picker
  12. Verify that the emoji was added to the section of frequently used emoji
  • Verify that no errors appear in the JS console

QA Steps

Same as tests

Screenshots

Web

Imagine me pressing enter repeatedly at the end of this video

2022-04-28_18-50-31.mp4

Mobile Web

Does not apply because there is no search box

Desktop

Imagine me pressing enter repeatedly at the end of this video

2022-04-28_18-53-33.mp4

iOS

Does not apply because there is no search box

Android

Does not apply because there is no search box

@tgolen tgolen requested review from marcaaron and a team as code owners April 29, 2022 00:54
@tgolen tgolen self-assigned this Apr 29, 2022
@melvin-bot melvin-bot bot requested review from yuwenmemon and removed request for a team April 29, 2022 00:55
Copy link
Contributor

@yuwenmemon yuwenmemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, all yours Marc!

@marcaaron marcaaron removed their request for review April 29, 2022 01:54
@marcaaron
Copy link
Contributor

Thanks I'm removing myself - just keeping an eye on the ReportActionsView but this isn't touching anything I'm interested in 😄

@tgolen
Copy link
Contributor Author

tgolen commented Apr 29, 2022

Kewl, I'll go ahead and merge this then.

@tgolen tgolen merged commit 64c173d into main Apr 29, 2022
@tgolen tgolen deleted the tgolen-fix-emojisearch branch April 29, 2022 15:18
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

OSBotify commented May 9, 2022

🚀 Cherry-picked to staging by @sketchydroide in version: 1.1.57-8 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @chiragsalian in version: 1.1.57-17 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants