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

Stop escaping codeFence data-code-raw attribute data #644

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

robertKozik
Copy link
Contributor

This PR is a follow up for live markdown preview web PR as there was discovered a bug which prevent parsing when special character was inside codeFence fe. ``` one > two ```.

To fix this issue we shouldn't escape data inside codeFence data-code-raw attribute so the input and output of the parser will be the same for the react-native-live-markdown library

Fixed Issues

$ Expensify/App#27977

Tests

  1. What unit/integration tests cover your change? What autoQA tests cover your change?
    There are already lots of unit tests present for covering this behaviour. All relevant ones were adjusted to new behaviour
  2. What tests did you perform that validates your changed worked?
    I've checked it on example app of react-native-live-markdown library which we are using for text input inside expensify app

To test the behaviour you should use example app of the react-native-live-markdown library. To do so:

  1. change the expensify-common version inside /parser directory and reinstall dependencies (npm install)
  2. build parser output file (npm run build)
  3. Build and open example app on desired platform (iOS/Android/Web)
  4. Type in code fence with special character inside it (fe. ``` one > two ```)
  5. Verify that input is parsed as codeFence properly

QA

  1. What does QA need to do to validate your changes?
  2. What areas to they need to test for regressions?
    There should be no regressions involved as I are only changing the behaviour of html attributed used only in react-native-live-markdown library. Any regression are possible only after new version of react-native-live-markdown library is present inside E/App

@shubham1206agra
Copy link

@robertKozik Is it possible to test this in NewDot now?
If not, I will approve this right away.

@robertKozik
Copy link
Contributor Author

Yeah you could but it's a long way. As you will need to locally update the expensify-common in parser directorymreact-native-live-markdown library. Then link whole lib to the NewDot. This way you should be able to test it

@robertKozik robertKozik marked this pull request as ready for review February 8, 2024 13:40
@robertKozik robertKozik requested a review from a team as a code owner February 8, 2024 13:40
@melvin-bot melvin-bot bot requested review from marcaaron and removed request for a team February 8, 2024 13:41
@marcaaron marcaaron merged commit c85dc9b into Expensify:main Feb 8, 2024
5 checks passed
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