mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-02 07:06:48 -06:00
Merge pull request #383 from vangel76/fix/sheetview-top-left-cell
View: restore and save the scroll position (topLeftCell) (#378) # Conflicts: # ROADMAP.md
This commit is contained in:
@@ -136,6 +136,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
|
||||
| 2026-10-11 | Merged cells draw their whole outline: right and bottom edges come from the last column's and last row's cells (#376) |
|
||||
| 2026-10-11 | Shrink to fit draws too-wide text and numbers in a smaller font inside the cell or merged area (#375) |
|
||||
| 2026-10-11 | Shift+Space and Ctrl+Space select the row / column without starting an edit; Control+Space on Mac (#338) |
|
||||
| 2026-10-11 | Workbooks reopen at their saved scroll position (`topLeftCell`, also with frozen panes) and save the current one back (#378) |
|
||||
| 2026-10-11 | Legacy Excel 97-2003 `.xls` (BIFF8) import with formulas, number formats, merges, sizes and names (#121) |
|
||||
| 2026-10-11 | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls and OLE objects are kept on save (#330, #324) |
|
||||
| 2026-10-11 | Data › Forecast › Forecast Sheet: a new sheet with the history, FORECAST.ETS forecast, confidence bounds, statistics and a chart (#322) |
|
||||
|
||||
@@ -139,6 +139,35 @@ impl DocState {
|
||||
pub fn sheet_index(&self) -> usize {
|
||||
self.wb.active_sheet
|
||||
}
|
||||
/// Records the view state that is saved with the file (`sheetView topLeftCell`, the active
|
||||
/// cell): the first visible cell of the scrolling area per sheet index in `tops`, and the
|
||||
/// active sheet's active cell. View state never makes the workbook dirty.
|
||||
pub fn store_view(&mut self, tops: &[(usize, CellRef)]) {
|
||||
let active = self.selection.active;
|
||||
let si = self.wb.active_sheet;
|
||||
let changed = tops.iter().any(|(i, c)| self.wb.sheets.get(*i).is_some_and(|sh| sh.view_top_left != *c))
|
||||
|| self.wb.sheets.get(si).is_some_and(|sh| sh.view_active != active);
|
||||
if !changed {
|
||||
return;
|
||||
}
|
||||
let dirty = self.is_dirty();
|
||||
let wb = Arc::make_mut(&mut self.wb);
|
||||
for (i, c) in tops {
|
||||
if let Some(sh) = wb.sheets.get_mut(*i)
|
||||
&& sh.view_top_left != *c
|
||||
{
|
||||
Arc::make_mut(sh).view_top_left = *c;
|
||||
}
|
||||
}
|
||||
if let Some(sh) = wb.sheets.get_mut(si)
|
||||
&& sh.view_active != active
|
||||
{
|
||||
Arc::make_mut(sh).view_active = active;
|
||||
}
|
||||
if !dirty {
|
||||
self.saved = self.wb.clone();
|
||||
}
|
||||
}
|
||||
pub fn display_title(&self) -> String {
|
||||
self.path
|
||||
.as_deref()
|
||||
|
||||
@@ -724,6 +724,7 @@ pub fn export_as(app: &mut SheetApp, fmt: &str) -> bool {
|
||||
}
|
||||
};
|
||||
}
|
||||
app.store_view_state();
|
||||
match app.session.run("file.saveBytes", json!({"format": fmt})) {
|
||||
Ok(r) => {
|
||||
let bytes = r.get("base64").and_then(Json::as_str).and_then(gridcraft_engine::io::base64_decode);
|
||||
|
||||
@@ -305,6 +305,9 @@ impl SheetApp {
|
||||
|
||||
/// Runs an engine command (or a UI command) and handles UI requests it makes.
|
||||
pub fn run(&mut self, id: &str, params: Json) -> Result<Json, String> {
|
||||
if matches!(id, "file.save" | "file.saveAs" | "file.saveBytes") {
|
||||
self.store_view_state();
|
||||
}
|
||||
if id.starts_with("file.")
|
||||
|| matches!(
|
||||
id,
|
||||
@@ -585,8 +588,9 @@ impl SheetApp {
|
||||
let path = if std::path::Path::new(&path).extension().is_none() { format!("{path}.xlsx") } else { path };
|
||||
self.run_or_alert("file.save", json!({"path": path}));
|
||||
}
|
||||
} else if let Some(dl) = &self.services.download
|
||||
&& let Ok(r) = self.session.run("file.saveBytes", json!({"format": "xlsx"}))
|
||||
} else if self.services.download.is_some()
|
||||
&& let Ok(r) = self.run("file.saveBytes", json!({"format": "xlsx"}))
|
||||
&& let Some(dl) = &self.services.download
|
||||
&& let Some(b) = r.get("base64").and_then(Json::as_str).and_then(gridcraft_engine::io::base64_decode)
|
||||
{
|
||||
let name = r.get("name").and_then(Json::as_str).unwrap_or("Book.xlsx").to_string();
|
||||
@@ -664,12 +668,45 @@ impl SheetApp {
|
||||
}
|
||||
|
||||
pub fn view(&self) -> SheetView {
|
||||
self.view_key().and_then(|k| self.views.get(&k).copied()).unwrap_or_default()
|
||||
self.view_key().and_then(|k| self.views.get(&k).copied()).unwrap_or_else(|| self.saved_view())
|
||||
}
|
||||
|
||||
pub fn view_mut(&mut self) -> Option<&mut SheetView> {
|
||||
let k = self.view_key()?;
|
||||
Some(self.views.entry(k).or_default())
|
||||
let saved = self.saved_view();
|
||||
Some(self.views.entry(k).or_insert(saved))
|
||||
}
|
||||
|
||||
/// The view a sheet opens with: scrolled to the file's saved first visible cell (`topLeftCell`).
|
||||
fn saved_view(&self) -> SheetView {
|
||||
let Some(sh) = self.session.active().and_then(|d| d.wb.active()) else { return SheetView::default() };
|
||||
if sh.view != gridcraft_engine::model::SheetViewMode::Normal {
|
||||
return SheetView::default();
|
||||
}
|
||||
let (fr, fc) = sh.freeze.unwrap_or((0, 0));
|
||||
let top = sh.view_top_left;
|
||||
let y = sh.row_top(top.row.max(fr)) - sh.row_top(fr);
|
||||
let x = sh.col_left(top.col.max(fc)) - sh.col_left(fc);
|
||||
SheetView { scroll: egui::vec2(x.max(0.0) as f32, y.max(0.0) as f32) }
|
||||
}
|
||||
|
||||
/// Hands the active document's scroll positions (first visible cell per sheet) and active
|
||||
/// cell to the engine so a save writes them (`sheetView topLeftCell`). Normal view only:
|
||||
/// Page Layout view scrolls over pages, not cells.
|
||||
pub fn store_view_state(&mut self) {
|
||||
let Some(d) = self.session.active_mut() else { return };
|
||||
let mut tops = Vec::new();
|
||||
for (&(uid, si), v) in &self.views {
|
||||
let Some(sh) = d.wb.sheets.get(si).filter(|sh| uid == d.uid && sh.view == gridcraft_engine::model::SheetViewMode::Normal) else {
|
||||
continue;
|
||||
};
|
||||
let (fr, fc) = sh.freeze.unwrap_or((0, 0));
|
||||
// A hair past the edge so a row that starts exactly at the scroll offset is the one found.
|
||||
let row = sh.row_at(sh.row_top(fr) + v.scroll.y.max(0.0) as f64 + 0.01);
|
||||
let col = sh.col_at(sh.col_left(fc) + v.scroll.x.max(0.0) as f64 + 0.01);
|
||||
tops.push((si, CellRef::new(row, col)));
|
||||
}
|
||||
d.store_view(&tops);
|
||||
}
|
||||
|
||||
/// Starts editing the active cell. `text` replaces the content (typing) or pre-fills it.
|
||||
@@ -936,6 +973,7 @@ impl SheetApp {
|
||||
&& self.session.active().is_some_and(|d| d.is_dirty() && d.path.as_deref().is_some_and(|p| p.ends_with(".xlsx")))
|
||||
{
|
||||
self.grid.last_autosave = now;
|
||||
self.store_view_state();
|
||||
let _ = self.session.run("file.save", json!({}));
|
||||
}
|
||||
// Window title.
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
//! The saved scroll position (`sheetView topLeftCell`) is restored on open and saved back (#378).
|
||||
|
||||
use gridcraft_engine::core::{CellRef, Value};
|
||||
use gridcraft_engine::model::Workbook;
|
||||
use gridcraft_engine::{DocState, Session};
|
||||
use gridcraft_ui_egui::{SheetApp, grid::Geo};
|
||||
use serde_json::json;
|
||||
|
||||
type Harness = egui_kittest::Harness<'static, SheetApp>;
|
||||
|
||||
fn harness(freeze: Option<(u32, u32)>) -> Harness {
|
||||
let mut wb = Workbook::new();
|
||||
let sh = wb.sheet_mut(0).unwrap();
|
||||
for r in 0..60 {
|
||||
sh.set_value(CellRef::new(r, 0), Value::text(format!("row {}", r + 1)));
|
||||
}
|
||||
sh.view_top_left = CellRef::parse("A31").unwrap();
|
||||
sh.view_active = CellRef::parse("A31").unwrap();
|
||||
sh.freeze = freeze;
|
||||
let mut session = Session::new();
|
||||
session.add_document(DocState::new(wb, None, "Book1".into()));
|
||||
let mut h = egui_kittest::Harness::builder().with_size(egui::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
|
||||
}
|
||||
|
||||
/// The first row painted at the top of the scrolling area.
|
||||
fn first_scrolled_row(h: &Harness) -> u32 {
|
||||
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);
|
||||
let (fr, _) = sheet.freeze.unwrap_or((0, 0));
|
||||
let top = geo.cells.top() + sheet.row_top(fr) as f32 * geo.z;
|
||||
(fr..200).find(|r| geo.y(sheet, *r) <= top + 0.5 && geo.y(sheet, *r + 1) > top + 0.5).unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saved_scroll_position_is_restored_and_saved_back() {
|
||||
for freeze in [None, Some((1, 0))] {
|
||||
let mut h = harness(freeze);
|
||||
assert_eq!(first_scrolled_row(&h), 30, "row 31 is the first visible row ({freeze:?})");
|
||||
// Scroll to row 51 and save: the file gets the new position, the document stays clean.
|
||||
{
|
||||
let app = h.state_mut();
|
||||
let sh = app.session.active().unwrap().wb.active().unwrap().clone();
|
||||
let fr = freeze.map_or(0, |f| f.0);
|
||||
app.view_mut().unwrap().scroll.y = (sh.row_top(50) - sh.row_top(fr)) as f32;
|
||||
app.run("file.saveBytes", json!({"format": "xlsx"})).unwrap();
|
||||
}
|
||||
let d = h.state().session.active().unwrap();
|
||||
assert_eq!(d.wb.active().unwrap().view_top_left, CellRef::parse("A51").unwrap());
|
||||
assert!(!d.is_dirty(), "scrolling and saving view state does not dirty the workbook");
|
||||
}
|
||||
}
|
||||
@@ -256,6 +256,10 @@ fn read_views(e: &El, sheet: &mut Sheet) {
|
||||
let rows = p.attr_f64("ySplit").unwrap_or(0.0).clamp(0.0, (MAX_ROWS - 1) as f64) as u32;
|
||||
if rows > 0 || cols > 0 {
|
||||
sheet.freeze = Some((rows, cols));
|
||||
// With frozen panes the scroll position is the scrolling pane's first cell.
|
||||
if let Some(c) = p.attr("topLeftCell").and_then(CellRef::parse) {
|
||||
sheet.view_top_left = c;
|
||||
}
|
||||
}
|
||||
}
|
||||
active_pane = p.attr("activePane");
|
||||
|
||||
@@ -182,7 +182,11 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> (
|
||||
let _ = write!(
|
||||
s,
|
||||
" topLeftCell=\"{}\" activePane=\"{pane}\" state=\"frozen\"/>",
|
||||
CellRef::new(rows.min(gridcraft_core::MAX_ROWS - 1), cols.min(gridcraft_core::MAX_COLS - 1)).a1()
|
||||
CellRef::new(
|
||||
rows.max(sheet.view_top_left.row).min(gridcraft_core::MAX_ROWS - 1),
|
||||
cols.max(sheet.view_top_left.col).min(gridcraft_core::MAX_COLS - 1)
|
||||
)
|
||||
.a1()
|
||||
);
|
||||
let _ = write!(s, "<selection pane=\"{pane}\" activeCell=\"{active}\" sqref=\"{active}\"/>");
|
||||
}
|
||||
|
||||
@@ -813,3 +813,28 @@ fn bad_comment_dates_are_left_out() {
|
||||
let c = &back.sheets[0].comments[&at("A1")];
|
||||
assert!(c.threaded && c.dates.is_empty());
|
||||
}
|
||||
|
||||
/// #378: the saved scroll position survives a round trip, with and without frozen panes (where it
|
||||
/// is the scrolling pane's `topLeftCell`).
|
||||
#[test]
|
||||
fn scroll_position_round_trips_with_and_without_frozen_panes() {
|
||||
let mut wb = Workbook::new();
|
||||
wb.sheet_mut(0).unwrap().view_top_left = at("A31");
|
||||
let (back, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap();
|
||||
assert_eq!(back.sheet(0).unwrap().view_top_left, at("A31"));
|
||||
|
||||
let s = wb.sheet_mut(0).unwrap();
|
||||
s.freeze = Some((1, 0));
|
||||
s.view_top_left = at("C40");
|
||||
let bytes = write_xlsx(&wb).unwrap();
|
||||
let (xml, _) = sheet1_xml(&bytes);
|
||||
assert!(xml.contains(r#"<pane ySplit="1" topLeftCell="C40""#), "{xml}");
|
||||
let (back, _) = read_xlsx(&bytes).unwrap();
|
||||
assert_eq!(back.sheet(0).unwrap().view_top_left, at("C40"));
|
||||
assert_eq!(back.sheet(0).unwrap().freeze, Some((1, 0)));
|
||||
|
||||
// A scroll position inside the frozen rows is written as the first scrolling row.
|
||||
wb.sheet_mut(0).unwrap().view_top_left = at("A1");
|
||||
let (xml, _) = sheet1_xml(&write_xlsx(&wb).unwrap());
|
||||
assert!(xml.contains(r#"topLeftCell="A2""#), "{xml}");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user