From 63d4e29a1286213796228df9812b585ff74c49d0 Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Wed, 12 Aug 2026 17:33:28 +0000 Subject: [PATCH] markdown: Auto-size table columns to content width (#61773) Closes https://github.com/zed-industries/zed/issues/62498 Fixes #50044: | before | after | | --- | --- | | before | after | ~Submitted as a draft PR because it relies on https://github.com/DioxusLabs/taffy/pull/1001. I temporarily pinned our `taffy` dependency to that particular branch to make the new tests pass.~ That fix got released! This fix is similar to the original fix in #50839 which got partially reverted by #52864 due to a regression, and `test_table_never_renders_past_its_available_width` ensures this doesn't regress again. ## 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 adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Markdown table columns are now sized based on their content. --- Cargo.lock | 12 +- crates/gpui/Cargo.toml | 2 +- crates/gpui/src/elements/div.rs | 39 ++++++ crates/markdown/src/html/html_rendering.rs | 9 +- crates/markdown/src/markdown.rs | 138 ++++++++++++++++++++- 5 files changed, 180 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fd9ca87a3b..1e69f12677 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8175,12 +8175,6 @@ dependencies = [ "tree-sitter-yaml", ] -[[package]] -name = "grid" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b40ca9252762c466af32d0b1002e91e4e1bc5398f77455e55474deb466355ff5" - [[package]] name = "group" version = "0.12.1" @@ -18419,14 +18413,14 @@ dependencies = [ [[package]] name = "taffy" -version = "0.12.2" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "340a09581f29809fc0df82a3955501dc7f2a21f887e5d1c13dbe288fe1c0bef4" +checksum = "c034e05f6ee85a12daa63863c2245797715075c70649947aa0da54f3f2ab1d0f" dependencies = [ "arrayvec", - "grid", "serde", "slotmap", + "smallvec", ] [[package]] diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index 0b833fef9f..bc5aab22b3 100644 --- a/crates/gpui/Cargo.toml +++ b/crates/gpui/Cargo.toml @@ -92,7 +92,7 @@ async-channel.workspace = true stacksafe.workspace = true strum.workspace = true sum_tree.workspace = true -taffy = "=0.12.2" +taffy = "=0.13.0" thiserror.workspace = true tracing.workspace = true gpui_util.workspace = true diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index 0fb4b202ed..308263e15d 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -5155,4 +5155,43 @@ mod tests { assert_eq!(focused, Some(item_b.id)); } + + struct ContentSizedGrid; + + impl Render for ContentSizedGrid { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + let widths = [px(100.), px(200.), px(50.)]; + div().size_full().child( + div() + .w_full() + .grid() + .grid_cols_max_content(widths.len() as u16) + .children(widths.into_iter().enumerate().map(|(index, width)| { + div() + .debug_selector(move || format!("cell-{index}")) + .w(width) + .h(px(10.)) + })), + ) + } + } + + #[gpui::test] + fn grid_cols_max_content_sizes_columns_to_their_content(cx: &mut TestAppContext) { + let window = cx.add_window(|_, _| ContentSizedGrid); + cx.update_window(window.into(), |_, window, cx| window.draw(cx).clear(cx)) + .unwrap(); + + let mut bounds = |selector: &'static str| { + cx.update_window(window.into(), |_, window, _| { + window.rendered_frame.debug_bounds.get(selector).copied() + }) + .unwrap() + .unwrap_or_else(|| panic!("{selector} was not rendered")) + }; + + assert_eq!(bounds("cell-0").origin.x, px(0.)); + assert_eq!(bounds("cell-1").origin.x, px(100.)); + assert_eq!(bounds("cell-2").origin.x, px(300.)); + } } diff --git a/crates/markdown/src/html/html_rendering.rs b/crates/markdown/src/html/html_rendering.rs index f9c1568445..e1c2ba0599 100644 --- a/crates/markdown/src/html/html_rendering.rs +++ b/crates/markdown/src/html/html_rendering.rs @@ -213,18 +213,18 @@ impl MarkdownElement { let total_rows = table.header.len() + table.body.len(); let mut grid_occupied = vec![vec![false; max_column_count]; total_rows]; + builder.push_div(div().flex(), &table.source_range, markdown_end); builder.push_div( div() .id(("html-table", table.source_range.start)) + .min_w_0() .grid() - .grid_cols(max_column_count as u16) .when(self.style.table_columns_min_size, |this| { - this.grid_cols_min_content(max_column_count as u16) + this.w_full().grid_cols_min_content(max_column_count as u16) }) .when(!self.style.table_columns_min_size, |this| { - this.grid_cols(max_column_count as u16) + this.grid_cols_max_content(max_column_count as u16) }) - .w_full() .mb_2() .border(px(1.5)) .border_color(cx.theme().colors().border) @@ -342,6 +342,7 @@ impl MarkdownElement { } builder.pop_div(); + builder.pop_div(); } fn render_html_paragraph( diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index 55ccd4dfea..4a33f4f5e1 100644 --- a/crates/markdown/src/markdown.rs +++ b/crates/markdown/src/markdown.rs @@ -2800,18 +2800,19 @@ impl Element for MarkdownElement { builder.table.start(alignments.clone()); let column_count = alignments.len(); + builder.push_div(div().flex(), range, markdown_end); builder.push_div( div() .id(("table", range.start)) + .debug_selector(|| "markdown_table".into()) + .min_w_0() .grid() - .grid_cols(column_count as u16) .when(self.style.table_columns_min_size, |this| { - this.grid_cols_min_content(column_count as u16) + this.w_full().grid_cols_min_content(column_count as u16) }) .when(!self.style.table_columns_min_size, |this| { - this.grid_cols(column_count as u16) + this.grid_cols_max_content(column_count as u16) }) - .w_full() .mb_2() .border(px(1.5)) .border_color(cx.theme().colors().border) @@ -3000,6 +3001,7 @@ impl Element for MarkdownElement { } } MarkdownTagEnd::Table => { + builder.pop_div(); builder.pop_div(); builder.table.end(); } @@ -4585,6 +4587,7 @@ mod tests { markdown: Entity, style: MarkdownStyle, code_span_link: Option, + width: Option, cx: &mut TestAppContext, ) -> RenderedText { let rendered_text = Rc::new(RefCell::new(None)); @@ -4597,6 +4600,9 @@ mod tests { rendered_text, } }); + if let Some(width) = width { + cx.simulate_resize(size(width, px(600.))); + } cx.run_until_parked(); rendered_text @@ -4788,7 +4794,7 @@ mod tests { let markdown = cx.new(|cx| Markdown::new(markdown.to_string().into(), None, None, cx)); cx.run_until_parked(); - render_markdown_entity_in_view(markdown, style, Some(Arc::new(callback)), cx) + render_markdown_entity_in_view(markdown, style, Some(Arc::new(callback)), None, cx) } fn render_markdown_with_language_registry( @@ -4799,6 +4805,18 @@ mod tests { render_markdown_with_options(markdown, language_registry, MarkdownOptions::default(), cx) } + fn render_markdown_at_width( + markdown: &str, + width: Pixels, + cx: &mut TestAppContext, + ) -> RenderedText { + ensure_theme_initialized(cx); + + let markdown = cx.new(|cx| Markdown::new(markdown.to_string().into(), None, None, cx)); + cx.run_until_parked(); + render_markdown_entity_in_view(markdown, MarkdownStyle::default(), None, Some(width), cx) + } + fn render_markdown_with_options( markdown: &str, language_registry: Option>, @@ -4817,7 +4835,7 @@ mod tests { ) }); cx.run_until_parked(); - render_markdown_entity_in_view(markdown, MarkdownStyle::default(), None, cx) + render_markdown_entity_in_view(markdown, MarkdownStyle::default(), None, None, cx) } fn render_markdown_with_image_resolver( @@ -5165,6 +5183,114 @@ mod tests { assert_eq!(second_word, "b"); } + const REVIEW_TABLE: &str = concat!( + "| ID | Sev | File | Line | Summary |\n", + "|---|---|---|---|---|\n", + "| R1 | High | src/main.rs | 42 | Unwrap on user input can panic |\n", + "| R2 | Low | src/lib.rs | 7 | Prefer iterators over index loops |\n", + ); + + #[gpui::test] + fn test_table_columns_are_sized_to_their_content(cx: &mut TestAppContext) { + fn cells(row: &str) -> impl Iterator { + row.trim().trim_matches('|').split('|') + } + + fn table_cell_widths(rendered: &RenderedText, column_count: usize) -> Vec { + rendered.lines[..column_count] + .iter() + .map(|line| line.layout.bounds().size.width) + .collect() + } + + let header = REVIEW_TABLE.lines().next().unwrap(); + let column_count = cells(header).count(); + + let expected: Vec = (0..column_count) + .map(|column| { + let source: String = REVIEW_TABLE + .lines() + .map(|row| format!("|{}|\n", cells(row).nth(column).unwrap())) + .collect(); + *table_cell_widths(&render_markdown_at_width(&source, px(800.), cx), 1) + .first() + .unwrap() + }) + .collect(); + + let widths = table_cell_widths( + &render_markdown_at_width(REVIEW_TABLE, px(800.), cx), + column_count, + ); + for (index, (width, expected)) in widths.iter().zip(&expected).enumerate() { + assert_eq!( + width, expected, + "column {index} width does not match expected width" + ); + } + } + + #[gpui::test] + fn test_table_never_renders_past_its_available_width(cx: &mut TestAppContext) { + for width in [800., 600., 500., 400., 300., 200.] { + let rendered = render_markdown_at_width(REVIEW_TABLE, px(width), cx); + for (index, line) in rendered.lines.iter().enumerate() { + let bounds = line.layout.bounds(); + assert!( + bounds.right() <= px(width), + "cell {index} ends at {:?} which is past the available width of {width}px", + bounds.right() + ); + } + } + } + + #[gpui::test] + fn test_table_border_hugs_its_columns(cx: &mut TestAppContext) { + ensure_theme_initialized(cx); + + struct TableView { + markdown: Entity, + } + + impl Render for TableView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div().size_full().child(MarkdownElement::new( + self.markdown.clone(), + MarkdownStyle::default(), + )) + } + } + + let table_width = |width: f32, cx: &mut TestAppContext| { + let window = cx.open_window(size(px(width), px(600.)), |_, cx| { + let markdown = cx.new(|cx| Markdown::new(REVIEW_TABLE.into(), None, None, cx)); + TableView { markdown } + }); + cx.run_until_parked(); + gpui::VisualTestContext::from_window(window.into(), cx) + .debug_bounds("markdown_table") + .expect("table should have been rendered") + .size + .width + }; + + assert!( + table_width(800., cx) < px(800.), + "the table should hug its columns instead of filling the window" + ); + assert_eq!( + table_width(800., cx), + table_width(1200., cx), + "the table should keep its width when there is more room available" + ); + assert_eq!( + table_width(400., cx), + px(400.), + "the table should shrink to the available width" + ); + } + #[test] fn test_table_state_current_cell_alignment_centers_headers() { let mut table = TableState::default();