Markdown links don't work properly when there is text in brackets at the beginning of the line

[isagoico]

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

---

Action Performed:

1. Navigate to a conversation
2. Send the following message

[Text text] more text ([link here](www.google.com))

Expected Result:

Message should be displayed like this -> [Text text] more text (link here)

Actual Result:

Message is not correctly hyperlinked. Please check the screenshot

The more text part is incorrectly included in a hyperlink.

Workaround:

N/A

Platform:

Where is this issue occurring?

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

Version Number: 1.0.83-0

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

Notes/Photos/Videos:
Please check the screenshot in Actual Result.

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @rafecolton https://expensify.slack.com/archives/C01GTK53T8Q/p1628560869141100

Markdown links don't work properly when there is text in brackets at the beginning of the line.

[MelvinBot]

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

[mateusbra]

If it could be external I would like to post my proposal 😅

[parasharrajat]

Unfortunately, this is a regression from Expensify/expensify-common#401. We expected a couple of regressions from this PR Expensify/expensify-common#400 (comment).

[mananjadhav]

Yeah this is a regression from m PR Expensify/expensify-common#400. I'll fix it.

[mananjadhav]

I've tried a few regex but some of the test cases break. Give me a day or two to find a solution and close this. @tgolen can you please assign this to me?

[tgolen]

OK, assigned!

[mananjadhav]

@isagoico @tgolen Can you guys help me with additional test cases for Markdown links?
While I was able to fix, [Text text] more text ([link here](www.google.com)) but the following nested cases fail.

1. [link `[inside another link](https://google.com)`](https://google.com)
2. [link with [brackets] inside of it](https://google.com)

I can find a solution to get both the nested cases work but if additional test cases are provided I can give a better solution to avoid any regression later.

[tgolen]

Are nested links a requirement? I don't think so.

I think case 2 in your comment is something we want to support though. How is that different than the case that spawned this issue with brackets at the beginning of the text?

[mananjadhav]

@tgolen I've raised the PR with fixes.
Expensify/expensify-common#407

What I mean by the earlier question was, by default JS regex doesn't support recursion and hence are we looking at any nested options. Anyway, I've got the answer. Please review this PR so that I can update the hash in App and close it.

[tgolen]

@mananjadhav I think there was some miscommunication here. When you asked me to assign it to you, I didn't know that you thought that meant it was OK to work on. This needs to go back through our normal process, so I am going to unassign it from you and we will walk some of this back before I can evaluate your PR.

[MelvinBot]

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

[tgolen]

@arielgreen we goofed up a little bit on the process here. I am OK with the proposal that @mananjadhav gave, but we need to get this to go through UpWork before he can work on it I believe (if @mananjadhav wants to be paid for the fix).

[arielgreen]

@mananjadhav I've just sent over an offer via Upwork.

[mananjadhav]

@tgolen I had worked on the first task of handling few conditions on MD Hyperlink conversion. This PR is meant to handle additional cases, which I treated as a regression from that PR.

This PR covers multiple additional nested cases of brackets etc. as pointed in this comment too (Expensify/expensify-common#407 (review)). Whether this change is eligible for payment or not I am not sure about it.

[parasharrajat]

Actually this issue is caused as a regression from the main PR. That's why @mananjadhav actively participated in it. #4526 (comment)

But it is up to you guys to decide.

[tgolen]

Oh, ha... well, welcome to my chaos roller coaster! Sorry about that confusion, I get it now.

OK, if it's a regression from a previous PR, then this would be part of the original job and there would be no additional payment, so I think we are OK here and I'm assigning back to @mananjadhav and we can go ahead and get the fix merged when it's done being reviewed.

[mananjadhav]

@tgolen I've raised the PR with the screenshots.

There seems to be a problem in rendering the hyperlinks on mobile Apps. You'll notice that they're aligned to the top. IIt isn't due to this PR. I've attached a screenshot where even if I paste a regular link it is aligned at the top.

@parasharrajat Could this be because of the recent upgrade of react-native-render-html

[parasharrajat]

@mananjadhav It could be but You don't need to worry about that in the Current PR. I am in discussions about the issues from the upgrade of react-native-render-html. Hopefully, soon they will be fixed.

[MelvinBot]

@tgolen, @mananjadhav Huh... This is 4 days overdue. Who can take care of this?

[mananjadhav]

PR is merged. Under QA.

[MelvinBot]

@tgolen, @mananjadhav Eep! 4 days overdue now. Issues have feelings too...

[MelvinBot]

@tgolen, @mananjadhav 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[MelvinBot]

@tgolen, @mananjadhav 10 days overdue. I'm getting more depressed than Marvin.

[mananjadhav]

@tgolen Is this safe to close? or add some label? It is deployed to production already as the comment on the PR

[tgolen]

Yeah, I think this is OK to closed since it went to production. The reason it wasn't automatically closed was because in the PR description, it needs the full URL to the GH issue: