Live Markdown Preview #34292

tomekzaw · 2024-01-10T20:51:03Z

Details

This PR adds Live Markdown Preview feature using @expensify/react-native-live-markdown library (which is not open-sourced yet). Currently, the feature targets Android and iOS, but we are already working on Web support.

Fixed Issues

$ #27977
$ #32626
PROPOSAL:

Tests

Testing Replacement Rules

Code Fence:
Type: ```your text```
Expected: Text appears in a separate block formatted as code.

Inline Code Block:
Type: ``your text``
Expected: Text appears inline, formatted as code.

Email Link:
Type: test@email.com
Expected: Email address turns into a clickable mailto link.

URL Link:
Type: https://github.com/Expensify/App/pull/34292
Expected: Displays "#34292" as a clickable web link.

Italics
Type: _your text_
Expected: Text appears in italics.

Bold Text:
Type: *your text*
Expected: Text appears in bold.

Strikethrough Text:
Type: ~your text~
Expected: Text appears with a strikethrough.

Block Quote:
Type: > your text
Expected: Text appears as a block quote.

Heading:
Type: # Your Heading
Expected: Text appears as a large, bold heading.

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

android.mp4

Android: mWeb Chrome

iOS: Native

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-01-31.at.22.21.25.mp4

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

thienlnam · 2024-01-11T02:48:14Z

https://expensify.slack.com/archives/C06BDSWLDPB/p1704940031487879?thread_ts=1704468677.556529&cid=C06BDSWLDPB

thienlnam · 2024-01-11T09:15:36Z

I'm running into this pod install error

