From 084b76d9def920f519e43c26a9eb5f59c6231df7 Mon Sep 17 00:00:00 2001 From: Divyanshu Gupta Date: Thu, 22 Oct 2026 00:04:20 +0530 Subject: [PATCH] fix(engine): preserve existing table cell margins when side is omitted In table.cellMargins, resolve existing margins from the table, its style, or defaults and only overwrite the sides explicitly provided in the command parameters. Closes #3490 --- crates/engine/src/cmd/tools.rs | 15 ++++++++++----- crates/engine/src/tests.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/tools.rs b/crates/engine/src/cmd/tools.rs index b02408dd2..582189473 100644 --- a/crates/engine/src/cmd/tools.rs +++ b/crates/engine/src/cmd/tools.rs @@ -629,14 +629,19 @@ pub fn specs() -> Vec { .params(r#"{"header"?: pt, "footer"?: pt}"#), CommandSpec::new("table.cellMargins", "Cell Margins", "Table Layout › Alignment", |s, v| { let Some((tp, _, _)) = s.sel.focus.path.cell() else { return Err(CmdError::Disabled("not in a table".into())) }; + let story = s.sel.focus.story; + let current = { + let table = s.doc.table(story, &tp).ok_or_else(|| CmdError::Failed("no table".into()))?; + let style = table.props.style.as_deref().and_then(|id| s.doc.styles.table_style(id)); + table.props.cell_margins.or(style.and_then(|st| st.parts.cell_margins)).unwrap_or([0.0, 5.4, 0.0, 5.4]) + }; let m = [ - p::f32(v, "top").unwrap_or(0.0), - p::f32(v, "left").unwrap_or(5.4), - p::f32(v, "bottom").unwrap_or(0.0), - p::f32(v, "right").unwrap_or(5.4), + p::f32(v, "top").unwrap_or(current[0]), + p::f32(v, "left").unwrap_or(current[1]), + p::f32(v, "bottom").unwrap_or(current[2]), + p::f32(v, "right").unwrap_or(current[3]), ] .map(|x| x.clamp(0.0, 144.0)); - let story = s.sel.focus.story; s.doc.table_mut(story, &tp)?.props.cell_margins = Some(m); for path in s.doc.para_paths(story).into_iter().filter(|p| p.0.starts_with(&tp.0) && p.0.len() > tp.0.len()) { s.doc.para_mut(story, &path)?.touch(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 4031e230a..b3231be92 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -8491,3 +8491,36 @@ fn rtf_list_round_trip_keeps_text() { assert_eq!(d.plain_text(StoryRef::Body), "first\nsecond", "{cmd}"); } } + +/// #3490: `table.cellMargins` preserves existing margins for any sides not passed. +#[test] +fn table_cell_margins_preserves_unpassed_sides() { + let mut s = s(); + run(&mut s, "insert.table", json!({"rows": 1, "cols": 1})); + let (tp, _, _) = s.sel.focus.path.cell().unwrap(); + + // 1. Updating 'top' only retains default left (5.4), bottom (0.0), and right (5.4). + run(&mut s, "table.cellMargins", json!({"top": 20.0})); + let margins = s.doc.table(StoryRef::Body, &tp).unwrap().props.cell_margins.unwrap(); + assert_eq!(margins, [20.0, 5.4, 0.0, 5.4]); + + // 2. Updating 'left' subsequently retains previously set top (20.0), bottom (0.0), and right (5.4). + run(&mut s, "table.cellMargins", json!({"left": 10.0})); + let margins = s.doc.table(StoryRef::Body, &tp).unwrap().props.cell_margins.unwrap(); + assert_eq!(margins, [20.0, 10.0, 0.0, 5.4]); + + // 3. Updating 'bottom' retains top (20.0), left (10.0), and right (5.4). + run(&mut s, "table.cellMargins", json!({"bottom": 8.0})); + let margins = s.doc.table(StoryRef::Body, &tp).unwrap().props.cell_margins.unwrap(); + assert_eq!(margins, [20.0, 10.0, 8.0, 5.4]); + + // 4. Updating 'right' retains top (20.0), left (10.0), and bottom (8.0). + run(&mut s, "table.cellMargins", json!({"right": 15.0})); + let margins = s.doc.table(StoryRef::Body, &tp).unwrap().props.cell_margins.unwrap(); + assert_eq!(margins, [20.0, 10.0, 8.0, 15.0]); + + // 5. Updating multiple sides at once retains unpassed sides. + run(&mut s, "table.cellMargins", json!({"top": 0.0, "bottom": 0.0})); + let margins = s.doc.table(StoryRef::Body, &tp).unwrap().props.cell_margins.unwrap(); + assert_eq!(margins, [0.0, 10.0, 0.0, 15.0]); +}