markdown_preview: Fix HTML alignment styles not being applied (#53196)
## What This PR Does This PR adds support for HTML alignment styles to be applied to Paragraph and Heading elements and their children. Here is what this looks like before vs after this PR (both images use the same markdown below): ```markdown <p style="text-align: center;"> <a target="_blank" href="https://github.com/"><img width="150" height="150" src="https://upload.wikimedia.org/wikipedia/commons/c/c2/GitHub_Invertocat_Logo.svg"></a> </p> ``` **BEFORE:** <img width="742" height="242" alt="image" src="https://github.com/user-attachments/assets/4ca8b8d9-0606-45f5-8a0e-cafaaac47d97" /> **AFTER:** <img width="1274" height="267" alt="image" src="https://github.com/user-attachments/assets/2c347ce7-75b9-4ef6-9598-b1eda7272ef5" /> ## Notes I used `style="text-align: center|left|right;"` instead of `align="center|right|left"` since `align` has been [deprecated in HTML5](https://www.w3.org/TR/2011/WD-html5-author-20110809/obsolete.html) for block-level elements. The issue this PR solves mentioned that github supports the `align="center|right|left"` attribute, so I'm unsure if the Zed team would want to have parity there. Feel free to let me know if that would be something that should be added, however for now I've decided to follow the HTML5 standard. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes https://github.com/zed-industries/zed/issues/51062 Release Notes: - Fixed HTML alignment styles not being applied in markdown previews --------- Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>
This commit is contained in:
1 parent
0f7cd1c58f
commit
72a7753bfe
3 files changed
+172
-32
No files matched your search
@@ -1,6 +1,6 @@
|
||||
use std::{cell::RefCell, collections::HashMap, mem, ops::Range};
|
||||
|
||||
use gpui::{DefiniteLength, FontWeight, SharedString, px, relative};
|
||||
use gpui::{DefiniteLength, FontWeight, SharedString, TextAlign, px, relative};
|
||||
use html5ever::{
|
||||
Attribute, LocalName, ParseOpts, local_name, parse_document, tendril::TendrilSink,
|
||||
};
|
||||
@@ -24,10 +24,17 @@ pub(crate) enum ParsedHtmlElement {
|
||||
List(ParsedHtmlList),
|
||||
Table(ParsedHtmlTable),
|
||||
BlockQuote(ParsedHtmlBlockQuote),
|
||||
Paragraph(HtmlParagraph),
|
||||
Paragraph(ParsedHtmlParagraph),
|
||||
Image(HtmlImage),
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
#[cfg_attr(test, derive(PartialEq))]
|
||||
pub(crate) struct ParsedHtmlParagraph {
|
||||
pub text_align: Option<TextAlign>,
|
||||
pub contents: HtmlParagraph,
|
||||
}
|
||||
|
||||
impl ParsedHtmlElement {
|
||||
pub fn source_range(&self) -> Option<Range<usize>> {
|
||||
Some(match self {
|
||||
@@ -35,7 +42,7 @@ impl ParsedHtmlElement {
|
||||
Self::List(list) => list.source_range.clone(),
|
||||
Self::Table(table) => table.source_range.clone(),
|
||||
Self::BlockQuote(block_quote) => block_quote.source_range.clone(),
|
||||
Self::Paragraph(text) => match text.first()? {
|
||||
Self::Paragraph(paragraph) => match paragraph.contents.first()? {
|
||||
HtmlParagraphChunk::Text(text) => text.source_range.clone(),
|
||||
HtmlParagraphChunk::Image(image) => image.source_range.clone(),
|
||||
},
|
||||
@@ -83,6 +90,7 @@ pub(crate) struct ParsedHtmlHeading {
|
||||
pub source_range: Range<usize>,
|
||||
pub level: HeadingLevel,
|
||||
pub contents: HtmlParagraph,
|
||||
pub text_align: Option<TextAlign>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -236,20 +244,21 @@ fn parse_html_node(
|
||||
consume_children(source_range, node, elements, context);
|
||||
}
|
||||
NodeData::Text { contents } => {
|
||||
elements.push(ParsedHtmlElement::Paragraph(vec![
|
||||
HtmlParagraphChunk::Text(ParsedHtmlText {
|
||||
elements.push(ParsedHtmlElement::Paragraph(ParsedHtmlParagraph {
|
||||
text_align: None,
|
||||
contents: vec![HtmlParagraphChunk::Text(ParsedHtmlText {
|
||||
source_range,
|
||||
highlights: Vec::default(),
|
||||
links: Vec::default(),
|
||||
contents: contents.borrow().to_string().into(),
|
||||
}),
|
||||
]));
|
||||
})],
|
||||
}));
|
||||
}
|
||||
NodeData::Comment { .. } => {}
|
||||
NodeData::Element { name, attrs, .. } => {
|
||||
let mut styles = if let Some(styles) =
|
||||
html_style_from_html_styles(extract_styles_from_attributes(attrs))
|
||||
{
|
||||
let styles_map = extract_styles_from_attributes(attrs);
|
||||
let text_align = text_align_from_attributes(attrs, &styles_map);
|
||||
let mut styles = if let Some(styles) = html_style_from_html_styles(styles_map) {
|
||||
vec![styles]
|
||||
} else {
|
||||
Vec::default()
|
||||
@@ -270,7 +279,10 @@ fn parse_html_node(
|
||||
);
|
||||
|
||||
if !paragraph.is_empty() {
|
||||
elements.push(ParsedHtmlElement::Paragraph(paragraph));
|
||||
elements.push(ParsedHtmlElement::Paragraph(ParsedHtmlParagraph {
|
||||
text_align,
|
||||
contents: paragraph,
|
||||
}));
|
||||
}
|
||||
} else if matches!(
|
||||
name.local,
|
||||
@@ -303,6 +315,7 @@ fn parse_html_node(
|
||||
_ => unreachable!(),
|
||||
},
|
||||
contents: paragraph,
|
||||
text_align,
|
||||
}));
|
||||
}
|
||||
} else if name.local == local_name!("ul") || name.local == local_name!("ol") {
|
||||
@@ -589,6 +602,30 @@ fn html_style_from_html_styles(styles: HashMap<String, String>) -> Option<HtmlHi
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_text_align(value: &str) -> Option<TextAlign> {
|
||||
match value.trim().to_ascii_lowercase().as_str() {
|
||||
"left" => Some(TextAlign::Left),
|
||||
"center" => Some(TextAlign::Center),
|
||||
"right" => Some(TextAlign::Right),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn text_align_from_styles(styles: &HashMap<String, String>) -> Option<TextAlign> {
|
||||
styles
|
||||
.get("text-align")
|
||||
.and_then(|value| parse_text_align(value))
|
||||
}
|
||||
|
||||
fn text_align_from_attributes(
|
||||
attrs: &RefCell<Vec<Attribute>>,
|
||||
styles: &HashMap<String, String>,
|
||||
) -> Option<TextAlign> {
|
||||
text_align_from_styles(styles).or_else(|| {
|
||||
attr_value(attrs, local_name!("align")).and_then(|value| parse_text_align(&value))
|
||||
})
|
||||
}
|
||||
|
||||
fn extract_styles_from_attributes(attrs: &RefCell<Vec<Attribute>>) -> HashMap<String, String> {
|
||||
let mut styles = HashMap::new();
|
||||
|
||||
@@ -770,6 +807,7 @@ fn extract_html_table(node: &Node, source_range: Range<usize>) -> Option<ParsedH
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use gpui::TextAlign;
|
||||
|
||||
#[test]
|
||||
fn parses_html_styled_text() {
|
||||
@@ -783,7 +821,7 @@ mod tests {
|
||||
let ParsedHtmlElement::Paragraph(paragraph) = &parsed.children[0] else {
|
||||
panic!("expected paragraph");
|
||||
};
|
||||
let HtmlParagraphChunk::Text(text) = ¶graph[0] else {
|
||||
let HtmlParagraphChunk::Text(text) = ¶graph.contents[0] else {
|
||||
panic!("expected text chunk");
|
||||
};
|
||||
|
||||
@@ -851,7 +889,7 @@ mod tests {
|
||||
let ParsedHtmlElement::Paragraph(paragraph) = &first_item.content[0] else {
|
||||
panic!("expected first item paragraph");
|
||||
};
|
||||
let HtmlParagraphChunk::Text(text) = ¶graph[0] else {
|
||||
let HtmlParagraphChunk::Text(text) = ¶graph.contents[0] else {
|
||||
panic!("expected first item text");
|
||||
};
|
||||
assert_eq!(text.contents.as_ref(), "parent");
|
||||
@@ -866,7 +904,7 @@ mod tests {
|
||||
else {
|
||||
panic!("expected nested item paragraph");
|
||||
};
|
||||
let HtmlParagraphChunk::Text(nested_text) = &nested_paragraph[0] else {
|
||||
let HtmlParagraphChunk::Text(nested_text) = &nested_paragraph.contents[0] else {
|
||||
panic!("expected nested item text");
|
||||
};
|
||||
assert_eq!(nested_text.contents.as_ref(), "child");
|
||||
@@ -875,9 +913,58 @@ mod tests {
|
||||
let ParsedHtmlElement::Paragraph(second_paragraph) = &second_item.content[0] else {
|
||||
panic!("expected second item paragraph");
|
||||
};
|
||||
let HtmlParagraphChunk::Text(second_text) = &second_paragraph[0] else {
|
||||
let HtmlParagraphChunk::Text(second_text) = &second_paragraph.contents[0] else {
|
||||
panic!("expected second item text");
|
||||
};
|
||||
assert_eq!(second_text.contents.as_ref(), "sibling");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_paragraph_text_align_from_style() {
|
||||
let parsed = parse_html_block("<p style=\"text-align: center\">x</p>", 0..40).unwrap();
|
||||
let ParsedHtmlElement::Paragraph(paragraph) = &parsed.children[0] else {
|
||||
panic!("expected paragraph");
|
||||
};
|
||||
assert_eq!(paragraph.text_align, Some(TextAlign::Center));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_heading_text_align_from_style() {
|
||||
let parsed = parse_html_block("<h2 style=\"text-align: right\">Title</h2>", 0..45).unwrap();
|
||||
let ParsedHtmlElement::Heading(heading) = &parsed.children[0] else {
|
||||
panic!("expected heading");
|
||||
};
|
||||
assert_eq!(heading.text_align, Some(TextAlign::Right));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_paragraph_text_align_from_align_attribute() {
|
||||
let parsed = parse_html_block("<p align=\"center\">x</p>", 0..24).unwrap();
|
||||
let ParsedHtmlElement::Paragraph(paragraph) = &parsed.children[0] else {
|
||||
panic!("expected paragraph");
|
||||
};
|
||||
assert_eq!(paragraph.text_align, Some(TextAlign::Center));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_heading_text_align_from_align_attribute() {
|
||||
let parsed = parse_html_block("<h2 align=\"right\">Title</h2>", 0..30).unwrap();
|
||||
let ParsedHtmlElement::Heading(heading) = &parsed.children[0] else {
|
||||
panic!("expected heading");
|
||||
};
|
||||
assert_eq!(heading.text_align, Some(TextAlign::Right));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prefers_style_text_align_over_align_attribute() {
|
||||
let parsed = parse_html_block(
|
||||
"<p align=\"left\" style=\"text-align: center\">x</p>",
|
||||
0..50,
|
||||
)
|
||||
.unwrap();
|
||||
let ParsedHtmlElement::Paragraph(paragraph) = &parsed.children[0] else {
|
||||
panic!("expected paragraph");
|
||||
};
|
||||
assert_eq!(paragraph.text_align, Some(TextAlign::Center));
|
||||
}
|
||||
}
|
||||
@@ -79,9 +79,20 @@ impl MarkdownElement {
|
||||
|
||||
match element {
|
||||
ParsedHtmlElement::Paragraph(paragraph) => {
|
||||
self.push_markdown_paragraph(builder, &source_range, markdown_end);
|
||||
self.render_html_paragraph(paragraph, source_allocator, builder, cx, markdown_end);
|
||||
builder.pop_div();
|
||||
self.push_markdown_paragraph(
|
||||
builder,
|
||||
&source_range,
|
||||
markdown_end,
|
||||
paragraph.text_align,
|
||||
);
|
||||
self.render_html_paragraph(
|
||||
¶graph.contents,
|
||||
source_allocator,
|
||||
builder,
|
||||
cx,
|
||||
markdown_end,
|
||||
);
|
||||
self.pop_markdown_paragraph(builder);
|
||||
}
|
||||
ParsedHtmlElement::Heading(heading) => {
|
||||
self.push_markdown_heading(
|
||||
@@ -89,6 +100,7 @@ impl MarkdownElement {
|
||||
heading.level,
|
||||
&heading.source_range,
|
||||
markdown_end,
|
||||
heading.text_align,
|
||||
);
|
||||
self.render_html_paragraph(
|
||||
&heading.contents,
|
||||
|
||||
@@ -36,8 +36,8 @@ use gpui::{
|
||||
FocusHandle, Focusable, FontStyle, FontWeight, GlobalElementId, Hitbox, Hsla, Image,
|
||||
ImageFormat, ImageSource, KeyContext, Length, MouseButton, MouseDownEvent, MouseEvent,
|
||||
MouseMoveEvent, MouseUpEvent, Point, ScrollHandle, Stateful, StrikethroughStyle,
|
||||
StyleRefinement, StyledText, Task, TextLayout, TextRun, TextStyle, TextStyleRefinement,
|
||||
actions, img, point, quad,
|
||||
StyleRefinement, StyledText, Task, TextAlign, TextLayout, TextRun, TextStyle,
|
||||
TextStyleRefinement, actions, img, point, quad,
|
||||
};
|
||||
use language::{CharClassifier, Language, LanguageRegistry, Rope};
|
||||
use parser::CodeBlockMetadata;
|
||||
@@ -1025,8 +1025,17 @@ impl MarkdownElement {
|
||||
width: Option<DefiniteLength>,
|
||||
height: Option<DefiniteLength>,
|
||||
) {
|
||||
let align = builder.text_style().text_align;
|
||||
builder.modify_current_div(|el| {
|
||||
el.items_center().flex().flex_row().child(
|
||||
let mut image_container = el.flex().flex_row().items_center();
|
||||
|
||||
image_container = match align {
|
||||
TextAlign::Left => image_container.justify_start(),
|
||||
TextAlign::Center => image_container.justify_center(),
|
||||
TextAlign::Right => image_container.justify_end(),
|
||||
};
|
||||
|
||||
image_container.child(
|
||||
img(source)
|
||||
.max_w_full()
|
||||
.when_some(height, |this, height| this.h(height))
|
||||
@@ -1041,14 +1050,29 @@ impl MarkdownElement {
|
||||
builder: &mut MarkdownElementBuilder,
|
||||
range: &Range<usize>,
|
||||
markdown_end: usize,
|
||||
text_align_override: Option<TextAlign>,
|
||||
) {
|
||||
builder.push_div(
|
||||
div().when(!self.style.height_is_multiple_of_line_height, |el| {
|
||||
el.mb_2().line_height(rems(1.3))
|
||||
}),
|
||||
range,
|
||||
markdown_end,
|
||||
);
|
||||
let align = text_align_override.unwrap_or(self.style.base_text_style.text_align);
|
||||
let mut paragraph = div().when(!self.style.height_is_multiple_of_line_height, |el| {
|
||||
el.mb_2().line_height(rems(1.3))
|
||||
});
|
||||
|
||||
paragraph = match align {
|
||||
TextAlign::Center => paragraph.text_center(),
|
||||
TextAlign::Left => paragraph.text_left(),
|
||||
TextAlign::Right => paragraph.text_right(),
|
||||
};
|
||||
|
||||
builder.push_text_style(TextStyleRefinement {
|
||||
text_align: Some(align),
|
||||
..Default::default()
|
||||
});
|
||||
builder.push_div(paragraph, range, markdown_end);
|
||||
}
|
||||
|
||||
fn pop_markdown_paragraph(&self, builder: &mut MarkdownElementBuilder) {
|
||||
builder.pop_div();
|
||||
builder.pop_text_style();
|
||||
}
|
||||
|
||||
fn push_markdown_heading(
|
||||
@@ -1057,15 +1081,26 @@ impl MarkdownElement {
|
||||
level: pulldown_cmark::HeadingLevel,
|
||||
range: &Range<usize>,
|
||||
markdown_end: usize,
|
||||
text_align_override: Option<TextAlign>,
|
||||
) {
|
||||
let align = text_align_override.unwrap_or(self.style.base_text_style.text_align);
|
||||
let mut heading = div().mb_2();
|
||||
heading = apply_heading_style(heading, level, self.style.heading_level_styles.as_ref());
|
||||
|
||||
heading = match align {
|
||||
TextAlign::Center => heading.text_center(),
|
||||
TextAlign::Left => heading.text_left(),
|
||||
TextAlign::Right => heading.text_right(),
|
||||
};
|
||||
|
||||
let mut heading_style = self.style.heading.clone();
|
||||
let heading_text_style = heading_style.text_style().clone();
|
||||
heading.style().refine(&heading_style);
|
||||
|
||||
builder.push_text_style(heading_text_style);
|
||||
builder.push_text_style(TextStyleRefinement {
|
||||
text_align: Some(align),
|
||||
..heading_text_style
|
||||
});
|
||||
builder.push_div(heading, range, markdown_end);
|
||||
}
|
||||
|
||||
@@ -1571,10 +1606,16 @@ impl Element for MarkdownElement {
|
||||
}
|
||||
}
|
||||
MarkdownTag::Paragraph => {
|
||||
self.push_markdown_paragraph(&mut builder, range, markdown_end);
|
||||
self.push_markdown_paragraph(&mut builder, range, markdown_end, None);
|
||||
}
|
||||
MarkdownTag::Heading { level, .. } => {
|
||||
self.push_markdown_heading(&mut builder, *level, range, markdown_end);
|
||||
self.push_markdown_heading(
|
||||
&mut builder,
|
||||
*level,
|
||||
range,
|
||||
markdown_end,
|
||||
None,
|
||||
);
|
||||
}
|
||||
MarkdownTag::BlockQuote => {
|
||||
self.push_markdown_block_quote(&mut builder, range, markdown_end);
|
||||
@@ -1826,7 +1867,7 @@ impl Element for MarkdownElement {
|
||||
current_img_block_range.take();
|
||||
}
|
||||
MarkdownTagEnd::Paragraph => {
|
||||
builder.pop_div();
|
||||
self.pop_markdown_paragraph(&mut builder);
|
||||
}
|
||||
MarkdownTagEnd::Heading(_) => {
|
||||
self.pop_markdown_heading(&mut builder);
|
||||
|
||||
Reference in new issue
Block a user