[HOLD for payment 2024-06-28] [$250] Task - Checkmark box is higher then the task title if font size is larger

[lanitochka17]

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: 1.4.78-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+jp_e_category@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

Precondition: font size of your device should be larger

1. Go to https://staging.new.expensify.com/
2. Log in
3. Open any chat
4. Create a task without assignee

Expected Result:

Checkmark box is on the same level as the task title if font size is larger

Actual Result:

Checkmark box is higher then the task title in the chat if font size is larger

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [x] 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/~01c835d73e1fb0a508
• Upwork Job ID: 1798750213257011740
• Last Price Increase: 2024-06-06
• Automatic offers:
• bernhardoj | Contributor | 102718395

Issue Owner

Current Issue Owner: @alexpensify

[melvin-bot[bot]]

Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@alexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-vsp

[bernhardoj]

Proposal

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

Checkbox position is higher than the task title when the font is large.

What is the root cause of that problem?

When we increase the font size of the device, the line height of the font is also increased. In this case, we use fontSizeNormalHeight as the line height. The initial value is 21 and the max value is 28. https://github.com/Expensify/App/blob/0cb2dc052f01b6a024ac872ad8bf628159fe7873/src/styles/variables.ts#L66

The alignment of both checkbox and the text is flex-start. When the line height is increased it make sense that the text is "pushed down".

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

1. Wrap the checkbox in a view https://github.com/Expensify/App/blob/a451de460c45611f64bcd06961afb78b4e2f3c9b/src/components/ReportActionItem/TaskPreview.tsx#L97-L110
2. Set the view wrapper height to be the same as the task text line height height: variables.fontSizeNormalHeight
3. Center the checkbox vertically with justifyContentCenter style.

[melvin-bot[bot]]

@alexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[alexpensify]

I'll run a test tomorrow and update with the next steps.

[alexpensify]

I'm able to replicate the experience on my Android device:

[melvin-bot[bot]]

Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.

[alexpensify]

@dannymcclain I'd like some Design feedback on how we should handle when the Android device has the increased font size. After that, I'll assign this to External to get help. Thanks!

[dannymcclain]

This isn't only happening when the font size is increased though, right? I'm seeing it in the mac desktop app (with default font size). Ideally we want the checkbox centered with the first line of text in all cases. If we can't figure out an elegant way to make that happen, I'd say we should just center the checkbox with the task title (regardless of size/number of lines/etc.)

[dannymcclain]

Here's a quick mock to show the alignment options:

cc @Expensify/design I very much prefer aligning with the top line of text, but curious what you all think is acceptable. Also, I thought our default text line-height was 20px, not 21px?

[bernhardoj]

This isn't only happening when the font size is increased though, right?

Yep, I see it too without the font size increase.

Btw, here is how it looks in real by centering it to the first line. Before:

After:

(with a scaled font)

[dannymcclain]

@bernhardoj oh I think your afters look good!

[alexpensify]

@dannymcclain - This GH is only for Android, so thanks for flagging that this might happen on other platforms. I only tested Android 🤦🏼‍♂️ .

[dannymcclain]

Ah dang yeah it looks like it's happening on all the platforms 😬

[shawnborton]

Hmm yeah, our default line height should be 20px. If it's not, let's fix it. I also prefer top alignment. From your mocks here though, should we add a bit more space between checkbox and text though?

[shawnborton]

Also just want to flag @thienlnam here - do you remember this from when you implemented Tasks, or maybe a regression has happened that caused the misalignment?

[thienlnam]

I think this is a regression, we had an issue like this we had fixed in the past. This may have been from recent bumps of react-native-live-markdown

[dannymcclain]

From your mocks https://github.com/Expensify/App/issues/42934#issuecomment-2152615595 though, should we add a bit more space between checkbox and text though?

Yeah I think it should have a little more space, but in these screenshots, the spacing looks good to me. Looks like in the product the gap is 8px and I just erroneously had it at 4px in my Figma doodles. So I think we just keep it at 8px (like it currently is in the product).

Hmm yeah, our default line height should be 20px. If it's not, let's fix it.

