From 7deb382f3d6e3828cebb7224f8ead4fad4f7e3d5 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:22:31 +0300 Subject: [PATCH] xlsx: keep a table's filter criteria when the sheet AutoFilter coincides with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine's live filter state is always sheet.autofilter — apply_filter, filter_values, clear_filter, reapply and the slicer command all read and write it, and a table's header dropdown installs one over the table's own range — while Table::filter is only ever filled by the reader. Writing suppressed the sheet whenever any table merely intersected it and wrote the table part from Table::filter, which was empty: a filtered table saved with its rows still hidden and no criteria anywhere, so on reopen there was no filter to clear and Excel showed an inactive filter over mysteriously hidden rows. A slicer's selection was lost the same way. Fold the coinciding sheet filter's criteria into the table part (colId offsets are relative to the range's first column, and both ranges start at the same column, so they carry over verbatim); a filter that only partially covers a table still has nowhere valid to go and keeps being suppressed. Regression test: a_table_filter_criteria_survive_a_round_trip — on main the table part carries no criteria after a save; with the fold they round trip, the hidden row stays, and the sheet keeps no duplicate autoFilter. --- crates/xlsx/src/sheet_write.rs | 25 +++++++++++-- crates/xlsx/src/tests/roundtrip.rs | 57 ++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 78e1afaae..98946f6ac 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -76,10 +76,29 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( let nstyles = wb.styles.len() as u32; let sid = |s: gridcraft_model::StyleId| if s.0 < nstyles { s.0 } else { 0 }; + // A table's dropdown filter is kept in `sheet.autofilter`, not in `Table::filter` (the + // engine's only live filter state, which the reader also fills). Where the two coincide, + // fold the sheet filter's criteria into the table so they reach the file: the stored + // `colId` offsets are relative to the range's first column, and both ranges start at the + // same column, so they carry over verbatim. A filter that only partially covers a table + // has nowhere valid to go, so it keeps being suppressed below. + let folded: Vec = match &sheet.autofilter { + Some(af) if !af.criteria.is_empty() => sheet + .tables + .iter() + .cloned() + .map(|mut t| { + if t.header_row && t.filter_button && t.filter.is_empty() && t.range.contains_range(&af.range) { + t.filter = af.criteria.clone(); + } + t + }) + .collect(), + _ => sheet.tables.clone(), + }; // Table header cells must hold the column names. let mut overrides: BTreeMap = BTreeMap::new(); - let tables: Vec<(&Table, Vec)> = sheet - .tables + let tables: Vec<(&Table, Vec)> = folded .iter() .filter(|t| t.range.height() > t.header_row as u32 + t.totals_row as u32 || (t.header_row && t.range.height() > t.totals_row as u32)) .map(|t| (t, table_columns(t))) @@ -341,7 +360,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( s.push_str(&protected_ranges_xml(sheet)); // autoFilter (a sheet filter overlapping a table would be invalid). if let Some(af) = &sheet.autofilter - && !sheet.tables.iter().any(|t| t.range.intersects(&af.range)) + && !folded.iter().any(|t| t.range.contains_range(&af.range) && t.header_row && t.filter_button) { s.push_str(&autofilter_xml(af.range, &af.criteria, out)); } diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 901802905..b2789a3dd 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -1164,3 +1164,60 @@ fn doc_props_keep_boundary_whitespace() { let (back, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); assert_eq!(back.props, wb.props); } + +/// A table's dropdown filter lives in `sheet.autofilter` (the engine's only live filter state), +/// not in `Table::filter`. Writing used to suppress the sheet `` because a table +/// intersects it and write the table part from `Table::filter`, which was empty: the criteria +/// were lost on every save, leaving the rows hidden with no filter to clear on reopen. +#[test] +fn a_table_filter_criteria_survive_a_round_trip() { + let mut wb = Workbook::new(); + let s = wb.sheet_mut(0).unwrap(); + for (r, v) in [(0u32, "Region"), (1, "East"), (2, "West"), (3, "East")] { + s.set_value(at(&format!("A{}", r + 1)), Value::text(v)); + } + s.tables.push(Table { + id: 1, + name: "Sales".into(), + range: rr("A1:A4"), + header_row: true, + totals_row: false, + columns: vec![TableColumn { + name: "Region".into(), + totals: TotalsFn::None, + totals_label: None, + formula: None, + unique_name: None, + xml: None, + }], + style: "TableStyleMedium2".into(), + banded_rows: true, + banded_cols: false, + first_col: false, + last_col: false, + filter_button: true, + filter: vec![], + table_type: None, + connection_id: None, + }); + // The dropdown of a table's header cell installs the sheet AutoFilter over the table's + // range, and `data.filterBy` stores its criteria there. + s.autofilter = Some(AutoFilter { + range: rr("A1:A4"), + criteria: vec![(0, FilterCriterion::Values { values: vec!["East".into()], blanks: false })], + }); + s.rows.entry(1).or_default().hidden = true; + + let (back, rep) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); + assert!(rep.warnings.is_empty(), "{:?}", rep.warnings); + let b = back.sheet(0).unwrap(); + let t = b.tables.first().expect("the table survived"); + let (_, crit) = t.filter.first().expect("the table's filter criteria were kept on save"); + assert!( + matches!(crit, FilterCriterion::Values { values, .. } if values == &["East".to_string()]), + "the criteria were dropped: {:?}", + t.filter + ); + assert!(b.is_row_hidden(1), "the row the filter hid is still hidden"); + assert!(b.autofilter.is_none(), "a table's filter belongs to the table part, not the sheet"); +}