[!] Invalid `Podfile` file: cannot load such file -- /Users/jack/Expensidev/App/node_modules/react-native/scripts/react_native_pods.rb

 #  from /Users/jack/Expensidev/App/ios/Podfile:8
 #  -------------------------------------------
 #    # Resolve script with node to allow for hoisting
 >    require Pod::Executable.execute_command('node', ['-p',
 #      "require.resolve(
 #  -------------------------------------------

tomekzaw · 2024-01-11T09:19:44Z

@thienlnam Do you have node_modules installed?

thienlnam · 2024-01-11T09:26:10Z

I do, I've tried removing it and re-installing it but still no dice.
Let's see if @c3024 @shubham1206agra have better luck

shubham1206agra · 2024-01-11T16:41:51Z

I want to make a design recommendation. Maybe we can adapt the design of live markdown shown here of link and quotes.
Right now, quotes have both standing line and > present. Maybe adapt from this design and use a lighter font tone to represent quotes.

c3024 · 2024-01-11T17:32:59Z

For code blocks the markdown is not quite the same as a sent message. I think we can use a different background color like this.

c3024 · 2024-01-11T17:51:13Z

src/hooks/useMarkdownStyle.ts

+            },
+            code: {
+                color: theme.text,
+                backgroundColor: theme.textBackground,


We might consider using a different background color for code blocks so that the markdown in composer appears more consistent with the markdown in sent messages.

tomekzaw · 2024-01-12T09:23:43Z

@shubham1206agra Thanks for your input! We can easily change the blockquote text color if needed, we just need to decide on the actual color.

For the reference, here's what it currently looks like:

tomekzaw · 2024-01-12T09:32:24Z

@c3024 Thanks for your review. I believe the background color is the same in the composer and in the sent message rgb(247,244,240), it just feels different because there is no border around inline code/codeblock in the composer yet. Adding the border (especially with border radius) is not that easy so we skipped it for now.

Another inconsistency is the background under codeblocks. In a sent message, it appears as a single box with its width equal to the maximum width among all lines of text. Currently, both on Android and iOS, the background color appears only under the actual characters of the inline code/codeblock. This is also something that we need to change.

Finally, there is one more inconsistency on iOS related to font family – the text should be in monospace font but it is not. This might be related to the fact that ExpensifyMono is a different font family than ExpensifyNeue. Fortunately, on Android, it works correctly:

@thienlnam Do you think these inconsistencies (missing border, partial background and incorrect font on iOS) are blocking for us? Should we focus on them right now or can we solve them iteratively?

Also, I have one question regarding codeblocks. Currently, ExpensiMark allows to write code directly after opening ```. This is not consistent with this design which seems to exclude the characters directly after opening ``` from the bordered block with background. How should we approach this?

thienlnam · 2024-01-12T12:50:21Z

@thienlnam Do you think these inconsistencies (missing border, partial background and incorrect font on iOS) are blocking for us? Should we focus on them right now or can we solve them iteratively?

Most of these are not blockers and we can keep working on iteratively, but the incorrect font on iOS would say is probably the closest to a blocker

As for the partial background - it is kind of obvious at the moment. I don't think it's a huge deal, but in the meantime until we get the border added could we set the background to be transparent for code blocks so it just doesn't show? (It would just show the enclosing quotes in the different font color similar to how block quotes look right now)

Also, I have one question regarding codeblocks. Currently, ExpensiMark allows to write code directly after opening . This is not consistent with this design which seems to exclude the characters directly after opening from the bordered block with background. How should we approach this?

I haven't been able to take this for a test run yet - but are you saying currently in this build if you type out triple backticks it will automatically start formatting the enclosed text even before the user has added the ending triple backticks? If so, I think having this just for the codeblocks is fine. This is how slack also does it so it will be familiar

tomekzaw · 2024-01-12T13:32:37Z

Most of these are not blockers and we can keep working on iteratively, but the incorrect font on iOS would say is probably the closest to a blocker

Thanks for the response. Okay, let's treat missing monospace font on iOS as a blocker (meaning we need to fix it before this PR gets merged).

As for the partial background - it is kind of obvious at the moment. I don't think it's a huge deal, but in the meantime until we get the border added could we set the background to be transparent for code blocks so it just doesn't show? (It would just show the enclosing quotes in the different font color similar to how block quotes look right now)

Sure, we can set background color to transparent for now until we come up with a proper implementation of background with border.

are you saying currently in this build if you type out triple backticks it will automatically start formatting the enclosed text even before the user has added the ending triple backticks?

No, currently the text following the opening triple backticks is not formatted until you close the triple backticks. However, this idea sounds like a reasonable thing to implement, i.e. the closing backticks for the last blockquote in the message are optional. This way, if the user enters triple backticks, the following text will be formatted with monospace font and we avoid a content "jump" when the last closing backtick is added. I think this would be beneficial for user experience. Let us know what you think, we could implement it in ExpensiMark. cc @robertKozik

What I meant to ask in the original question was where exactly should we place the rectangle with border and background for the following input:

As you can see, aaa is placed in the same line as opening triple backticks, so we cannot start the background in the next line (bbb) because aaa would be outside of it. Similarly, ddd is placed in the same line as closing triple backticks, so we also need to include it in the rectangle.

I think the best approach here would be to disallow any characters after opening triple backticks. Effectively, this would mean that the user needs to add a newline directly after triple backticks in order for the text to be formatted. Similarly, we would need to disallow any characters in the same line before closing triple backticks.

Alternatively, we can place the triple backticks inside the rectangle with background and border (either always or only if there are any characters directly after opening triple brackets).

robertKozik · 2024-01-12T21:02:40Z

Formatting the non-enclosed code block is feasible. However, currently, the user's intent can be ambiguous. For instance, typing ``` followed by any string with Markdown tags might lead to a situation where inside the Markdown input, it is formatted as a code block (with other tags inside not being parsed). Yet, after sending, it will appear as just triple backticks followed by a formatted string.

shubham1206agra · 2024-01-15T15:16:53Z

I think the best approach here would be to disallow any characters after opening triple backticks. Effectively, this would mean that the user needs to add a newline directly after triple backticks in order for the text to be formatted. Similarly, we would need to disallow any characters in the same line before closing triple backticks.

In my opinion, this should be preferred given every platform markdown works like this.

thienlnam · 2024-01-16T03:11:04Z

Thanks, for this PR then let's aim to

fix missing monospace font on iOS
set background color to transparent for codeblocks
Resolve any more major bugs that we find from QA / testing

Re the automatic formatting - we've had a previous discussion in the pre-design regarding this and decided not to automatically format any of the text until the ending text for the initial version (discussion here. Something I think we can revisit once we've had some time using this

I think the best approach here would be to disallow any characters after opening triple backticks. Effectively, this would mean that the user needs to add a newline directly after triple backticks in order for the text to be formatted. Similarly, we would need to disallow any characters in the same line before closing triple backticks.

Agreed, I think we can go with this

tomekzaw · 2024-01-18T21:57:26Z

I've pushed fixes for these two issues:

fix missing monospace font on iOS
set background color to transparent for codeblocks

iOS (dark theme):

iOS (light theme):

Android (dark theme):

Android (light theme):

tomekzaw · 2024-01-19T22:00:11Z

I've pushed a commit that updates the dependency. Can we please run an adhoc build and test the feature? cc @thienlnam

shubham1206agra · 2024-01-21T18:14:03Z

@thienlnam @tomekzaw I am unable to access the live markdown repo now. Can you give us access?

github-actions · 2024-01-22T01:24:47Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
❌ FAILED ❌	❌ FAILED ❌
The QR code can't be generated, because the android build failed	The QR code can't be generated, because the iOS build failed
Desktop 💻	Web 🕸️
❌ FAILED ❌	❌ FAILED ❌
The QR code can't be generated, because the Desktop build failed	The QR code can't be generated, because the web build failed

👀 View the workflow run that generated this build 👀

shubham1206agra · 2024-01-23T06:39:45Z

@tomekzaw Can you merge main?

tomekzaw · 2024-01-23T09:30:02Z

@shubham1206agra Done

shubham1206agra · 2024-01-23T09:50:22Z

Underline seems to be on wrong place on using []() syntax for link. Should be discussed as slack has the original one.

shubham1206agra · 2024-01-23T09:59:57Z

Screen.Recording.2024-01-23.at.3.28.08.PM.mov

@tomekzaw Typing mention tries to open mention dialog again and again. It has a weird stuttering too when I tried to clear input using Backspace

shubham1206agra · 2024-01-23T10:03:50Z

Screen.Recording.2024-01-23.at.3.33.22.PM.mov

shubham1206agra · 2024-02-01T07:59:42Z

This weird thing happening with display name when I send this particular message

This is still not fixed

@tomekzaw I am able to repro this on desktop main. You can skip this for now.

shubham1206agra · 2024-02-01T08:01:42Z

We have difference in output and preview for this. May need to decide which is correct. Strike and italics have same as preview.

This is also not fixed

I believe this is a problem with ExpensiMark instead of preview. But we need to fix this to make it consistent across.

tomekzaw · 2024-02-01T08:13:17Z

Thanks @shubham1206agra for pointing out these bugs, I must have overlooked them!

Invalid layout of nested blockquotes on Android Live Markdown Preview #34292 (comment) – this is indeed a bug but also an edge-case so let's treat is as non-blocking and fix in a follow-up PR
Inconsistency of formatting between composer and history – this is because Live Markdown uses a different (newer) version of ExpensiMark than App; there's already a PR that bumps ExpensiMark in the app and we want to avoid git conflicts; we will align the versions in a follow-up PR
Missing display name when sending particular message Live Markdown Preview #34292 (comment) – we were also not able to reproduce; skipping this as you suggested

tomekzaw · 2024-02-01T09:05:41Z

Merged main. FYI the tests are failing because of this error:

npm WARN tarball tarball data for react-native-flipper@https://gitpkg.now.sh/facebook/flipper/react-native/react-native-flipper?9cacc9b59402550eae866e0e81e5f0c2f8203e6b
(sha512-M784S/qPuN/HqjdvXg98HIDmfm0sF8mACc56YNg87nzEF90zKSKp0XyOE83SEW+UJX2Gq/rf9BvM2GZeXlrhnQ==)
seems to be corrupted. Trying again.

robertKozik · 2024-02-01T11:54:19Z

We have difference in output and preview for this. May need to decide which is correct. Strike and italics have same as preview.

This is also not fixed

I believe this is a problem with ExpensiMark instead of preview. But we need to fix this to make it consistent across.

@shubham1206agra Can you check one more time? I've build the latest version of this branch and now output and preview is consistent

shubham1206agra · 2024-02-01T13:22:52Z

@robertKozik This is fixed now.

c3024 · 2024-02-01T17:24:54Z

I tried all common patterns and whatever combinations I could think of

livemarkdownChrome.mp4

LGTM

thienlnam · 2024-02-01T17:28:05Z

We're close to merging - checks are failing for all PRs here https://expensify.slack.com/archives/C01GTK53T8Q/p1706808000490579 but once that is resolved we should be good

c3024 · 2024-02-01T18:01:05Z

Sorry, I just found this issue.

Screen.Recording.2024-02-01.at.11.28.52.PM.mov

If the last line happens to be with triple back ticks previous lines' formatting is lost. In this video the website link's formatting is lost.

Please check.

@tomekzaw @robertKozik

tomekzaw · 2024-02-01T18:11:22Z

@c3024 Thanks for finding and reporting this issue. There is one case when we deliberately skip formatting: when we are unable to restore the original Markdown string from HTML returned by ExpensiMark. It looks like you have found a Markdown message that, once converted into HTML and back to Markdown, is different than the original input. This is not an issue with MarkdownTextInput component itself; it's an issue with our ExpensiMark->JS parser pipeline. This is something that we can fix after merging or even skip fixing this if we want to use a different Markdown parser in near future.

cc @thienlnam

OSBotify · 2024-02-01T19:23:54Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-02-05T22:53:18Z

🚀 Deployed to production by https://github.com/marcaaron in version: 1.4.36-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

alitoshmatov · 2024-06-19T10:26:23Z

src/hooks/useMarkdownStyle.ts

+                borderColor: theme.border,
+                borderWidth: 4,
+                marginLeft: 0,
+                paddingLeft: 6,


Blockquote element has style of inline-block which is forcing cursor to be inside of this block and overlap with characters. We solved this by adding padding-right so that cursor has some space for itself

FYI paddingRight is a no-op on mobile (Android & iOS), it works only on web.

thienlnam self-requested a review January 11, 2024 02:09

thienlnam assigned tomekzaw Jan 11, 2024

thienlnam added the Ready To Build label Jan 11, 2024

This comment has been minimized.

Sign in to view

thienlnam requested review from shubham1206agra and c3024 January 11, 2024 09:33

c3024 reviewed Jan 11, 2024

View reviewed changes

AndrewGable mentioned this pull request Jan 22, 2024

Testing Live Markdown Preview #34905

Closed

Merge branch 'main' into @tomekzaw/live-markdown

5ed7798

tomekzaw added 3 commits February 1, 2024 11:28

Re-run tests

908dbb9

Merge branch 'main' into @tomekzaw/live-markdown

f3f778f

Merge branch 'main' into @tomekzaw/live-markdown

f475b98

Merge branch 'main' into @tomekzaw/live-markdown

d768bee

Merge branch 'main' into @tomekzaw/live-markdown

e8c51e2

thienlnam merged commit 63ba91f into Expensify:main Feb 1, 2024
16 checks passed

tomekzaw mentioned this pull request Feb 2, 2024

[HOLD for payment 2024-04-09] Live Markdown Preview #27977

Closed

20 tasks

tomekzaw deleted the @tomekzaw/live-markdown branch February 2, 2024 10:51

github-actions bot mentioned this pull request Feb 2, 2024

Deploy Checklist: New Expensify 2024-02-02 #35673

Closed

Skalakid mentioned this pull request Feb 5, 2024

Live Markdown Preview for web #35837

Merged

50 tasks

robertKozik mentioned this pull request Feb 6, 2024

Reintroduce animated input for Markdown composer #35973

Merged

50 tasks

kbecciv mentioned this pull request Feb 9, 2024

Composer - Email is not turned to a clickable link, and GitHub link is not parsed to number #36253

Closed

6 tasks

tomekzaw mentioned this pull request Feb 12, 2024

Text in the composer is very small when device font size is minimum #35787

Closed

6 tasks

thienlnam mentioned this pull request Feb 16, 2024

[$500] Payment for review PR #34292 #36650

Closed

joekaufmanexpensify mentioned this pull request Mar 5, 2024

Chat - Quote in live preview isn't replaced by quote sign but is supplemented by this sign #37735

Closed

6 tasks

alitoshmatov reviewed Jun 19, 2024

View reviewed changes

alitoshmatov mentioned this pull request Jun 19, 2024

[HOLD for payment 2024-06-20] [$250] Web - Markdown - Cursor overlaps with certain letters if quote markdown is applied #42196

Closed

6 tasks

Live Markdown Preview #34292

Live Markdown Preview #34292

Conversation

tomekzaw commented Jan 10, 2024 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

This comment has been minimized.

thienlnam commented Jan 11, 2024

thienlnam commented Jan 11, 2024 • edited Loading

tomekzaw commented Jan 11, 2024

thienlnam commented Jan 11, 2024

shubham1206agra commented Jan 11, 2024

c3024 commented Jan 11, 2024 • edited Loading

c3024 Jan 11, 2024

Choose a reason for hiding this comment

tomekzaw commented Jan 12, 2024

tomekzaw commented Jan 12, 2024 • edited Loading

thienlnam commented Jan 12, 2024

tomekzaw commented Jan 12, 2024

robertKozik commented Jan 12, 2024

shubham1206agra commented Jan 15, 2024

thienlnam commented Jan 16, 2024

tomekzaw commented Jan 18, 2024

tomekzaw commented Jan 19, 2024

shubham1206agra commented Jan 21, 2024

github-actions bot commented Jan 22, 2024

shubham1206agra commented Jan 23, 2024

tomekzaw commented Jan 23, 2024

shubham1206agra commented Jan 23, 2024 • edited Loading

shubham1206agra commented Jan 23, 2024 • edited Loading

shubham1206agra commented Jan 23, 2024

shubham1206agra commented Feb 1, 2024

shubham1206agra commented Feb 1, 2024

tomekzaw commented Feb 1, 2024

tomekzaw commented Feb 1, 2024 • edited Loading

robertKozik commented Feb 1, 2024

shubham1206agra commented Feb 1, 2024

c3024 commented Feb 1, 2024

thienlnam commented Feb 1, 2024

c3024 commented Feb 1, 2024 • edited Loading

tomekzaw commented Feb 1, 2024

OSBotify commented Feb 1, 2024

OSBotify commented Feb 5, 2024

alitoshmatov Jun 19, 2024

Choose a reason for hiding this comment

tomekzaw Jun 20, 2024

Choose a reason for hiding this comment

tomekzaw commented Jan 10, 2024 •

edited

Loading

thienlnam commented Jan 11, 2024 •

edited

Loading

c3024 commented Jan 11, 2024 •

edited

Loading

tomekzaw commented Jan 12, 2024 •

edited

Loading

shubham1206agra commented Jan 23, 2024 •

edited

Loading

shubham1206agra commented Jan 23, 2024 •

edited

Loading

tomekzaw commented Feb 1, 2024 •

edited

Loading

c3024 commented Feb 1, 2024 •

edited

Loading