Down to fix this too.

[shawnborton]

Okay cool, so I guess we should open this up for proposals then? cc @alexpensify

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01c835d73e1fb0a508

[melvin-bot[bot]]

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

[alexpensify]

Thanks, team! I've updated the title to avoid confusion and ensure that we go with a plan that supports all platforms.

@aimane-chnaif please review the proposals and feedback from the Design team. Thanks!

[oXSMo]

do you want to place the checkbox in the middle !!? becaus it's ez

[melvin-bot[bot]]

📣 @oXSMo! 📣 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>

[bernhardoj]

Based on this commit message, we update the line height to 21 to fix emojis being cut off.

Here is what I got when update it to 20.

@dannymcclain @shawnborton

[aimane-chnaif]

I think the clean solution is to set checkbox container height same as text lineHeight and center checkbox vertically inside container.

[bernhardoj]

Yes, that's in my alternative solution too.

[shawnborton]

I think we need to think of a different way to handle the emojis while still keeping our line height at 20px. Perhaps it's an overflow issue or something, but either way, I think that was a mistake to make that change :( cc @roryabraham

[alexpensify]

@aimane-chnaif - do we have enough feedback to carry on with the review process? Thanks for the update.

[melvin-bot[bot]]

@dannymcclain, @alexpensify, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[roryabraham]

I'm generally all for it, but just FWIW I don't think New Expensify is really set up to "officially" support different OS font sizes. So part of me wonders if we should take bug reports for that, or if we should try to holistically start supporting multiple OS font fizes, and include some automated UI and/or regression tests to cover it.

Anyways, that's not the case with this issue, as it's happening with normal font sizes too.

I think the clean solution is to set checkbox container height same as text lineHeight and center checkbox vertically inside container.

Agree with this.

I think we need to think of a different way to handle the emojis while still keeping our line height at 20px. Perhaps it's an overflow issue or something, but either way, I think that was a mistake to make that change

Created an issue to address this: https://github.com/Expensify/App/issues/43449 (though it doesn't sound like the root cause of this problem?)

[shawnborton]

Thanks! Though I already brought this up in the SWM Slack channel, and made this GH issue here.

[aimane-chnaif]

I think the clean solution is to set checkbox container height same as text lineHeight and center checkbox vertically inside container.

Thanks. Let's go with ^ solution here. @bernhardoj please update your proposal to reflect that.

[alexpensify]

We are waiting for a proposal update here.

[bernhardoj]

Huh, weird. Pretty sure I already included that solution in my alternative before. Maybe I forget to press the update button.

Updated my proposal.

[aimane-chnaif]

@bernhardoj's proposal looks good to me. 🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[bernhardoj]

PR is ready

cc: @aimane-chnaif

[alexpensify]

This PR is going through the review process.

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/43646

If no regressions arise, payment will be issued on 2024-06-28. :confetti_ball:

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

• @bernhardoj requires payment automatic offer (Contributor)
• @aimane-chnaif requires payment through NewDot Manual Requests

[melvin-bot[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:

• [ ] [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [ ] [@aimane-chnaif] 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:
• [ ] [@aimane-chnaif] 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:
• [ ] [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [ ] [@aimane-chnaif] 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.
• [ ] [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[alexpensify]

Automation worked here, I'll work on the payment process tomorrow.

[alexpensify]

Payouts due: 2024-06-28

• [x] Contributor: $250 @bernhardoj
• [ ] Contributor+: $250 @aimane-chnaif

Upwork job is here.

Status: @aimane-chnaif please submit a request in Chat. @bernhardoj has been paid in Upwork.

[aimane-chnaif]

@alexpensify I am still using upwork. Can you please reopen and sort payment? Thanks

[alexpensify]

Reopening so that I can review this notice in more detail.

[alexpensify]

Ok, I found the notice here: https://expensify.slack.com/archives/C01SKUP7QR0/p1727830262798869. I'll need to create a new Upwork job for this payment.

[alexpensify]

I need to hold on creating the job because there is a payment issue with Upwork right now: https://expensify.slack.com/archives/C01GTK53T8Q/p1728319262830679