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

[HOLD for payment 2022-01-17] Chat - Indentation is not preserved in comments #6769

Closed
mvtglobally opened this issue Dec 15, 2021 · 14 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Improvement Item broken or needs improvement. Weekly KSv2

Comments

@mvtglobally
Copy link

mvtglobally commented Dec 15, 2021

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


Action Performed:

  1. Navigate to any chat
  2. Send a message with indentation
  3. Some item
    i. Some sub-item
    ii. Some other sub-item
  4. Some other item

Expected Result:

Message should be received with indentation

Actual Result:

Format is not persisting

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.20-1
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screen Shot 2021-12-14 at 9 23 12 PM

Expensify/Expensify Issue URL:
Issue reported by: @roryabraham
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1639498323234800

View all open jobs on GitHub

view this job on upwork

@MelvinBot
Copy link

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

@stitesExpensify stitesExpensify added Weekly KSv2 Improvement Item broken or needs improvement. External Added to denote the issue can be worked on by a contributor and removed Daily KSv2 labels Dec 16, 2021
@MelvinBot
Copy link

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

@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Dec 16, 2021
@stitesExpensify stitesExpensify removed their assignment Dec 16, 2021
@jboniface
Copy link

@botify botify removed the Daily KSv2 label Dec 17, 2021
@MelvinBot MelvinBot added the Weekly KSv2 label Dec 17, 2021
@MelvinBot
Copy link

Triggered auto assignment to @parasharrajat (Exported)

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 17, 2021
@MelvinBot
Copy link

Triggered auto assignment to @deetergp (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@parasharrajat
Copy link
Member

Aha, it seems straightforward so I like to propose here.

  1. Enable this flag
    dangerouslyDisableWhitespaceCollapsing={false}
    .

It will disable the whitespace collapsing.
image

@roryabraham
Copy link
Contributor

@jsamr Any chance you can provide context on why that dangerouslyDisableWhitespaceCollapsing is dangerous?

@deetergp
Copy link
Contributor

@parasharrajat Your proposal seems like an easy fix, let's do it!

I'm curious why it's marked dangerous. Following it to its source in react-native-render-html module it looks like they have a convention of naming any of their experimental methods and properties as dangerous….

@MelvinBot MelvinBot removed the Overdue label Dec 27, 2021
@parasharrajat
Copy link
Member

I can't give you the proper reason why it was marked as dangerous but I think works great for our app and there are no apparant regressions to me.

@jsamr
Copy link
Collaborator

jsamr commented Dec 27, 2021

@jsamr Any chance you can provide context on why that dangerouslyDisableWhitespaceCollapsing is dangerous?

Good spot, that is not documented. It is marked dangerous because its side effects are undetermined (have not been assessed thoroughly, appart from the obvious effect of preserving all whitespaces). "dangerous" here means "use it knowingly and with care, strange things could happen"...

@deetergp deetergp added the Reviewing Has a PR in review label Jan 5, 2022
@MelvinBot MelvinBot removed the Overdue label Jan 5, 2022
@deetergp deetergp removed the Reviewing Has a PR in review label Jan 5, 2022
@deetergp
Copy link
Contributor

deetergp commented Jan 5, 2022

Reviewed, approved, & merged.

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 10, 2022
@botify
Copy link

botify commented Jan 10, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.26-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2022-01-17. 🎊

@botify botify changed the title Chat - Indentation is not preserved in comments [HOLD for payment 2022-01-17] Chat - Indentation is not preserved in comments Jan 10, 2022
@jboniface
Copy link

@parasharrajat looks like we never hired on upwork and the job closed itself. i sent a new one.

@jboniface
Copy link

paid but forgot to close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants