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.

Sign up for GitHub

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

Jump to bottom

[Pay 5/19] Wrap text when it includes markup #2790

Closed
3 of 5 tasks
twisterdotcom opened this issue May 11, 2021 · 14 comments · Fixed by #2826
Closed
3 of 5 tasks

[Pay 5/19] Wrap text when it includes markup #2790

twisterdotcom opened this issue May 11, 2021 · 14 comments · Fixed by #2826
Assignees
@roryabraham
Labels
DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2

Comments

@twisterdotcom
Copy link
Contributor

twisterdotcom commented May 11, 2021 •
edited
Loading

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Expected Result:

When sending a message with a hyperlink, any text within that message/paragraph which won't fit in the current window should wrap onto the next line within the current window. When resizing the window, this should change where the wrap in the text occurs.

Actual Result:

When sending a message with a hyperlink, the text sent remains on one line and cannot be scrolled to the right to read the remaining text.

Action Performed:

  1. Open staging.expensify.cash
  2. Sign in if not already
  3. Enter a chat with anybody
  4. Send the message:
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum
  1. This should wrap the text perfectly.
  2. Send the message:
www.expensify.com Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum
  1. This will send the message on just one line.

Workaround:

Try to copy the line and paste it into the compose box (or some other text editor) in order to read what was written.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Haven't been able to repro on iOS or Android. Not sure what the PR that introduced it was. Perhaps #2320? @yuwenmemon?

Version Number: 1.0.41-9 (1.0.41-9) on staging.expensify.cash
Logs: N/A
Notes/Photos/Videos:

image

Expensify/Expensify Issue URL: Do I need this? I thought I could just create this here? cc @Expensify/contributor-management

View all open jobs on Upwork
@twisterdotcom twisterdotcom added DeployBlockerCash This issue or pull request should block deployment AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 Improvement Item broken or needs improvement. labels May 11, 2021
@MelvinBot
Copy link

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

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label May 11, 2021
@OSBotify OSBotify mentioned this issue May 11, 2021
Closed
36 tasks
@twisterdotcom
Copy link
Contributor Author

Looks like this is any markup not just hyperlinks:

image

@twisterdotcom twisterdotcom changed the title Wrap text when it includes a hyperlink Wrap text when it includes markup May 11, 2021
@arielgreen
Copy link
Contributor

Expensify/Expensify Issue URL: Do I need this? I thought I could just create this here?

You are correct, our template needs to be updated.

@parasharrajat
Copy link
Member

Wow. We missed it for so long. But relatively, the fix is very simple.
we just need to supply flex: 1 to base style here
https://github.com/Expensify/Expensify.cash/blob/3997ae6620f8a104d08ec059e734934d85550f06/src/components/RenderHTML.js#L182

@JmillsExpensify
Copy link

Seeing a bunch of this in v1.0.40-0. Reported here: https://expensify.slack.com/archives/C01GTK53T8Q/p1620796475097600. Releasing to the pool.

@MelvinBot
Copy link

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

@roryabraham roryabraham added External Added to denote the issue can be worked on by a contributor and removed Engineering labels May 12, 2021
@roryabraham
Copy link
Contributor

I was able to verify that @parasharrajat's fix works, so I'm going to move this along by adding the External label so we get an Upwork job for this ASAP. @parasharrajat Can you have a PR ready?

@roryabraham roryabraham added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels May 12, 2021
@roryabraham
Copy link
Contributor

Seems like the External auto-assigner isn't working atm. cc @rafecolton any idea why that might be?

In the meantime, I'm going to create an Upwork job for this myself.

@roryabraham
Copy link
Contributor

Upwork job is here: https://www.upwork.com/jobs/~0195cf4074210779ba

@JmillsExpensify
Copy link

Oh thank you! I was hawking this one as well, not sure what happened (maybe because I was assigned)? I'm happy to help push from here btw.

@parasharrajat
Copy link
Member

parasharrajat commented May 12, 2021 •
edited
Loading

@roryabraham It's ready. Will be listed in few minutes.

@parasharrajat parasharrajat mentioned this issue May 12, 2021
Merged
5 tasks
@roryabraham roryabraham reopened this May 12, 2021
@roryabraham roryabraham changed the title Wrap text when it includes markup [Pay 5/19] Wrap text when it includes markup May 12, 2021
@roryabraham
Copy link
Contributor

@isagoico This should be fixed as of 1.0.42-4

@roryabraham roryabraham added Weekly KSv2 and removed Daily KSv2 labels May 12, 2021
@roryabraham
Copy link
Contributor

Verified this fix in 1.0.43-3:

  • Web
  • Desktop
  • iOS
  • iOS safari

@roryabraham
Copy link
Contributor

Paid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roryabraham roryabraham

Labels
DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fixed text-wrapping with Markup parasharrajat/App
7 participants
@iwiznia @JmillsExpensify @twisterdotcom @MelvinBot @parasharrajat @arielgreen @roryabraham