ON HOLD FOR #36655 [$500] iphone 13 Pro - Workspace name is cutoff at the top

[m-natarajan]

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

---

Version Number: Reproducible in staging?: happening with iphone 13 Pro Reproducible in production?: needs reproduction 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: @AndrewGable Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707240198084589

Action Performed:

1. Go to workspace
2. click on wrench to go to settings

   Expected Result:

   Workspace name should appear without any cutoff

   Actual Result:

   Workspace name is cutoff at the top

   Workaround:

   visual

   Platforms:

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

   • [ ] Android: Native
   • [ ] Android: mWeb Chrome
   • [x] 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/~0190df84185c63613f
• Upwork Job ID: 1757800156429877248
• Last Price Increase: 2024-03-06

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@zanyrenney Eep! 4 days overdue now. Issues have feelings too...

[zanyrenney]

I do not have an iphone 13 pro so i can't repro.

[zanyrenney]

but adding external as Andrew tested/ confirmed this in chat.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @brandonhenry! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

   Contributor details
   Your Expensify account email: <REPLACE EMAIL HERE>
   Upwork Profile Link: <REPLACE LINK HERE>

[brandonhenry]

Contributor details Your Expensify account email: itsbhenry@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01568e6425f3546d5f

[melvin-bot[bot]]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[brandonhenry]

Proposal

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

The workspace name is cut off at the top when viewed in the settings view of the workspace on an iPhone 13 Pro. This issue affects the user experience by not displaying the full name of the workspace, which can be confusing and aesthetically unpleasing.

What is the root cause of that problem?

The root cause of this problem is likely related to the layout and styling of the workspace name container not accounting for the unique screen dimensions and notch of the iPhone 13 Pro. This results in the workspace name being rendered outside the visible area of the screen.

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

• Adjust the CSS styling for the workspace name container to ensure proper padding and margin settings. This will prevent the workspace name from being cut off at the top.
• Utilize React Native's SafeAreaView component to ensure that the workspace name is rendered within the safe area boundaries of the iPhone 13 Pro's screen, accommodating the notch and other screen features.
• Test the solution on an iPhone 13 Pro device to ensure that the workspace name is displayed correctly without any cutoff.
• Additionally, test the solution on other devices and platforms to verify compatibility and consistent behavior.

What alternative solutions did you explore? (Optional)

Instead of using SafeAreaView, we could manually calculate the safe area insets and apply them as padding to the workspace name container. However, this approach is less reliable and more complex than using SafeAreaView.

[zanyrenney]

@parasharrajat what do you think of the proposal above?

[parasharrajat]

The proposal lacks clear instructions and does not confirm if it solves the issue. @brandonhenry Can you please update your proposal to be specific to our App?

[brandonhenry]

@parasharrajat

Proposal

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

The workspace name is cut off at the top when viewed in the settings view of the workspace on an iPhone 13 Pro. This issue affects the user experience by not displaying the full name of the workspace, which can be confusing and aesthetically unpleasing.

What is the root cause of that problem?

The root cause of this problem is related to how the text inside of the Breadbrumbs component is styled. By default, it has a line-height of 23px. This is super close to it's font-size (22px).

lineHeight can render differently across different devices and platforms due to variations in font rendering and device pixel ratios. This can potentially lead to layout issues like text being cut off or misaligned - especially when the line-height is really close to font-size

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

The lineHeight should typically be greater than the fontSize to ensure adequate spacing for descenders and ascenders in the text, which are the parts of letters that extend below the baseline (like 'p', 'q', 'y') or above the x-height (like 'd', 'h', 'l').

1. Import the variables from @styles/variables.
2. Calculate the lineHeight based on the font size and desired ratio.
3. Apply the calculated lineHeight to the text styles.

Example:

const lineHeightRatio = 1.3; // factor that is safe line height area
const primaryFontSize = variables.fontSizeNormal; // used by https://github.com/Expensify/App/blob/5d45f0f6c86e97d4ae58ac167b6c162a610b817c/src/components/Text.tsx
const primaryLineHeight = primaryFontSize * lineHeightRatio;

// add extra styling to header text so line-height is properly calculated

{primaryBreadcrumb.type === CONST.BREADCRUMB_TYPE.ROOT ? (
    <View style={styles.breadcrumbLogo}>
        <Header
            title={
                <ImageSVG
                    contentFit="contain"
                    src={LogoComponent}
                    fill={theme.text}
                    width={variables.lhnLogoWidth}
                    height={variables.lhnLogoHeight}
                />
            }
            shouldShowEnvironmentBadge
        />
    </View>
) : (
    <Text
        numberOfLines={1}
        style={[
            styles.flexShrink1,
            styles.breadcrumb,
            styles.breadcrumbStrong,
            {lineHeight: primaryLineHeight}, // Apply calculated lineHeight
        ]}
    >
        {primaryBreadcrumb.text}
    </Text>
)}

