Skip to content

remove truncation when convert html to markdown#870

Merged
justinpersaud merged 2 commits into
Expensify:mainfrom
nkdengineer:remove-truncation-when-convert-html-to-markdown
Jul 10, 2025
Merged

remove truncation when convert html to markdown#870
justinpersaud merged 2 commits into
Expensify:mainfrom
nkdengineer:remove-truncation-when-convert-html-to-markdown

Conversation

@nkdengineer

Copy link
Copy Markdown
Contributor

Fixed Issues

$ Expensify/App#63711

Tests

  1. What unit/integration tests cover your change? What autoQA tests cover your change?
  2. What tests did you perform that validates your changed worked?

QA

  1. What does QA need to do to validate your changes?
  2. What areas to they need to test for regressions?

Result

Screen.Recording.2025-07-09.at.21.28.34.mov

@srikarparsi srikarparsi requested review from justinpersaud and removed request for srikarparsi July 9, 2025 16:52
@srikarparsi

Copy link
Copy Markdown
Contributor

Hey @justinpersaud, reassigning you here if that's okay since it deals with this issue

@justinpersaud justinpersaud left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nkdengineer your proposal mentioned we would

Truncate the final result

Is that no longer necessary?

Also, are there any other tests we should do with regards to pasting now that truncation is being removed?

@nkdengineer

Copy link
Copy Markdown
Contributor Author

Truncate the final result

It's just the add-on. Here're the reasons I think we don't need to truncate the final result:

  1. User still can enter more than maximum length (10.000)

image

so if we truncate the final result here, it can cause the inconsistency

  1. User can be confused if we truncate the final result without any warnings.

Also, are there any other tests we should do with regards to pasting now that truncation is being removed?

We removed the maxBodyLength in htmlToMarkdown, it's more generic now so I think no test is needed

@justinpersaud justinpersaud left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks, the 10k char limit seems good to me

@justinpersaud justinpersaud merged commit 79d18d8 into Expensify:main Jul 10, 2025
6 checks passed
@os-botify

os-botify Bot commented Jul 10, 2025

Copy link
Copy Markdown
Contributor

🚀 Published to npm in 2.0.147 🎉

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.

4 participants