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

Allow more lenient code fencing with new lines #376

Merged
merged 5 commits into from
May 24, 2021

Conversation

jasperhuangg
Copy link
Contributor

@jasperhuangg jasperhuangg commented May 24, 2021

Details

Our code fencing previously only worked if there were newlines surrounding the wrapped text. This changes it so that you don't need newlines (just like what Slack offers). See the issue for more context.

e.g:

```
hi
```
and ```hi``` are the same.

Fixed Issues

$ Expensify/App#3039

Tests

  • Expensimark-test covers all my changes. I've updated each of the tests for code fencing to include each possible case (see QA)

QA

In Expensify.cash, try to send this as a message, and verify it renders the same as the screenshot:

```hi```
```hi
```
```
hi```
```
hi
```

Screen Shot 2021-05-24 at 8 38 41 PM

Screen Captures

Web

2021-05-24_19-20-04.mp4

Mobile Web

2021-05-24_19-42-48.mp4

iOS

2021-05-24_19-40-34.mp4

Android

1621913590738637.mp4

Desktop

2021-05-24_19-23-36.mp4

@jasperhuangg jasperhuangg self-assigned this May 24, 2021
@github-actions
Copy link

github-actions bot commented May 24, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@jasperhuangg
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@jasperhuangg jasperhuangg requested a review from a team May 24, 2021 12:34
@MelvinBot MelvinBot requested review from bondydaa and removed request for a team May 24, 2021 12:34
@jasperhuangg jasperhuangg marked this pull request as ready for review May 24, 2021 12:42
@jasperhuangg jasperhuangg requested a review from a team as a code owner May 24, 2021 12:42
@MelvinBot MelvinBot removed the request for review from a team May 24, 2021 12:42
Copy link
Contributor

@bondydaa bondydaa left a comment

Choose a reason for hiding this comment

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

comparing the old regex
image

and the new one
image

seems to check out 👍

I'm going to tap the resident regexer @timszot to double check me though (I'm 🤞 that he also has his android dot cash environment set up properly to test as well)

@bondydaa bondydaa requested a review from timszot May 24, 2021 21:21
Copy link
Contributor

@timszot timszot left a comment

Choose a reason for hiding this comment

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

My android e.cash dev setup is also not working, but with the added tests here this looks good to me.

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.

3 participants