Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert line-breaks to spaces in LHN message preview #3059

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented May 21, 2021

Details

Fixed Issues

Fixes #2965

Tests / QA Steps

  1. Navigate to a conversation
  2. Send this message
Cool
Test
Of
New line
  1. Check the message preview in the LHN.
  2. There should be a space between newlines text in the preview.
  3. Multiple new lines should only add one space.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

lhn-w.mp4

Mobile Web

Desktop

iOS

Android

@parasharrajat parasharrajat marked this pull request as ready for review May 21, 2021 21:25
@parasharrajat parasharrajat requested a review from a team as a code owner May 21, 2021 21:25
@MelvinBot MelvinBot requested review from deetergp and removed request for a team May 21, 2021 21:25
@parasharrajat
Copy link
Member Author

parasharrajat commented May 21, 2021

Blocker: We also need to modify the backend so that it convert <Br> to spaces while sending data from Pusher & API

@parasharrajat
Copy link
Member Author

Bumping here so that blocker can be resolved. Thanks.

@parasharrajat
Copy link
Member Author

@deetergp Have you got a chance to review it? And could you please tag the right guy for backend blocker. Thanks

@deetergp
Copy link
Contributor

Hey @parasharrajat 👋 Been a bit swamped with other stuff, but getting up to speed on this. I'm pretty sure the answer to this is "no" but do you know whether there was a GH made for the back end portion of this? If there is, then I don't want to step on someone else's toes, but if one hasn't been made, I will put one together and probably just do the work myself. Thanks!

@deetergp
Copy link
Contributor

deetergp commented May 28, 2021

@parasharrajat Can you explain why backend changes are needed? Can you show examples of where not having these changes is causing problems?

@parasharrajat
Copy link
Member Author

parasharrajat commented May 28, 2021

Yes. @deetergp. When we save the report using the Report_AddComment we send all the html and this api returns a response back which includes text as well. I saw that this text is different from what we have send. Also, other user will receive the Pusher event with the message payload that also contains the text. Let me know if I am missing something here.

Copy link
Contributor

@deetergp deetergp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The blocking back-end portion has been merged, un-blocking this one. We're good to go! 🎉

@deetergp deetergp merged commit 6b7902f into Expensify:main Jun 1, 2021
@OSBotify
Copy link
Contributor

OSBotify commented Jun 1, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@isagoico
Copy link

isagoico commented Jun 2, 2021

@parasharrajat @deetergp
We found this issue in Android and iOS apps

2 spaces are displayed in LHN for a line break

Expected result

Only 1 space should be displayed for a line break

Actual result

2 spaces are displayed for a line break

Action Performed

  1. Navigate to a conversation
  2. Send this message
Cool
Test
Of
New line
  1. Check the message preview in the LHN.

Platform

iOS ✔️
Android ✔️

Images/Notes/Video

@isagoico
Copy link

isagoico commented Jun 2, 2021

Will open a separate issue for the above 🔝 Following Rory's comment here #3059 (comment)

@parasharrajat parasharrajat deleted the LHN-preview branch November 20, 2023 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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