Original / Culprit

https://github.com/Expensify/App/blob/d2259b5b869afb7495018e0e3e93040e70698239/src/components/Breadcrumbs.tsx#L58

[melvin-bot[bot]]

@parasharrajat @zanyrenney 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!

[zanyrenney]

bump @parasharrajat please review.

[melvin-bot[bot]]

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

[parasharrajat]

@brandonhenry Thanks for the proposal.

const lineHeightRatio = 1.3; // factor that is safe line height area

How did you come up with this?

• Please explain code suggestions.
• Can you confirm if the above suggestion does not impact UI? If it does how?

[brandonhenry]

How did you come up with this?

Can't remember where I read it, but this site has info about recommend line-height ratios (original linked). We can do anything 1.2, 1.5 but 1:3 ratio is a common practice in typography and is considered a safe and effective way to calculate lineHeight for optimal readability and aesthetics. Checking now to confirm no impact to UI

Please explain code suggestions.

const lineHeightRatio = 1.3; // factor that is safe line height area
const primaryFontSize = variables.fontSizeNormal; // used by https://github.com/Expensify/App/blob/5d45f0f6c86e97d4ae58ac167b6c162a610b817c/src/components/Text.tsx
const primaryLineHeight = primaryFontSize * lineHeightRatio;

The default Text fontSize is pulled in from variables.fontSizeNormal.

https://github.com/Expensify/App/blob/5d45f0f6c86e97d4ae58ac167b6c162a610b817c/src/components/Text.tsx#L37

That default size is too close to the lineHeight of this element (in this case, looks like we're setting them to 1px off...). When the height of the element shrinks too much, we start to see this clipping. To avoid the height shrinking like that, we need to calculate a new lineHeight based on the standard ratio (in this case, I used 1.3).

This will give us a new lineHeight that should prevent any clipping of the text element, we can then apply to the styles of the Text element here:

<Text
    numberOfLines={1}
    style={[
        styles.flexShrink1,
        styles.breadcrumb,
        styles.breadcrumbStrong,
        {fontSize: primaryFontSize}, // make sure to use dynamic fontSize that adjusts with lineHeight
        {lineHeight: primaryLineHeight}, // Apply calculated lineHeight
    ]}
>
    {primaryBreadcrumb.text}
</Text>

[agent3bood]

Proposal

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

iphone 13 Pro - Workspace name is cutoff at the top

What is the root cause of that problem?

The breadcrumb component applies two styles to the text, styles.breadcrumb, styles.breadcrumbStrong, the second one sets the fontSize without setting the lineHeight.

https://github.com/Expensify/App/blob/e3e0b2db46a1d8fa2097ba8cf51b045b690d5c26/src/components/Breadcrumbs.tsx#L60

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

Add lineHeight to styles.breadcrumbStrong

        breadcrumbStrong: {
            color: theme.text,
            fontSize: variables.fontSizeXLarge,
            lineHeight: variables.lineHeightXXLarge,
        },

What alternative solutions did you explore? (Optional)

[parasharrajat]

@agent3bood By fontHeight, you mean line height?

@brandonhenry As the Text component can take any font size for Text, how will this logic handle that dynamic value?

[agent3bood]

@parasharrajat sorry for the typo, is is lineHeight as shown in the code snippet. Corrected on the proposal comment.

[brandonhenry]

@parasharrajat good point - we should also go ahead and pass in the textSize we are using to calculate the lineHeight. I updated my example: {fontSize: primaryFontSize}

[parasharrajat]

@brandonhenry you indeed have an interesting suggestion but I have seen a simple solution to be chosen over dynamic line height in the past. I will reconfirm if this is still the case.

[melvin-bot[bot]]

@parasharrajat @zanyrenney this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot[bot]]

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

[parasharrajat]

Looks like we are already solving such issues in one of the PRs. https://github.com/Expensify/App/pull/36655.

We should hold on that PR @zanyrenney

[lanitochka17]

Hello We found a similar issue in Android and with emojis. Is this the same problem or a new one? Thank you

[parasharrajat]

Same

[zanyrenney]

on hold, amended the title of the issue

[melvin-bot[bot]]

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

[zanyrenney]

On HOLD for: https://github.com/Expensify/App/pull/36655

[zanyrenney]

Still On HOLD for: https://github.com/Expensify/App/pull/36655 it is open, not yet merged and deployed.

[parasharrajat]

This might have been fixed. I can't reproduce this. Can we please test this again?

[AndrewGable]

It works for me on latest TestFlight 👍