-
Notifications
You must be signed in to change notification settings - Fork 136
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
Prevent bold, italic, strikethrough, and auto link when the content is a codefence #663
Prevent bold, italic, strikethrough, and auto link when the content is a codefence #663
Conversation
@puneetlath adding you as reviewer since you're assigned to the linked issue |
@allroundexperts can you please take the first round of review? |
Yes. On it today. |
Still reviewing this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great!
Screen.Recording.2024-03-18.at.3.08.16.AM.mov
Screen.Recording.2024-03-18.at.3.08.46.AM.mov
Is the next step to open an App PR to apply this expensify-common update? |
@puneetlath @allroundexperts we would need to update the
I see that this issue will update the |
Ok sounds good to me. Mind commenting there? |
I have commented here. In the meantime, I open a live markdown PR to update the expensify-common Expensify/react-native-live-markdown#242 |
Fixed Issues
$ Expensify/App#37533
Tests
What unit/integration tests cover your change? What autoQA tests cover your change?
The ExpensiMark-HTML-test.js is updated to include the new changes case.
What tests did you perform that validates your changed worked?
Ran the unit test.
Testing in App (Android & iOS only):
to be able to test these changes, you need to apply the code change to react-native-live-markdown-parser.js
find all occurrences of
includes('<pre>')
and replace it withincludes('</pre>')
Screen.Recording.2024-03-13.at.15.05.10.mov
QA
Same as Test above