LHN - Message preview doesn't treat new line as space

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Expected Result:

Message preview should display a space if there is a new line in the message.

Actual Result:

Message preview doesn't treat new line as space

Action Performed:

1. Navigate to a conversation
2. Send this message

Cool
Test
Of
New line

3. Check the message preview in the LHN

Workaround:

No need, visual issue.

Platform:

Where is this issue occurring?

Web ✔️
iOS
Android
Desktop App
Mobile Web

Version Number: 1.0.47-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @puneetlath https://expensify.slack.com/archives/C01GTK53T8Q/p1621222051218200

LHN doesn’t treat new line as a space. Notice how in the LHN image there is no space between “Hello!” and the sentence before it. But in the actual message, it is on its own line. We should treat that new line as a space when showing the text in the LHN.

[MelvinBot]

Triggered auto assignment to @zanyrenney (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[quinthar]

This might overlap with the discussion elsewhere about getting rid of the "text" version of the message stored in the reportAction, and just generating a stripped version of the HTML "on demand". During this stripping process, we can just ignore newlines.

…

On Mon, May 17, 2021 at 9:13 AM MelvinBot ***@***.***> wrote: Triggered auto assignment to @zanyrenney <https://github.com/zanyrenney> ( AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2965 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEMNUWEQDVZVRAEHEHHAFTTOFTGRANCNFSM45BBNLSQ> .

[MelvinBot]

Triggered auto assignment to @tgolen (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[tgolen]

Looks like a good contributor issue.

[MelvinBot]

Triggered auto assignment to @jliexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[jliexpensify]

Can confirm this is happening on v1.0.48-0:

Will get this to Upwork.

p.s. Puneet brought up the issue here - https://expensify.slack.com/archives/C01GTK53T8Q/p1621222051218200

[marcaaron]

I thought maybe we could build this into ExpensiMark, but since we just need to convert the HTML into plain text + converts line breaks into spaces it can probably be a simple utility function like this but swap out the \n for a single space character?

[jliexpensify]

Posted to upwork: https://www.upwork.com/jobs/~01335a9f088d223b91

[parasharrajat]

Here is my proposal.

1. As Marc said, its fairly simple. We need to replace <br> tags with spaces before converting the HTML markup to text for lastMessageText in report object creation.
   https://github.com/Expensify/Expensify.cash/blob/ff61c1aaf2309ed4fcaf1264ff149acbeb9bfa9b/src/libs/actions/Report.js#L157
   TO

 const lastMessageText = lodashGet(lastReportAction, ['message', 'html'], '').replace(/(<(br[^>]*)>)/gi, ' ').replace(/(<([^>]+)>)/gi, '');

will just do the trick. This lastMessageText is further used to create Option for LHN.

.

[pranshuchittora]

This is very much related to #2847

---

Proposal 📄

Message preview should display a space if there is a new line in the message.

Investigation 🕵🏻‍♂️

#2847

Approach 👨🏼‍💻

File of concern : Report.js and expensify-common

• Use the parser to convert markdown to text
  Edit comment - Spaces are not displayed in edit box #2847 (comment)

Best Practices 💃🏼

• No hard-coded regexes
• No inline styling
• Follow React Native best practices

Testing Strategy 🧪

• Unit tests for the underlying modules

Expected Delivery Time 📦

Approx 1 week.

Previous Experience 🙅🏼‍♂️

• [Hold - for payment on Wed 26th] Add an About and App download links page to Expensify.cash #2811

[marcaaron]

That solution looks good @parasharrajat 👍

[marcaaron]

I'll let @tgolen weigh in though as he is assigned to the PR. I think this is something that would happen in the server eventually if we move to reduce processing logic for API data in the client.

[parasharrajat]

I think this is something that would happen in the server eventually if we move to reduce processing logic for API data in the client.

Then it makes sense

[tgolen]

Yep, I think it's OK for now. That logic isn't going anywhere today. If/when it's moved to the server-side then this replacement will come along with the logic.

[parasharrajat]

@tgolen Are you giving me a 🟢 signal? PR is one-click away?

[tgolen]

Yep! 🟢 Click away on your PR!

[jliexpensify]

Thanks @tgolen - I'll go ahead and hire @parasharrajat

[jliexpensify]

Looks like PR is blocked atm, will check again later.

[jliexpensify]

PR approved and deployed 6 hours ago

[parasharrajat]

@jliexpensify which PR? mine is still open.

[jliexpensify]

Sorry for the confusion @parasharrajat - I was referring to this one: https://github.com/Expensify/Web-Expensify/pull/31071

It looks like that PR has been deployed, and should have a flow-on effect for your PR to be unblocked soon.

[jliexpensify]

Contributor paid!