[HOLD for payment 2022-11-11] [$1500] some emojis are not displayed correctly as they should be - reported by Karim mostafa

[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- Type emoji's keyword in the chat box
3- observe the shape of the emoji appeared

Expected Result:

Emojis should appear in the normal shapes as they are supposed to

Actual Result:

Emojis appeared in different shape than they are supposed to

Workaround:

unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.88-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

VID-20220720-WA0004.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Karim-30 (Karim mostafa)
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1658404495570899

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @johncschuster (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot]

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

[parasharrajat]

Seems like this will be solved in #9551. We are already tackling this in #9123 since some time.

[aldo-expensify]

At least for them moment, #9551 is not solving it:

This can be reproduced in Chrome / web (macos). I have no idea where these emojis come from, are they characters in the font we use?

Update: maybe from here:

App/assets/emojis.js

Lines 8779 to 8784 in e3c8897

	code: '☪',
	keywords: [
	'islam',
	'muslim',
	'religion',
	],

[melvin-bot]

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

[bfitzexpensify]

@parasharrajat #9123 is referencing a particular subset of emoji problems (skin tone), but if I'm understanding correctly, you're saying that it will address this problem too?

[aldo-expensify]

I got the feeling that it was something different. This happens even in chrome and I think it is coming from the hardcoded list of emojis in this file:

App/assets/emojis.js

Lines 8779 to 8784 in e3c8897

	code: '☪',
	keywords: [
	'islam',
	'muslim',
	'religion',
	],

I tested manually changing the symbol, and I saw it changing in the app.

[thesahindia]

#9123 has two issues in it.

1. Hand emoji at skin tone picker shows as black and white (safari only)
2. Some emojis look different at windows/Ubuntu because they need some system specific font.

This one is same as 2 but the system is different so we will need another font for this (Apple Color Emoji)

[bfitzexpensify]

Great, thanks for the explanation!

Upwork job here
Reporting job here - please apply to this @Karim-30

[melvin-bot]

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

[melvin-bot]

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

[Karim-30]

Proposal

By replacing the incorrect emojis inside App/assets/emojis.js with the correct emojis (see the attached emojis.txt), all Emojis will appear in the correct shapes in Safari, Firefox.

but in Chrome all Emojis will appear in the correct shapes except these 8 arrow emojis.

to solve these 8 emojis we will need to change the fontFamily for some components from GTAmericaExp-Regular to sans-serif or roboto as the following :

in src/styles/fontFamily/index.js:

    MONOSPACE_BOLD: 'GTAmericaExpMono-Bd',
    MONOSPACE_BOLD_ITALIC: 'GTAmericaExpMono-BdIt',
+   SANS_SERIF: 'sans-serif',

in src/styles/styles.js :

    baseTextInput: {
-       fontFamily: fontFamily.GTA,
+       fontFamily: fontFamily.SANS_SERIF,
        fontSize: variables.fontSizeNormal,
        lineHeight: variables.fontSizeNormalHeight,
        color: themeColors.text,


    textLabelSupporting: {
-       fontFamily: fontFamily.GTA,
+       fontFamily: fontFamily.SANS_SERIF,
        fontSize: variables.fontSizeLabel,



   textInputCompose: addOutlineWidth({
        backgroundColor: themeColors.componentBG,
        borderColor: themeColors.border,
        color: themeColors.text,
-       fontFamily: fontFamily.GTA,
+       fontFamily: fontFamily.SANS_SERIF,
        fontSize: variables.fontSizeNormal,
        borderWidth: 0,


    emojiText: {
-       fontFamily: fontFamily.GTA_BOLD,
+       fontFamily: fontFamily.SANS_SERIF,
        textAlign: 'center',

in src/pages/home/report/ReportActionItemFragment.js :

 import canUseTouchScreen from '../../../libs/canUseTouchscreen';
 import compose from '../../../libs/compose';
+import fontFamily from '../../../styles/fontFamily';


                    <Text
                        selectable={!canUseTouchScreen() || !props.isSmallScreenWidth}
-                       style={EmojiUtils.containsOnlyEmojis(props.fragment.text) ? styles.onlyEmojisText : undefined}
+                       style={[{fontFamily: fontFamily.SANS_SERIF}, EmojiUtils.containsOnlyEmojis(props.fragment.text) ? styles.onlyEmojisText : undefined]}
                    >

Result :

chrome.results.mov

Don't forget to replace App/assets/emojis.js content with the attached file content.
emojis.txt

[melvin-bot]

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

• Update Emoji Datasource and fix uniform rendering across platforms #12293

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

[MitchExpensify]

Made a note to pay on my calendar!

[puneetlath]

@MitchExpensify SO is updated.

[akshayasalvi]

@tgolen @mananjadhav Can I please request to increase the compensation for this one by some amount?

I didn't factor in the changes to process the emojis and refactor of the Tests. I know the tests are to be part of the PR, but the new emojis did break the current test, and I cleaned up some of the unwanted to code while fixing the tests not related to the change.

Would be great if you can consider the request.

[MitchExpensify]

@mananjadhav, following these instructions - Mind pointing to me to the PR that cause this bug? Thank you

[mananjadhav]

@MitchExpensify This has been a problem since the inception. What I mean is our emoji datasource didn't support them since the beginning. #1991 here's the PR when we added the previous emojis with the emoji picker.

[MitchExpensify]

@puneetlath "A discussion in #contributor-plus 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."

Curious for your thoughts as I try to follow these instructions, the PR that introduced the emoji picker didn't have a C+ or CME. Does it still make sense to post in #contributor-plus in a similar manner tagging internal engineers involved? Thank you!

[tgolen]

Can I please request to increase the compensation for this one by some amount?

I can appreciate that there was more work required to complete this than was figured in the original proposal. However, I was pretty disappointed that it took almost a month to get the PR created for this.

You originally said on Sep 30:

PR should be ready for review by weekend or Monday.

The PR was taken out of draft and marked ready to review on Oct. 30 and there was a gap of nearly a month where the branch didn't have any commits.

If the work would have been done with greater urgency, I would be OK doubling the price of this issue, but due to the solution taking so long to get to completion, I am only approving a $500 increase to the price of this issue to a total of $1500.

[akshayasalvi]

Thank you for the consideration @tgolen.

I agree with the speed. I had some issues with the conflicts, and then I had to reset the branch. Because emojis files even a small change, is causing the conflict, as I was replacing the whole file. I also had to reupdate the emojis because another PR also had the datasource with the new key added. I should've been more mindful in giving weekly updates on the issue.

I totally agree if you don't want to increase the budget too if you're unhappy with the experience.

[mananjadhav]

@akshayasalvi Few points of feedback in addition to @tgolen mentioned.

1. While we understand conflicts arise, and if you see any impeding PR pushing frequent updates which causes you to resolve conflicts, just highlight so that we can put this on Hold.
2. As @tgolen mentioned, there was a long gap between commits, if you're going OOO or not working then please inform. Communication goes a long way in setting up expectations.
3. I also saw some commits with misses in the script, so if you felt you're stuck somewhere, clarify, and we can resolve.
4. Ideally we're looking for longer relationships with the contributors, so we don't want bad experiences neither for you or ourselves. Always remember two rules: Get shit done and Don't ruin it for everyone else

Thank you so much for taking it to the finish line! Appreciate that you refactored some of the code.

[MitchExpensify]

Making a note for myself:

• Reporting payment will be issued via this Upwork Job
• C+ and Contributor payment will be issued via this Upwork Job

[mananjadhav]

@MitchExpensify I am already hired for this job with this link. When I click on the link you've shared it says Job no longer available.

I am guessing it's the same link, but still fyi.

[MitchExpensify]

@mananjadhav Yes all set! Issuing payment now

[MitchExpensify]

@Karim-30 mins accepting the offer here? Thank you!

[MitchExpensify]

Paid @mananjadhav & @akshayasalvi ! Thank you!

[mananjadhav]

Thanks @MitchExpensify

[Karim-30]

@MitchExpensify Accepted the offer, Thanks.

[Karim-30]

@tgolen I got paid, we can close it.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[MitchExpensify]

Conversation started in #contributor-plus here