diff --git a/ROADMAP.md b/ROADMAP.md index 59b3bf2d8..7a794e06e 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-11-06 | A sheet keeps its own scroll position when sheets are inserted, deleted or moved before it: a new PivotTable or inserted sheet opens at the top instead of where its neighbour was scrolled; a closed window's positions are let go (#3485) | | 2026-11-02 | Form controls: Insert › Controls places buttons, sliders (scroll bars) and spin buttons on the sheet. A slider or spin button holds a whole number from 0 to 30000 in a linked cell (a drag or a held arrow is one undo step, and editing the cell moves the control); a button runs a macro or one command through the macro policy, asking first in a workbook opened from a file; Assign Action and Format Control edit them, and XLSX files read and write them (control properties, `` and VML), alongside the form controls GridCraft keeps unchanged | | 2026-11-02 | Dialogs take ranges by selection on the grid: reference boxes (RefEdit) with a collapse button that shrinks the dialog while the person selects, in Goal Seek, Solver (including the Model Range of Load/Save) and Data Analysis; while collapsed, the arrow keys move the selection and Shift+arrow extends it, and F4 cycles a reference's anchors | | 2026-11-02 | Python mode in the formula bar: Ctrl+Alt+Shift+P, `=PY` then Tab, or editing a Python cell edits its code in the formula bar, with Enter for a new line, Tab to indent and Ctrl+Enter to commit through `python.set`. The code still doesn't run | @@ -259,6 +260,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-11-06 | trivial | Progress log: scroll positions follow their sheet through sheet inserts, deletes and moves (#3485) | | 2026-11-02 | minor | Form controls; catalog 277 / 292 → 281 / 296 (Developer › Controls); progress log row | | 2026-11-02 | minor | Progress log: RefEdit reference boxes in dialogs | | 2026-11-02 | minor | Progress log: Python mode in the formula bar | diff --git a/crates/ui-egui/src/app/commands.rs b/crates/ui-egui/src/app/commands.rs index 4aba80024..6dff45cb2 100644 --- a/crates/ui-egui/src/app/commands.rs +++ b/crates/ui-egui/src/app/commands.rs @@ -302,6 +302,7 @@ impl SheetApp { // The requests belong to the window the command left active. self.follow_active_window(); self.prune_selected_object(); + self.prune_views(); for req in self.session.take_ui_requests() { match req { UiRequest::Dialog(name, params) => self.open_dialog(&name, params), diff --git a/crates/ui-egui/src/app/view.rs b/crates/ui-egui/src/app/view.rs index aea36abc2..482372448 100644 --- a/crates/ui-egui/src/app/view.rs +++ b/crates/ui-egui/src/app/view.rs @@ -7,8 +7,8 @@ impl SheetApp { } /// The active window and sheet: each window scrolls on its own. - fn scroll_key(&self) -> Option<(u64, usize)> { - Some((self.session.active_window()?, self.session.active()?.wb.active_sheet())) + fn scroll_key(&self) -> Option<(u64, u64)> { + Some((self.session.active_window()?, self.session.active()?.wb.active()?.uid.get())) } pub fn view(&self) -> SheetView { @@ -41,8 +41,10 @@ impl SheetApp { let window = self.session.active_window(); let Some(d) = self.session.active_mut() else { return }; let mut tops = Vec::new(); - for (&(w, si), v) in &self.views { - let Some(sh) = d.wb.sheets.get(si).filter(|sh| Some(w) == window && sh.view == gridcraft_engine::model::SheetViewMode::Normal) else { + for (si, sh) in d.wb.sheets.iter().enumerate() { + let Some(v) = + window.and_then(|w| self.views.get(&(w, sh.uid.get()))).filter(|_| sh.view == gridcraft_engine::model::SheetViewMode::Normal) + else { continue; }; let (fr, fc) = sh.freeze.unwrap_or((0, 0)); @@ -53,4 +55,10 @@ impl SheetApp { } d.store_view(&tops); } + + /// Drops the scroll positions of windows that have closed: window ids are never reused. + pub(super) fn prune_views(&mut self) { + let session = &self.session; + self.views.retain(|&(w, _), _| session.window(w).is_some()); + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e773a207d..dffe3ed89 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -222,8 +222,9 @@ pub struct SheetApp { pub ui: UiState, /// The window chrome to draw (see [`Chrome`]). pub chrome: Chrome, - /// Scroll position per (window id, sheet index). - pub views: HashMap<(u64, usize), SheetView>, + /// Scroll position per (window id, sheet uid): each sheet keeps its own when sheets are + /// inserted, deleted or moved. + pub views: HashMap<(u64, u64), SheetView>, pub editor: Option, pub text_box_editor: Option, pub dialog: Option, diff --git a/crates/ui-egui/tests/it/scroll_position.rs b/crates/ui-egui/tests/it/scroll_position.rs index 520bb1691..ffcb56dd6 100644 --- a/crates/ui-egui/tests/it/scroll_position.rs +++ b/crates/ui-egui/tests/it/scroll_position.rs @@ -1,4 +1,6 @@ //! The saved scroll position (`sheetView topLeftCell`) is restored on open and saved back (#378). +//! Each window keeps each sheet's own scroll position while sheets are inserted, deleted or +//! moved (#3485). use gridcraft_engine::core::{CellRef, Value}; use gridcraft_engine::model::Workbook; @@ -58,3 +60,84 @@ fn saved_scroll_position_is_restored_and_saved_back() { assert!(!d.is_dirty(), "scrolling and saving view state does not dirty the workbook"); } } + +#[test] +fn a_pivottable_sheet_opens_at_the_top_and_its_source_sheet_keeps_its_place() { + // A new PivotTable sheet goes in front of its source sheet. Scroll positions were kept by + // sheet index, so the new sheet took over the source sheet's position and opened at row 51. + let mut h = harness(None); + h.state_mut().run("range.setValues", json!({"range": "C1:D3", "values": [["Region", "Sales"], ["North", 10], ["South", 20]]})).unwrap(); + { + let app = h.state_mut(); + let sh = app.session.active().unwrap().wb.active().unwrap().clone(); + app.view_mut().unwrap().scroll.y = sh.row_top(50) as f32; + } + h.run_steps(2); + assert_eq!(first_scrolled_row(&h), 50, "setup: Sheet1 is scrolled to row 51"); + h.state_mut().run("insert.pivotTable", json!({"source": "Sheet1!$C$1:$D$3", "rows": ["Region"]})).unwrap(); + h.run_steps(4); + let wb = &h.state().session.active().unwrap().wb; + assert!(!wb.active().unwrap().pivots.is_empty(), "setup: the PivotTable's new sheet is active"); + assert_eq!(first_scrolled_row(&h), 0, "the new sheet opens at row 1"); + h.state_mut().run("sheet.next", json!({})).unwrap(); + h.run_steps(4); + assert_eq!(h.state().session.active().unwrap().wb.active().unwrap().name, "Sheet1"); + assert_eq!(first_scrolled_row(&h), 50, "Sheet1 is still scrolled to row 51"); +} + +/// Scrolls the active sheet so `row` (0-based) is the first one in view. +fn scroll_to_row(h: &mut Harness, row: u32) { + let app = h.state_mut(); + let sh = app.session.active().unwrap().wb.active().unwrap().clone(); + app.view_mut().unwrap().scroll.y = sh.row_top(row) as f32; + h.run_steps(2); +} + +fn activate(h: &mut Harness, sheet: &str) { + h.state_mut().run("sheet.activate", json!({"sheet": sheet})).unwrap(); + h.run_steps(4); + assert_eq!(h.state().session.active().unwrap().wb.active().unwrap().name, sheet); +} + +#[test] +fn a_sheet_keeps_its_scroll_position_when_sheets_are_inserted_deleted_or_moved() { + let mut h = harness(None); + scroll_to_row(&mut h, 50); + h.state_mut().run("home.insertSheet", json!({"name": "Two", "before": 0})).unwrap(); + h.run_steps(4); + assert_eq!(first_scrolled_row(&h), 0, "the inserted sheet opens at row 1"); + scroll_to_row(&mut h, 10); + activate(&mut h, "Sheet1"); + assert_eq!(first_scrolled_row(&h), 50, "Sheet1 moved one place right and is still at row 51"); + h.state_mut().run("home.deleteSheet", json!({"sheet": "Two"})).unwrap(); + h.run_steps(4); + assert_eq!(first_scrolled_row(&h), 50, "Sheet1 moved back and is still at row 51"); + h.state_mut().run("edit.undo", json!({})).unwrap(); + h.run_steps(4); + activate(&mut h, "Two"); + assert_eq!(first_scrolled_row(&h), 10, "the sheet undo brings back is where it was left"); + h.state_mut().run("sheet.move", json!({"sheet": 0, "to": 1})).unwrap(); + h.run_steps(4); + activate(&mut h, "Two"); + assert_eq!(first_scrolled_row(&h), 10, "a moved sheet keeps its place"); + activate(&mut h, "Sheet1"); + assert_eq!(first_scrolled_row(&h), 50, "and so does the sheet it moved past"); +} + +#[test] +fn each_window_scrolls_on_its_own_and_a_closed_windows_positions_are_dropped() { + let mut h = harness(None); + scroll_to_row(&mut h, 50); + let first = h.state().session.active_window().unwrap(); + h.state_mut().run("view.newWindow", json!({})).unwrap(); + h.run_steps(4); + let second = h.state().session.active_window().unwrap(); + assert_ne!(first, second); + scroll_to_row(&mut h, 10); + assert!(h.state().views.keys().any(|(w, _)| *w == second), "setup: the second window has its own position"); + h.state_mut().run("window.close", json!({"window": second})).unwrap(); + h.run_steps(4); + assert_eq!(h.state().session.active_window(), Some(first)); + assert_eq!(first_scrolled_row(&h), 50, "the first window kept its own position"); + assert!(h.state().views.keys().all(|(w, _)| *w == first), "nothing is kept for the closed window: {:?}", h.state().views.keys()); +}