Expensify / react-native-live-markdown

Drop-in replacement for React Native's TextInput component with Markdown formatting.
https://www.npmjs.com/package/@expensify/react-native-live-markdown
MIT License
685 stars 46 forks source link

Update expensify-common and remove appending the newline to the codefence content #413

Closed bernhardoj closed 2 weeks ago

bernhardoj commented 2 weeks ago

Details

The new expensify-common includes the codefence newline as the content/children, so we don't need to append the newline manually anymore.

Related Issues

https://github.com/Expensify/App/issues/43750

Manual Tests

Local test in live-markdown example:

  1. Type a codefence with \r\n as the newline
  2. Verify there is no Parsing error: the processed text does not match the original Markdown input. error.

Expensify test:

  1. Submit an expense with code block as description
  2. Open the transaction thread
  3. Open the description
  4. Verify there is no Parsing error: the processed text does not match the original Markdown input. error.
tomekzaw commented 2 weeks ago

cc @Skalakid

s77rt commented 2 weeks ago

@bernhardoj I just noticed that \r\n is not supported. That's something we should fix.

Screenshot 2024-07-03 at 1 06 34 PM Screenshot 2024-07-03 at 1 06 46 PM
s77rt commented 2 weeks ago

Btw did you get the parsing error on main? (without the fix)

bernhardoj commented 2 weeks ago

I can't get the native example app running, keep running into error, but I get the parsing error on web,

https://github.com/Expensify/react-native-live-markdown/assets/50919443/735e73af-2a45-4277-8910-9ba389216d3f

and the markdown style is also applied in web.

image
s77rt commented 2 weeks ago

On web it works fine. On native the newlines are formatted

Screenshot 2024-07-03 at 6 59 33 PM Screenshot 2024-07-03 at 6 59 37 PM
bernhardoj commented 2 weeks ago

I can finally run the examples on native by reinstalling the node_modules.

On native the newlines are formatted

I found that it's an issue on the native side itself.

image
Skalakid commented 2 weeks ago

Hmm, interesting I didn't caught this bug on Android, @bernhardoj can you look into this? cc @tomekzaw

tomekzaw commented 2 weeks ago

For now, E/App doesn't use backgroundColor for codeblocks so that's not an actual issue.

Another fix might be to move the start index of the codeblock to the next position so it doesn't include the newline character. This can be done on the parser/formatter side in JS.

Finally, we're working on improving the behavior of backgroundColor for inline code and codeblocks so that the whole block has a background color.