shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

#833: pin hidden-row formatting, grow-only restore branch, tighten tests

This commit is contained in:
Eduardo Soares 2026-10-12 20:05:58 -03:00
parent dc3ccc4ea0
commit 26c6e3df44
3 changed files with 44 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/engine/src/cmd/format.rs
View File

@ -241,7 +241,7 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) ->
let new = *cache.entry(*st).or_insert_with(|| cx.wb.styles.derive(*st, &f));
cx.sheet_mut(sheet)?.set_style(*c, new);
}
// Formatting changed these cells' fonts: re-fit their rows (Excel auto-fits).
// Formatting touched these cells: re-fit their rows (Excel auto-fits).
cx.fit_rows.extend(cells.iter().map(|(c, _)| (sheet, c.row)));
continue;
}

42
crates/engine/src/tests.rs
View File

@ -1008,17 +1008,20 @@ fn spill_references_roundtrip_through_xlsx() {
}
/// Fixture for the open-time row fit (issue #833): `row_info` = (size, custom, hidden) for
/// row 5 (index 4), `None` = no row entry at all. Cell A5 = "plain big bold", whole-cell
/// font 20pt. 11pt cell in A1 for contrast rows. Built with the model API, never through
/// commands — commands would auto-fit the rows — and exported with `write_xlsx`, which
/// omits `ht` for rows without a size: exactly the bug's input.
fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec<u8> {
/// row 5 (index 4), `None` = no row entry at all. Cell A5 = "plain big bold" — whole-cell
/// font 20pt with `big_font`, plain 11pt without (a tall plain row's fit removes its entry).
/// 11pt cell in A1 for contrast rows. Built with the model API, never through commands —
/// commands would auto-fit the rows — and exported with `write_xlsx`, which omits `ht` for
/// rows without a size: exactly the bug's input.
fn big_font_workbook(row_info: Option<(f32, bool, bool)>, big_font: bool) -> Vec<u8> {
let mut wb = gridcraft_model::Workbook::new();
let big = wb.styles.derive(gridcraft_model::StyleId::DEFAULT, |s| s.font.size = 20.0);
let sh = wb.sheet_mut(0).unwrap();
sh.set_value(CellRef::new(0, 0), Value::text("small"));
sh.set_value(CellRef::new(4, 0), Value::text("plain big bold"));
sh.set_style(CellRef::new(4, 0), big);
if big_font {
sh.set_style(CellRef::new(4, 0), big);
}
if let Some((size, custom, hidden)) = row_info {
let e = sh.rows.entry(4).or_default();
e.size = Some(size);
@ -1032,7 +1035,7 @@ fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec<u8> {
fn open_grows_default_height_rows_to_their_font() {
let default_h = gridcraft_model::Workbook::new().active().unwrap().row_height(0);
let mut s = Session::new();
s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap();
s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None, true))})).unwrap();
let h = s.doc().unwrap().wb.active().unwrap().row_height(4);
assert!(h >= 30.0, "20pt cell must grow its default-height row on open, got {h}");
assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(0), default_h, "11pt row stays default");
@ -1041,35 +1044,48 @@ fn open_grows_default_height_rows_to_their_font() {
#[test] // B — Review Focus 1
fn open_keeps_custom_height_rows() {
let mut s = Session::new();
s.execute("file.open", json!({"name": "custom.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((25.0, true, false))))}))
s.execute("file.open", json!({"name": "custom.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((25.0, true, false)), true))}))
.unwrap();
assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows are left alone on open");
}
#[test] // C — Review Focus 2
#[test] // C — Review Focus 2 (Some→Some: the fit shrinks the row, the clamp keeps the taller height)
fn open_never_shrinks_taller_auto_rows() {
let mut s = Session::new();
s.execute("file.open", json!({"name": "tall.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false))))})).unwrap();
s.execute("file.open", json!({"name": "tall.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false)), true))}))
.unwrap();
assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 60.0, "open is grow-only: a 60px auto-height row keeps its height");
}
#[test] // C′ — Review Focus 2 (Some→None: a plain tall row's entry is removed by the fit, the clamp restores it)
fn open_restores_height_of_plain_rows_the_fit_removed() {
let mut s = Session::new();
s.execute(
"file.open",
json!({"name": "tall-plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false)), false))}),
)
.unwrap();
assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 60.0, "a tall plain row keeps its height after the fit removes its entry");
}
#[test] // D — Review Focus 5
fn open_fitting_keeps_hidden_rows_hidden() {
let mut s = Session::new();
s.execute("file.open", json!({"name": "hidden.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((20.0, false, true))))}))
s.execute("file.open", json!({"name": "hidden.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((20.0, false, true)), true))}))
.unwrap();
let sh = s.doc().unwrap().wb.active().unwrap();
let info = sh.rows.get(&4).unwrap();
assert!(info.size.is_none_or(|h| h >= 20.0), "hidden row grows or keeps its height, got {:?}", info.size);
assert!(info.size.is_some_and(|h| h >= 20.0), "hidden row grows or keeps its height, got {:?}", info.size);
assert!(info.hidden, "fitting a hidden row must not unhide it");
}
#[test] // E — Review Focus 4
fn open_leaves_plain_workbooks_untouched() {
let mut s = Session::new();
s.execute("file.open", json!({"name": "plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap();
s.execute("file.open", json!({"name": "plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None, false))})).unwrap();
let sh = s.doc().unwrap().wb.active().unwrap();
assert!(sh.rows.get(&0).is_none_or(|i| i.size.is_none()), "11pt default rows get no height entry");
assert!(sh.rows.values().all(|i| i.size.is_none()), "no row gains a height entry");
}
#[test]

14
crates/engine/tests/row_autofit.rs
View File

@ -45,3 +45,17 @@ fn whole_row_format_does_not_resize_custom_height_rows() {
s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap();
assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows keep their height");
}
#[test]
fn whole_row_format_keeps_hidden_rows_hidden() {
let mut s = gridcraft_engine::Session::new();
s.new_workbook();
s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap();
let before = s.doc().unwrap().wb.active().unwrap().row_height(4);
sheet_mut(&mut s).rows.entry(4).or_default().hidden = true;
s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap();
let sh = s.doc().unwrap().wb.active().unwrap();
let info = sh.rows.get(&4).unwrap();
assert!(info.size.unwrap_or(before) > before + 10.0, "hidden row still grows to its font: before={before}, size={:?}", info.size);
assert!(info.hidden, "formatting a hidden row must not unhide it");
}