[HOLD for payment 2024-11-29] [$250] Auto suggestions for emoji is not sorted alphabetically

[m-natarajan]

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

---

Version Number: 9.0.43-1
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
Expensify/Expensify Issue URL:
Issue reported by: @flodnv
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727863255553599

Action Performed:

1. Go to staging.new.expensify.com
2. Open any DM
3. Type :disap in the compose box for emoji suggestions

Expected Result:

Suggestions displayed are sorted alphabetically

Actual Result:

Suggestions displayed are not sorted alphabetically

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021841781443095645269
• Upwork Job ID: 1841781443095645269
• Last Price Increase: 2024-10-03
• Automatic offers:
• suneox | Reviewer | 104528548
• nkdengineer | Contributor | 104528551

Issue Owner

Current Issue Owner: @adelekennedy

[melvin-bot]

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

[cretadn22]

Proposal

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

Auto suggestions for emoji is not sorted alphabetically

What is the root cause of that problem?

We don't arrange the emoji list before showing it

App/src/pages/home/report/ReportActionCompose/SuggestionEmoji.tsx

Line 169 in 1764d10

const newSuggestedEmojis = EmojiUtils.suggestEmojis(leftString, preferredLocale);

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

We should sort the emoji list alphabetically by name before displaying it

const newSuggestedEmojis = lodashSortBy(EmojiUtils.suggestEmojis(leftString, preferredLocale), (emoji) => emoji.name) 

What alternative solutions did you explore? (Optional)

Alternatively, we can prioritize emojis that match the search term first and then sort the remaining ones alphabetically

[nkdengineer]

Proposal

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

Suggestions displayed are not sorted alphabetically

What is the root cause of that problem?

App/src/libs/EmojiUtils.ts

Line 423 in 1764d10

const nodes = trie.getAllMatchingWords(emojiData[0].toLowerCase().slice(1), limit);

Here we are calculating nodes based on the text entered by the user and the keywords defined in this file

The reason when we enter :dis the 2 icons "melting_face" and "dotted_line_face" are displayed first is because they have the keyword "disappear" here and here

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

Since the suggestEmojis function is used in many other places, we should return the match sorted by the emoji name in this function suggestEmojis

App/src/libs/EmojiUtils.ts

Line 408 in 1764d10

function suggestEmojis(text: string, lang: Locale, limit: number = CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_SUGGESTIONS): Emoji[] | undefined {

    return lodashSortBy(matching, (emoji) => !emoji.name.includes(emojiData[0].toLowerCase().slice(1)));

What alternative solutions did you explore? (Optional)

Screen.Recording.2024-10-03.at.02.42.20.mov

[melvin-bot]

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

[melvin-bot]

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

[suneox]

Thanks for all the proposals.

Based on the expected result, Suggestions displayed are sorted alphabetically the outcome appears like this

However, I believe this is not a common pattern. Another platforms with markdown editors like Slack and GitHub prioritize emoji suggestions that match the text more closely, giving them higher priority.

Screen.Recording.2024-10-03.at.22.22.54.mov

So @flodnv, I would like to confirm whether the expected behavior should continue sorting by alphabetically or prioritize those that more closely match the text.

cc: @adelekennedy

[suneox]

@nkdengineer Your RCA is correct and detailed, but could you please double-check your implementation? When searching with only 2 characters, the result is incorrect

Screen.Recording.2024-10-03.at.22.44.58.mp4

[nkdengineer]

@suneox sorry for not being more clear in the proposal, I guess you missed update matching in here and here

Please check again

[flodnv]

The expected behavior is Slack's behavior, which I think you are referencing as prioritize those that more closely match the text. @suneox

[suneox]

The expected behavior is Slack's behavior, which I think you are referencing as prioritize those that more closely match the text. @suneox

@flodnv Yes, that is the expected behavior I want to confirm, as the current expected behavior is confusing when compared to Slack’s behavior

Based on the expected behavior, we can proceed with @nkdengineer proposal

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @srikarparsi, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[srikarparsi]

Hi @hannojg! I saw here that you were thinking about applying some findings with the recent search algorithm to emoji recommendations. Do you think you could provide a little more detail about that before we go through with the above proposal? It would definitely work but I'm not sure if there's a more performant way to do this.

[melvin-bot]

@suneox, @srikarparsi, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[suneox]

We’re still waiting for internal feedback about the order logic. The current selected proposal behaves like Slack, It only sorts by placing results that include the search text at the top.

Screen.Recording.2024-10-03.at.22.22.54.mov

cc: @hannojg

[melvin-bot]

@suneox, @srikarparsi, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[srikarparsi]

Hey @hannojg! Friendly bump on this message

[melvin-bot]

@suneox, @srikarparsi, @adelekennedy 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@suneox @srikarparsi @adelekennedy this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

This issue has not been updated in over 15 days. @suneox, @srikarparsi, @adelekennedy, @nkdengineer eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[flodnv]

What is the latest update here?

[adelekennedy]

@suneox it looks like @srikarparsi left a comment on the PR a while ago that needs to addressed?

[suneox]

@suneox it looks like @srikarparsi left a comment on the PR a while ago that needs to addressed?

Yes, we’re still working on a PR to handle lost order in some use cases.

[melvin-bot]

@suneox, @srikarparsi, @adelekennedy, @nkdengineer Whoops! This issue is 2 days overdue. Let's get this updated quick!

[srikarparsi]

PRs almost ready

[melvin-bot]

@suneox, @srikarparsi, @adelekennedy, @nkdengineer Huh... This is 4 days overdue. Who can take care of this?

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

• fix: sorted suggestion emoji #51220

If no regressions arise, payment will be issued on 2024-11-29. 🎊

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

• @suneox requires payment automatic offer (Reviewer)
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

@suneox @adelekennedy @suneox The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[suneox]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other: Improvement

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: This is an improvement, so we don’t have an offending PR.

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
  N/A: It isn't an impactful bug

[adelekennedy]

Payouts due:

• Contributor: $250 @nkdengineer
• Reviewer: $250 @suneox

Upwork job is here.