User can use basic markdown

[holmesworcester]

Jason and I decided we should use other open source apps as inspiration for styles. For example https://github.com/mattermost/

It should look good on desktop and mobile, which means finding libraries for both React and React Native.

Related to #1025

The most important thing is that this should work and

this...
should work

Slack does not implement "#" and "##" and "------" for H1, H2, etc and I think we should also not do this.

Bold and [link](https://link.com) should work too.

[josephlacey]

@holmesworcester A couple questions about the scope for this feature based on the implementation in GitHub, Mattermost and Slack.

• You've indicated a couple of parts of the Markdown spec to exclude. Do we just blacklist these and include everything else? Or do we only whitelist selected parts? These are the common parts of the spec implemented by the three comparative examples.
  • Bold
  • Italics
  • Strikethrough
  • Order list
  • Unordered list
  • Links
  • Inline code
  • Code blocks
  • Quotes
• Are we including a preview option?
  • GitHub includes a preview tab.
  • Mattermost doesn't offer a preview, but you can edit messages in case the styling isn't correct.
  • Slack sort of fakes a preview by changing the input as you type, so typing **This is bold text** changes to This is bold text before sending.
• Are we incorporating key bindings, like Ctrl+B for bold? GitHub, Mattermost and Slack all do this.
• Are we including a toolbar with icons? Github, Mattermost and Slack all have one.

---

Possible options for rendering libraries.

Desktop

• https://github.com/remarkjs/react-markdown, seems to be the ubiquitous one.
• https://github.com/uiwjs/react-md-editor, a simpler alternative
• https://github.com/ianstormtaylor/slate, a generic text editor, not markdown-specific

Mobile

• https://github.com/iamacup/react-native-markdown-display
  • Fork, with some updated compatibilities: https://github.com/jonasmerlin/react-native-markdown-display/
• https://github.com/andangrd/react-native-markdown-package
• https://github.com/CharlesMangwa/react-native-simple-markdown

[holmesworcester]

Someone we want to try quiet specifically requested the stuff I mention in the description: inline code, code blocks, links, and bold. We could throw in Italic and strikethrough, or quotes, if those are easy.

Preview, key bindings, and toolbar are definitely features we can tackle separately and not necessary here.

It seems like there are two approaches here: implement the features one by one, or grab some library and do what's easy with the library. The requirement that we get the above features working means we should focus on rending the markdown and not creating it... we can let the user do that themselves in a text field, and later we can implement editing and the other stuff.

We might want to see what library Mattermost uses and use that. I'm relatively agnostic as to the library. We'll need to pick one for React Native as well, because we can't just use webviews for each message in React Native, I don't think.

[josephlacey]

Preview, key bindings, and toolbar are definitely features we can tackle separately and not necessary here.

Agreed.

grab some library and do what's easy with the library. The requirement that we get the above features working means we should focus on rending the markdown and not creating it... we can let the user do that themselves in a text field, and later we can implement editing and the other stuff.

This second approach sounds right to me. I'll test what's out of the box with the libraries I found, and compare parity between the desktop and mobile options.

[josephlacey]

@holmesworcester Just an intermediate update on this. I've got a working version for desktop locally. The react-markdown library proved most compatible to our build. (I also tested react-remark, but it didn't have some of the customizations we needed.)

The implementation includes all the features of the markdown library except the blacklisted elements, namely heading tags and the horizontal rule. I also had to remove paragraph tags because it was adding them to all markdown converted messages and creating a design spacing issue.

Handling links required a little bit of work. It's not possible to pass the converted markdown to Linkify, so my working version removes that library from the TextMessage component and reimplements links with the markdown library.

My next steps are to find a React Native library that will handle the mobile implementation similarly.

[josephlacey]

For the mobile implementation, I went with react-native-markdown-package because it has commits within the last year and allows the configurability we need to maintain our own design and keep it synced with the desktop implementation.

I've rewritten the default styles to remove additional heading styles and to maintain the current design. One still missing parts is removing the horizontal rule rendering. I'll need to dig into this a little more to see if I can write a custom rule for rendering it.

