From 614f67ed2aa7378e5f11359ea01ba873b6a2a103 Mon Sep 17 00:00:00 2001 From: "Angel P." Date: Tue, 7 Apr 2026 05:00:22 -0400 Subject: [PATCH] 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

``` **BEFORE:** image **AFTER:** image ## 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 --- crates/markdown/src/html/html_parser.rs | 117 ++++++++++++++++++--- crates/markdown/src/html/html_rendering.rs | 18 +++- crates/markdown/src/markdown.rs | 69 +++++++++--- 3 files changed, 172 insertions(+), 32 deletions(-) diff --git a/crates/markdown/src/html/html_parser.rs b/crates/markdown/src/html/html_parser.rs index 20338ec2abef2314b7cd6ca91e45ee05be909745..8aa5da0cea7ea160721875fa889a720fe4c8bed1 100644 --- a/crates/markdown/src/html/html_parser.rs +++ b/crates/markdown/src/html/html_parser.rs @@ -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, + pub contents: HtmlParagraph, +} + impl ParsedHtmlElement { pub fn source_range(&self) -> Option> { 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, pub level: HeadingLevel, pub contents: HtmlParagraph, + pub text_align: Option, } #[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) -> Option Option { + 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) -> Option { + styles + .get("text-align") + .and_then(|value| parse_text_align(value)) +} + +fn text_align_from_attributes( + attrs: &RefCell>, + styles: &HashMap, +) -> Option { + 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>) -> HashMap { let mut styles = HashMap::new(); @@ -770,6 +807,7 @@ fn extract_html_table(node: &Node, source_range: Range) -> Optionx

", 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("

Title

", 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("

x

", 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("

Title

", 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( + "

x

", + 0..50, + ) + .unwrap(); + let ParsedHtmlElement::Paragraph(paragraph) = &parsed.children[0] else { + panic!("expected paragraph"); + }; + assert_eq!(paragraph.text_align, Some(TextAlign::Center)); + } } diff --git a/crates/markdown/src/html/html_rendering.rs b/crates/markdown/src/html/html_rendering.rs index 103e2a6accb7dce9bc429419aafd27cbdf5080ce..6ae25eff0b4ba2ec8dedde8118ebd8d60e8fce7d 100644 --- a/crates/markdown/src/html/html_rendering.rs +++ b/crates/markdown/src/html/html_rendering.rs @@ -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, diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index 247c082d223005a7e0bd6d57696751ce76cc4d86..e6ad1b1f2ac9154eaabc6d18dbcb9c8695ae019d 100644 --- a/crates/markdown/src/markdown.rs +++ b/crates/markdown/src/markdown.rs @@ -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, height: Option, ) { + 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, markdown_end: usize, + text_align_override: Option, ) { - 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, markdown_end: usize, + text_align_override: Option, ) { + 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);