[HOLD for payment 2024-03-22] [$500] IOU - The cursor isn't centered

[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.33.0
Reproducible in staging?: Y
Reproducible in production?: Y
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: Applause - Internal Team
Slack conversation:

Issue found when executing PR #35119

Action Performed:

1. Open app
2. Tap FAB - Request money

Expected Result:

The cursor should be centered

Actual Result:

The cursor isn't centered. It affects the manual IOU and custom time

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6361353_1706644512181.IOU.mp4

Bug6361353_1706644512173.Time.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01501e5a4487cb18d9
• Upwork Job ID: 1752428207590764544
• Last Price Increase: 2024-03-06

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01501e5a4487cb18d9

[melvin-bot]

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

[melvin-bot]

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

[BhuvaneshPatil]

Proposal

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

Cursor for time selection is not centered. It's at end

What is the root cause of that problem?

We are setting cursor position at 2, when setting state in TimePicker

App/src/components/TimePicker/TimePicker.js

Line 122 in eb031dc

const [selectionMinute, setSelectionMinute] = useState({start: 2, end: 2}); // we focus it by default so need to have selection on the end

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

We shall change it to

 const [selectionMinute, setSelectionMinute] = useState({start: 0, end: 0}); /

What alternative solutions did you explore? (Optional)

[tienifr]

Proposal

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

The cursor isn't vertically centered in IOU amount input and also focus custom time

What is the root cause of that problem?

For those inputs we're using the custom EXP_NEW_KANSAS_MEDIUM font

This font has a problem, when exported it, it seems we've exported it with incorrect vertical metrics, particularly we exported it with some intrinsic bottom padding. That's why when rendering, in some platform like Android Chrome, it will take this bottom padding into account and render the text inside the input with that bottom padding, thus the cursor looks like it's not vertically centered and instead closer to the top. See more explanation on a similar issue here and here.

To validate this, we can just change EXP_NEW_KANSAS_MEDIUM here to EXP_NEUE and we'll see the problem doesn't occur, because the EXP_NEUE font was exported correctly, with correct vertical metrics.

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

We need to fix the incorrect vertical matrix in the EXP_NEW_KANSAS_MEDIUM font.

Update: We have the new set of updated font files from the font foundry here and we should use that in assets/fonts/web

More explanation:
I initially did this by:

1. Go to https://transfonter.org/
2. Upload that font, the relative file path is assets/fonts/web/ExpensifyNewKansas-Medium.woff2
3. Enable Fix vertical metrics option
4. Export the font, and replace the fonts in the app with the new exported font files (both the .woff and .woff2 font file)

I believe the design team probably has an internal process to design these fonts, we can also follow that internal process and tools we're using, to fix the vertical metrics similar to what's done above. And we'll raise a PR here to replace the font files.

Below is the result on Android Chrome where this issue occurs (I also tested in other platforms as well and still working fine):

Before:

After (cursor is vertically centered now):

This problem could be happening for other fonts as well, we might want to check and fix them (if any)

What alternative solutions did you explore? (Optional)

NA

[ntdiary]

The cursor should be centered

Hi, @lanitochka17, this means that the cursor should be vertically centered, right?

[melvin-bot]

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

[mallenexpensify]

Same question as @ntdiary

The cursor should be centered

Hi, @lanitochka17, this means that the cursor should be vertically centered, right?

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

also assigning to @lanitochka17 to help get 👀 on the above.

[melvin-bot]

@ntdiary, @mallenexpensify, @lanitochka17 Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@ntdiary @mallenexpensify @lanitochka17 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!

[tienifr]

@shawnborton The zip here only had .woff2 fonts while we need to replace both .woff and .woff2 in the repo. Please contact the font foundry and ask them to update the missing .woff files (i.e. ExpensifyNewKansas-MediumItalic.woff and ExpensifyNewKansas-Medium.woff). Also remind them to test the metrics in https://vertical-metrics.netlify.app/ as well. Thanks in advance!

[shawnborton]

Are you sure we need the .woff file though? I feel like it's becoming more standard to just use a .woff2. Either way, I can ask for it.

[shawnborton]

Okay, I have the .woff files but our type foundry confirmed my thinking that these aren't really used anymore... are you sure we need to implement them?

Either way, here it is: ENK_Medium_Woff.zip

[melvin-bot]

@dannymcclain, @ntdiary, @chiragsalian, @mallenexpensify, @lanitochka17, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick!

[tienifr]

PR ready for review #38116.

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

• Fix: Misaligned vertical metrics ExpensifyNewKansas font on OS/2 #38116

If no regressions arise, payment will be issued on 2024-03-22. 🎊

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

• @ntdiary requires payment (Needs manual offer from BZ)
• @tienifr 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:

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

[mallenexpensify]

@ntdiary @tienifr can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~014cdc2015b019d9f8

@ntdiary , please complete the BZ checklist above too.

[tienifr]

@mallenexpensify I’ve accepted the offer, thanks!

[ntdiary]

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:

• [@ntdiary] The PR that introduced the bug has been identified. Link to the PR: Add new header font #13714
• [@ntdiary] 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: https://github.com/Expensify/App/pull/13714/files#r1540395407
• [@ntdiary] 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: No need
• [@ntdiary] Determine if we should create a regression test for this bug. No need
• [@ntdiary] 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. N/A
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

No need for regression test, after the font is fixed, this issue shouldn't occur again. :)

[ntdiary]

@ntdiary @tienifr can you please accept the job and reply here once you have? https://www.upwork.com/jobs/~014cdc2015b019d9f8

@ntdiary , please complete the BZ checklist above too.

@mallenexpensify, thanks! Have accepted. 😄

[mallenexpensify]

Contributor: @tienifr paid $500 via Upwork
Contributor+: @ntdiary paid $500 via Upwork.

@ntdiary I created the TestRail GH because QA doesn't monthly runs for design and edge case issues and they might want to do one for this. I prefer to err on the side of letting QA make the decision.

• https://github.com/Expensify/Expensify/issues/383750