Skip to content

fix: Remove duplicate emojis from search#7323

Merged
MonilBhavsar merged 6 commits intoExpensify:mainfrom
mananjadhav:fix/rem-duplicate-emojis-from-search
Jan 20, 2022
Merged

fix: Remove duplicate emojis from search#7323
MonilBhavsar merged 6 commits intoExpensify:mainfrom
mananjadhav:fix/rem-duplicate-emojis-from-search

Conversation

@mananjadhav
Copy link
Collaborator

@mananjadhav mananjadhav commented Jan 19, 2022 •
edited
Loading

Details

  • Emojis when searched were picked twice - once from "frequently used" and the other relevant category. This PR ignores the emojis from "frequently used" and search only in the original data categories.

Fixed Issues

$ #7273

Tests

  1. Test by filtering the text
  2. Test when the emoji exists in original category as well as frequently used category, then the search result should be one of them only
  3. Tested without frequently used
  • Verify that no errors appear in the JS console

QA Steps

  1. Launch any chat
  2. Click on EmojiPicker action
  3. If Frequenly Used emojis don't exist then add a few emojis in the chat, if it exists then go to step 4
  4. In the search menu, filter the emoji say "smile", "laugh", etc.
  5. It should show each emoji only once.
  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

frequently-used-emojis-reference

web-remove-duplicate-emojis-in-search

Mobile Web

NA

Desktop

desktop-remove-duplicate-emojis-in-search

iOS

NA

Android

NA

@mananjadhav mananjadhav marked this pull request as ready for review January 19, 2022 16:17
@mananjadhav mananjadhav requested a review from a team as a code owner January 19, 2022 16:17
@MelvinBot MelvinBot requested review from MonilBhavsar and removed request for a team January 19, 2022 16:17
parasharrajat
parasharrajat reviewed Jan 19, 2022
View reviewed changes
src/pages/home/report/EmojiPickerMenu/index.js

// Move in the prescribed direction until we reach an element that isn't a header
const isHeader = e => e.header || e.code === CONST.EMOJI_SPACER;
const isHeader = e => e.header || e.spacer;
Copy link
Member

@parasharrajat parasharrajat Jan 19, 2022

Choose a reason for hiding this comment

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

Good catch..

parasharrajat
parasharrajat suggested changes Jan 19, 2022
View reviewed changes
@mananjadhav @parasharrajat
fix: Changed comment
bf00fb3 
Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>
parasharrajat
parasharrajat previously approved these changes Jan 20, 2022
View reviewed changes
Copy link
Member

@parasharrajat parasharrajat left a comment

Choose a reason for hiding this comment

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

LGTM.

cc: @MonilBhavsar

🎀 👀 🎀 C+ reviewed

MonilBhavsar
MonilBhavsar approved these changes Jan 20, 2022
View reviewed changes
Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good

@MonilBhavsar MonilBhavsar merged commit db697aa into Expensify:main Jan 20, 2022
@OSBotify
Copy link
Contributor

OSBotify commented Jan 20, 2022

✋ 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 OSBotify mentioned this pull request Jan 20, 2022
Closed
33 tasks
@OSBotify
Copy link
Contributor

OSBotify commented Jan 21, 2022

🚀 Deployed to staging by @MonilBhavsar in version: 1.1.31-2 🚀

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

@botify botify mentioned this pull request Jan 25, 2022
Closed
@OSBotify
Copy link
Contributor

OSBotify commented Jan 25, 2022

🚀 Deployed to production by @AndrewGable in version: 1.1.32-0 🚀

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

Reviewers

@MonilBhavsar MonilBhavsar MonilBhavsar approved these changes

+1 more reviewer

@parasharrajat parasharrajat parasharrajat approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mananjadhav @OSBotify @parasharrajat @MonilBhavsar