From 3fa6cb470b408f3613f7412d036f0445d551b2b6 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 08:47:51 -0700 Subject: [PATCH 1/2] PivotTables: right-clicking a value cell offers Show Details in the cell menu The cell menu of a PivotTable's value, subtotal or grand total cell now has a Show Details item after Sort Z to A. It runs pivot.showDetails on the active cell, which lists the cell's source records on a new sheet, as double-clicking the cell does. Labels and headers of a current report, and cells outside a report, do not offer it. In a report that is out of date every cell of the report offers the item, and running it shows the message that asks for a refresh. Whether the menu offers the item is cached per document, revision, sheet and active cell. --- ROADMAP.md | 1 + crates/ui-egui/src/grid.rs | 32 ++++++ crates/ui-egui/tests/it/main.rs | 1 + .../tests/it/pivot_show_details_menu.rs | 106 ++++++++++++++++++ docs/ui-parity.md | 5 +- 5 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 crates/ui-egui/tests/it/pivot_show_details_menu.rs diff --git a/ROADMAP.md b/ROADMAP.md index da06359b5..e0a3c218c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -135,6 +135,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-24 | PivotTables: right-clicking a value, subtotal or grand total cell offers Show Details in the cell menu, which lists the cell's source records on a new sheet as double-clicking does | | 2026-10-23 | Accounting negatives in text: `$(1,234)`, `$ (5)` and `(5)%` read as negative numbers when typed, coerced by arithmetic or passed to VALUE, like Excel 365; `(-5)`, `((5))`, `$5%` and dates or times in parentheses stay text | | 2026-10-23 | Calc: `INDEX(A1:C3,2)` (one index on a reference of several rows and columns) is `#REF!` as in Excel, instead of the whole row; `INDEX(A1:C3,2,)` still gives the row | | 2026-10-22 | Formula hardening: digits too many for a finite number are text, so `"1 <400 digits>/<400 digits>"` no longer coerces to NaN; a NaN column index in VLOOKUP, HLOOKUP and the D-functions is `#VALUE!` instead of an index underflow; REGEXREPLACE stops building a result as soon as it can only be too long (a worst case went from 1 GB to 7 MB) (#2163, #2162) | diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index b485104b1..abe8bcf42 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -163,6 +163,9 @@ pub struct GridState { /// The PivotTable Fields pane's `pivot.fields` answer, cached per (doc uid, revision, pivot): /// it scans the whole source list, far too slow to repeat every frame (#2113). pub pivot_fields: Option<((u64, u64, String), serde_json::Value)>, + /// Whether the cell menu offers Show Details, cached per (doc uid, revision, sheet, active + /// cell): finding out computes the whole PivotTable report. + pub show_details_item: Option<((u64, u64, usize, CellRef), bool)>, pub header_menu: Option<(Pos2, bool)>, /// Page Layout view pagination, cached per (doc uid, revision, sheet, rows, columns). pub paper: Option, @@ -3353,6 +3356,12 @@ fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { if let Some(item) = insert_copied { items.insert(6, item); } + if show_details_offered(app) + && let Some(i) = items.iter().position(|(_, id)| *id == "data.sortDescending") + { + items.insert(i + 1, ("-", "")); + items.insert(i + 2, ("Show Details", "pivot.showDetails")); + } let lang = crate::i18n::current(ui.ctx()); for (label, id) in items { if label == "-" { @@ -3409,6 +3418,29 @@ fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { } } +/// Whether the cell menu offers Show Details (Excel's right-click › Show Details): the active cell +/// is a value, subtotal or grand total cell of a PivotTable, or a cell of a report that is out of +/// date, where running the command says so. Labels and headers of a current report don't offer it. +fn show_details_offered(app: &mut SheetApp) -> bool { + let Some(d) = app.session.active() else { return false }; + let (si, at) = (d.wb.active_sheet, d.selection.active); + let key = (d.uid, d.revision, si, at); + if let Some((k, on)) = app.grid.show_details_item + && k == key + { + return on; + } + let on = d.wb.active().and_then(|sh| sh.pivots.iter().find(|pt| pt.last_range.is_some_and(|r| r.contains(at)))).is_some_and(|pt| { + let origin = pt.last_range.map(|r| r.start).unwrap_or_default(); + !matches!( + gridcraft_engine::pivot::detail_records(&d.wb, pt, si, at.row.saturating_sub(origin.row), at.col.saturating_sub(origin.col)), + Ok(None) + ) + }); + app.grid.show_details_item = Some((key, on)); + on +} + /// The ways to fill a dragged range, as the fill menu and the Auto Fill Options button list them. const FILL_MODES: [(&str, &str); 4] = [("Copy Cells", "copy"), ("Fill Series", "extend"), ("Fill Formatting Only", "formats"), ("Fill Without Formatting", "values")]; diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 3a9769c1c..4a70a8f7a 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -62,6 +62,7 @@ mod page_setup_margins; mod paste_special_dialog; mod pivot_pane; mod pivot_show_details; +mod pivot_show_details_menu; mod popup_dismissal; mod protect_password; mod right_to_left; diff --git a/crates/ui-egui/tests/it/pivot_show_details_menu.rs b/crates/ui-egui/tests/it/pivot_show_details_menu.rs new file mode 100644 index 000000000..02527d8a3 --- /dev/null +++ b/crates/ui-egui/tests/it/pivot_show_details_menu.rs @@ -0,0 +1,106 @@ +//! Right-clicking a PivotTable's value cell offers Show Details in the cell menu (Excel's +//! right-click › Show Details); labels of the report and ordinary cells don't offer it. + +use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::Session; +use gridcraft_engine::core::CellRef; +use gridcraft_ui_egui::{SheetApp, grid::Geo}; +use serde_json::json; + +fn harness() -> egui_kittest::Harness<'static, SheetApp> { + let mut session = Session::new(); + session.new_workbook(); + session.execute("range.setValues", json!({"range": "A1", "values": [["Region", "Sales"], ["East", 100], ["West", 80], ["East", 30]]})).unwrap(); + // Report at D3: D4 East, E4 its Sum of Sales, E6 the Grand Total. + session + .execute("insert.pivotTable", json!({"source": "Sheet1!A1:B4", "destination": "Sheet1!D3", "rows": ["Region"], "values": ["Sales"]})) + .unwrap(); + session.execute("selection.set", json!({"cell": "A1"})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_step_dt(1.0 / 60.0).with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +fn center(h: &egui_kittest::Harness<'_, SheetApp>, cell: &str) -> Pos2 { + let app = h.state(); + let sheet = app.session.active().unwrap().wb.active().unwrap(); + let geo = Geo::new(sheet, app.grid.rect.unwrap(), app.view().scroll); + geo.cell_rect(sheet, CellRef::parse(cell).unwrap()).center() +} + +fn right_click(h: &mut egui_kittest::Harness<'_, SheetApp>, cell: &str) { + let pos = center(h, cell); + h.input_mut().events.push(Event::PointerMoved(pos)); + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Secondary, pressed, modifiers: Modifiers::NONE }); + h.run_steps(2); + } + assert!(h.state().grid.context_menu.is_some(), "the cell menu opens on {cell}"); +} + +#[test] +fn show_details_in_the_cell_menu_lists_a_value_cells_records_on_a_new_sheet() { + let mut h = harness(); + right_click(&mut h, "E4"); + h.get_by_label("Show Details").click(); + h.run_steps(2); + + let app = h.state(); + assert!(app.grid.context_menu.is_none(), "the menu closes"); + let wb = &app.session.active().unwrap().wb; + assert_eq!(wb.sheets.len(), 2); + let sheet = wb.active().unwrap(); + let text = |a: &str| sheet.value(CellRef::parse(a).unwrap()).display(); + assert_eq!( + [text("A1"), text("B1"), text("A2"), text("B2"), text("A3"), text("B3"), text("A4")], + ["Region", "Sales", "East", "100", "East", "30", ""] + ); +} + +#[test] +fn show_details_in_the_cell_menu_on_the_grand_total_takes_every_record() { + let mut h = harness(); + right_click(&mut h, "E6"); + h.get_by_label("Show Details").click(); + h.run_steps(2); + + let wb = &h.state().session.active().unwrap().wb; + assert_eq!(wb.sheets.len(), 2); + let sheet = wb.active().unwrap(); + assert_eq!(sheet.value(CellRef::parse("B4").unwrap()).display(), "30"); +} + +#[test] +fn labels_of_the_report_and_ordinary_cells_have_no_show_details_item() { + let mut h = harness(); + for cell in ["D4", "D3", "A2"] { + right_click(&mut h, cell); + assert!(h.query_by_label("Sort A to Z").is_some(), "the cell menu is up on {cell}"); + assert!(h.query_by_label("Show Details").is_none(), "{cell} offers no Show Details"); + let escape = Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }; + h.input_mut().events.push(escape); + h.run_steps(2); + } +} + +#[test] +fn show_details_in_an_out_of_date_report_asks_for_a_refresh() { + let mut h = harness(); + // West goes away without a refresh: the report on the sheet is no longer what the source gives. + h.state_mut().session.execute("range.setValues", json!({"range": "Sheet1!A3", "values": [["East"]]})).unwrap(); + h.run_steps(2); + right_click(&mut h, "E4"); + h.get_by_label("Show Details").click(); + h.run_steps(2); + + let app = h.state(); + assert!(app.message.as_ref().is_some_and(|(_, m)| m.contains("out of date")), "{:?}", app.message); + assert_eq!(app.session.active().unwrap().wb.sheets.len(), 1); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 5d5f1efee..d7715c3aa 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-23 · **Change:** minor (New Formatting Rule's formula box takes references clicked or dragged on the grid, #2348; Page Up and Ctrl+Up into frozen rows, Ctrl+Left and Alt+Page Up into frozen columns bring the scrolled area back; Auto Fill Options button after a left-drag of the fill handle (#1403); Show Outline Symbols, Ctrl+8; a too-wide number in a repeat-fill format such as Accounting shows #### instead of losing its leading digits; Esc and Enter close a data validation alert over the cell editor; Ctrl+Enter fill gives each cell the automatic number format typing gives it; keytip letters typed with Alt held, #1846; Number Stored as Text indicator and Convert to Number) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-24 · **Change:** minor (Show Details in the cell menu of a PivotTable value cell; New Formatting Rule's formula box takes references clicked or dragged on the grid, #2348; Page Up and Ctrl+Up into frozen rows, Ctrl+Left and Alt+Page Up into frozen columns bring the scrolled area back; Auto Fill Options button after a left-drag of the fill handle (#1403); Show Outline Symbols, Ctrl+8; a too-wide number in a repeat-fill format such as Accounting shows #### instead of losing its leading digits; Esc and Enter close a data validation alert over the cell editor; Ctrl+Enter fill gives each cell the automatic number format typing gives it; keytip letters typed with Alt held, #1846; Number Stored as Text indicator and Convert to Number) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Interaction, controls and feel, against Excel. Part of [target-app-parity.md](target-app-parity.md). **UI/UX fidelity: ~55% ready, 50–80 h** (estimated). Ribbon command coverage is measured @@ -64,7 +64,7 @@ this logic, with one undo step. | Freeze panes, split panes | freeze done; split to verify | | A Table's header names and filter buttons in the column headings once its header row scrolls away | done (#1423): with the active cell in the Table, as in Excel; the button opens the column's filter menu, the rest of the heading still selects the column. Plain AutoFilter ranges keep their letters (Excel does the same) | | Copy as HTML table for other apps | done (#36, 10-10) | -| Context menu on cells | 16 items (Cut, Copy, Paste, Paste Special, Insert, Delete, Clear Contents, Filter by value, Sort, New Comment, New Note, Format Cells, Pick From Drop-down List, Define Name, Link). Insert Copied Cells (Insert Cut Cells for whole rows/columns) while the clipboard holds a copy (#21). Missing: Insert Cut Cells for a cell block, paste-option icons, Quick Analysis, Get Data from Table, Show Changes, Smart Lookup | +| Context menu on cells | 16 items (Cut, Copy, Paste, Paste Special, Insert, Delete, Clear Contents, Filter by value, Sort, New Comment, New Note, Format Cells, Pick From Drop-down List, Define Name, Link). Insert Copied Cells (Insert Cut Cells for whole rows/columns) while the clipboard holds a copy (#21). Show Details on a PivotTable's value, subtotal or grand total cell (`pivot.showDetails`). Missing: Insert Cut Cells for a cell block, paste-option icons, Quick Analysis, Get Data from Table, Show Changes, Smart Lookup | | Move and resize charts, pictures and shapes | charts can't be resized (#190) | | Context menus on row/column headers, sheet tabs, charts, shapes | headers and tabs done; objects partial | | Quick Analysis (Ctrl+Q) | missing | @@ -178,6 +178,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-24 | minor | Right-clicking a PivotTable's value, subtotal or grand total cell offers Show Details in the cell menu, as in Excel ("Right-click a field in the values area of the PivotTable, and then click Show Details"): it lists the cell's source records on a new sheet, as double-clicking the cell does (`pivot.showDetails`). Labels and headers of a current report don't offer it; in a report that is out of date every cell offers it, and choosing it asks for a refresh | | 2026-10-23 | minor | Conditional Formatting ▸ New Rule, formula: clicking a cell on the grid writes its absolute reference at the formula box's caret (`=$B$2`, an empty box starts with `=`), a drag writes a range (`$B$2:$C$4`) and a second click replaces the reference just written, as in Excel ("Selecting cells on the worksheet inserts absolute cell references"). The selection the rule applies to doesn't move, and the box keeps the keyboard. After an operand (`=A1>5`) the click does nothing (#2348) | | 2026-10-23 | minor | Frozen panes: Page Up, Ctrl+Up and Shift variants that move from the scrolled rows into the frozen ones bring the scrolled rows back to the top, so the rows in between are seen again (Page Up from far down used to end on row 1 with the view still a page down); Ctrl+Left and Alt+Page Up do the same for frozen columns. Other ways of selecting a frozen cell (click, Find, Go To, Undo, formula pointing) leave the view where it is | | 2026-10-23 | minor | After a left-drag of the fill handle an Auto Fill Options button sits below the filled cells; its menu redoes the fill as Copy Cells, Fill Series, Fill Formatting Only or Fill Without Formatting (undoes the drag's fill, then `edit.autoFill` in that mode). It goes away when the workbook changes or the selection moves (#1403) | From 010eddd25e329e47e8a893541a52bd5414127372 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 11:27:58 -0700 Subject: [PATCH 2/2] PivotTables: the cell menu computes a report once for all its cells The cell menu checked whether to offer Show Details by computing the whole report again for each new active cell. pivot::detail_cells now returns every value, subtotal and grand total cell of a report from one computation, and the menu caches that set per document, revision, sheet and PivotTable. --- crates/engine/src/cmd/pivot.rs | 31 ++++++++++ crates/engine/src/pivot.rs | 56 +++++++++++++------ crates/ui-egui/src/grid.rs | 33 ++++++----- .../tests/it/pivot_show_details_menu.rs | 19 ++++++- 4 files changed, 106 insertions(+), 33 deletions(-) diff --git a/crates/engine/src/cmd/pivot.rs b/crates/engine/src/cmd/pivot.rs index 229ff22b6..ae5446715 100644 --- a/crates/engine/src/cmd/pivot.rs +++ b/crates/engine/src/cmd/pivot.rs @@ -2580,4 +2580,35 @@ mod tests { } assert_eq!(s.doc().unwrap().wb.sheets.len(), 2); } + + #[test] + fn detail_cells_are_the_cells_show_details_takes() { + let mut s = sales(); + s.execute("insert.pivotTable", json!({"rows": ["Region"], "columns": ["Product"], "values": ["Sales"]})).unwrap(); + for layout in 0..2 { + if layout == 1 { + // A two-level row axis, with subtotals at the top of each group. + s.execute("pivot.moveField", json!({"field": "Product", "from": "columns", "to": "rows"})).unwrap(); + } + let d = s.doc().unwrap(); + let si = d.wb.active_sheet; + let pt = &d.wb.active().unwrap().pivots[0]; + let r = pt.last_range.unwrap(); + let cells = crate::pivot::detail_cells(&d.wb, pt, si).unwrap(); + let mut values = 0; + for row in 0..r.height() { + for col in 0..r.width() { + let one = crate::pivot::detail_records(&d.wb, pt, si, row, col).unwrap(); + assert_eq!(cells.contains(&(row, col)), one.is_some(), "layout {layout}, cell ({row}, {col})"); + values += usize::from(one.is_some()); + } + } + assert!(values > 4, "layout {layout} has value cells"); + } + // An out-of-date report answers as Show Details does. + s.execute("range.setValues", json!({"range": "Sheet1!A2", "values": [["Central"]]})).unwrap(); + let d = s.doc().unwrap(); + let pt = &d.wb.active().unwrap().pivots[0]; + assert!(crate::pivot::detail_cells(&d.wb, pt, d.wb.active_sheet).unwrap_err().contains("out of date")); + } } diff --git a/crates/engine/src/pivot.rs b/crates/engine/src/pivot.rs index 397dd5ad2..a490824da 100644 --- a/crates/engine/src/pivot.rs +++ b/crates/engine/src/pivot.rs @@ -816,11 +816,30 @@ pub const NOT_A_VALUE_CELL: &str = "select a value cell in a PivotTable"; /// on the sheet is no longer what the source computes: the cell's records would be another cell's. pub fn detail_records(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize, row: u32, col: u32) -> Result)>, String> { let src = read_source(wb, &pt.source, pivot_sheet)?; - let mut probe = Probe { at: (row as usize, col as usize), hit: None, paths: Vec::new() }; + let mut probe = Probe { at: Some((row as usize, col as usize)), hit: None, paths: Some(Vec::new()), cells: None }; let out = compute_from(wb, pt, &src, Some(&mut probe))?; - // Excel reads its PivotTable cache; without one, the report written by the last refresh - // must still be what the source gives. Checked first: in a stale report even a cell that - // computes as a label may show a value. + // Checked first: in a stale report even a cell that computes as a label may show a value. + check_current(wb, pt, pivot_sheet, &out)?; + let Some((rp, cp)) = probe.hit else { return Ok(None) }; + let records = + probe.paths.unwrap_or_default().into_iter().filter(|(_, r, c)| r.starts_with(&rp) && c.starts_with(&cp)).map(|(i, _, _)| i).collect(); + Ok(Some((src, records))) +} + +/// The cells of the report grid (relative to its top-left) where [`detail_records`] finds +/// records: its value, subtotal and grand total cells, from one computation of the report. The +/// same errors as [`detail_records`]. +pub fn detail_cells(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize) -> Result, String> { + let src = read_source(wb, &pt.source, pivot_sheet)?; + let mut probe = Probe { at: None, hit: None, paths: None, cells: Some(HashSet::new()) }; + let out = compute_from(wb, pt, &src, Some(&mut probe))?; + check_current(wb, pt, pivot_sheet, &out)?; + Ok(probe.cells.unwrap_or_default().into_iter().map(|(r, c)| (r as u32, c as u32)).collect()) +} + +/// An error when the report on the sheet is not what `out` computes. Excel reads its PivotTable +/// cache; without one, the report written by the last refresh must still be what the source gives. +fn check_current(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize, out: &Output) -> Result<(), String> { let sh = wb.sheet(pivot_sheet).ok_or("There's no such sheet.")?; let current = pt.last_range.is_some_and(|r| { r.height() == out.height().max(1) @@ -834,17 +853,17 @@ pub fn detail_records(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize, row: u if !current { return Err("The PivotTable report is out of date. Refresh it first.".into()); } - let Some((rp, cp)) = probe.hit else { return Ok(None) }; - let records = probe.paths.into_iter().filter(|(_, r, c)| r.starts_with(&rp) && c.starts_with(&cp)).map(|(i, _, _)| i).collect(); - Ok(Some((src, records))) + Ok(()) } -/// What [`detail_records`] asks of a report computation: the row and column item paths of the -/// value cell at `at`, and the paths of every record the filters let through. +/// What [`detail_records`] and [`detail_cells`] ask of a report computation: the row and column +/// item paths of the value cell at `at`, the paths of every record the filters let through (when +/// `paths` is set), and every value cell (when `cells` is set). struct Probe { - at: (usize, usize), + at: Option<(usize, usize)>, hit: Option<(Vec, Vec)>, - paths: Vec<(usize, Vec, Vec)>, + paths: Option, Vec)>>, + cells: Option>, } fn compute_from(wb: &Workbook, pt: &PivotTable, src: &Source, mut probe: Option<&mut Probe>) -> Result { @@ -959,8 +978,8 @@ fn compute_from(wb: &Workbook, pt: &PivotTable, src: &Source, mut probe: Option< if !col_leaves.contains(&cp[..shown]) { col_leaves.insert(cp[..shown].to_vec()); } - if let Some(pr) = probe.as_deref_mut() { - pr.paths.push((ri, rp.clone(), cp.clone())); + if let Some(paths) = probe.as_deref_mut().and_then(|pr| pr.paths.as_mut()) { + paths.push((ri, rp.clone(), cp.clone())); } row_set.insert(rp); col_set.insert(cp); @@ -1326,10 +1345,13 @@ fn compute_from(wb: &Workbook, pt: &PivotTable, src: &Source, mut probe: Option< _ => raw, }, }; - if let Some(pr) = probe.as_deref_mut() - && pr.at == (r, l + j) - { - pr.hit = Some((rp.to_vec(), cp.to_vec())); + if let Some(pr) = probe.as_deref_mut() { + if pr.at == Some((r, l + j)) { + pr.hit = Some((rp.to_vec(), cp.to_vec())); + } + if let Some(cells) = pr.cells.as_mut() { + cells.insert((r, l + j)); + } } g.set(r, l + j, shown, ck.1, 0, data_fmt.get(*vi).cloned().flatten()); } diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index abe8bcf42..8b36f520b 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -163,9 +163,11 @@ pub struct GridState { /// The PivotTable Fields pane's `pivot.fields` answer, cached per (doc uid, revision, pivot): /// it scans the whole source list, far too slow to repeat every frame (#2113). pub pivot_fields: Option<((u64, u64, String), serde_json::Value)>, - /// Whether the cell menu offers Show Details, cached per (doc uid, revision, sheet, active - /// cell): finding out computes the whole PivotTable report. - pub show_details_item: Option<((u64, u64, usize, CellRef), bool)>, + /// The cells of a PivotTable report where the cell menu offers Show Details, relative to the + /// report's top-left and cached per (doc uid, revision, sheet, pivot): finding them computes + /// the whole report. `None` when the report can't be computed or is out of date: every cell + /// of it offers Show Details, and running it says why. + pub show_details_cells: Option<((u64, u64, usize, usize), Option>)>, pub header_menu: Option<(Pos2, bool)>, /// Page Layout view pagination, cached per (doc uid, revision, sheet, rows, columns). pub paper: Option, @@ -3424,20 +3426,21 @@ fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { fn show_details_offered(app: &mut SheetApp) -> bool { let Some(d) = app.session.active() else { return false }; let (si, at) = (d.wb.active_sheet, d.selection.active); - let key = (d.uid, d.revision, si, at); - if let Some((k, on)) = app.grid.show_details_item - && k == key + let Some((pi, pt)) = d.wb.active().and_then(|sh| sh.pivots.iter().enumerate().find(|(_, pt)| pt.last_range.is_some_and(|r| r.contains(at)))) + else { + return false; + }; + let origin = pt.last_range.map(|r| r.start).unwrap_or_default(); + let cell = (at.row.saturating_sub(origin.row), at.col.saturating_sub(origin.col)); + let key = (d.uid, d.revision, si, pi); + if let Some((k, cells)) = &app.grid.show_details_cells + && *k == key { - return on; + return cells.as_ref().is_none_or(|c| c.contains(&cell)); } - let on = d.wb.active().and_then(|sh| sh.pivots.iter().find(|pt| pt.last_range.is_some_and(|r| r.contains(at)))).is_some_and(|pt| { - let origin = pt.last_range.map(|r| r.start).unwrap_or_default(); - !matches!( - gridcraft_engine::pivot::detail_records(&d.wb, pt, si, at.row.saturating_sub(origin.row), at.col.saturating_sub(origin.col)), - Ok(None) - ) - }); - app.grid.show_details_item = Some((key, on)); + let cells = gridcraft_engine::pivot::detail_cells(&d.wb, pt, si).ok(); + let on = cells.as_ref().is_none_or(|c| c.contains(&cell)); + app.grid.show_details_cells = Some((key, cells)); on } diff --git a/crates/ui-egui/tests/it/pivot_show_details_menu.rs b/crates/ui-egui/tests/it/pivot_show_details_menu.rs index 02527d8a3..73172155e 100644 --- a/crates/ui-egui/tests/it/pivot_show_details_menu.rs +++ b/crates/ui-egui/tests/it/pivot_show_details_menu.rs @@ -4,7 +4,7 @@ use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; use egui_kittest::kittest::Queryable as _; use gridcraft_engine::Session; -use gridcraft_engine::core::CellRef; +use gridcraft_engine::core::{CellRef, Value}; use gridcraft_ui_egui::{SheetApp, grid::Geo}; use serde_json::json; @@ -90,6 +90,23 @@ fn labels_of_the_report_and_ordinary_cells_have_no_show_details_item() { } } +#[test] +fn the_cell_menu_computes_the_report_once_for_all_its_cells() { + let mut h = harness(); + right_click(&mut h, "E4"); + assert!(h.query_by_label("Show Details").is_some()); + let escape = Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }; + h.input_mut().events.push(escape); + h.run_steps(2); + // West goes away behind the session's back, so the revision stays the same. A menu that + // computed the report again for D4 would find it out of date and offer Show Details there. + let doc = h.state_mut().session.active_mut().unwrap(); + let si = doc.wb.active_sheet; + std::sync::Arc::make_mut(&mut doc.wb).sheet_mut(si).unwrap().set_value(CellRef::parse("A3").unwrap(), Value::text("East")); + right_click(&mut h, "D4"); + assert!(h.query_by_label("Show Details").is_none(), "D4 is a label of the report computed for E4"); +} + #[test] fn show_details_in_an_out_of_date_report_asks_for_a_refresh() { let mut h = harness();