[$500] IOU - The cursor isn't centered

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] 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 https://github.com/Expensify/App/pull/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?

• [x] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/68a63a44-fe4d-4129-974d-62c643f18510

https://github.com/Expensify/App/assets/78819774/db2196c4-e6f0-4fd5-8f23-1b16663cba32

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-02-06

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[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 https://github.com/Expensify/App/blob/eb031dc2ee42def13f84e47dd978992659a11a05/src/components/TimePicker/TimePicker.js#L122

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)

[Krishna2323]

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 have default focus on the minutes input with selectionMinute set to useState({start: 0, end: 0});

https://github.com/Expensify/App/blob/eb031dc2ee42def13f84e47dd978992659a11a05/src/components/TimePicker/TimePicker.js#L122

https://github.com/Expensify/App/blob/eb031dc2ee42def13f84e47dd978992659a11a05/src/components/TimePicker/TimePicker.js#L131

https://github.com/Expensify/App/blob/eb031dc2ee42def13f84e47dd978992659a11a05/src/components/TimePicker/TimePicker.js#L498

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

IMO, we should initially focus on the hour input with selectTextOnInput prop. This default behavior is likely familiar to most users. We will only select text for the first time only, we need to add a ref to track if the input is already focused or not.

Result

https://github.com/Expensify/App/assets/85894871/7d93e730-ded7-42a5-863e-2fd22a644786

Result with optional suggestion

https://github.com/Expensify/App/assets/85894871/214bbb19-29ea-4573-ac6e-fcc7cc0e7b38

Optional

We can also select text in the minute input when it will be focused for the first time.

[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[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[bot]]

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

[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? 💸

[mallenexpensify]

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

[melvin-bot[bot]]

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

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

[mallenexpensify]

@lanitochka17 👀 plz. Also checking with QA on a process for pinging QA in GHs.

[isagoico]

The cursor should be centered

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

To answer this question, yes, we raised this issue since the cursor is not vertically centered. This is affecting both the BNP and the time selector in setting a status.

IOU:

Time selector:

[melvin-bot[bot]]

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

[mallenexpensify]

@dannymcclain please see screenshots above. I was planning to close this because it's a small bug that's not directly related to a VIP or wave project. Let me know if you disagree.

[dannymcclain]

@mallenexpensify I agree we should close and :donothing:

[tienifr]

@dannymcclain @mallenexpensify This happens 100% of the time on Android Chrome, and is pretty visible, also it's a font issue that can be fixed quickly (details here). So I think we should make an effort to polish it.

[dannymcclain]

It makes me a little uncomfortable to edit our font files (my main concern is other unintended consequences). Let's see if the @Expensify/design team has any other thoughts/opinions about this.

[shawnborton]

Hmm I can reach out to our font foundry and get their opinion on this, but part of me thinks this is not really a "bug" and we can close. Either way, I'll send an email and cc the @Expensify/design team.

[dannymcclain]

Thanks @shawnborton! It's interesting too because it looks perfectly centered on Mac web/desktop. So it can't be fully inherent in the font file, ya know?

[shawnborton]

This is what our foundry said:

In the screenshot you attached there seems to be something strange going on with the vertical alignment of the text. The colon is low or the numbers are high. Perhaps correcting this will help - in the font the colon is correctly positioned with the bottom dot aligned on the baseline, so this must be something in the web code.

Also, vertical metrics in the font are set to accommodate (as best possible) all the glyphs, so the cursor may look low when next to 123ABC, but high when next to gjpy.

[shawnborton]

Is there something we're doing with line heights or something else in the app? Sounds like the issue is NOT our fonts, phew!

[melvin-bot[bot]]

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

[mallenexpensify]

Bumping to weekly, I wanna see if we get any feedback on

Is there something we're doing with line heights or something else in the app?

Posted about it in #expensify-open-source

[melvin-bot[bot]]

@dannymcclain @ntdiary @mallenexpensify @lanitochka17 this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

Current assignee @ntdiary is eligible for the Internal assigner, not assigning anyone new.

[melvin-bot[bot]]

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