[HOLD for payment 2024-06-05] Search - Expense preview with multiline description shows two spacings between each line

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

1. Go to staging.new.expensify.com
2. Submit an expense to a user with no unsettled expense, and include a multiline description in the expense
3. Note the expense preview in LHN shows one spacing between each line
4. Click on the search icon
5. Note that expense preview in search list

Expected Result:

The expense preview in search list will also show one spacing between each line

Actual Result:

The expense preview in search list alsos two spacings between each line

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

Bug6469800_1714725584145.20240503_163336.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @mallenexpensify

[melvin-bot]

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

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

[lanitochka17]

@mallenexpensify 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

[melvin-bot]

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

[mallenexpensify]

@dannymcclain , do you happen to know if this is expected behaviour?

[dannymcclain]

@mallenexpensify I don't think it's expected—I would imagine the LHN and search rows would look the same. At the same time, I would say this is extremely minor.

[bernhardoj]

Proposal

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

The multi-line description of an expense request shows extra spacing between lines on the search page, but not in LHN.

What is the root cause of that problem?

This issue happens after my PR here. In the PR, we are fixing an issue where the LHN subtitle is shown as multiline when we set the expense description to multiline by replacing all new line characters with a whitespace.

App/src/libs/ReportUtils.ts

Line 1630 in 2f5173c

return String(lastMessageText).trim().replace(CONST.REGEX.LINE_BREAK, ' ').substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim();

The LINE_BREAK regex is then updated to consider a case where the new line character is \r\n as found here.

App/src/CONST.ts

Line 1879 in 2f5173c

LINE_BREAK: /\r|\n/g,

But the new regex (\r|\n) matches each \r and \n and replaces it with whitespace, so if we have \r\n, we will end up with 2 whitespaces. This isn't noticeable in LHN because the text style set the white-space to nowrap which collapses the whitespace into a single whitespace.

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

Update the LINE_BREAK regex replace both \r\n into a single whitespace.

// stricter, if \n\r is given, then we will have 2 whitespaces, which I believe is expected
LINE_BREAK: /\r\n|\r|\n/g,

OR

// shorter, but will match \n\r too
LINE_BREAK: /[\r\n]+/g,

we can optionally set the white-space back to pre for the LHN that gets removed in this PR

[dannymcclain]

Not overdue Melvin.

[mallenexpensify]

@dannymcclain If we updated it so that
Hello
Hello

Didn't show as Hello Hello in LHN, it'd just show Hello. In another example. If someone sent.
Hi,
What's the ETA on the project
If we fixed this to recognize the line break, we'd only show Hi, in LHN. I don't think that's better than showing ``Hi, What's the ETA on the project'

OOOOOOOF... check this weirdness, it shows all the text in LHN then the second line gets removed for some reason

2024-05-07_12-02-56.mp4

[dannymcclain]

Whoaaaa, that is weird haha. I totally agree that "fixing" it to just show Hi, is not better than Hi, What's the ETA on the project

In the video posted on the issue description, the LHN looks fine to me.

The expense description is

Hello
Hello

And the LHN displays Hello Hello.

But the search row displays Hello Hello with an extra space between the lines.

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[mallenexpensify]

Bumping to weekly and adding Needs Reproduction because this isn't a top priority and I want to suss things out next week once the site's doing better, then add it to a project. We should def align on what we think is best for how we should multi-line posts in LHN from comments on reports as well as expenses, tasks, ???

[dannymcclain]

We should def align on what we think is best for how we should multi-line posts in LHN from comments on reports as well as expenses, tasks, ???

cc @Expensify/design in case y'all have any strong feelings about this. (My feelings are all pretty mild right now.)

[dubielzyk-expensify]

Yeah agree, but very mild here too. I think showing it the preview on 1 line is desirable, but without double spaces. That being said it's an extremely minor issue

[dannymcclain]

I think showing it the preview on 1 line is desirable, but without double spaces. That being said it's an extremely minor issue

Totally agree.

[shawnborton]

Yup, similar feelings here as well.

Interesting that we even allow multi-line descriptions though and render them that way on the preview card... just flagging @trjExpensify for vis

[trjExpensify]

Interesting that we even allow multi-line descriptions though and render them that way on the preview card... just flagging @trjExpensify for vis

I think for this one we have a cap at something like 40 characters or something like that, I can't recall exactly what it is but I remember it from before.

[trjExpensify]

As for the bug in the OP, I agree it's really minor. @bernhardoj has identified his PR that caused it though, so can't we fix it as a follow-up to remove the second white space in Search? @parasharrajat for the C+ review as well to keep both on it.

[parasharrajat]

I think updating the regex is a better solution. I think @bernhardoj's proposal works great.

🎀 👀 🎀 C+ reviewed

[carlosmiceli]

What's the status of this? @dannymcclain @mallenexpensify

[parasharrajat]

PR is merged.

[bernhardoj]

I think we need a payment here?

cc: @parasharrajat

[parasharrajat]

@carlosmiceli yes payment is pending.

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

• Fix expense preview with multiline description shows two spacings between each line in search page #42243

If no regressions arise, payment will be issued on 2024-06-05. 🎊

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

• @parasharrajat requires payment through NewDot Manual Requests
• @bernhardoj 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:

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

@carlosmiceli , when contributors are assigned to the issue (or, in the rare instance where they're not but they reviewed the associated/linked PR) we want to keep the issue open for payment. The BZ will manage the payment as well as ensuring a TestRail case is created, when needed.

[carlosmiceli]

My bad everyone, still learning some of the contributors' systems, thanks for the lesson @mallenexpensify 🙏

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @parasharrajat owed $250 via NewDot
• ROLE: @bernhardoj paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@mallenexpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[parasharrajat]

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:

• [@parasharrajat] The PR that introduced the bug has been identified. Link to the PR: Fix report preview LHN subtitle shown as multiline #39525
• [@parasharrajat] 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/39525/files#r1628296997
• [@parasharrajat] 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: Not needed
• [@parasharrajat] Determine if we should create a regression test for this bug. Yes
• [@parasharrajat] 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 Steps

1. Go to 1:1 DM with user that has no unsettled IOU.
2. Create a money request with multiline description.
3. Verify the LHN subtitle is shown as a single line.
4. Open the search page and make sure the chat subtitle is shown as single line.

Do you agree 👍 or 👎 ?

[mallenexpensify]

@bernhardoj can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01aab5a64c2284bdc2

TR GH created, thanks @parasharrajat
https://github.com/Expensify/Expensify/issues/402725

[bernhardoj]

@mallenexpensify accepted!

[mallenexpensify]

Contributor: @bernhardoj paid $250 via Upwork
Contributor+: @parasharrajat owed $250 via NewDot

Thanks!

[parasharrajat]

Payment requested as per #41587 (comment)

[JmillsExpensify]

$250 approved for @parasharrajat