-
Notifications
You must be signed in to change notification settings - Fork 5.6k
markdown: Add support for inline HTML img
tags inside text
#37264
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
markdown: Add support for inline HTML img
tags inside text
#37264
Conversation
img
tags inside text
img
tags inside text
Ooo sorry @maxdeviant for changing the title at the same time. Feel free to change it back. |
img
tags inside text
We have to find a better html minifier, because html5minify has many issues with removing spaces, and it has a cve on a dependency. Or write our own in a next PR. |
Should be ready now, In a next RemcoSmitsDev#120 I will fix that, we have support for styling tags like b and strong etc. After that, I will start working on adding li / ol support or fix the html minifier issues. |
Should i just revert that change so we can handle it correctly in the near future? So this pr just adds the groundwork for nested elements inside a p tag. |
Yes, I think you should do the other ones first. |
@SomeoneToIgnore It's ready if you are ok with the changes. |
We will later fix the wrapping issues
|
||
fn render_markdown_paragraph(parsed: &MarkdownParagraph, cx: &mut RenderContext) -> AnyElement { | ||
cx.with_common_p(h_flex().flex_wrap()) | ||
cx.with_common_p(div()) |
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.
@huacnlee I'm re-adding this, because adding a flex & flex-col here breaks the keyboard shortcut code for example:
Before removing h_flex

After removing h_flex

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.
I would rather see the wrapping break than it looks like this.
Reverting this because see: zed-industries@8b8186a#r2322518759 This reverts commit 8b8186a.
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.
Nice, thank you for the follow-up.
I have 2 more PRs coming :) |
Sorry, I have busy on Input changed in GPUI Compopnent recently days, not reply this. @RemcoSmitsDev may not understand I was said issue, the Expected![]() Now merged in main![]() |
Sorry for overlooking: it is a great point that we should not break the "main" page for Zed's markdown rendering: its own release notes. Given that today is a release day and people will look at this, I'll revert the PR meanwhile and will be looking future PRs more on rendering markdown from Zed and from rust-analyzer's repo. Still interested to get those image renders in. |
…37264)" (#37893) This reverts commit e1a5d29. This have regressed Zed release notes' wrapping which we do not want to do on a release day: #37264 (comment) Release Notes: - N/A
…ustries#37264) Follow-up: zed-industries#36700 This PR adds basic support for showing images inline inside a text. As you can see inside the before screenshot, the image was displayed right below the `Some inline text` text. This was because we didn't consider the image to be inline with the text (paragraph). Now we do :) All the test changes are making sure it is not more than 1 element parsed, instead of only checking for the first parsed element. This could work out bad when we return more than 1 result. **Before** <img width="1717" height="1344" alt="Screenshot 2025-08-31 at 13 49 45" src="https://github.com/user-attachments/assets/13c5f9dd-0e0a-4e08-b2a6-28e9a4e0cab8" /> **After** <img width="1719" height="1343" alt="Screenshot 2025-08-31 at 13 42 14" src="https://github.com/user-attachments/assets/bf7aa82f-3743-4fb3-87aa-4a97a550c4d1" /> **Code example**: ```markdown <p>some inline text <img src="https://picsum.photos/200/300" alt="Description of image" style="height: 100px" /> asdjkflsadjfl</p> # Html Tag <img src="https://picsum.photos/200/300" alt="Description of image" /> # Html Tag with width and height <img src="https://picsum.photos/200/300" alt="Description of image" width="100" height="200" /> # Html Tag with style attribute with width and height <img src="https://picsum.photos/200/300" alt="Description of image" style="width: 100px; height: 200px" /> # Normal Tag  ``` Release Notes: - Markdown: Added support for inline HTML `img` tags inside paragraphs
…ed-industries#37264)" (zed-industries#37893) This reverts commit e1a5d29. This have regressed Zed release notes' wrapping which we do not want to do on a release day: zed-industries#37264 (comment) Release Notes: - N/A
Yeah, I knew it was not 100% perfect but didn't know It was that broken. Sorry for that, should have checked it better. @huacnlee and I both don't really know what the best way is to support wrapping with images. I'm trying to implement the inline display feature from CSS inside taffy. As I'm not really familiar with the code base, it will probably take me some time to get it working as expected. @SomeoneToIgnore Do you want me to just push the HTML image support without adding the flex wrap code? So we don't break the wrapping but have HTML image support without them being inline yet? |
Yeah, I think that would be a nice step forward. Overall, a baseline are all md files in Zed, esp. the release notes preview (which is autogenerated IIRC?) — if those do not break, it is fine to move on + test on a few extra documents. |
Re-adds: #37264 This PR re-adds basic support for showing HTML images, without touching the display mode for images. The initial PR changed the `div().flex().flex_col()` to `h_flex().flex_wrap()` but this broke the text wrapping in almost all cases. **Note**: This does not add support for showing the images inline, because we haven't figured out how they correctly do this. I'm working on adding the CSS `inline` display feature support to taffy that hopefully allows us to correctly show images/other elements inline without breaking the text wrapping. **Before (nightly) and after (dev) for the README file inside Zed. (nothing has changed, which is good)** <img width="3440" height="1380" alt="Screenshot 2025-09-13 at 12 49 08" src="https://github.com/user-attachments/assets/9cbdcb07-dbe9-4236-9d20-e59acc0e955e" /> **Result** <img width="1717" height="1314" alt="Screenshot 2025-09-13 at 12 51 54" src="https://github.com/user-attachments/assets/1c0f8507-c63d-472e-8e82-a654a63f7153" /> cc @SomeoneToIgnore Release Notes: - markdown preview: Added support for HTML `img` tags inside paragraphs
Follow-up: #36700
This PR adds basic support for showing images inline inside a text.
As you can see inside the before screenshot, the image was displayed right below the
Some inline text
text. This was because we didn't consider the image to be inline with the text (paragraph). Now we do :)All the test changes are making sure it is not more than 1 element parsed, instead of only checking for the first parsed element. This could work out bad when we return more than 1 result.
Before

After

Code example:
cc @huacnlee As you are also familiar with this code.
Release Notes:
img
tags inside paragraphs