Skip to content
This repository was archived by the owner on Sep 10, 2025. It is now read-only.

Conversation

@marcin-kozinski
Copy link
Collaborator

I started working on displaying Markdown again after a while. With a new round comes a new look at the Markdown handling/formatting API I created previously. As it often happens it felt.. more complicated than it needed to be:

  • setMarkdownString defined as an extension on TextView was cute in theory, but because it's not a top level helper method, but has to be used with a class instance it resulted in a convoluted usage pattern: with (markdown) { textView.setMarkdownString(content) } . And it required passing the handler/formatter all the way to where the view is.
  • I also tied it to MarkdownString which in theory is just a general value holder, but in practice lives in a sync engine module, so while it's not in principle, in practice it is or feels tied to the sync engine. Also, looks like all usages before this new addition in Notes, actually had to artificially create this "sync engine" wrapper, just to use Markdown formatting.

The change to a method that converts a string with markdown formatting to Spanned:

  • makes the API shape simpler,
  • allows to format markdown further up from the view and pass what is almost a data description of the formatted text,
  • means we give up on some "auto-magic" handling Markwon (the Markdown library we use) can apply when it has access to the TextView, but I think it's a good trade-off that we have to for example explicitly set link movement methods or add more explicit binding from our TextViews to Markwon,
  • while it's still a type native to legacy views, I think it opens up the possibility of converting it to composables.

Note

By the way this fixes a long standing bug in Collections, where collection story excerpts weren't clickable. So to open a collection story you had to click on the card anywhere except on the excerpt (which was usually the largest part). This is an issue we fixed ages ago, where the default link "movement method" eagerly eats all clicks on a TextView, even outside of links and even if there are no links

So this is the trade-off of switching away from Markwon auto-magic. We switched to our own link movement method, which has had a fix/workaround for this issue for ages.

References

PR Checklist

Setup:

  • Described changes for automated release notes in PR title using
    Conventional Commits standard
  • Self Review (review, clean up, documentation)
  • Basic Self QA

Review:

  • Code Review approved

@marcin-kozinski marcin-kozinski changed the title refactor(markdown): simplify markdown formatting api fix(markdown): simplify markdown formatting api Jan 27, 2025
@marcin-kozinski marcin-kozinski added this pull request to the merge queue Jan 27, 2025
Merged via the queue into main with commit c7484b1 Jan 27, 2025
5 checks passed
@marcin-kozinski marcin-kozinski deleted the markdown-improvements branch January 27, 2025 16:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants