[HOLD for payment 2024-01-04] [$1000] IOS - Emoji chopped off from top in workspace name

[kbecciv]

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. Go to settings-> Workspaces
2. Click New Workspace -> Click Settings
3. Rename with “:vertical_traffic_light::vertical_traffic_light::vertical_traffic_light::vertical_traffic_light:” -> Click Save

Expected Result:

Emoji with some height is being chopped off from top

Actual Result:

🚦 Emoji shouldn’t be chopped off

💥 Please note, this seems to be device-specific and will likely only affect smaller phones (for example, the issue was reproduced on an iPhone 11)💥

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.57.5
Reproducible in staging?: n/a
Reproducible in production?: n/a
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
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @DinalJivani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692310024902729

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0119f528f80030b332
• Upwork Job ID: 1696626363709296640
• Last Price Increase: 2023-09-19

Issue Owner

Current Issue Owner: @johncschuster

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[DinalJivani]

@johncschuster
I am able to reproduce this issue on iPhone 11 iOS/Native App.
Not sure for about other devices though...

[johncschuster]

Thanks for calling that out, @DinalJivani! I've updated the OP to indicate that this might be device-specific.

[melvin-bot]

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

[melvin-bot]

Current assignee @johncschuster is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[dantastisk]

Proposal

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

The 🚦-emoji is cut-off when used in the name of a workspace.

What is the root cause of that problem?

The root cause of the problem is the styles.textHeadline being applied to the element. The style combines variables.fontSizeXLarge with variables.lineHeightXXLarge, which causes the clipping as the line-height is simply not large enough for the font size. This happens everywhere that styles.textHeadline is used. Another example can be seen with the user displayname as can be seen here:

Screenshot of user page

The style gets applied here:

App/src/pages/workspace/WorkspaceInitialPage.js

Lines 239 to 244 in 96a0db1

	<Text
	numberOfLines={1}
	style={[styles.textHeadline, styles.alignSelfCenter, styles.pre]}
	>
	{policy.name}
	</Text>

The style:
https://github.com/Expensify/App/blob/96a0db1aa2e239530f926b9bf62951f3c3408918/src/styles/styles.js#L382C1-L388

The variables in question:

App/src/styles/variables.ts

Line 54 in 96a0db1

fontSizeXLarge: 22,

App/src/styles/variables.ts

Line 95 in 96a0db1

lineHeightXXLarge: getValueUsingPixelRatio(27, 32),

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

A clean and simple fix is to change styles.textHeadline.lineHeight from variables.lineHeightXXLarge to variables.lineHeightXXXLarge. This ensures that it gets fixed everywhere and looks perfect as shown in the screenshots below.

Screenshot of workspace with fix

Screenshot of user page with fix

What alternative solutions did you explore? (Optional)

Alternative solutions include introducing a new style specifically for where styles.textHeadline is used and emojis might appear.

[neonbhai]

Proposal

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

Emoji chopped off from top in workspace name

What is the root cause of that problem?

The 🚦 emoji in iOS is one of the tallest emojis and the headline Text elements that have to render it do not have enough lineHeight
This happens because the fontSize (referenced from variables.fontSizeXLarge and is hardcoded 22) is mismatched with lineHeight (referenced by variables.lineHeightXXLarge and calculated by:

App/src/styles/variables.ts

Line 95 in 9701fbe

lineHeightXXLarge: getValueUsingPixelRatio(27, 32),

App/src/styles/variables.ts

Lines 10 to 12 in 9701fbe

	function getValueUsingPixelRatio(defaultValue: number, maxValue: number): number {
	return PixelRatio.getFontScale() * defaultValue > maxValue ? maxValue : defaultValue * PixelRatio.getFontScale();
	}

which is a function)

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

We should these modifiations to the Headline lineheight.

Solution 1:

We should add a parameter to textheadline that would conditionally increase the fontsize if the parameter is true:

textHeadline: (shouldBeTall) = {
    ...headlineFont,
    ...whiteSpace.preWrap,
    color: themeColors.heading,
    fontSize: variables.fontSizeXLarge,

    //lineHeight conditionally set:
    lineHeight: shouldBeTall ? variables.lineHeightXXXLarge : variables.lineHeightXXLarge,
},

Solution 2:

We should add a CSS style called textHeadlineTall and increase the lineHeight in it:

textHeadline: {
    ...headlineFont,
    ...whiteSpace.preWrap,
    color: themeColors.heading,
    fontSize: variables.fontSizeXLarge,
    lineHeight: variables.lineHeightXXXLarge, // <--- lineHeight increased
},

This style should be applied to:

• WorkspaceInitialPage.js:
  

  App/src/pages/workspace/WorkspaceInitialPage.js

  Line 241 in 96a0db1

  style={[styles.textHeadline, styles.alignSelfCenter, styles.pre]}

• DetailsPage.js:
  

  App/src/pages/DetailsPage.js

  Line 170 in 9701fbe

  style={[styles.textHeadline, styles.mb6, styles.pre]}

• Section.js:
  

  App/src/components/Section.js

  Line 51 in 9701fbe

  <Text style={[styles.textHeadline, styles.cardSectionTitle]}>{props.title}</Text>