Links also needed a reimplementation, like they did with the desktop. I removed Linkify and made the basic styling changes, but the openUrl function that's passed into the component isn't compatible with the link handling of the markdown library. The latter is written to expect a Promise when clicking a link. Instead of using our openUrl function, I've switched over to the default react-native openURL function, which the markdown library recommends. But I'm not sure what our custom function does other than opening URLs, so this change may have implications I'm not aware of.

[holmesworcester]

Great! For openURL I think we had our own function there because in an earlier version of the app we were catching external links and displaying a warning, similar to Discord. We have an open issue to add this back in but no immediate plans to work on it, so we can worry about this in the future.

[holmesworcester]

Also I'm okay with a horizontal line if it's easier to keep it than to remove it.

Though we should make it consistent.

Can you post screenshots of a complex markdown message? We'll probably want some chromatic/storybook tests to make sure we don't mess up this style.

[josephlacey]

Can you post screenshots of a complex markdown message? We'll probably want some chromatic/storybook tests to make sure we don't mess up this style.

Here are the screenshots of the testing input without markdown rendering.

[josephlacey]

Here's the desktop rendering,

[josephlacey]

Here's the mobile rendering

[josephlacey]

The differences between the libraries with their out-of-the-box rendering that I'm seeing with these testing snippets,

• List indentation
• Code block rendering: type face, background color and styling
• Horizontal rule spacing
• Quote blocks without a space between preceding text. Desktop renders the quote correctly, mobile doesn't.
• Table rendering: header background color, column spacing and borders
• Image justification

One other detail: it looks like current desktop rendering converts tildes to dashes.

If there's an existing style guide I can reference, I can start implementing that. Otherwise I can make some inferences from the existing style or check the style implementations from GitHub, Mattermost and Slack.

[holmesworcester]

The only thing that looks really bad to me here is the quote styling on desktop. Just indenting like this might work for longer form text but not within short messages. Making desktop look like mobile in this respect would be great. This is what I mean, to be clear:

The lack of consistency with when quotes are rendered seems like a problem too, but a somewhat less serious one. Is there an easy fix?

We should also disable the rendering of images, since that's a security issue (you could use it to get an IP, right now, or perhaps send a malicious image) and pay close attention to how this is done so that it's not possible to work around it.

I think other than that we can start with the default styling and be responsive to user requests for changes. Maybe code blocks will need some styling too; not sure.

Also, can you put some real code (a long chunk) in the code block so I can see something more realistic? It's difficult to tell now if I think that code blocks are good enough to release.

[holmesworcester]

Also if it's easy to style these we should standardize around what Mattermost does.

But just using the library defaults might be easier to maintain going forward and more reliable?

[holmesworcester]

Also we currently support in-line LaTeX formulae in messages using $$...$$. How does this play with markdown, currently? It would be good to make that part of our storybook / test messages.

[MrLuxuri]

Hey guys @holmesworcester @josephlacey! Coming from discord, this is how markdown is used by everyone there, maybe we can takeaway something good from here :)

[holmesworcester]

@MrLuxuri do people use the headings a lot? I guess I have seen those but how big a thing is it? Slack chose not to use them so I'm on the fence.

[maxvonhippel]

@MrLuxuri do people use the headings a lot? I guess I have seen those but how big a thing is it? Slack chose not to use them so I'm on the fence.

I don't use headings very often, but they could be useful e.g. if you have a long pinned post at the top of a channel? For instance if people were using Quiet as something more like a weird mix between Discord and Reddit, I could imagine certain communities having laundry lists of information or rules that they pin to the top of a channel, and headings would help with that. But for day-to-day communication, my opinion is that code, italics, bold, underline, strikethrough, quote blocks, and links are the most useful features.

[maxvonhippel]

One other thought - this might cause some unexpected issues when combined with the latex feature. For example, who wins in this scenario?

`$x_0$`

Or this one?

$`x`$

It would be worth testing a bit for strange edge cases before merging.

[josephlacey]

@holmesworcester Inline updates for each of these. I think these address the other comments from the initial set of screenshots except LaTeX, which I'll respond to separately.

