Skip to content

Tables: text wrapping options in Table Properties (#5884) #5907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
batistalucasv wants to merge 1 commit into
base: main
Choose a base branch
from
+370 −3
Open

Tables: text wrapping options in Table Properties (#5884) #5907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ROADMAP.md
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion crates/engine/src/cmd/table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ fn in_table(s: &Session) -> Option<&'static str> {
if s.sel.focus.path.cell().is_some() { None } else { Some("the cursor isn't in a table") }
}

/// Register Table Design and Table Layout commands (including text wrapping options in Table Properties), their parameters and ribbon locations.
pub fn specs() -> Vec<CommandSpec> {
let t = |c: CommandSpec| c.when(in_table);
vec![
Expand Down Expand Up @@ -225,7 +226,7 @@ pub fn specs() -> Vec<CommandSpec> {
})
.params(r#"{"value"?: bool} (true: the first column stands at the right; without a value it toggles)"#)),
t(CommandSpec::new("table.properties", "Properties", "Table Layout › Table", properties).params(
r#"{"align"?: "left|center|right", "rtl"?: bool, "width"?: pt|null, "widthPct"?: percent, "indent"?: pt, "rowHeight"?: pt|null, "rowHeightRule"?: "atLeast|exact", "allowBreak"?: bool, "headerRow"?: bool, "columnWidth"?: pt, "cellWidth"?: pt|null, "valign"?: "top|center|bottom"} (row, column and cell settings apply to the selected rows, columns and cells, or the caret's; without settings it returns the caret's current ones)"#,
r#"{"align"?: "left|center|right", "rtl"?: bool, "width"?: pt|null, "widthPct"?: percent, "indent"?: pt, "rowHeight"?: pt|null, "rowHeightRule"?: "atLeast|exact", "allowBreak"?: bool, "headerRow"?: bool, "columnWidth"?: pt, "cellWidth"?: pt|null, "valign"?: "top|center|bottom", "wrapping"?: "none|around"} (row, column and cell settings apply to the selected rows, columns and cells, or the caret's; without settings it returns the caret's current ones)"#,
)),
CommandSpec::new("table.fromText", "Convert Text to Table", "Insert › Tables", from_text)
.params(r#"{"separator"?: "tab|comma|paragraph|other", "char"?: string (with other), "columns"?: n, "autofit"?: "initial|fixed|contents|window", "width"?: pt (initial column width, 0 = Auto, at most 1584)}"#),
Expand Down
76 changes: 76 additions & 0 deletions crates/engine/src/cmd/table/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ pub(super) fn properties(s: &mut Session, v: &Value) -> CmdResult {
"columnWidth",
"cellWidth",
"valign",
"wrapping",
"wrap",
];
if !keys.iter().any(|k| v.get(*k).is_some()) {
let t = s.doc.table(s.sel.focus.story, &tp).cloned().unwrap_or_default();
Expand All @@ -102,6 +104,7 @@ pub(super) fn properties(s: &mut Session, v: &Value) -> CmdResult {
o.insert("row".into(), row.map(|x| serde_json::to_value(&x.props).unwrap_or(Value::Null)).unwrap_or(Value::Null));
o.insert("columnWidth".into(), t.grid.get(t.grid_col(r, c)).map_or(Value::Null, |w| json!(w)));
o.insert("cell".into(), cl.map(|x| serde_json::to_value(&x.props).unwrap_or(Value::Null)).unwrap_or(Value::Null));
o.insert("wrapping".into(), json!(if t.props.float.is_some() { "around" } else { "none" }));
}
return Ok(out);
}
Expand All @@ -125,6 +128,29 @@ pub(super) fn properties(s: &mut Session, v: &Value) -> CmdResult {
Some("bottom") => Some(VAlign::Bottom),
Some(x) => return Err(CmdError::Params(format!("unknown vertical alignment `{x}`"))),
};
let parse_wrap = |val: &Value| -> Result<Option<&'static str>, CmdError> {
match val {
Value::Null => Ok(Some("none")),
Value::String(s) => match s.as_str() {
"none" => Ok(Some("none")),
"around" => Ok(Some("around")),
x => Err(CmdError::Params(format!("unknown table text wrapping `{x}`"))),
},
_ => Err(CmdError::Params("expected a string for table text wrapping (`none` or `around`)".into())),
}
};
Comment thread
batistalucasv marked this conversation as resolved.
Resolved
let wrapping = match (v.get("wrapping"), v.get("wrap")) {
(None, None) => None,
(Some(w1), Some(w2)) => {
let p1 = parse_wrap(w1)?;
let p2 = parse_wrap(w2)?;
if p1 != p2 {
return Err(CmdError::Params("conflicting `wrapping` and `wrap` parameters".into()));
}
p1
}
(Some(w), None) | (None, Some(w)) => parse_wrap(w)?,
};
// `null` clears a preferred size (automatic); a number sets it.
let size = |k: &str, lo: f32, hi: f32| -> Option<Option<f32>> {
match v.get(k)? {
Expand All @@ -146,15 +172,65 @@ pub(super) fn properties(s: &mut Session, v: &Value) -> CmdResult {
let mut rows: Vec<usize> = cells.iter().map(|&(r, _)| r).collect();
rows.sort_unstable();
rows.dedup();
if wrapping == Some("around") && !(s.sel.focus.story == wordcraft_doc::StoryRef::Body && tp.0.len() == 1) {
return Err(CmdError::Disabled("only top-level tables in the body text can wrap text".into()));
}
with_table(s, |t| {
if let Some(w) = wrapping {
if w == "none" {
t.props.float = None;
} else if w == "around" {
// Reuse existing float settings when present; only initialize a default when float is None.
if t.props.float.is_none() {
let h_align = if !t.props.bidi_visual {
match t.props.align {
Some(Align::Center) => Some(wordcraft_doc::para::FloatAlign::Center),
Some(Align::Right) => Some(wordcraft_doc::para::FloatAlign::End),
_ => None,
}
} else {
None
};
t.props.float = Some(wordcraft_doc::props::TableFloat {
h_rel: wordcraft_doc::para::Anchor::Column,
v_rel: wordcraft_doc::para::Anchor::Paragraph,
h_align,
x: if h_align.is_none() { t.props.indent.unwrap_or(0.0) } else { 0.0 },
dist: [9.0, 0.0, 9.0, 0.0],
overlap: true,
..Default::default()
});
}
}
}
if let Some(a) = align {
t.props.align = Some(a);
if let Some(f) = &mut t.props.float
&& !t.props.bidi_visual
{
f.h_align = match a {
Align::Center => Some(wordcraft_doc::para::FloatAlign::Center),
Align::Right => Some(wordcraft_doc::para::FloatAlign::End),
_ => None,
};
if a == Align::Left {
f.x = t.props.indent.unwrap_or(0.0);
}
}
}
if let Some(on) = rtl {
t.props.bidi_visual = on;
if on && let Some(f) = &mut t.props.float {
f.h_align = None;
}
}
if let Some(i) = indent {
t.props.indent = Some(i);
if let Some(f) = &mut t.props.float
&& f.h_align.is_none()
{
f.x = i;
}
}
if let Some(cw) = column_width {
set_column_widths(t, &cells, cw);
Expand Down
243 changes: 243 additions & 0 deletions crates/engine/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2893,6 +2893,249 @@ fn table_properties_apply_in_one_step() {
assert!(s.run("table.properties", &json!({"valign": "middle"})).is_err());
}

/// #5884: Table Properties supports text wrapping options: "none" (inline) and "around" (floating).
#[test]
fn table_properties_text_wrapping() {
let mut s = s();
run(&mut s, "insert.table", json!({"rows": 2, "cols": 2}));
let query0 = run(&mut s, "table.properties", json!({}));
assert_eq!(query0["wrapping"], "none");
assert_eq!(s.doc.body.iter().find_map(|b| b.as_table()).unwrap().props.float, None);

// Set wrapping to around.
run(&mut s, "table.properties", json!({"wrapping": "around"}));
let query_around = run(&mut s, "table.properties", json!({}));
assert_eq!(query_around["wrapping"], "around");
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert!(t.props.float.is_some());

// Undo restores none.
run(&mut s, "edit.undo", json!({}));
assert_eq!(run(&mut s, "table.properties", json!({}))["wrapping"], "none");
assert_eq!(s.doc.body.iter().find_map(|b| b.as_table()).unwrap().props.float, None);

// Redo restores around.
run(&mut s, "edit.redo", json!({}));
assert_eq!(run(&mut s, "table.properties", json!({}))["wrapping"], "around");
assert!(s.doc.body.iter().find_map(|b| b.as_table()).unwrap().props.float.is_some());

// Set wrapping back to none.
run(&mut s, "table.properties", json!({"wrapping": "none"}));
assert_eq!(run(&mut s, "table.properties", json!({}))["wrapping"], "none");
assert_eq!(s.doc.body.iter().find_map(|b| b.as_table()).unwrap().props.float, None);

// "wrap" alias also works.
run(&mut s, "table.properties", json!({"wrap": "around"}));
assert_eq!(run(&mut s, "table.properties", json!({}))["wrapping"], "around");
run(&mut s, "table.properties", json!({"wrap": "none"}));
assert_eq!(run(&mut s, "table.properties", json!({}))["wrapping"], "none");

// Both "wrapping" and "wrap" matching succeeds.
run(&mut s, "table.properties", json!({"wrapping": "around", "wrap": "around"}));
assert_eq!(run(&mut s, "table.properties", json!({}))["wrapping"], "around");
run(&mut s, "table.properties", json!({"wrapping": "none", "wrap": "none"}));
assert_eq!(run(&mut s, "table.properties", json!({}))["wrapping"], "none");

// Conflicting "wrapping" and "wrap" parameters return an error.
assert!(s.run("table.properties", &json!({"wrapping": "around", "wrap": "none"})).is_err());
assert!(s.run("table.properties", &json!({"wrapping": "none", "wrap": "around"})).is_err());

// Invalid wrapping value returns error (unknown strings and non-string types).
assert!(s.run("table.properties", &json!({"wrapping": "tight"})).is_err());
assert!(s.run("table.properties", &json!({"wrap": "tight"})).is_err());
assert!(s.run("table.properties", &json!({"wrapping": 123})).is_err());
assert!(s.run("table.properties", &json!({"wrap": true})).is_err());
assert!(s.run("table.properties", &json!({"wrapping": []})).is_err());
assert!(s.run("table.properties", &json!({"wrap": {}})).is_err());

// Nested tables cannot wrap text around.
run(&mut s, "insert.table", json!({"rows": 1, "cols": 1}));
let (nested_path, _, _) = s.sel.focus.path.cell().unwrap();
assert_eq!(nested_path.0.len(), 4, "inside nested table");
let err = s.run("table.properties", &json!({"wrapping": "around"}));
assert!(matches!(err, Err(crate::CmdError::Disabled(_))), "nested table text wrapping around rejected");
// Nested table setting wrapping to none is permitted.
assert!(s.run("table.properties", &json!({"wrapping": "none"})).is_ok());

// Non-body tables (e.g. in headers/footers) also cannot wrap text around.
let hid = s.doc.add_part(
wordcraft_doc::PartKind::Header,
vec![std::sync::Arc::new(wordcraft_doc::Block::Para(wordcraft_doc::Paragraph::with_text("header", Default::default())))],
);
s.doc.last_section.headers.default = Some(hid);
s.sel = crate::Selection::caret(s.doc.start_of(wordcraft_doc::StoryRef::Part(hid)));
run(&mut s, "insert.table", json!({"rows": 1, "cols": 1}));
let err_header = s.run("table.properties", &json!({"wrapping": "around"}));
assert!(matches!(err_header, Err(crate::CmdError::Disabled(_))), "header table text wrapping around rejected");
assert!(s.run("table.properties", &json!({"wrapping": "none"})).is_ok());
}

/// #5884: Table Properties text wrapping preserves custom float settings and initializes alignment.
#[test]
fn table_properties_text_wrapping_float_alignment_and_preservation() {
let mut s = s();
run(&mut s, "insert.table", json!({"rows": 2, "cols": 2}));

// A pre-existing custom float is preserved when "around" is re-applied,
// and is restored intact after "none" via undo.
let custom_float = wordcraft_doc::props::TableFloat {
h_rel: wordcraft_doc::para::Anchor::Margin,
v_rel: wordcraft_doc::para::Anchor::Page,
x: 42.0,
y: 108.0,
dist: [12.0, 15.0, 12.0, 15.0],
overlap: false,
..Default::default()
};
let (tp, _, _) = s.sel.focus.path.cell().unwrap();
s.doc.table_mut(s.sel.focus.story, &tp).unwrap().props.float = Some(custom_float);
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float, Some(custom_float));

// Re-applying "around" preserves custom float settings without resetting to default.
run(&mut s, "table.properties", json!({"wrapping": "around"}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float, Some(custom_float));

// Changing non-alignment properties with "around" preserves the custom float.
run(&mut s, "table.properties", json!({"width": 200.0, "wrapping": "around"}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float, Some(custom_float));

// Changing alignment on a wrapped table updates the float's h_align and position.
run(&mut s, "table.properties", json!({"align": "center"}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float.unwrap().h_align, Some(wordcraft_doc::para::FloatAlign::Center));

run(&mut s, "table.properties", json!({"align": "right"}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float.unwrap().h_align, Some(wordcraft_doc::para::FloatAlign::End));

run(&mut s, "table.properties", json!({"align": "left", "indent": 18.0}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float.unwrap().h_align, None);
assert_eq!(t.props.float.unwrap().x, 18.0);

// Setting "none" clears the float.
run(&mut s, "table.properties", json!({"wrapping": "none"}));
assert_eq!(s.doc.body.iter().find_map(|b| b.as_table()).unwrap().props.float, None);

// Undo restores the previous float settings.
run(&mut s, "edit.undo", json!({}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert!(t.props.float.is_some());
run(&mut s, "table.properties", json!({"wrapping": "none", "align": "left", "indent": 0.0}));

// When enabling around wrapping, float position is initialized from table alignment for LTR tables.
run(&mut s, "table.properties", json!({"align": "center", "wrapping": "around"}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float.unwrap().h_align, Some(wordcraft_doc::para::FloatAlign::Center));
run(&mut s, "table.properties", json!({"wrapping": "none"}));

run(&mut s, "table.properties", json!({"align": "right", "wrapping": "around"}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float.unwrap().h_align, Some(wordcraft_doc::para::FloatAlign::End));
run(&mut s, "table.properties", json!({"wrapping": "none"}));

// RTL table retains None for float h_align.
run(&mut s, "table.properties", json!({"align": "right", "rtl": true, "wrapping": "around"}));
let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.props.float.unwrap().h_align, None);
run(&mut s, "table.properties", json!({"wrapping": "none", "rtl": false, "align": "left"}));

// Left-aligned indented table retains position when wrapped around.
run(&mut s, "table.columnWidth", json!({"width": 80.0}));
run(&mut s, "table.properties", json!({"indent": 36.0, "wrapping": "none"}));
let x_left_none = s.layout().pages[0]
.items
.iter()
.find_map(|it| match it {
wordcraft_layout::Placed::Cell { rect, row: 0, cell: 0, .. } => Some(rect.x),
_ => None,
})
.unwrap();
run(&mut s, "table.properties", json!({"wrapping": "around"}));
let x_left_around = s.layout().pages[0]
.items
.iter()
.find_map(|it| match it {
wordcraft_layout::Placed::Cell { rect, row: 0, cell: 0, .. } => Some(rect.x),
_ => None,
})
.unwrap();
assert!((x_left_none - x_left_around).abs() < 1.0, "left-aligned indented table retains position when wrapped around");

// Narrow centered/right-aligned tables retain alignment in page layout when wrapped around.
run(&mut s, "table.properties", json!({"align": "center", "indent": 0.0, "wrapping": "none"}));
let x_center_none = s.layout().pages[0]
.items
.iter()
.find_map(|it| match it {
wordcraft_layout::Placed::Cell { rect, row: 0, cell: 0, .. } => Some(rect.x),
_ => None,
})
.unwrap();
run(&mut s, "table.properties", json!({"wrapping": "around"}));
let x_center_around = s.layout().pages[0]
.items
.iter()
.find_map(|it| match it {
wordcraft_layout::Placed::Cell { rect, row: 0, cell: 0, .. } => Some(rect.x),
_ => None,
})
.unwrap();
assert!((x_center_none - x_center_around).abs() < 1.0, "centered table retains position when wrapped around");

run(&mut s, "table.properties", json!({"align": "right", "wrapping": "none"}));
let x_right_none = s.layout().pages[0]
.items
.iter()
.find_map(|it| match it {
wordcraft_layout::Placed::Cell { rect, row: 0, cell: 0, .. } => Some(rect.x),
_ => None,
})
.unwrap();
run(&mut s, "table.properties", json!({"wrapping": "around"}));
let x_right_around = s.layout().pages[0]
.items
.iter()
.find_map(|it| match it {
wordcraft_layout::Placed::Cell { rect, row: 0, cell: 0, .. } => Some(rect.x),
_ => None,
})
.unwrap();
assert!((x_right_none - x_right_around).abs() < 1.0, "right-aligned table retains position when wrapped around");
}

/// #5884: DOCX round-trip preserves table text wrapping ("around" and "none").
#[test]
fn table_properties_text_wrapping_docx_round_trip() {
let mut s = s();
run(&mut s, "insert.table", json!({"rows": 2, "cols": 2}));
run(&mut s, "table.properties", json!({"wrapping": "around"}));
let bytes = wordcraft_docx::write(&s.doc).unwrap();
let back = wordcraft_docx::read(&bytes).unwrap();
let t_back = back.body.iter().find_map(|b| b.as_table()).unwrap();
assert!(t_back.props.float.is_some(), "table float persists across DOCX round-trip");
let f_back = t_back.props.float.unwrap();
assert_eq!(f_back.h_rel, wordcraft_doc::para::Anchor::Column);
assert_eq!(f_back.v_rel, wordcraft_doc::para::Anchor::Paragraph);
assert!(f_back.overlap);

let mut s_back = Session::new(back);
// Put caret into table
let tp = s_back.doc.body.iter().position(|b| b.as_table().is_some()).unwrap();
s_back.sel.focus = Pos::new(wordcraft_doc::StoryRef::Body, wordcraft_doc::Path(vec![tp as u32, 0, 0, 0]), 0);
s_back.sel.anchor = s_back.sel.focus.clone();
assert_eq!(run(&mut s_back, "table.properties", json!({}))["wrapping"], "around");

run(&mut s_back, "table.properties", json!({"wrapping": "none"}));
let bytes2 = wordcraft_docx::write(&s_back.doc).unwrap();
let back2 = wordcraft_docx::read(&bytes2).unwrap();
let t_back2 = back2.body.iter().find_map(|b| b.as_table()).unwrap();
assert!(t_back2.props.float.is_none(), "table float cleared across DOCX round-trip");
}

/// #4857: Table Properties' row settings apply to every selected row (the whole table, say), the
/// column width to every selected column and the cell settings to every selected cell, as in
/// Word, in one undo step; with nothing selected, to the caret's alone.
Expand Down
Loading
Loading