This is the cleaner approach and allows us to handpick where we need the Tall Headline (since the other pages DownloadAppModal.js, PDFInfoMessage.js, FeatureList.js, SaveTheWorldPage.js, JustSignedInModal.js, have hardcoded content in-app that doesn't need this)

What alternative solutions did you explore? (Optional)

xx

[melvin-bot]

@johncschuster, @fedirjh Huh... This is 4 days overdue. Who can take care of this?

[fedirjh]

Hey @neonbhai, @dantastisk Does this bug affect other emojis ? or it's just that specific emoji? to me, it seems like an emoji-device-specific edge case.

The proposed changes do not really make sense to me, They will affect other platforms and may cause different regression.

@johncschuster This bug seems to be an emoji/device-specific case, and it's an extremely edge case, I suggest we do nothing here.

[dantastisk]

@fedirjh It affects quite a few tall emojis. Also the issue gets increasingly worse as text size is decreased on the device and affects text too on the lower settings, especially letters with accents (see the Á in the screenshots below).

As for potential regressions, I've checked everywhere that the textHeadLine style is used, and my proposal doesn't seem to break anything. On the contrary it actually fixes the issue in other places as well (like the user displayname and the participant details in a report).

This is with the lowest text size setting:

Same settings with the proposed fix:

[fedirjh]

especially letters with accents (see the Á in the screenshots below).

@dantastisk what device have you tested on?

This is what I get on iPhone 11

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[dantastisk]

@dantastisk what device have you tested on?

The screenshots are from a physical iPhone XS.

I was not able to reproduce the bug on a simulated iPhone, initially, but I realize now, that this is because the bug is related to the device text size setting (in Settings -> Accessibility -> Display & Text Size -> Larger Text). As far as I can tell, the chopping only happens when this is set to the 3rd tick or lower, worsening as the text size is decreased.

I do not know how common it is to adjust these settings and what the policy is for supporting this, but this makes it seem like it should be supported:

App/src/styles/variables.ts

Lines 3 to 12 in cbda2b0

	/**
	* Calculate the fontSize, lineHeight and padding when the device font size is changed, In most cases users do not change their device font size so PixelRatio.getFontScale() = 1 and this
	* method always returns the defaultValue (first param). When the device font size increases/decreases, the PixelRatio.getFontScale() value increases/decreases as well.
	* This means that if you have text and its 'fontSize' is 19, the device font size changed to the 5th level on the iOS slider and the actual fontSize is 19 * PixelRatio.getFontScale()
	* = 19 * 1.11 = 21.09. Since we are disallowing font scaling we need to calculate it manually. We calculate it with: PixelRatio.getFontScale() * defaultValue > maxValue ? maxValue :
	* defaultValue * PixelRatio getFontScale() This means that the fontSize is increased/decreased when the device font size changes up to maxValue (second param)
	*/
	function getValueUsingPixelRatio(defaultValue: number, maxValue: number): number {
	return PixelRatio.getFontScale() * defaultValue > maxValue ? maxValue : defaultValue * PixelRatio.getFontScale();
	}

[DinalJivani]

@dantastisk

I checked this Text size thing, default selection will be 3rd tick, When we go for larger size it actually works fine, But when you go to smallest size it chopped the emoji even more... Adding video for same below.

RPReplay_Final1694002465.mp4

[melvin-bot]

@johncschuster, @fedirjh Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@johncschuster, @fedirjh Eep! 4 days overdue now. Issues have feelings too...

[johncschuster]

@johncschuster This bug seems to be an emoji/device-specific case, and it's an extremely edge case, I suggest we do nothing here.

@fedirjh sorry I missed this comment! I'm a bit torn. Yes, I agree that part of this behavior seems a bit device-specific, but the Universal Font Size setting in iOS's Accessibility features might make this behavior more common. We can't really know how many of our users have increased the font size on their device, so we have no way of knowing how many people might be affected by this.

@Expensify/design, what are your thoughts on fixing this behavior? I kinda feel like we should fix it to ensure all folks are seeing New Expensify as we intend it to be seen.

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

• Fix tall emojis are chopped off from top #32818

If no regressions arise, payment will be issued on 2024-01-04. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @fedirjh requires payment (Needs manual offer from BZ)
• @rezkiy37 does not require payment (Contractor)
• @DinalJivani requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] 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:
• [@fedirjh] A discussion in #expensify-bugs 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:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@johncschuster / @stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[johncschuster]

Looks like I need to manually create the Upwork job since it wasn't automatically created. We were just discussing a new process for this today. I'll follow the process there and get this paid out.

[melvin-bot]

@johncschuster, @arosiclair, @fedirjh, @rezkiy37 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[johncschuster]

Payment Summary

@fedirjh requires payment by manual request of $1000
@rezkiy37 does not require payment (Contractor)
@DinalJivani - bug reporter - requires payment of $50 via Upwork

[situchan]

There was regression

[DinalJivani]

@johncschuster
Can you please create Upwork job and send me offer?

[melvin-bot]

@johncschuster, @arosiclair, @fedirjh, @rezkiy37 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@johncschuster, @arosiclair, @fedirjh, @rezkiy37 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[johncschuster]

@DinalJivani, I invited you to the Upwork job. Can you please accept that so I can issue payment for the bug report?

[DinalJivani]

@johncschuster
You sent offer for $50 instead of $250. This is old bug report.

[fedirjh]

@johncschuster Could you please invite me as well? I am already ineligible for the manual requests.

[DinalJivani]

@johncschuster
I had just accepted offer. Please approve $250 reporting bonus.

Thanks

[johncschuster]

You sent offer for $50 instead of $250. This is old bug report.

@DinalJivani sorry about that. I just double-checked when the reporting bonus change was made, and I see it was August 30, while this was reported on August 17th. I've issued the corrected amount of $250.

[johncschuster]

@johncschuster Could you please invite me as well? I am already ineligible for the manual requests.

Sure thing, @fedirjh, I'll invite you now.

[fedirjh]

@johncschuster Thank you accepted.

[johncschuster]

Payment has been issued! Thanks everyone!