markdown: Auto-size table columns to content width (#61773)
Closes https://github.com/zed-industries/zed/issues/62498 Fixes #50044: | before | after | | --- | --- | | <img width="1260" height="1449" alt="before" src="https://github.com/user-attachments/assets/84dfca5a-66da-4179-90f3-8408255346d4" /> | <img width="1260" height="1449" alt="after" src="https://github.com/user-attachments/assets/d63f9128-1397-4f1a-87a5-4e681e9b65f0" /> | ~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.
This commit is contained in:
1 parent
3c8231e1c7
commit
63d4e29a12
5 files changed
+180
-20
No files matched your search
Generated
+3
-9
@@ -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]]
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<Self>) -> 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.));
|
||||
}
|
||||
}
|
||||
@@ -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(
|
||||
|
||||
@@ -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<Markdown>,
|
||||
style: MarkdownStyle,
|
||||
code_span_link: Option<CodeSpanLinkCallback>,
|
||||
width: Option<Pixels>,
|
||||
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<Arc<LanguageRegistry>>,
|
||||
@@ -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<Item = &str> {
|
||||
row.trim().trim_matches('|').split('|')
|
||||
}
|
||||
|
||||
fn table_cell_widths(rendered: &RenderedText, column_count: usize) -> Vec<Pixels> {
|
||||
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<Pixels> = (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<Markdown>,
|
||||
}
|
||||
|
||||
impl Render for TableView {
|
||||
fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> 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();
|
||||
|
||||
Reference in new issue
Block a user