Fix: Added emoji category translations #7125

mananjadhav · 2022-01-11T09:27:51Z

Details

Added translations for emoji categories
Added Food & Drinks category
Removed hardcoded spaces in the list and added a dynamic spacing logic.

Fixed Issues

$ #7060
$ #7074

Tests

Tested for English category names
Tested for Spanish category names

Verify that no errors appear in the JS console

Errors are not related to my code

QA Steps

Go to home page, and settings, change the language preference to Spanish
Go to any chat, and click on emoji button on compose input
Category names should have translated values

Verify that no errors appear in the JS console

Tested On

Screenshots

Web

Translation

Food & Drinks

Mobile Web

Translation

Food and Drinks

Desktop

Translation

Food and Drinks

iOS

Translations

Food and Drinks

Android

Translations

Food and Drinks

mananjadhav · 2022-01-11T11:48:13Z

@parasharrajat @rushatgabhane, Because @stitesExpensify is a reviewer for #7074 as well, I've created a common PR.

It also helps me avoid conflicts that I will only have to resolve. I hope it isn't a big trouble in reviewing it.

src/libs/EmojiUtils.js

rushatgabhane · 2022-01-11T14:42:12Z

@mananjadhav Is it just me or the emoji picker is bugged.

Screencast.from.11-01-22.05-40-09.PM.+03.mp4

Head is at the latest commit.

mananjadhav · 2022-01-11T14:47:57Z

Let me check and get back

mananjadhav · 2022-01-11T15:02:54Z

@rushatgabhane I checked and I am not able to reproduce this. Can you probably hard refresh and check?

emoji-picker.mp4

rushatgabhane · 2022-01-11T15:29:20Z

Can you probably hard refresh and check?

Thanks, nothing worked but for some reason npm ci again helped 🥴
This bug isn't reproducible anymore.

rushatgabhane

Looks good! some suggestions.

src/libs/EmojiUtils.js

stitesExpensify · 2022-01-11T16:12:09Z

Is it just me or the emoji picker is bugged.

This is what happens if the dynamic spacer elements are off FYI. I wonder if you had a cached emoji list or something? Weird regardless, glad it's not actually a problem 😄

src/languages/es.js

src/libs/EmojiUtils.js

stitesExpensify · 2022-01-11T16:20:12Z

Also, I believe if you pull main the tests will pass now

…oji-category-translations

parasharrajat

LGTM. C+ Linked issue for ref #7074

cc: @stitesExpensify
🎀 👀 🎀 C+ reviewed

src/languages/es.js

stitesExpensify · 2022-01-11T18:27:19Z

Hmm i'm actually getting this too... I did an npm ci and a hard refresh/empty cache and it's still happening. I'm worried that the same problem is going to happen to all of our users if this goes live like this...

parasharrajat · 2022-01-11T18:39:31Z

Time for me to test it again. I try to reproduce that broken UI..

rushatgabhane · 2022-01-11T18:41:11Z

Interesting, gonna dig deeper.

mananjadhav · 2022-01-12T04:33:11Z

Ohhh 🤔 For some reason, I didn’t get this at all 😕

Let me try to check the issue as well.

parasharrajat · 2022-01-12T11:01:26Z

I don't see that issue. cc: @rushatgabhane

Co-authored-by: Ionatan Wiznia <[email protected]>

rushatgabhane · 2022-01-12T13:44:18Z

src/libs/EmojiUtils.js

