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

[JEWEL-495] Fix nested inlines in Markdown #2994

Closed

Conversation

rock3r
Copy link
Collaborator

@rock3r rock3r commented Mar 17, 2025

Properly supporting nested inline styling in Markdown requires a lot of changes. Luckily, they're mostly internal, but they do require us to deprecate and hide the old APIs because the behaviour has changed.

image (note how the `UI` part is italic, inheriting the emphasis, and the next paragraph also renders as expected now)

How does this fix work? Essentially, we need a smarter way to merge the styling as we visit nodes:

  1. We carry the "current" text style with us, that we can merge additional styles into
  2. We change the inline styles to only carry the minimum amount of style changes, so that we can them stack-merge them as we visit, without overriding important properties
  3. We have a custom smartMerge algorithm, used in the default inline renderer, that works the way we'd expect it to for Markdown

In addition to that:

  • We've removed the renderHtml property from the inline Markdown, since it's weird that would override the renderer; now you can actually decide it in the inline renderer
  • Default to render the inline HTML, as disabled-looking monospace, so that no information is lost during render
  • In the default inline renderer we've extracted rendering functions so that other implementations can replace one or more of them easily

@rock3r rock3r requested a review from hamen March 17, 2025 16:20
@rock3r rock3r self-assigned this Mar 17, 2025
@rock3r rock3r added the Jewel label Mar 17, 2025
Properly supporting nested inline styling in Markdown requires a lot of
changes. Luckily, they're mostly internal, but they do require us to
deprecate and hide the old APIs because the behaviour has changed.

How does this fix work? Essentially, we need a smarter way to merge the
styling as we visit nodes:
 1. We carry the "current" text style with us, that we can merge
    additional styles into
 2. We change the inline styles to only carry the minimum amount of
    style changes, so that we can them stack-merge them as we visit,
    without overriding important properties
 3. We have a custom `smartMerge` algorithm, used in the default inline
    renderer, that works the way we'd expect it to for Markdown

In addition to that:
 * We've removed the renderHtml property from the inline Markdown, since
   it's weird that would override the renderer; now you can actually
   decide it in the inline renderer
 * Default to render the inline HTML, as disabled-looking monospace, so
   that no information is lost during render
 * In the default inline renderer we've extracted rendering functions so
   that other implementations can replace one or more of them easily
@rock3r rock3r force-pushed the sebp/fix-markdown-combined-styles branch from 5b6c825 to dd5d329 Compare March 18, 2025 14:24
@rock3r rock3r deleted the sebp/fix-markdown-combined-styles branch March 27, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants