[C+ Checklist Needs Completion] [$250] Split - Values after decimal point displayed on second line if splitting certain amounts

[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.73-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: N/A
Issue reported by: Applause - Internal Team

Action Performed:

precondition: the device text size is set to default (middle)

1. Open the app and log in
2. Tap FAB > Split expense
3. Enter certain amount (1, 6-9, 60, 70, 80, 90)
4. On the next screen select one participant
5. Proceed to the confirmation screen

Expected Result:

The values after decimal point are displayed in one line

Actual Result:

The values after decimal point are displayed on the second line. It happens with certain amounts (1, 6-9, 60, 70, 80, 90) if splitting between 2 users

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

Bug6479893_1715633663967.IMG_6856.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011b4523bc0bb34adf
• Upwork Job ID: 1793049519934955520
• Last Price Increase: 2024-06-04
• Automatic offers:
• dukenv0307 | Contributor | 102671868
• tienifr | Contributor | 102690052

Issue Owner

Current Issue Owner: @dukenv0307

[melvin-bot]

Triggered auto assignment to @greg-schroeder (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]

@greg-schroeder 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-split

[allgandalf]

Proposal

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

Decimal point values go on second line for split requests.

What is the root cause of that problem?

We have extra padding which causes the values after decimal point to go to next line:

App/src/components/TextInputWithCurrencySymbol/BaseTextInputWithCurrencySymbol.tsx

Line 64 in f9abf1e

style={[styles.pr1, style]}

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

Remove the extra padding : styles.pr1

Result:

[bernhardoj]

Proposal

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

Decimal value is put at the second line.

What is the root cause of that problem?

Currently, we set a fixed width for the split amount input.

App/src/components/MoneyRequestConfirmationList.tsx

Line 492 in a6cb60c

const amountWidth = StyleUtils.getWidthStyle(formattedTotalAmount.length * 9 + prefixPadding);

It assumes each digit width is 9 and it adds the prefix padding. However, the input has a padding right style.

App/src/components/TextInputWithCurrencySymbol/BaseTextInputWithCurrencySymbol.tsx

Line 64 in a6cb60c

style={[styles.pr1, style]}

If the input is 35.00 and we don't use the fixed width, the width is 64.3334...

but the fixed width value is 64.

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

We should consider the padding-right into the calculation by adding pr1.paddingRight.

And to fix #42279, we should also consider the padding-left value of the input padding here.

App/src/components/TextInput/BaseTextInput/index.tsx

Line 365 in f16005f

!!prefixCharacter && StyleUtils.getPaddingLeft(StyleUtils.getCharacterPadding(prefixCharacter) + styles.pl1.paddingLeft),

[bernhardoj]

Updated proposal to include fix for #42279

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~011b4523bc0bb34adf

[greg-schroeder]

Applied External so we can get proposal review going

[melvin-bot]

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

[tienifr]

Proposal

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

The values after decimal point are displayed on the second line. It happens with certain amounts (1, 6-9, 60, 70, 80, 90) if splitting between 2 users

What is the root cause of that problem?

In here, we're calculating a fixed amountWidth based on the number of digits in the amount. This is intended to be the width of the "content" of the input.

However, by default, React Native views operate with box-sizing of border-box style, that means the width specified above there is inclusive of the paddings and borders.

So when there're paddings in the input here and here, it will compress the inner content of the input and the text will overflow to the next line.

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

We should avoid doing calculations on the content width, padding, margin, ... We should instead make the text input align itself so that it will work with any value of width, padding, margin. To do this, we need to introduce box-sizing content-box behavior to the BaseTextInput. That means the with specified here is correctly the width of the amount, whatever paddings of the input will be excluded from that amount width and the TextInput will adjust its total width accordingly.

1. Introduce an contentWidth property to the BaseTextInput, this will be the width of the content only
2. Extract the paddingLeft here to a variable

const paddingLeft = !!prefixCharacter && StyleUtils.getPaddingLeft(StyleUtils.getCharacterPadding(prefixCharacter) + styles.pl1.paddingLeft);

3. If contentWidth is specified, the Text here should be wrapped in a View, which also has the styles of Text. The View will have the width: 'auto' so that it will size itself according to the content width and paddings. It should also use the paddingLeft above

Pseudo code:

<View
    style={[
        inputStyle,
        styles.hiddenElementOutsideOfWindow,
        styles.visibilityHidden,
        {width: 'auto', ...paddingLeft}
    ]}
    onLayout={(e) => {
        if (e.nativeEvent.layout.width === 0 && e.nativeEvent.layout.height === 0) {
            return;
        }
        setTextInputWidth(e.nativeEvent.layout.width);
        setTextInputHeight(e.nativeEvent.layout.height);
    }}
>
    <Text
        style={[
            inputStyle,
            autoGrowHeight && styles.autoGrowHeightHiddenInput(width ?? 0, typeof maxAutoGrowHeight === 'number' ? maxAutoGrowHeight : undefined),
            {width: contentWidth}
        ]}
    >
        {/* \u200B added to solve the issue of not expanding the text input enough when the value ends with '\n' (https://github.com/Expensify/App/issues/21271) */}
        {value ? `${value}${value.endsWith('\n') ? '\u200B' : ''}` : placeholder}
    </Text>
</View>

4. In here, pass the contentWidth equal to formattedTotalAmount.length * 9 (taken from here).

5. Then we won't need the amountWidth here and here

As we can see, there's no manual calculation based on paddings, content, ... required. The TextInput will automatically adjust. Minor changes can be handled in the PR phase.

What alternative solutions did you explore? (Optional)

NA

[DylanDylann]

@tienifr Could you provide a test branch for your solution?

[melvin-bot]

@greg-schroeder @DylanDylann 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!

[DylanDylann]

Hi Melv, the @tienifr's proposal looks promising. I am requesting a test branch to ensure it works well on devices. @tienifr Any update on this issue?

[melvin-bot]

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

[melvin-bot]

@greg-schroeder, @DylanDylann Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

📣 @tienifr 🎉 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 📖

[dangrous]

How are we looking on this one @tienifr @dukenv0307?

[dukenv0307]

@dangrous I'm waiting for @tienifr to update his PR

[dangrous]

Got it - @tienifr do you think you can take a look and updated as needed today? Would love to move this forward and it's been about a week. Thanks!

[greg-schroeder]

Last update from @tienifr was yesterday:

On it now. I was working on a related PR and thought it's better to finish it first before this so we wouldn't have expectation conflicts.

[greg-schroeder]

PR is on staging, awaiting deploy to prod

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

• fix: split amount is put into a second line #43624

If no regressions arise, payment will be issued on 2024-07-24. 🎊

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

• @tienifr requires payment automatic offer (Contributor)
• @dukenv0307 requires payment automatic offer (Contributor)

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

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

[greg-schroeder]

Processing

[greg-schroeder]

Payment summary:

Contributor: @tienifr - $250 - You can make a manual request via ND (rescinded Upwork offer)
C+: @dukenv0307 - $250 - Paid via Upwork

[greg-schroeder]

Next up is the checklist from @dukenv0307!

[JmillsExpensify]

$250 approved for @tienifr

[dukenv0307]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: N/A
• 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: N/A
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• 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.

Regression test:

1. Tap FAB > Split expense
2. Enter $199.98 amount
3. Proceed to the confirmation screen with another participant
4. Verify the split amounts for each participant display in one line

Do we 👍 or 👎