The only thing that looks really bad to me here is the quote styling on desktop. Just indenting like this might work for longer form text but not within short messages. Making desktop look like mobile in this respect would be great. This is what I mean, to be clear:

Here's a bunch of the styling cleaned on Desktop.

The lack of consistency with when quotes are rendered seems like a problem too, but a somewhat less serious one. Is there an easy fix?

I've basically done the above styling clean up for mobile, but there's a current gotcha with it. More below. The short answer though is yes, we have pretty fine-grained control over the rendering of any element once the Markdown library converts them to HTML.

We should also disable the rendering of images, since that's a security issue (you could use it to get an IP, right now, or perhaps send a malicious image) and pay close attention to how this is done so that it's not possible to work around it.

This is the gotcha. With desktop rendering, this is fixed. You'll see this in the above the screenshot, but with mobile rendering, the library doesn't allow this manipulation as easily. I'm going to look at other libraries to see if another will allow this more easily.

Also, can you put some real code (a long chunk) in the code block so I can see something more realistic? It's difficult to tell now if I think that code blocks are good enough to release.

More realistic code blocks included in the above screenshot. If you want something more complex or with a particular language, let me know.

[josephlacey]

@holmesworcester @maxvonhippel

Also we currently support in-line LaTeX formulae in messages using $$...$$. How does this play with markdown, currently? It would be good to make that part of our storybook / test messages.

One other thought - this might cause some unexpected issues when combined with the latex feature. For example, who wins in this scenario?

`$x_0$`

Or this one?

$`x`$

It would be worth testing a bit for strange edge cases before merging.

The LaTeX implementation has precedence over Markdown. On desktop the rendering checks if LaTeX is present, and if so, handles that, and then passes the remaining parts of the message to the regular text handling. The Markdown library is wrapper around that.

These were the test cases,

