[HOLD for payment 2022-01-27] [HOLD for payment 2022-01-25] Food & Drink emojis and currently they are categorized in Animals & Nature - reported by @thesahindia

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open app
2. Navigate to any chat
3. Click on Emoji picker
4. Scroll to food emojis

Expected Result:

Food & Drink emojis should have its own category

Actual Result:

Food & Drink emojis and currently they are categorized in Animals & Nature

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.26-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1641235742418900

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[thesahindia]

Proposal

We need to add this before line 3618 -

    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: 'Food & Drink',
        header: true,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },

App/assets/emojis.js

Lines 3618 to 3623 in 667570c

	{
	code: '🍇',
	keywords: [
	'fruit',
	'grape',
	'plant',

and we need to add this before line 4368

    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },

App/assets/emojis.js

Lines 4368 to 4385 in 667570c

	{
	code: CONST.EMOJI_SPACER,
	},
	{
	code: CONST.EMOJI_SPACER,
	},
	{
	code: CONST.EMOJI_SPACER,
	},
	{
	code: CONST.EMOJI_SPACER,
	},
	{
	code: 'Travel & Places',
	header: true,
	},
	{
	code: CONST.EMOJI_SPACER,

[mananjadhav]

Proposal

What I would recommend is instead of manually adjusting such spaces we should remove all EMOJI_SPACER from emojis.js and dynamically add them. I had already added getDynamicSpacing while working on frequently used emojis. Can reuse the logic.

function addSpacesToEmojis(emojis) {
    const updatedEmojis = [];
    emojis.forEach((emoji, i) => {
        if(emoji.header) {
            updatedEmojis = updateEmojis.concat(getDynamicSpacing(updatedEmojis.length), [emoji], getDynamicSpacing(1));
        } else {
            updatedEmojis.push(emoji);
        }
    });
    return updatedEmojis;
}

[thesahindia]

@mananjadhav, I think your proposal is almost the same as mine except it's missing one crucial thing.
And I think changing the current way of adding EMOJI_SPACER could be a suggestion but it's not related to the issue itself.

[mananjadhav]

except it's missing one crucial thing.

What's that?

could be a suggestion but it's not related to the issue itself

Indeed, but creating issues for a dev function doesn't make sense. Hence, generally, I've pushed such enhancements as a part of any feature development. For instance, we removed hardcoded emoji header indices when we worked on frequently used emojis.

[thesahindia]

except it's missing one crucial thing.

What's that?

we need to add this before line 4368 as well

    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },
    {
        code: CONST.EMOJI_SPACER,
    },

could be a suggestion but it's not related to the issue itself

Indeed, but creating issues for a dev function doesn't make sense. Hence, generally, I've pushed such enhancements as a part of any feature development. For instance, we removed hardcoded emoji header indices when we worked on frequently used emojis.

Yes, and I just think that you could've just shared the suggestion instead of duplicating an already existing proposal.

[mananjadhav]

instead of duplicating an already existing proposal.

Don't worry I won't be anyway taking up/getting the job if the proposal is duplicated 😃

[mananjadhav]

@thesahindia I've updated the proposal to only my recommendation. Hope this helps.

[stitesExpensify]

Thank you for noticing this and creating the issue @thesahindia.

@mananjadhav do you know if this was a regression? I thought everything was categorized correctly at one point..

Regardless, I agree that dynamic spacing is a good improvement so that every time we add/remove emojis we don't need to manually change the amount of spaces in the whole file.

[MelvinBot]

Triggered auto assignment to @dylanexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[mananjadhav]

do you know if this was a regression?

@stitesExpensify I checked the history and couldn't find Food & Drinks as the header. I also checked our original data source https://github.com/amurani/unicode-emoji-list/, it is missing there as well, guessing hence we've missed it here.

every time we add/remove emojis we don't need to manually change the amount of spaces in the whole file.

Exactly. Current structure also stops us from changing the number of emojis per row if we need that in the future.

[dylanexpensify]

Posted!

Internal: https://www.upwork.com/ab/applicants/1480481665394843648/job-details
External: https://www.upwork.com/jobs/~01bacbeab1b11e4bfa

[MelvinBot]

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

[MelvinBot]

Current assignee @stitesExpensify is eligible for the Exported assigner, not assigning anyone new.

[parasharrajat]

Hmm... Tough call. I would differently recommend dynamic spacing and it's a nice clean-up.
But I see that @mananjadhav you didn't specify adding a new category Food & Drink in the emoji file. But that is understood 🤓.

Based on the weightage, Manan has proposed a nice improvement to the code for a long-term basis and it saves space. I like @mananjadhav proposal. @thesahindia you have proposed a working solution but Manan's proposed improvement has added advantage.

cc: @stitesExpensify

🎀 👀 🎀 C+ reviewed

[stitesExpensify]

I agree with @parasharrajat. @mananjadhav you can go ahead and apply to the job!

[mananjadhav]

@parasharrajat @stitesExpensify PR raised.

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.30-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: Added emoji category translations #7125
• [No QA] fix: Fixed error with frequently used emojis not showing up #7255

If no regressions arise, payment will be issued on 2022-01-25. 🎊

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.31-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [No QA] fix: Fixed error with frequently used emojis not showing up #7255

If no regressions arise, payment will be issued on 2022-01-27. 🎊

[dylanexpensify]

@mananjadhav offer sent, please apply and I'll pay you out ASAP!

[dylanexpensify]

@thesahindia offer sent for reporting the issue! Please apply and I'll pay you out ASAP!

[mananjadhav]

Done @dylanexpensify

[parasharrajat]

@dylanexpensify I have applied as C+. Thanks.

[dylanexpensify]

Everyone paid out, contracts ended, and job closed!