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

Implement code and pre blocks support on web #341

Closed
wants to merge 12 commits into from

Conversation

BartoszGrajdek
Copy link
Collaborator

@BartoszGrajdek BartoszGrajdek commented May 7, 2024

Details

Adds support for displaying and customising code and pre blocks on web.

Related Issues

GH_LINK

Manual Tests

  1. Change regex for codeFence in ExpensiMark to /(```(?:\r\n|\n))((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))(```)/g
  2. Run npm run build in /parser
  3. You should now be able to test it out 🙌🏻

Linked PRs

@@ -49,12 +52,17 @@ function addStyling(targetElement: HTMLElement, type: MarkdownType, markdownStyl
});
break;
case 'code':
Object.assign(node.style, markdownStyle.code);
Object.assign(node.style, {...markdownStyle.code, lineHeight: 1.4});
Copy link
Collaborator Author

@BartoszGrajdek BartoszGrajdek May 7, 2024

Choose a reason for hiding this comment

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

I believe this is something we should discuss. We can't set the display to inline-block or block because it will break how we are positioning inline-code blocks.

Since we're not able to do so margin won't work on inline-code block. That's why I decided to set the line-height to 1.4 - In my opinion it looks alright but let me know what do you think 😄

Screen Shot image

src/web/parserUtils.ts Outdated Show resolved Hide resolved
@Skalakid
Copy link
Collaborator

Skalakid commented May 9, 2024

Found a couple of bugs while testing:

1. It's impossible to remove the newline between the first syntax `backticks` and `code block`
Screen.Recording.2024-05-09.at.09.35.48.mov
2. After removing whole content from code block, there are some new lines added
Screen.Recording.2024-05-09.at.09.44.26.mov
3. Strange selection behavior when trying to select whole `codeblock`
Screen.Recording.2024-05-09.at.09.46.43.mov
4. Cursor is wrongly positioned when removing syntax backticks at the end of `codeblock`
Screen.Recording.2024-05-09.at.09.51.02.mov

@BartoszGrajdek BartoszGrajdek requested a review from Skalakid May 20, 2024 11:46
@BartoszGrajdek BartoszGrajdek marked this pull request as ready for review May 20, 2024 11:46
@Skalakid
Copy link
Collaborator

Skalakid commented May 21, 2024

The bugs that I previously reported have been fixed, thank you! After your changes I've only found the problem with cursor positioning when entering newline after first backticks:

Video https://github.com/Expensify/react-native-live-markdown/assets/39538890/beeed721-10ae-49a6-8727-cda96d746bfd

Copy link
Collaborator

@Skalakid Skalakid left a comment

Choose a reason for hiding this comment

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

Left some questions and comments, but overall looks good

src/MarkdownTextInput.web.tsx Outdated Show resolved Hide resolved
src/MarkdownTextInput.web.tsx Outdated Show resolved Hide resolved
src/MarkdownTextInput.web.tsx Show resolved Hide resolved
src/web/MarkdownTextInput.css Outdated Show resolved Hide resolved
src/web/cursorUtils.ts Outdated Show resolved Hide resolved
src/web/parserUtils.ts Outdated Show resolved Hide resolved
src/web/parserUtils.ts Outdated Show resolved Hide resolved
src/web/parserUtils.ts Outdated Show resolved Hide resolved
@Skalakid
Copy link
Collaborator

Skalakid commented Jun 4, 2024

Hi, when testing I found a problem with cursor positioning when writing on Firefox:

Screen.Recording.2024-06-04.at.09.09.35.mov

@Skalakid
Copy link
Collaborator

Skalakid commented Jun 4, 2024

Firefox: after pasting code block text is still highlighted

Test string
Hello, *world*!
https://expensify.com
# Lorem ipsum
> Hello world
`foo`
test test
@here
@[email protected]
#room-mention
Screen.Recording.2024-06-04.at.09.29.51.mov

@BartoszGrajdek
Copy link
Collaborator Author

Hi, when testing I found a problem with cursor positioning when writing on Firefox:

Screen.Recording.2024-06-04.at.09.09.35.mov

This is related to a regression caused by another PR - we'll work on solving it in another issue

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