• $$f(x) = x^2$$
• $$F(x) = \int^a_b \frac{1}{3}x^3$$
• $$x_0$$
• $$``x``$$ Note: just a single tick surrounding the x. I'm just compensating for GitHub's Markdown here.
• This is **bold text** along some LaTeX $$f(x) = x^2$$. And [here's a link](https://github.com/TryQuiet/quiet/issues/1088)

Mobile rending though doesn't pass the non-LaTeX parts to the Markdown renderer. I need to check this.

[holmesworcester]

Also I'm persuaded by max that headings are worth allowing. Let's allow them if that's easy, and use font sizes from Discord or Mattermost.

[josephlacey]

@holmesworcester

We could ship markdown on desktop first, if the image loading issue requires finding a different library on mobile. Markdown is designed to be backwards compatible with plaintext so it's not so bad as a temporary state of affairs and having it on desktop adds value.

I've been working on them side by side, so the commits are entangled on the feature branch. But I've basically got it all ready for review. There's one final issue that the new library introduced. A weird text wrapping problem. Everything else though, including the custom handling of images, is working locally. I pushed a bunch of commits for that work yesterday and will hopefully have the wrapping issue fixed and new screenshots up later today.

I did get to the bottom of the mixed LaTeX and Markdown issue on mobile. There's a conditional in the message rendering that sends the message down either path, not both like on Desktop. I assume we'll push this to another issue.

Also I'm persuaded by max that headings are worth allowing. Let's allow them if that's easy, and use font sizes from Discord or Mattermost.

I've reenabled Headings.

[holmesworcester]

Can you post a screenshot of the wrapping problem? How bad is it?

You have a clear path to fixing it?

[josephlacey]

Can you post a screenshot of the wrapping problem? How bad is it?

Yeah, it's pretty bad. Seem the line with the inline code and the image line below that. No lines are wrapping.

You have a clear path to fixing it?

I do. The surrounding Typography component seems to be the culprit. Still getting to the bottom of exactly what's amiss.

[josephlacey]

Update desktop screenshots

[josephlacey]

Updated mobile screenshots

[josephlacey]

@holmesworcester

Can you post a screenshot of the wrapping problem? How bad is it?

Yeah, it's pretty bad. Seem the line with the inline code and the image line below that. No lines are wrapping.

You have a clear path to fixing it?

I do. The surrounding Typography component seems to be the culprit. Still getting to the bottom of exactly what's amiss.

Fixes pushed. I migrated the Typography component to within a custom paragraph rendering rule.

I think that addresses all everything on the list from earlier in this ticket, but send over any changes or thoughts.

I am going to review everything in these two libraries to make sure they aren't introducing any security holes with unaudited features. Namely I saw a boolean flag for includeHTML. I tested some obvious cases to check that this didn't just render HTML, and it didn't, but I want to take a closer look.

[josephlacey]

@holmesworcester

I am going to review everything in these two libraries to make sure they aren't introducing any security holes with unaudited features. Namely I saw a boolean flag for includeHTML. I tested some obvious cases to check that this didn't just render HTML, and it didn't, but I want to take a closer look.

A couple of notable features.

• Desktop
  • Links support tel: and mailto:. In general I've always disliked these two, but nothing happens without a secondary confirmation, so they should be fine.
  • To replicate the features of Linkify, we're using the remark-gfm package, which adds the GitHub Markdown spec. In addition to autolinking URI's, strike-through and tables, it also includes footnotes and task lists. Neither of these additional features seem worrisome.
• Mobile
  • Links support tel: and mailto: as well.
  • The package we're using on mobile includes Linkify. Enabling it relies on markdown-it, which provides the additional GitHub spec support for strike-through and tables. Footnotes and task lists aren't supported by default though, so these would be cases where the rendering would differ between desktop and mobile. All other options for this secondary package are disabled, including the default enabled typographer feature which handles special character transformations, like copyright (c), trademark (tm), etc.

Two general notes about both packages,

• Both ignore HTML rendering by default and state XSS holes have been closed with the default configuration, so we should be fine here. The desktop library notes a secondary package option for sanitizing the markdown if there's a desire to be doubly sure.
• Both have methods to simply disable support for specific types. That disabling though, in our case for images, doesn't render anything. That could be seen as a bug, so I wrote custom handling rules to just output the text of the intended image. Those custom rules are copies of the package implementation with requisite changes to output text.

Otherwise I think this is ready for review.

[holmesworcester]

@josephlacey I'm sorry we've been so slow on getting this reviewed. I didn't want it to block the latest release, and this release has dragged out. Will get review on this soon and hopefully we can release very soon.

@EmiM are there likely to be interactions with your typescript work? Can you lead on getting that resolved?

[holmesworcester]

@josephlacey we have to fix failing tests here to get this through. Can you follow up on this as you have time? I'm moving this back to "in progress."

[josephlacey]

@josephlacey we have to fix failing tests here to get this through. Can you follow up on this as you have time? I'm moving this back to "in progress."

@holmesworcester I took a look at these. I think they fall into three categories. I'll order these easiest to hardest to fix

• The Visual Regression test is Chromatic, and I just need to send that manually. I'll do that after I get these others sorted.
• The Scroll Regression test I couldn't see the details of because my local copy is missing the base images for comparison. Not sure why that is, but I pushed a couple of new commits to rerun the CI tests to pull down the assets from those, but there's currently an error on all the tests now. Is this happening on all commits?

ERR! bootstrap The "bootstrap" command was removed by default in v7, and is no longer maintained.
ERR! bootstrap Learn more about this change at https://lerna.js.org/docs/legacy-package-management.

• The Desktop and Mobile Tests have the same error, "SyntaxError: Unexpected token 'export'". There's an issue on the library related to it that offers three possible solutions to fix it. Only 1 and 3 are feasible. Solution 1 is just to suspend the usage of markdown during the Unit Testing. This would allow the testing to go forward but as noted has the downside of not testing the functionality. Solution 3 requires some refactoring of the testing-related configs and then include the parts of the library to Jest without transformations. Looking for guidance about what y'all think is the best approach here.
  • If solution 3, that begs the question of creating tests and what exactly we want to test. Bold, Italics, Quotes, Code Blocks, Images and Links?

[holmesworcester]

The solution in the link above seems to be to downgrade to a previous version. Let's do that until ESM isn't an issue here. @josephlacey you can do this or we can do this on our end.

[holmesworcester]

We are okay with the solution where react-markdown is mocked, also.

[josephlacey]

@holmesworcester

The solution in the link above seems to be to downgrade to a previous version. Let's do that until ESM isn't an issue here. @josephlacey you can do this or we can do this on our end.

We are okay with the solution where react-markdown is mocked, also.

As I thought might be the case, version 6 of the library requires React 17, but we're on React 18, downgrading isn't a possible solution. Mocking the tests is probably the quickest solution, so I can start working on that.

[holmesworcester]

@vinkabuki you worked on this, correct? Can you add your notes to the issue?

[holmesworcester]

@josephlacey an update: vinkabuki has been out for a bit but will return to this once he's back.

[kingalg]

Below you can find some markdowns with a comment if they are working or not. It will be divided between multiple comments as Github doesn't accept so many attachments in one comment.

Testing examples from: https://www.markdownguide.org/basic-syntax/#:~:text=To%20create%20an%20unordered%20list,to%20create%20a%20nested%20list.

1. Headings 

Working on desktop and mobile

Heading level 1 | Heading level 1

-- | --

Heading level 2 | Heading level 2

Heading level 3 | Heading level 3

Heading level 4 | Heading level 4

Heading level 5 | Heading level 5

Heading level 6 | Heading level 6

[kingalg]

Line Breaks

Working on desktop but not on mobile

To create a line break or new line end a line with two or more spaces, and then type return.

Markdown	Rendered Output
This is the first line.  And this is the second line.	This is the first line.And this is the second line.

[kingalg]

Italic

Working on desktop and mobile

To italicize text, add one asterisk or underscore before and after a word or phrase. To italicize the middle of a word for emphasis, add one asterisk without spaces around the letters.

Markdown	Rendered Output
Italicized text is the cat's meow.	Italicized text is the cat’s meow.
Italicized text is the cat's meow.	Italicized text is the cat’s meow.
Acatmeow	Acatmeow

[kingalg]

 Paragraphs

Not working on desktop and mobile

Markdown	Rendered Output
I really like using Markdown.I think I'll use it to format all of my documents from now on.	I really like using Markdown.I think I'll use it to format all of my documents from now on.

[kingalg]

Bold and Italic

To emphasise text with bold and italics at the same time, add three asterisks or underscores before and after a word or phrase. To bold and italicize the middle of a word for emphasis, add three asterisks without spaces around the letters.

Markdown	Rendered Output
This text is really important.	This text is really important.
This text is really important.	This text is really important.
This text is really important.	This text is really important.
This text is really important.	This text is really important.
This is reallyveryimportant text.	This is reallyveryimportant text.

[kingalg]

Ordered Lists

Working on desktop and mobile

To create an ordered list, add line items with numbers followed by periods. The numbers don’t have to be in numerical order, but the list should start with the number one.

Markdown	Rendered Output

1. First item2. Second item3. Third item4. Fourth item | First itemSecond itemThird itemFourth item
2. First item1. Second item1. Third item1. Fourth item | First itemSecond itemThird itemFourth item
3. First item8. Second item3. Third item5. Fourth item | First itemSecond itemThird itemFourth item
4. First item2. Second item3. Third item    1. Indented item    2. Indented item4. Fourth item | First itemSecond itemThird itemIndented itemIndented itemFourth item

[kingalg]

Unordered Lists

To create an unordered list, add dashes (-), asterisks (*), or plus signs (+) in front of line items. Indent one or more items to create a nested list.

Working on desktop and mobile (except for the last item in the table). 

[kingalg]

Links
To create a link, enclose the link text in brackets (e.g., [Duck Duck Go]) and then follow it immediately with the URL in parentheses (e.g., (https://duckduckgo.com)).
My favorite search engine is Duck Duck Go.

[kingalg]

Fenced Code Blocks
Working on desktop and mobile

[kingalg]

Version: 2.0.0-alpha.7
Comments above - working.