+    _.each(emojis, (emoji, index) => {
+        if (emoji.header) {
+            updatedEmojis = updatedEmojis.concat(getDynamicSpacing(updatedEmojis.length, index), [emoji], getDynamicSpacing(1, index));
+        } else {


return early here

I am not sure when you say return early. There's an if and else case to it. Neither it gave an error for the same.

I mean to say - return at end of if
So you don't need to use else

Just realised and commit pushed.

rushatgabhane

Still investigating the UI bug, having a hard time replicating it consistently for debugging.
Weird thing is, it fixes itself sometimes after reopening emoji drawer.
Dry ran the code but couldn't find any edge cases yet.

Small change requested.

mananjadhav · 2022-01-12T15:39:38Z

Well, team, I was able to reproduce this consistently for one case -> "When we don't have any frequently used emojis."

App/src/libs/EmojiUtils.js

Lines 112 to 115 in 7e50a8d

    
           function mergeEmojisWithFrequentlyUsedEmojis(emojis, frequentlyUsedEmojis = []) { 
        
               if (frequentlyUsedEmojis.length === 0) { 
        
                   return emojis; 
        
               }

This code should ideally be :

if (frequentlyUsedEmojis.length === 0) {
    return addSpacesToEmojiCategories(emojis);
}

…mananjadhav/App into fix/emoji-category-translations

mananjadhav · 2022-01-12T15:48:11Z

@stitesExpensify @parasharrajat @rushatgabhane PR updated.

Hopefully you shouldn't get the spacing issue now 😃

parasharrajat · 2022-01-12T15:58:59Z

Oh, Nice. I didn't notice the frequent emojis. Glad you found it.

parasharrajat

LGTM.

rushatgabhane

LGTM
cc: @stitesExpensify

stitesExpensify · 2022-01-12T20:47:18Z

Well, team, I was able to reproduce this consistently for one case -> "When we don't have any frequently used emojis."

Nice find!

stitesExpensify

LGTM!

Fixed

OSBotify · 2022-01-12T20:53:11Z

✋ 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 · 2022-01-14T20:01:11Z

🚀 Deployed to staging by @stitesExpensify in version: 1.1.29-6 🚀

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

OSBotify · 2022-01-18T22:03:43Z

🚀 Deployed to production by @roryabraham in version: 1.1.30-3 🚀

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

mananjadhav added 3 commits January 11, 2022 14:39

fix: Changed the title to translation key

14c145d

fix: replaced the code with translate function

b3ee832

fix: Updated translations for es

e9853d4

mananjadhav requested a review from a team as a code owner January 11, 2022 09:27

MelvinBot requested review from rushatgabhane and stitesExpensify and removed request for a team January 11, 2022 09:27

mananjadhav added 4 commits January 11, 2022 15:23

fix: Added translation to native code as well

a2734a0

fix: Added food & drinks

b57f01c

fix: Added dynamic spacing for category names

2aa425f

fix: Removed hardcoded spaces and added new category

1aecb6b

parasharrajat suggested changes Jan 11, 2022

View reviewed changes

src/libs/EmojiUtils.js Outdated Show resolved Hide resolved

rushatgabhane suggested changes Jan 11, 2022

View reviewed changes

src/libs/EmojiUtils.js Show resolved Hide resolved

stitesExpensify reviewed Jan 11, 2022

View reviewed changes

src/languages/es.js Show resolved Hide resolved

src/libs/EmojiUtils.js Show resolved Hide resolved

mananjadhav added 2 commits January 11, 2022 22:49

Merge branch 'main' of https://github.com/mananjadhav/App into fix/em…

abac75c

…oji-category-translations

fix: Code cleanup

ea2c6bd

mananjadhav requested review from stitesExpensify, rushatgabhane and parasharrajat January 11, 2022 17:27

parasharrajat previously approved these changes Jan 11, 2022

View reviewed changes

iwiznia previously requested changes Jan 11, 2022

View reviewed changes

src/languages/es.js Outdated Show resolved Hide resolved

fix: Updated spanish translation

39853fb

Co-authored-by: Ionatan Wiznia <[email protected]>

mananjadhav dismissed parasharrajat’s stale review via 39853fb January 12, 2022 12:18

rushatgabhane reviewed Jan 12, 2022

View reviewed changes

rushatgabhane suggested changes Jan 12, 2022

View reviewed changes

mananjadhav added 3 commits January 12, 2022 21:10

fix: Add dynamic spaces when no frequently used emojis exist

deb283d

Merge branch 'fix/emoji-category-translations' of https://github.com/…

d25031b

…mananjadhav/App into fix/emoji-category-translations

fix: Added early return

dcf4dde

parasharrajat approved these changes Jan 12, 2022

View reviewed changes

rushatgabhane approved these changes Jan 12, 2022

View reviewed changes

stitesExpensify approved these changes Jan 12, 2022

View reviewed changes

stitesExpensify merged commit c226b8e into Expensify:main Jan 12, 2022

OSBotify mentioned this pull request Jan 14, 2022

Deploy Checklist: New Expensify 2022-01-14 #7235

Closed

66 tasks

mananjadhav mentioned this pull request Jan 15, 2022

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

Merged

7 tasks

kavimuru mentioned this pull request Jan 15, 2022

Emojis - "Frequently Used" header is missing in emojis categories #7258

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Added emoji category translations #7125

Fix: Added emoji category translations #7125

mananjadhav commented Jan 11, 2022 •

edited

Loading

mananjadhav commented Jan 11, 2022 •

edited

Loading

rushatgabhane commented Jan 11, 2022 •

edited

Loading

mananjadhav commented Jan 11, 2022

mananjadhav commented Jan 11, 2022

rushatgabhane commented Jan 11, 2022

rushatgabhane left a comment

stitesExpensify commented Jan 11, 2022

stitesExpensify commented Jan 11, 2022

parasharrajat left a comment •

edited

Loading

stitesExpensify commented Jan 11, 2022

parasharrajat commented Jan 11, 2022

rushatgabhane commented Jan 11, 2022 •

edited

Loading

mananjadhav commented Jan 12, 2022

parasharrajat commented Jan 12, 2022

rushatgabhane Jan 12, 2022

mananjadhav Jan 12, 2022

rushatgabhane Jan 12, 2022

mananjadhav Jan 12, 2022

rushatgabhane left a comment •

edited

Loading

mananjadhav commented Jan 12, 2022

mananjadhav commented Jan 12, 2022

parasharrajat commented Jan 12, 2022 •

edited

Loading

parasharrajat left a comment

rushatgabhane left a comment

stitesExpensify commented Jan 12, 2022

stitesExpensify left a comment

OSBotify commented Jan 12, 2022

OSBotify commented Jan 14, 2022

OSBotify commented Jan 18, 2022

Fix: Added emoji category translations #7125

Fix: Added emoji category translations #7125

Conversation

mananjadhav commented Jan 11, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mananjadhav commented Jan 11, 2022 • edited Loading

rushatgabhane commented Jan 11, 2022 • edited Loading

mananjadhav commented Jan 11, 2022

mananjadhav commented Jan 11, 2022

rushatgabhane commented Jan 11, 2022

rushatgabhane left a comment

Choose a reason for hiding this comment

stitesExpensify commented Jan 11, 2022

stitesExpensify commented Jan 11, 2022

parasharrajat left a comment • edited Loading

Choose a reason for hiding this comment

stitesExpensify commented Jan 11, 2022

parasharrajat commented Jan 11, 2022

rushatgabhane commented Jan 11, 2022 • edited Loading

mananjadhav commented Jan 12, 2022

parasharrajat commented Jan 12, 2022

rushatgabhane Jan 12, 2022

Choose a reason for hiding this comment

mananjadhav Jan 12, 2022

Choose a reason for hiding this comment

rushatgabhane Jan 12, 2022

Choose a reason for hiding this comment

mananjadhav Jan 12, 2022

Choose a reason for hiding this comment

rushatgabhane left a comment • edited Loading

Choose a reason for hiding this comment

mananjadhav commented Jan 12, 2022

mananjadhav commented Jan 12, 2022

parasharrajat commented Jan 12, 2022 • edited Loading

parasharrajat left a comment

Choose a reason for hiding this comment

rushatgabhane left a comment

Choose a reason for hiding this comment

stitesExpensify commented Jan 12, 2022

stitesExpensify left a comment

Choose a reason for hiding this comment

OSBotify commented Jan 12, 2022

OSBotify commented Jan 14, 2022

OSBotify commented Jan 18, 2022

mananjadhav commented Jan 11, 2022 •

edited

Loading

mananjadhav commented Jan 11, 2022 •

edited

Loading

rushatgabhane commented Jan 11, 2022 •

edited

Loading

parasharrajat left a comment •

edited

Loading

rushatgabhane commented Jan 11, 2022 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

parasharrajat commented Jan 12, 2022 •

edited

Loading