From 5722dc5355a6e19e3b24889eeffc7a37522d5a35 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sat, 10 Oct 2026 23:45:21 +0200 Subject: [PATCH 01/42] Do not draw the selection over the frozen pane when it lies only in the scrolling pane (#198) --- crates/ui-egui/src/chartview.rs | 5 +- crates/ui-egui/src/grid.rs | 173 ++++++++++++++++++++++++++++---- 2 files changed, 155 insertions(+), 23 deletions(-) diff --git a/crates/ui-egui/src/chartview.rs b/crates/ui-egui/src/chartview.rs index adeeddb..bb40a4a 100644 --- a/crates/ui-egui/src/chartview.rs +++ b/crates/ui-egui/src/chartview.rs @@ -303,7 +303,10 @@ pub fn paint_objects(app: &SheetApp, p: &Painter, geo: &Geo, wb: &Workbook, si: && rf.sheet_name().is_none_or(|n| n.eq_ignore_ascii_case(&sh.name)) { let rr = geo.range_rect(sh, rf.range()); - p.rect_stroke(rr, 0.0, Stroke::new(1.5, Color32::from_rgb(0x2E, 0x6F, 0xD8)), StrokeKind::Inside); + for pane in geo.range_panes(rf.range()) { + let pp = p.with_clip_rect(pane.intersect(p.clip_rect())); + pp.rect_stroke(rr, 0.0, Stroke::new(1.5, Color32::from_rgb(0x2E, 0x6F, 0xD8)), StrokeKind::Inside); + } } } } diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 72d33e2..93a6709 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -186,6 +186,33 @@ impl Geo { let c1 = self.ci.at(left + ((self.cells.width() - self.frozen_w) / self.z) as f64).min(MAX_COLS - 1); (c0, c1) } + /// Screen areas of the panes (frozen rows/columns or the scrolling part) that hold part of `r`. + /// The panes share one coordinate system, so a range that is not clipped to these is also drawn + /// over the frozen rows/columns when it is scrolled under them. + pub fn range_panes(&self, r: RangeRef) -> Vec { + let split = pos2(self.cells.left() + self.frozen_w, self.cells.top() + self.frozen_h); + let mut xs = Vec::new(); + if r.start.col < self.fc { + xs.push((self.cells.left(), split.x)); + } + if r.end.col >= self.fc { + xs.push((split.x, self.cells.right())); + } + let mut ys = Vec::new(); + if r.start.row < self.fr { + ys.push((self.cells.top(), split.y)); + } + if r.end.row >= self.fr { + ys.push((split.y, self.cells.bottom())); + } + ys.iter().flat_map(|&(y0, y1)| xs.iter().map(move |&(x0, x1)| Rect::from_min_max(pos2(x0, y0), pos2(x1, y1)))).collect() + } + /// True when cell `c` is in the scrolling pane but scrolled entirely under the frozen rows or columns. + pub fn hidden_under_frozen(&self, sh: &Sheet, c: CellRef) -> bool { + let r = self.cell_rect(sh, c); + (self.fc > 0 && c.col >= self.fc && r.right() <= self.cells.left() + self.frozen_w) + || (self.fr > 0 && c.row >= self.fr && r.bottom() <= self.cells.top() + self.frozen_h) + } } /// Resolved look of a cell for painting. @@ -396,7 +423,7 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { } let p = painter.with_clip_rect(*clip); paint_quadrant(&p, &geo, &wb, si, sh, *rows, *cols, &mut cf, &t); - paint_selection(&p, &geo, sh, &sel, &t, app); + paint_selection(&p, &geo, sh, &sel, &t, app, *rows, *cols); } // Freeze lines. if geo.fr > 0 { @@ -901,21 +928,40 @@ fn paint_cf_icon(p: &Painter, r: Rect, set: &str, idx: usize) { } } -fn paint_selection(p: &Painter, geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::Selection, t: &Tokens, app: &SheetApp) { +/// True when `r` covers at least one cell of the pane that shows `rows` x `cols` (inclusive). +/// The panes of a frozen sheet share one coordinate system, so a range that lives only in the +/// scrolling pane would otherwise be drawn over the frozen pane as well. +fn range_in_pane(r: RangeRef, rows: (u32, u32), cols: (u32, u32)) -> bool { + r.start.row <= rows.1 && r.end.row >= rows.0 && r.start.col <= cols.1 && r.end.col >= cols.0 +} + +#[allow(clippy::too_many_arguments)] +fn paint_selection( + p: &Painter, + geo: &Geo, + sh: &Sheet, + sel: &gridcraft_engine::Selection, + t: &Tokens, + app: &SheetApp, + rows: (u32, u32), + cols: (u32, u32), +) { // Fill all areas except the active cell. for r in &sel.ranges { + if !range_in_pane(*r, rows, cols) { + continue; + } if r.is_single() && sel.ranges.len() == 1 { continue; } let rect = geo.range_rect(sh, *r); p.rect_filled(rect, 0.0, t.sel_fill); } - let active = geo.cell_rect(sh, sh.merge_at(sel.active).map(|m| m.start).unwrap_or(sel.active)); - let active = match sh.merge_at(sel.active) { - Some(m) => geo.range_rect(sh, m), - None => active, - }; - if !(sel.ranges.len() == 1 && sel.current().is_single()) { + let active_range = sh.merge_at(sel.active).unwrap_or_else(|| RangeRef::cell(sel.active)); + let active = geo.range_rect(sh, active_range); + let active_here = range_in_pane(active_range, rows, cols); + let cur_here = range_in_pane(sel.current(), rows, cols); + if active_here && !(sel.ranges.len() == 1 && sel.current().is_single()) { p.rect_filled(active.shrink(1.0), 0.0, t.grid_bg.gamma_multiply(0.0)); } let cur = geo.range_rect(sh, sel.current()); @@ -927,6 +973,9 @@ fn paint_selection(p: &Painter, geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::S if sheet.as_deref().is_some_and(|s| !s.eq_ignore_ascii_case(&sh.name)) { continue; } + if !range_in_pane(r, rows, cols) { + continue; + } let rr = geo.range_rect(sh, r); let col = REF_COLORS[idx]; p.rect_filled(rr, 0.0, col.gamma_multiply(0.12)); @@ -936,18 +985,22 @@ fn paint_selection(p: &Painter, geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::S } } } - p.rect_stroke(cur.expand(0.5), 0.0, Stroke::new(2.0, t.sel_border), StrokeKind::Middle); - if sel.ranges.len() > 1 { + if cur_here { + p.rect_stroke(cur.expand(0.5), 0.0, Stroke::new(2.0, t.sel_border), StrokeKind::Middle); + } + if active_here && sel.ranges.len() > 1 { p.rect_stroke(active, 0.0, Stroke::new(1.0, t.sel_border), StrokeKind::Inside); } // Fill handle (bottom-right of the current area). - if app.editor.is_none() { + if app.editor.is_none() && range_in_pane(RangeRef::cell(sel.current().end), rows, cols) { let h = Rect::from_center_size(cur.right_bottom(), vec2(6.0, 6.0)); p.rect_filled(h.expand(1.0), 0.0, Color32::WHITE); p.rect_filled(h, 0.0, t.sel_border); } // Fill-handle drag preview. - if let crate::grid::Drag::Fill { target: Some(tr) } = &app.grid.drag { + if let crate::grid::Drag::Fill { target: Some(tr) } = &app.grid.drag + && range_in_pane(*tr, rows, cols) + { let rr = geo.range_rect(sh, *tr); p.extend(egui::Shape::dashed_line( &[rr.left_top(), rr.right_top(), rr.right_bottom(), rr.left_bottom(), rr.left_top()], @@ -959,12 +1012,15 @@ fn paint_selection(p: &Painter, geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::S if let crate::grid::Drag::Move { to: Some(to), .. } = &app.grid.drag { let r = sel.current(); let moved = RangeRef::new(*to, to.offset_clamped(r.height() as i64 - 1, r.width() as i64 - 1)); - let rr = geo.range_rect(sh, moved); - p.rect_stroke(rr, 0.0, Stroke::new(2.0, Color32::from_gray(110)), StrokeKind::Middle); + if range_in_pane(moved, rows, cols) { + let rr = geo.range_rect(sh, moved); + p.rect_stroke(rr, 0.0, Stroke::new(2.0, Color32::from_gray(110)), StrokeKind::Middle); + } } // Marching ants around the copied range. if let Some(clip) = &app.session.clipboard && app.session.active().is_some_and(|d| d.uid == clip.doc_uid && d.wb.active_sheet == clip.sheet) + && range_in_pane(clip.range, rows, cols) { let rr = geo.range_rect(sh, clip.range).expand(0.5); let phase = app.grid.marching_phase; @@ -1065,6 +1121,12 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & let sel = app.session.active().map(|d| d.selection.clone()).unwrap_or_default(); let cur_rect = geo.range_rect(sh, sel.current()); let handle = Rect::from_center_size(cur_rect.right_bottom(), vec2(9.0, 9.0)); + // The fill handle and the move border only react in the panes where they are painted, not + // over the frozen rows/columns that a scrolled selection lies under. + let handle_panes = geo.range_panes(RangeRef::cell(sel.current().end)); + let border_panes = geo.range_panes(sel.current()); + let on_handle = |p: Pos2| handle.contains(p) && handle_panes.iter().any(|r| r.contains(p)); + let on_cur_border = |p: Pos2| on_border(cur_rect, p) && border_panes.iter().any(|r| r.contains(p)); // Hover cursors. if resp.hovered() @@ -1076,12 +1138,12 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & ctx.set_cursor_icon(CursorIcon::ResizeColumn); } else if near_row_edge { ctx.set_cursor_icon(CursorIcon::ResizeRow); - } else if handle.contains(p) && app.editor.is_none() { + } else if on_handle(p) && app.editor.is_none() { ctx.set_cursor_icon(CursorIcon::Crosshair); if app.grid.drag == Drag::None && !pointer.any_down() { resp.clone().on_hover_text_at_pointer("Drag to fill cells. Drag from inside a cell to select a range."); } - } else if in_cells && app.editor.is_none() && on_border(cur_rect, p) { + } else if in_cells && app.editor.is_none() && on_cur_border(p) { ctx.set_cursor_icon(CursorIcon::Move); } else if in_cells { ctx.set_cursor_icon(CursorIcon::Cell); @@ -1258,11 +1320,11 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & if app.editor.is_some() && !app.commit_edit(0, 0, false, false) { return; } - if handle.contains(p) { + if on_handle(p) { app.grid.drag = Drag::Fill { target: None }; return; } - if on_border(cur_rect, p) && resp.drag_started() { + if on_cur_border(p) && resp.drag_started() { app.grid.drag = Drag::Move { grab: c, to: None }; return; } @@ -1442,7 +1504,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & // Double-click the fill handle. if resp.double_clicked() && let Some(p) = pos - && handle.contains(p) + && on_handle(p) { // Fill down as far as the neighbouring column goes. let src = sel.current(); @@ -1954,7 +2016,7 @@ pub fn point_move(app: &mut SheetApp, ed: &mut EditState, dr: i64, dc: i64, exte /// The dropdown arrow beside a cell with an in-cell list validation. pub fn validation_arrow(sh: &Sheet, geo: &Geo, c: CellRef) -> Option { let dv = sh.validations.iter().find(|d| d.ranges.iter().any(|r| r.contains(c)))?; - if dv.kind != gridcraft_engine::model::ValidationKind::List || !dv.in_cell_dropdown { + if dv.kind != gridcraft_engine::model::ValidationKind::List || !dv.in_cell_dropdown || geo.hidden_under_frozen(sh, c) { return None; } let r = geo.cell_rect(sh, c); @@ -1986,6 +2048,7 @@ fn paint_overlays( && let Some(dv) = sh.validations.iter().find(|d| d.ranges.iter().any(|r| r.contains(sel.active))) && dv.show_input && !(dv.input_title.is_empty() && dv.input_message.is_empty()) + && !geo.hidden_under_frozen(sh, sel.active) { let r = geo.cell_rect(sh, sel.active); let at = pos2(r.left() + 8.0, r.bottom() + 6.0); @@ -2007,7 +2070,7 @@ fn paint_overlays( continue; } let r = geo.cell_rect(sh, *c); - if !p.clip_rect().intersects(r) { + if !p.clip_rect().intersects(r) || geo.hidden_under_frozen(sh, *c) { continue; } let at = pos2(r.right() + 10.0, r.top()); @@ -2047,7 +2110,9 @@ fn paint_overlays( let Some(t) = target else { continue }; let tr = geo.range_rect(sh, t); if !t.is_single() { - p.rect_stroke(tr, 0.0, Stroke::new(1.5, blue), StrokeKind::Inside); + for pane in geo.range_panes(t) { + p.with_clip_rect(pane.intersect(p.clip_rect())).rect_stroke(tr, 0.0, Stroke::new(1.5, blue), StrokeKind::Inside); + } } let (from, to) = if it.get("range").is_some() { (tr.center(), active) } else { (active, tr.center()) }; p.line_segment([from, to], Stroke::new(1.5, blue)); @@ -2223,3 +2288,67 @@ fn paint_pages(app: &mut SheetApp, p: &Painter, geo: &Geo, sh: &Sheet) { } p.rect_stroke(pr, 0.0, Stroke::new(3.0, blue), StrokeKind::Outside); } + +#[cfg(test)] +mod pane_tests { + use super::*; + + fn rng(r0: u32, c0: u32, r1: u32, c1: u32) -> RangeRef { + RangeRef::new(CellRef::new(r0, c0), CellRef::new(r1, c1)) + } + + #[test] + fn selection_in_scrolling_pane_is_not_drawn_in_frozen_pane() { + // Frozen: 2 rows and 2 columns; the scrolling pane shows rows 2..=20 and columns 2..=10. + let frozen_cols = ((2, 20), (0, 1)); + let frozen_rows = ((0, 1), (2, 10)); + let corner = ((0, 1), (0, 1)); + let scrolled = ((2, 20), (2, 10)); + let sel = rng(5, 4, 7, 6); + assert!(range_in_pane(sel, scrolled.0, scrolled.1)); + assert!(!range_in_pane(sel, frozen_cols.0, frozen_cols.1)); + assert!(!range_in_pane(sel, frozen_rows.0, frozen_rows.1)); + assert!(!range_in_pane(sel, corner.0, corner.1)); + } + + #[test] + fn selection_across_the_freeze_line_is_drawn_in_both_panes() { + let sel = rng(5, 0, 7, 6); + assert!(range_in_pane(sel, (2, 20), (0, 1))); + assert!(range_in_pane(sel, (2, 20), (2, 10))); + } + + /// Two frozen rows and columns, scrolled right so that columns C:E lie under the frozen columns. + fn frozen_geo(sh: &mut Sheet) -> Geo { + sh.freeze = Some((2, 2)); + let scroll = vec2(3.0 * sh.default_col_width + 8.0, 0.0); + Geo::new(sh, Rect::from_min_size(pos2(0.0, 0.0), vec2(800.0, 600.0)), scroll) + } + + #[test] + fn overlays_skip_cells_scrolled_under_the_frozen_columns() { + let mut sh = Sheet::new("S"); + let geo = frozen_geo(&mut sh); + assert!(geo.hidden_under_frozen(&sh, CellRef::new(5, 2))); + assert!(geo.hidden_under_frozen(&sh, CellRef::new(5, 4))); + assert!(!geo.hidden_under_frozen(&sh, CellRef::new(5, 5))); + assert!(!geo.hidden_under_frozen(&sh, CellRef::new(5, 0))); + let unscrolled = Geo::new(&sh, geo.rect, vec2(0.0, 0.0)); + assert!(!unscrolled.hidden_under_frozen(&sh, CellRef::new(5, 2))); + } + + #[test] + fn ranges_are_clipped_to_the_panes_that_hold_them() { + let mut sh = Sheet::new("S"); + let geo = frozen_geo(&mut sh); + let split = pos2(geo.cells.left() + geo.frozen_w, geo.cells.top() + geo.frozen_h); + assert_eq!(geo.range_panes(rng(5, 3, 7, 6)), [Rect::from_min_max(split, geo.cells.max)]); + assert_eq!(geo.range_panes(rng(5, 0, 7, 6)).len(), 2); + assert_eq!(geo.range_panes(rng(0, 0, 7, 6)).len(), 4); + assert_eq!(geo.range_panes(rng(0, 0, 1, 1)), [Rect::from_min_max(geo.cells.min, split)]); + // The fill handle of D6:E7 sits over the frozen columns, where nothing of it is shown. + let handle = geo.range_rect(&sh, rng(5, 3, 6, 4)).right_bottom(); + assert!(handle.x < split.x); + assert!(!geo.range_panes(rng(6, 4, 6, 4)).iter().any(|r| r.contains(handle))); + } +} From 36c591883b88d1b8fda497719b6a7f51f81ce4bd Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sat, 10 Oct 2026 23:44:01 +0200 Subject: [PATCH 02/42] Resize charts from the corner handle and the Size fields, keep typed chart and table names (#190) The corner drag now grabs the chart where the press began and the selected object's corner shows a resize cursor. The Format Chart Area Size fields, chart and axis title fields and the Table Name field reloaded their value from the workbook every frame, so typed or dragged edits were thrown away; they now keep a draft until the field loses focus (Escape cancels, like sheet tab rename). --- crates/ui-egui/src/grid.rs | 16 ++- crates/ui-egui/src/panes.rs | 26 ++-- crates/ui-egui/src/ribbon.rs | 7 +- crates/ui-egui/src/widgets.rs | 19 +++ crates/ui-egui/tests/chart_resize.rs | 119 ++++++++++++++++++ crates/ui-egui/tests/committed_text_fields.rs | 81 ++++++++++++ docs/chart-parity.md | 2 +- 7 files changed, 254 insertions(+), 16 deletions(-) create mode 100644 crates/ui-egui/tests/chart_resize.rs create mode 100644 crates/ui-egui/tests/committed_text_fields.rs diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 72d33e2..d890f17 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -1047,6 +1047,9 @@ fn paint_headers(p: &Painter, geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::Sel // ------------------------------------------------------------------ interaction +/// How close (in points) a press must be to an object's bottom-right corner to resize it instead of moving it. +const OBJECT_RESIZE_RADIUS: f32 = 10.0; + fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &Geo, sh: &Sheet, wb: &Workbook) { // Text selection and editing must not also move the object or edit worksheet cells. if app.text_box_editor.is_some() { @@ -1072,7 +1075,12 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & { let near_col_edge = in_col_header && col_edge(geo, sh, p.x).is_some(); let near_row_edge = in_row_header && row_edge(geo, sh, p.y).is_some(); - if near_col_edge { + let selected_object = crate::chartview::hit(app, geo, sh, p).filter(|&(_, id, _)| app.selected_chart == Some(id)); + let near_corner = selected_object.is_some_and(|(_, _, rect)| (p - rect.right_bottom()).length() < OBJECT_RESIZE_RADIUS); + let near_object_corner = app.grid.drag == Drag::None && near_corner; + if near_object_corner || matches!(app.grid.drag, Drag::Object { resize: true, .. }) { + ctx.set_cursor_icon(CursorIcon::ResizeNwSe); + } else if near_col_edge { ctx.set_cursor_icon(CursorIcon::ResizeColumn); } else if near_row_edge { ctx.set_cursor_icon(CursorIcon::ResizeRow); @@ -1135,7 +1143,9 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & } // Objects (charts, pictures, shapes) take clicks first. if resp.drag_started() || primary_clicked { - if let Some(p) = pos + // A drag is recognised only after the pointer has moved, so grab the object where the press began. + let at = if resp.drag_started() { pointer.press_origin().or(pos) } else { pos }; + if let Some(p) = at && let Some((kind, id, rect)) = crate::chartview::hit(app, geo, sh, p) { app.selected_chart = Some(id); @@ -1151,7 +1161,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & } if resp.drag_started() { let a = object_anchor(sh, kind, id); - let resize = (p - rect.right_bottom()).length() < 10.0; + let resize = (p - rect.right_bottom()).length() < OBJECT_RESIZE_RADIUS; if let Some(a) = a { app.grid.drag = Drag::Object { kind, id, start: p, orig: (a.cell, a.dx, a.dy, a.width, a.height), resize }; } diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index fb8a9d7..47da52d 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -152,8 +152,8 @@ fn format_chart(app: &mut SheetApp, ui: &mut Ui) { }; let theme = app.session.active().map(|d| d.wb.theme.clone()).unwrap_or_default(); ui.collapsing("Chart Title", |ui| { - let mut title = chart.title.clone().unwrap_or_default(); - if ui.text_edit_singleline(&mut title).lost_focus() { + let field = egui::Id::new(("gridcraft.chart_title", id)); + if let Some(title) = crate::widgets::committed_text(ui, field, chart.title.as_deref().unwrap_or(""), None) { app.run_or_alert("chart.set", json!({"chart": id, "title": title})); } }); @@ -195,17 +195,17 @@ fn format_chart(app: &mut SheetApp, ui: &mut Ui) { if ui.checkbox(&mut gl, "Gridlines").changed() { app.run_or_alert("chart.set", json!({"chart": id, "gridlines": gl})); } - let mut xt = chart.x_title.clone().unwrap_or_default(); ui.horizontal(|ui| { ui.label("Horizontal axis title:"); - if ui.text_edit_singleline(&mut xt).lost_focus() { + let field = egui::Id::new(("gridcraft.chart_x_title", id)); + if let Some(xt) = crate::widgets::committed_text(ui, field, chart.x_title.as_deref().unwrap_or(""), None) { app.run_or_alert("chart.set", json!({"chart": id, "xTitle": if xt.is_empty() { Json::Null } else { json!(xt) }})); } }); - let mut yt = chart.y_title.clone().unwrap_or_default(); ui.horizontal(|ui| { ui.label("Vertical axis title:"); - if ui.text_edit_singleline(&mut yt).lost_focus() { + let field = egui::Id::new(("gridcraft.chart_y_title", id)); + if let Some(yt) = crate::widgets::committed_text(ui, field, chart.y_title.as_deref().unwrap_or(""), None) { app.run_or_alert("chart.set", json!({"chart": id, "yTitle": if yt.is_empty() { Json::Null } else { json!(yt) }})); } }); @@ -250,12 +250,20 @@ fn format_chart(app: &mut SheetApp, ui: &mut Ui) { }); }); ui.collapsing("Size", |ui| { - let mut w = chart.anchor.width; - let mut h = chart.anchor.height; + // Keep the value being typed or dragged between frames; reloading it from the chart every frame threw the edit away. + let draft = egui::Id::new(("gridcraft.chart_size_draft", id)); + let (mut w, mut h) = ui.data_mut(|d| d.get_temp::<(f32, f32)>(draft)).unwrap_or((chart.anchor.width, chart.anchor.height)); let a = ui.add(egui::DragValue::new(&mut w).prefix("Width ").range(40.0..=4000.0)); let b = ui.add(egui::DragValue::new(&mut h).prefix("Height ").range(40.0..=4000.0)); if a.drag_stopped() || a.lost_focus() || b.drag_stopped() || b.lost_focus() { - app.run_or_alert("chart.set", json!({"chart": id, "width": w, "height": h})); + ui.data_mut(|d| d.remove::<(f32, f32)>(draft)); + if !ui.input(|i| i.key_pressed(egui::Key::Escape)) { + app.run_or_alert("chart.set", json!({"chart": id, "width": w, "height": h})); + } + } else if a.dragged() || a.has_focus() || b.dragged() || b.has_focus() { + ui.data_mut(|d| d.insert_temp(draft, (w, h))); + } else { + ui.data_mut(|d| d.remove::<(f32, f32)>(draft)); } }); } diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 264479a..ede1acc 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -1733,9 +1733,10 @@ fn table_design(app: &mut SheetApp, ui: &mut Ui) { }; ui.vertical(|ui| { ui.label(egui::RichText::new("Table Name:").small()); - let mut name = tname.clone(); - let r = ui.add(egui::TextEdit::singleline(&mut name).desired_width(120.0)); - if r.lost_focus() && name != tname { + let field = egui::Id::new(("gridcraft.table_name", &tname)); + if let Some(name) = crate::widgets::committed_text(ui, field, &tname, Some(120.0)) + && name != tname + { act(app, "table.rename", json!({"table": tname, "name": name})); } }); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 18649e6..7369892 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -125,6 +125,25 @@ pub fn split_button(ui: &mut Ui, icon: Icon, accent: Option, tip: &str) (m.on_hover_text(tr(ui, tip)).clicked(), a) } +/// A one-line text field for a value stored in the workbook, which callers reload every frame. The text being typed is +/// kept in egui memory until the field loses focus, then returned to commit (`None` when Escape cancels the edit). +pub fn committed_text(ui: &mut Ui, id: egui::Id, current: &str, width: Option) -> Option { + let width = width.unwrap_or(ui.spacing().text_edit_width); + let draft = id.with("draft"); + let mut text = ui.data_mut(|d| d.get_temp::(draft)).unwrap_or_else(|| current.to_string()); + let r = ui.add(egui::TextEdit::singleline(&mut text).id(id).desired_width(width)); + if r.lost_focus() { + ui.data_mut(|d| d.remove::(draft)); + return (!ui.input(|i| i.key_pressed(egui::Key::Escape))).then_some(text); + } + if r.has_focus() { + ui.data_mut(|d| d.insert_temp(draft, text)); + } else { + ui.data_mut(|d| d.remove::(draft)); + } + None +} + /// Our colour palette (theme row + tints + standard colours), returns a picked hex colour, /// `Some("none")` for No Fill/Automatic. pub fn color_palette(ui: &mut Ui, theme_colors: &[u32; 12], none_label: &str) -> Option { diff --git a/crates/ui-egui/tests/chart_resize.rs b/crates/ui-egui/tests/chart_resize.rs new file mode 100644 index 0000000..5aa07e0 --- /dev/null +++ b/crates/ui-egui/tests/chart_resize.rs @@ -0,0 +1,119 @@ +//! Resizing a chart by dragging its bottom-right corner or with the Format Chart Area Size fields, through real input. + +use egui::{Event, Key, Modifiers, PointerButton, accesskit::Role, 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; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn fixture() -> (Harness, u32) { + let mut session = Session::new(); + session.new_workbook(); + session.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); + session.execute("cell.set", json!({"cell": "A2", "input": "2"})).unwrap(); + session.execute("selection.set", json!({"range": "A1:A2"})).unwrap(); + let chart = session.execute("insert.chart", json!({"type": "column", "at": "D3", "width": 300, "height": 200})).unwrap(); + let id = chart["chart"].as_u64().unwrap() as u32; + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + (h, id) +} + +fn size(h: &Harness, id: u32) -> (f32, f32) { + let a = h.state().session.doc().unwrap().wb.active().unwrap().charts.iter().find(|c| c.id == id).unwrap().anchor; + (a.width, a.height) +} + +fn corner(h: &Harness, id: u32) -> egui::Pos2 { + let app = h.state(); + let sh = app.session.doc().unwrap().wb.active().unwrap(); + let geo = Geo::new(sh, app.grid.rect.unwrap(), app.view().scroll); + let a = sh.charts.iter().find(|c| c.id == id).unwrap().anchor; + let min = geo.cell_rect(sh, CellRef::parse("D3").unwrap()).min + vec2(a.dx, a.dy) * geo.z; + min + vec2(a.width, a.height) * geo.z +} + +fn button(h: &mut Harness, pos: egui::Pos2, pressed: bool) { + h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.run_steps(2); +} + +#[test] +fn dragging_the_corner_handle_resizes_the_chart() { + let (mut h, id) = fixture(); + let (w0, h0) = size(&h, id); + let start = corner(&h, id); + // Select the chart first, as the user does before grabbing a handle. + let inside = start - vec2(40.0, 40.0); + h.input_mut().events.push(Event::PointerMoved(inside)); + h.run_steps(2); + button(&mut h, inside, true); + button(&mut h, inside, false); + assert_eq!(h.state().selected_chart, Some(id)); + + h.input_mut().events.push(Event::PointerMoved(start)); + h.run_steps(2); + assert_eq!(h.output().platform_output.cursor_icon, egui::CursorIcon::ResizeNwSe, "the corner shows a resize cursor"); + button(&mut h, start, true); + // The pointer moves well past the drag threshold in one step, like a quick drag. + let end = start + vec2(60.0, 30.0); + h.input_mut().events.push(Event::PointerMoved(pos2(end.x, end.y))); + h.run_steps(3); + button(&mut h, end, false); + let (w1, h1) = size(&h, id); + assert!(w1 > w0 + 40.0 && h1 > h0 + 20.0, "chart grew from {w0}x{h0} to {w1}x{h1}"); +} + +/// Opens Format Chart Area for the chart and expands its Size section; returns the Width field's rect. +fn open_size_fields(h: &mut Harness, id: u32) -> egui::Rect { + h.state_mut().selected_chart = Some(id); + h.state_mut().grid.pane = Some("formatChart".into()); + h.run_steps(2); + h.get_by_role_and_label(Role::Button, "Size").click(); + h.run_steps(2); + h.get_all_by_role(Role::SpinButton).next().unwrap().rect() +} + +#[test] +fn typing_a_width_in_the_size_field_resizes_the_chart() { + let (mut h, id) = fixture(); + let (_, h0) = size(&h, id); + let field = open_size_fields(&mut h, id); + h.input_mut().events.push(Event::PointerMoved(field.center())); + button(&mut h, field.center(), true); + button(&mut h, field.center(), false); + // Clicking the field selects its text; each keystroke must survive the next frame. + for digit in ["6", "4", "0"] { + h.input_mut().events.push(Event::Text(digit.into())); + h.run_steps(2); + } + h.key_press(Key::Enter); + h.run_steps(2); + assert_eq!(size(&h, id), (640.0, h0)); +} + +#[test] +fn dragging_the_size_field_resizes_the_chart() { + let (mut h, id) = fixture(); + let (w0, _) = size(&h, id); + let field = open_size_fields(&mut h, id); + h.input_mut().events.push(Event::PointerMoved(field.center())); + button(&mut h, field.center(), true); + for step in 1..=6 { + h.input_mut().events.push(Event::PointerMoved(field.center() + vec2(10.0 * step as f32, 0.0))); + h.run_steps(1); + } + button(&mut h, field.center() + vec2(60.0, 0.0), false); + let (w1, _) = size(&h, id); + assert!(w1 > w0 + 20.0, "width went from {w0} to {w1}"); +} diff --git a/crates/ui-egui/tests/committed_text_fields.rs b/crates/ui-egui/tests/committed_text_fields.rs new file mode 100644 index 0000000..565a3d1 --- /dev/null +++ b/crates/ui-egui/tests/committed_text_fields.rs @@ -0,0 +1,81 @@ +//! Text fields over workbook values (chart titles, table name) keep what the user types until they commit it. + +use egui::{Event, Key, Modifiers, accesskit::Role, vec2}; +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness(session: Session) -> Harness { + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +fn session() -> Session { + let mut session = Session::new(); + session.new_workbook(); + session.execute("range.setValues", json!({"range": "A1", "values": [["Name", "Qty"], ["a", 1], ["b", 2]]})).unwrap(); + session +} + +/// Clicks the field, replaces its text with `text` and presses Enter. +fn retype(h: &mut Harness, field: egui::Rect, text: &str) { + h.input_mut().events.push(Event::PointerMoved(field.center())); + h.run_steps(1); + h.get_all_by_role(Role::TextInput).find(|n| n.rect() == field).unwrap().click(); + h.run_steps(2); + h.key_press_modifiers(Modifiers::COMMAND, Key::A); + h.run_steps(1); + // One keystroke per frame, as a person types: each must survive the next frame. + for c in text.chars() { + h.input_mut().events.push(Event::Text(c.to_string())); + h.run_steps(2); + } + h.key_press(Key::Enter); + h.run_steps(2); +} + +#[test] +fn typing_a_chart_title_in_format_chart_area_sets_it() { + let mut session = session(); + session.execute("selection.set", json!({"range": "A1:B3"})).unwrap(); + let chart = session.execute("insert.chart", json!({"type": "column", "at": "D3"})).unwrap(); + let id = chart["chart"].as_u64().unwrap() as u32; + let mut h = harness(session); + h.state_mut().selected_chart = Some(id); + h.state_mut().grid.pane = Some("formatChart".into()); + h.run_steps(2); + h.get_by_role_and_label(Role::Button, "Chart Title").click(); + h.run_steps(2); + // The pane sits on the right; the formula bar and name box are further left. + let field = h.get_all_by_role(Role::TextInput).map(|n| n.rect()).find(|r| r.left() > 850.0).unwrap(); + retype(&mut h, field, "Sales"); + let title = h.state().session.active().unwrap().wb.active().unwrap().charts.iter().find(|c| c.id == id).unwrap().title.clone(); + assert_eq!(title.as_deref(), Some("Sales")); +} + +#[test] +fn typing_a_table_name_on_the_table_design_tab_renames_the_table() { + let mut session = session(); + session.execute("insert.table", json!({"range": "A1:B3", "header": true})).unwrap(); + session.execute("selection.set", json!({"range": "A2"})).unwrap(); + let mut h = harness(session); + h.state_mut().ui.ribbon_tab = "Table Design".into(); + h.run_steps(2); + let label = h.get_by_label("Table Name:").rect(); + let below_label = |r: &egui::Rect| r.top() >= label.bottom() - 2.0 && (r.left() - label.left()).abs() < 20.0; + let field = h.get_all_by_role(Role::TextInput).map(|n| n.rect()).find(below_label).unwrap(); + retype(&mut h, field, "Sales"); + let name = h.state().session.active().unwrap().wb.active().unwrap().tables[0].name.clone(); + assert_eq!(name, "Sales"); +} diff --git a/docs/chart-parity.md b/docs/chart-parity.md index 0577929..edfc95e 100644 --- a/docs/chart-parity.md +++ b/docs/chart-parity.md @@ -35,7 +35,7 @@ open PRs #158, #161, and issue #190. | Element | Excel | GridCraft | |---|---|---| -| Move and resize the chart object | yes | move yes; **resize broken** (#190) | +| Move and resize the chart object | yes | yes (corner handle, Format Chart Area Size fields) | | Chart title, axis titles | text, linked to cells, full formatting | text only | | Legend | position, overlay, formatting, per-entry delete | position only | | Data labels | value, category, series, percentage, from cells, callouts, position | one on/off flag | From ac87c269ffdfe6d6a0f104e0d7468936270842c7 Mon Sep 17 00:00:00 2001 From: Zarvinx Date: Sun, 11 Oct 2026 01:02:18 +0300 Subject: [PATCH 03/42] Fix #213: delete selected objects from keyboard --- ROADMAP.md | 2 ++ crates/ui-egui/src/grid.rs | 18 ++++++++++++++++ crates/ui-egui/tests/interaction.rs | 33 +++++++++++++++++++++++++++++ docs/ui-parity.md | 4 +++- 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 1189fd7..1bbae9a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -131,6 +131,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Selected charts, pictures and shapes can be deleted from the keyboard (#213) | | 2026-10-10 | Ribbon translated into Japanese, Chinese, Korean, Brazilian Portuguese and Russian with system-language default (#31, #104, #82); CJK/Thai/Hebrew font fallback (#38); clear messages for encrypted and `.xls` files (#17); smooth line charts (#62). Progress docs restructured to the craftrules standard; full re-measure (this page). Initial ribbon key tips (#43), in-place text box editing (#84), copy as HTML table (#36). ODS and XLSB data import (#100, #101); recalc linear in the number of range formulas (#66); RISC-V Linux build; Flatpak repository on Pages; web cell-commit trap fixed | | 2026-10-09 | Community fixes: spill range operator `A1#`, sheet-scoped names, protection refuses locked-cell edits, Insert/Delete Cells adjust references, `DATE(1900,2,29)`, SUMIF sizing, `ROUND` at 15 digits, `_xlpm.` LAMBDA parameters in XLSX, vertical scrollbar, system appearance, border previews; v0.4.0 | | 2026-10-08 | v0.2.0 and v0.3.0 releases; Flatpak, AppImage zsync, branded DMG; desktop log file; DirectX 12 default on Windows | @@ -142,6 +143,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-11 | minor | Selected objects can be deleted from the keyboard (#213) | | 2026-10-10 | minor | Readiness-by-audience table with hours to ~95% per audience; full number unchanged (additive weighted sum) | | 2026-10-10 | minor | Added Mainstream practitioner (~50%) and Essentials user (~63%); languages updated for the 10-10 merges (ribbon in ja, zh, ko, pt-BR, ru); localization 5%→10%; full number re-checked (51.6, unchanged) | | 2026-10-10 | minor | Checked against the alpha gate (six core workflows pass): stays alpha | diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 72d33e2..6b7530d 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -1608,6 +1608,22 @@ fn object_anchor(sh: &Sheet, kind: &str, id: u32) -> Option bool { + let Some(id) = app.selected_chart else { return false }; + let kind = if sh.charts.iter().any(|object| object.id == id) { + "chart" + } else if sh.images.iter().any(|object| object.id == id) { + "image" + } else if sh.shapes.iter().any(|object| object.id == id) { + "shape" + } else { + return false; + }; + app.run_or_alert("object.delete", json!({"kind": kind, "id": id})); + app.selected_chart = None; + true +} + pub fn follow_link(app: &mut SheetApp, target: &str) { if target.contains("://") || target.starts_with("mailto:") { if let Some(f) = &app.services.open_url { @@ -1708,7 +1724,9 @@ fn keyboard(app: &mut SheetApp, ctx: &egui::Context, resp: &egui::Response, geo: } Key::Tab => app.move_after_enter(0, if shift { -1 } else { 1 }), Key::F2 => app.begin_edit(None, false), + Key::Delete if delete_selected_object(app, sh) => {} Key::Delete => app.run_or_alert("edit.clearContents", json!({})), + Key::Backspace if delete_selected_object(app, sh) => {} Key::Backspace => { // Excel for Mac: Delete clears the cell and starts editing it. app.run_or_alert("edit.clearContents", json!({})); diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index 84f20a6..7558771 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -85,6 +85,21 @@ fn click_cell(h: &mut egui_kittest::Harness<'static, SheetApp>, cell: &str) { h.run_steps(2); } +fn click_chart(h: &mut egui_kittest::Harness<'static, SheetApp>, id: 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 anchor = sheet.charts.iter().find(|chart| chart.id == id).unwrap().anchor; + let pos = egui::pos2(geo.x(sheet, anchor.cell.col) + anchor.dx * geo.z, geo.y(sheet, anchor.cell.row) + anchor.dy * geo.z) + + egui::vec2(anchor.width, anchor.height) * geo.z / 2.0; + h.input_mut().events.extend([ + Event::PointerMoved(pos), + Event::PointerButton { pos, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }, + Event::PointerButton { pos, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }, + ]); + h.run_steps(2); +} + #[test] fn keyboard_navigation_does_not_replay_the_last_pointer_click() { let mut h = harness(blank()); @@ -170,6 +185,24 @@ fn shortcuts_format_and_extend() { assert_eq!(value(&h, "A1"), Value::Empty); } +#[test] +fn delete_key_removes_the_selected_chart() { + let mut session = blank(); + session.execute("range.setValues", json!({"range": "A1:B2", "values": [[1, 2], [3, 4]]})).unwrap(); + let id = session.execute("insert.chart", json!({"range": "A1:B2", "at": "D2"})).unwrap()["chart"].as_u64().unwrap() as u32; + let mut h = harness(session); + + click_cell(&mut h, "C10"); + click_chart(&mut h, id); + assert_eq!(h.state().selected_chart, Some(id)); + + key(&mut h, Key::Delete, Modifiers::NONE); + + assert!(h.state().session.active().unwrap().wb.active().unwrap().charts.is_empty()); + assert_eq!(h.state().selected_chart, None); + assert_eq!(value(&h, "A1"), Value::Number(1.0)); +} + #[test] fn renders_every_ribbon_tab_without_panicking() { let mut s = Session::new(); diff --git a/docs/ui-parity.md b/docs/ui-parity.md index e85bca1..6c341de 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-10 · **Change:** major (created from a source audit and the issue tracker) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (selected objects can be deleted from the keyboard) · **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 @@ -56,6 +56,7 @@ Sources: `crates/ui-egui/src` (ribbon, grid, editor, formula bar, dialogs, panes | Ctrl+1, Ctrl+; and Ctrl+Shift+; , Ctrl+D/R, Ctrl+E, Ctrl+T, Ctrl+K, Ctrl+Shift+L | yes | most done; audit pending | | F2, F4, F5, F9 family, Shift+F3, F11 | yes | most; F11 chart sheet missing (no chart sheets) | | Mac-specific (Cmd variants, Fn keys) | yes | Cmd mapping done | +| Delete selected charts, pictures and shapes | yes | done (#213) | | Requests | — | #110 (more shortcuts and hot keys), #33 (sequential Alt; initial support landed in #43) | Work: a shortcut table generated from the command catalog checked against Microsoft's published @@ -91,5 +92,6 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-11 | minor | Delete and Backspace now remove the selected chart, picture or shape (#213) | | 2026-10-10 | minor | Chart resize bug (#190) | | 2026-10-10 | major | Created: chrome, grid, keyboard, dialogs, accessibility; key tips, Page Layout view and accessibility tree flagged | From 706fe7b10c4fa2ab4a168cfd9a11646fb03d1bcf Mon Sep 17 00:00:00 2001 From: Zarvinx Date: Sun, 11 Oct 2026 01:20:03 +0300 Subject: [PATCH 04/42] macOS: use the padded app icon in the Dock --- apps/gridcraft/src/main.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs index 3c73a95..dc70a09 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -165,6 +165,9 @@ fn services() -> Services { } fn icon() -> Option { + #[cfg(target_os = "macos")] + let bytes = include_bytes!("../../../assets/app-icon/gridcraft-macos-512.png"); + #[cfg(not(target_os = "macos"))] let bytes = include_bytes!("../../../assets/app-icon/hicolor/256x256/apps/ai.storyteller.gridcraft.png"); let img = image::load_from_memory(bytes).ok()?.to_rgba8(); Some(egui::IconData { width: img.width(), height: img.height(), rgba: img.into_raw() }) From a0dc1abce1f5e8fab69430ddbf727ba4d1bd9f84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Koci=C4=87?= Date: Sun, 11 Oct 2026 00:35:45 +0200 Subject: [PATCH 05/42] fix: Save toolbar button and saveChanges dialog discard an in-progress cell edit Clicking the Save icon in the toolbar, or the Save button in the "save changes?" dialog, called file.save directly without first committing a cell edit still in progress (e.g. double-click a cell, type a new value, then click Save with the mouse instead of pressing Enter). The pending text in EditState was never flushed into the document, so the saved file still held the pre-edit value, silently. The File > Save menu item already guards against this (app.commit_edit(...) before running the command); this applies the same guard to the toolbar icon and the saveChanges dialog's Save button. --- crates/ui-egui/src/dialogs.rs | 3 ++- crates/ui-egui/src/ribbon.rs | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 7dd3acb..8d6c8ad 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -810,7 +810,8 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { if ui.button("Cancel").clicked() { open = false; } - if ui.button("Save").clicked() { + // File operations must include the cell the user is still editing (as the File > Save menu item already does). + if ui.button("Save").clicked() && app.commit_edit(0, 0, false, false) { app.run_or_alert("file.save", json!({})); if app.session.active().is_some_and(|x| !x.is_dirty()) { app.run_or_alert("file.close", json!({"force": true})); diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 264479a..7838316 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -44,7 +44,10 @@ pub fn title_bar(app: &mut SheetApp, ui: &mut Ui) { if icon_button(ui, Icon::Home, t.text_dim, "Home", vec2(26.0, 26.0)).clicked() { app.open_dialog("start", json!({})); } - if icon_button(ui, Icon::Save, t.text_dim, "Save (⌘S)", vec2(26.0, 26.0)).clicked() { + if icon_button(ui, Icon::Save, t.text_dim, "Save (⌘S)", vec2(26.0, 26.0)).clicked() + // File operations must include the cell the user is still editing (as the File > Save menu item already does). + && app.commit_edit(0, 0, false, false) + { app.run_or_alert("file.save", json!({})); } let can_undo = app.session.active().is_some_and(|d| !d.undo.is_empty()); From 83c7a3c2f5b582388b7436b8c48be76e5790a019 Mon Sep 17 00:00:00 2001 From: webdevtodayjason <115739170+webdevtodayjason@users.noreply.github.com> Date: Sat, 10 Oct 2026 22:56:15 +0000 Subject: [PATCH 06/42] Contributors: add Jason Brashear display name --- contributors/people.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 78683ae..9b0d8aa 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -28,3 +28,7 @@ sync_github_name = false [people.edurbs] sync_github_name = true + +[people.webdevtodayjason] +display_name = "Jason Brashear" +sync_github_name = false From a788c41faa5b94ab280cfa40c9e1e43d4b731ce5 Mon Sep 17 00:00:00 2001 From: webdevtodayjason <115739170+webdevtodayjason@users.noreply.github.com> Date: Sat, 10 Oct 2026 22:56:15 +0000 Subject: [PATCH 07/42] #200: keep formula helpers inside the worksheet Fixes #200 --- crates/ui-egui/src/formula_bar.rs | 6 +- crates/ui-egui/src/grid.rs | 12 ++- crates/ui-egui/tests/formula_popup.rs | 144 ++++++++++++++++++++++++++ docs/ui-parity.md | 5 +- 4 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 crates/ui-egui/tests/formula_popup.rs diff --git a/crates/ui-egui/src/formula_bar.rs b/crates/ui-egui/src/formula_bar.rs index 6cca110..63abc6f 100644 --- a/crates/ui-egui/src/formula_bar.rs +++ b/crates/ui-egui/src/formula_bar.rs @@ -336,9 +336,11 @@ pub fn editor_widget(app: &mut SheetApp, ui: &mut egui::Ui, id: egui::Id, font: ui.painter().galley(pos, g, Color32::WHITE); } // Autocomplete list and argument hint under the editor. - if mine && (!ed.autocomplete.is_empty() || ed.current_function().is_some()) { + if mine && ui.is_rect_visible(resp.rect) && (!ed.autocomplete.is_empty() || ed.current_function().is_some()) { let below = resp.rect.left_bottom() + vec2(0.0, 4.0); - egui::Area::new(id.with("ac")).fixed_pos(below).order(egui::Order::Tooltip).show(ui.ctx(), |ui| { + // Areas have their own clip rect: keep cell helpers inside the worksheet. + let bounds = if ed.from_formula_bar { ui.ctx().content_rect() } else { ui.clip_rect() }; + egui::Area::new(id.with("ac")).fixed_pos(below).constrain_to(bounds).order(egui::Order::Tooltip).show(ui.ctx(), |ui| { egui::Frame::popup(ui.style()).inner_margin(4.0).show(ui, |ui| { if !ed.autocomplete.is_empty() { let mut pick = None; diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 72d33e2..8cad6c6 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -1756,10 +1756,18 @@ fn in_cell_editor(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo, sh: &Sheet, PatternType::Solid => st.fill.fg.resolve(&wb.theme).map(theme::color32).unwrap_or(Color32::WHITE), _ => Color32::WHITE, }; - ui.painter().rect_filled(erect, 0.0, fill); - ui.painter().rect_stroke(erect.expand(1.0), 0.0, Stroke::new(2.0, Tokens::get(ui.ctx()).sel_border), StrokeKind::Middle); + let split = pos2(geo.cells.left() + geo.frozen_w, geo.cells.top() + geo.frozen_h); + let quadrant = Rect::from_min_max( + pos2(if cell.col >= geo.fc { split.x } else { geo.cells.left() }, if cell.row >= geo.fr { split.y } else { geo.cells.top() }), + pos2(if cell.col >= geo.fc { geo.cells.right() } else { split.x }, if cell.row >= geo.fr { geo.cells.bottom() } else { split.y }), + ); + let clip = ui.clip_rect().intersect(quadrant); + let painter = ui.painter().with_clip_rect(clip); + painter.rect_filled(erect, 0.0, fill); + painter.rect_stroke(erect.expand(1.0), 0.0, Stroke::new(2.0, Tokens::get(ui.ctx()).sel_border), StrokeKind::Middle); let id = egui::Id::new("gridcraft.cell_editor"); let mut child = ui.new_child(egui::UiBuilder::new().max_rect(erect.shrink2(vec2(2.0, 1.0)))); + child.set_clip_rect(clip); crate::formula_bar::editor_widget(app, &mut child, id, font, false, erect.width() - 4.0); } diff --git a/crates/ui-egui/tests/formula_popup.rs b/crates/ui-egui/tests/formula_popup.rs new file mode 100644 index 0000000..6734d30 --- /dev/null +++ b/crates/ui-egui/tests/formula_popup.rs @@ -0,0 +1,144 @@ +//! Formula helpers must stay in the worksheet while its editor scrolls. +use egui::{Context, Id, LayerId, Order, RawInput, Rect, pos2, vec2}; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; + +fn frame(ctx: &Context, app: &mut SheetApp) -> egui::FullOutput { + let mut output = + ctx.run_ui(RawInput { screen_rect: Some(Rect::from_min_size(pos2(0.0, 0.0), vec2(1200.0, 800.0))), ..Default::default() }, |ui| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }); + output.textures_delta.clear(); + output +} + +fn frames(ctx: &Context, app: &mut SheetApp) { + for _ in 0..4 { + frame(ctx, app); + } +} + +fn visible(ctx: &Context, editor: &str) -> bool { + ctx.memory(|m| m.areas().is_visible(&LayerId::new(Order::Tooltip, Id::new(editor).with("ac")))) +} + +#[test] +fn cell_formula_helpers_hide_when_scrolled_out_and_return_with_the_editor() { + for text in ["=SU", "=SUM("] { + let ctx = Context::default(); + let mut session = Session::new(); + session.new_workbook(); + let mut app = SheetApp::new(session, Default::default()); + frames(&ctx, &mut app); + app.begin_edit(Some(text.into()), false); + if text == "=SU" { + app.editor.as_mut().unwrap().update_autocomplete(&["SUM".into(), "SUMIF".into()]); + } + frames(&ctx, &mut app); + assert!(visible(&ctx, "gridcraft.cell_editor"), "visible cell shows {text} help"); + app.view_mut().unwrap().scroll.y = 400.0; + frames(&ctx, &mut app); + assert!(!visible(&ctx, "gridcraft.cell_editor"), "scrolled-out cell must hide {text} help"); + assert_eq!(app.editor.as_ref().unwrap().text, text, "scrolling preserves the edit"); + app.view_mut().unwrap().scroll.y = 0.0; + frames(&ctx, &mut app); + assert!(visible(&ctx, "gridcraft.cell_editor"), "scrolling back restores {text} help"); + } +} + +#[test] +fn formula_bar_hint_remains_visible_when_the_worksheet_scrolls() { + let ctx = Context::default(); + let mut session = Session::new(); + session.new_workbook(); + let mut app = SheetApp::new(session, Default::default()); + frames(&ctx, &mut app); + app.begin_edit(Some("=SUM(".into()), true); + frames(&ctx, &mut app); + assert!(visible(&ctx, "gridcraft.formula_bar")); + app.view_mut().unwrap().scroll.y = 400.0; + frames(&ctx, &mut app); + assert!(visible(&ctx, "gridcraft.formula_bar")); +} + +#[test] +fn cell_argument_hint_is_clipped_to_the_worksheet() { + let ctx = Context::default(); + let mut session = Session::new(); + session.new_workbook(); + let mut app = SheetApp::new(session, Default::default()); + frames(&ctx, &mut app); + app.begin_edit(Some("=SUM(".into()), false); + frames(&ctx, &mut app); + app.view_mut().unwrap().scroll.y = 10.0; + frames(&ctx, &mut app); + let output = frame(&ctx, &mut app); + let cells = app.grid.cells_rect.unwrap(); + let hint = output.shapes.iter().find(|s| matches!(&s.shape, egui::Shape::Text(t) if t.galley.text() == "SUM(")).expect("argument hint painted"); + assert!(cells.contains_rect(hint.clip_rect), "helper must not paint over the headers: {:?}", hint.clip_rect); +} + +#[test] +fn scrolled_out_cell_editor_still_handles_escape() { + let ctx = Context::default(); + let mut session = Session::new(); + session.new_workbook(); + let mut app = SheetApp::new(session, Default::default()); + frames(&ctx, &mut app); + app.begin_edit(Some("=SUM(".into()), false); + frames(&ctx, &mut app); + app.view_mut().unwrap().scroll.y = 400.0; + frames(&ctx, &mut app); + let mut output = ctx.run_ui( + RawInput { + screen_rect: Some(Rect::from_min_size(pos2(0.0, 0.0), vec2(1200.0, 800.0))), + events: vec![egui::Event::Key { + key: egui::Key::Escape, + physical_key: None, + pressed: true, + repeat: false, + modifiers: egui::Modifiers::NONE, + }], + ..Default::default() + }, + |ui| app.ui(ui), + ); + output.textures_delta.clear(); + assert!(app.editor.is_none(), "scrolling must not disable editor keys"); +} + +#[test] +fn cell_editor_and_helper_stay_below_the_frozen_top_row() { + let ctx = Context::default(); + let mut session = Session::new(); + session.new_workbook(); + session.run("view.freezeTopRow", serde_json::json!({})).unwrap(); + session.run("selection.set", serde_json::json!({"range": "B5"})).unwrap(); + let mut app = SheetApp::new(session, Default::default()); + frames(&ctx, &mut app); + let sh = app.session.active().unwrap().wb.active().unwrap(); + let frozen_h = sh.row_height(0); + let scroll = sh.row_top(4) as f32; + app.begin_edit(Some("=SUM(".into()), false); + frames(&ctx, &mut app); + assert!(visible(&ctx, "gridcraft.cell_editor")); + app.view_mut().unwrap().scroll.y = scroll - frozen_h / 2.0; + frames(&ctx, &mut app); + let output = frame(&ctx, &mut app); + let frozen_bottom = app.grid.cells_rect.unwrap().top() + frozen_h * gridcraft_ui_egui::grid::DISPLAY_SCALE; + let editor = output + .shapes + .iter() + .find(|s| s.clip_rect.bottom() > frozen_bottom && matches!(&s.shape, egui::Shape::Text(t) if t.galley.text() == "=SUM(")) + .expect("editor painted"); + assert!(editor.clip_rect.top() >= frozen_bottom, "editor must be clipped below the frozen row: {:?}", editor.clip_rect); + app.view_mut().unwrap().scroll.y = scroll; + frames(&ctx, &mut app); + assert!(!visible(&ctx, "gridcraft.cell_editor"), "cell hidden under the frozen row must hide its helper"); + assert_eq!(app.editor.as_ref().unwrap().text, "=SUM("); + app.view_mut().unwrap().scroll.y = 0.0; + frames(&ctx, &mut app); + assert!(visible(&ctx, "gridcraft.cell_editor"), "scrolling back restores the helper"); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index e85bca1..689a580 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-10 · **Change:** major (created from a source audit and the issue tracker) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-10 · **Change:** minor (formula helpers stay inside the worksheet, #200) · **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 @@ -36,7 +36,7 @@ Sources: `crates/ui-egui/src` (ribbon, grid, editor, formula bar, dialogs, panes | Drag-move and drag-copy cells (border drag, Ctrl/Option, Shift to insert) | done (M3) | | In-cell editing, Enter/Tab movement, Alt/Option+Enter line breaks | done | | Point mode (click/drag/arrow references), reference colouring, F4 | done | -| AutoComplete in columns, function autocomplete and argument tips | done | +| AutoComplete in columns, function autocomplete and argument tips | done; cell helpers clip to the worksheet and hide when the editor scrolls out (#200) | | Header resize and double-click AutoFit | done (#42, #88 for header dividers) | | Freeze panes, split panes | freeze done; split to verify | | Copy as HTML table for other apps | done (#36, 10-10) | @@ -91,5 +91,6 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-10 | minor | Keep formula helpers inside the worksheet when scrolling (#200) | | 2026-10-10 | minor | Chart resize bug (#190) | | 2026-10-10 | major | Created: chrome, grid, keyboard, dialogs, accessibility; key tips, Page Layout view and accessibility tree flagged | From 2f078962a0aa2d4692d04f6485d4a1aa6327c85e Mon Sep 17 00:00:00 2001 From: pkibiego Date: Sun, 11 Oct 2026 03:39:32 +0300 Subject: [PATCH 08/42] xlsx: bump quick-xml to 0.41 for the namespace-declaration DoS (RUSTSEC-2026-0195) The ODS and XLSB readers parse untrusted package metadata and content through `quick_xml::NsReader`. In 0.38.4 that is RUSTSEC-2026-0195 (CVSS 7.5): the namespace resolver pushes a binding for every `xmlns` / `xmlns:*` declaration on a start tag, with no cap on how many there are, and `NsReader::read_event` reaches that path - not only `read_resolved_event`. None of our caps bound it. MAX_PART (512 MiB), MAX_TOTAL (1 GiB), MAX_ENTRIES, MAX_CELLS, MAX_TEXT and MAX_DEPTH bound part size, entry count, cells, text and nesting, never declarations per tag. Measured with `gridcraft-cli info` on crafted .ods files, before this change: 754 B, 0 declarations -> 8.6 MB, exit 0 24 KB, 1,000,000 -> 52.6 MB, exit 0 187 KB, 8,000,000 -> 367.8 MB, exit 0 770 KB, 33,000,000 -> 1.46 GB, exit 0 About 45 bytes of resident memory per declaration, roughly 1,900x the file size, and no error at all. At MAX_PART a ~1 MB file reaches ~1.9 GB, and every part is held in memory while the package is read. Both readers share the path, so a downloaded .ods or .xlsb was enough. Nothing panicked, so `Session::execute` never saw it. 0.41.0 caps declarations per start tag at 256 (DEFAULT_MAX_DECLARATIONS_PER_ELEMENT, raisable through NamespaceResolver::set_max_declarations_per_element) and returns `NamespaceError::TooManyDeclarations` beyond it. The same fixtures after: 754 B -> 8.6 MB, ok 24 KB -> 19 MB, error 187 KB -> 101.8 MB, error 770 KB -> 397 MB, error with "start tag declares more than 256 namespace bindings". The residual peak is the part itself: MAX_PART still lets one part decompress to 256 MiB, the existing intended bound. API changes for 0.41 (0.x bumps may break; all four are in the xlsx crate): - `resolve_element` and `resolve_attribute` moved off `NsReader` on to the resolver: `reader.resolver_mut().resolve_element(..)`. `attr` and `tag` in ods.rs take `&mut NsReader` for that. - `Attribute::unescape_value` is deprecated in favour of `normalized_value`, which also applies the attribute-value normalization the XML specification requires. - `BytesText::xml_content` now takes the document's XML version. This also clears RUSTSEC-2026-0194, the quadratic duplicate-attribute check. xml.rs already passed `with_checks(false)`, but ods.rs and xlsb.rs used the checked iterator, so they were on the quadratic path as well. Regression test: `ods_tests::ods_namespace_declaration_bomb_is_rejected` asserts that a tag with thousands of declarations is an error and that a document with a handful still reads. It fails on 0.38.4 (the tag is accepted) and passes on 0.41.0. Verified: `cargo xtask ci`; the full xlsx suite (80 tests) plus the malformed-input tests; and the four measurements above, before and after, with no panics either way. --- Cargo.lock | 13 ++----------- Cargo.toml | 2 +- crates/xlsx/src/ods.rs | 23 ++++++++++++----------- crates/xlsx/src/tests/ods_tests.rs | 16 ++++++++++++++++ crates/xlsx/src/xlsb.rs | 7 ++++--- crates/xlsx/src/xml.rs | 5 +++-- 6 files changed, 38 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f772f83..16e2221 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1584,7 +1584,7 @@ dependencies = [ "gridcraft-core", "gridcraft-formula", "gridcraft-model", - "quick-xml 0.38.4", + "quick-xml", "thiserror 2.0.21", "zip", ] @@ -2739,15 +2739,6 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" -[[package]] -name = "quick-xml" -version = "0.38.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b66c2058c55a409d601666cffe35f04333cf1013010882cec174a7467cd4e21c" -dependencies = [ - "memchr", -] - [[package]] name = "quick-xml" version = "0.41.0" @@ -3725,7 +3716,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "338e30461b3a2b67d70eb30a6d89f8e0c93a833e07d2ae89085cd070c4a00ac0" dependencies = [ "proc-macro2", - "quick-xml 0.41.0", + "quick-xml", "quote", ] diff --git a/Cargo.toml b/Cargo.toml index d7fad0f..d826390 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ serde_json = { version = "1", features = ["float_roundtrip"] } log = "0.4" rayon = "1" regex = { version = "1", default-features = false, features = ["std", "unicode", "perf"] } -quick-xml = "0.38" +quick-xml = "0.41" zip = { version = "3", default-features = false, features = ["deflate"] } image = { version = "0.25", default-features = false, features = ["png", "jpeg"] } diff --git a/crates/xlsx/src/ods.rs b/crates/xlsx/src/ods.rs index 4f63fd1..8415ce3 100644 --- a/crates/xlsx/src/ods.rs +++ b/crates/xlsx/src/ods.rs @@ -10,6 +10,7 @@ use gridcraft_core::date::{days_in_month, serial_from_ymd}; use gridcraft_core::{CellRef, DateSystem, MAX_COLS, MAX_ROWS, RangeRef, Value}; use gridcraft_model::{Cell, NumFmt, Sheet, StyleId, Workbook}; use quick_xml::NsReader; +use quick_xml::XmlVersion; use quick_xml::events::{BytesStart, Event}; use quick_xml::name::ResolveResult; @@ -67,13 +68,13 @@ fn is_ns(ns: ResolveResult<'_>, expected: &[u8]) -> bool { matches!(ns, ResolveResult::Bound(n) if n.as_ref() == expected) } -fn attr(reader: &NsReader<&[u8]>, e: &BytesStart<'_>, ns: &[u8], name: &[u8]) -> Result, IoError> { +fn attr(reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>, ns: &[u8], name: &[u8]) -> Result, IoError> { let mut found = None; for a in e.attributes() { let a = a.map_err(|e| IoError::Xml(e.to_string()))?; - let (resolved, local) = reader.resolve_attribute(a.key); + let (resolved, local) = reader.resolver_mut().resolve_attribute(a.key); if is_ns(resolved, ns) && local.as_ref() == name { - let value = a.unescape_value().map_err(|e| IoError::Xml(e.to_string()))?; + let value = a.normalized_value(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?; if value.len() > MAX_CELL_TEXT { return Err(IoError::TooLarge("ODS attribute exceeds 1 MiB".into())); } @@ -106,7 +107,7 @@ fn check_manifest(bytes: &[u8]) -> Result<(), IoError> { let event = reader.read_event().map_err(xml_error)?; match &event { Event::Start(e) | Event::Empty(e) => { - let (ns, name) = reader.resolve_element(e.name()); + let (ns, name) = reader.resolver_mut().resolve_element(e.name()); if is_ns(ns.clone(), MANIFEST) && name.as_ref() == b"encryption-data" { return Err(IoError::Format("encrypted ODS files are not supported".into())); } @@ -153,8 +154,8 @@ enum Tag { Other, } -fn tag(reader: &NsReader<&[u8]>, e: &BytesStart<'_>) -> Tag { - let (ns, name) = reader.resolve_element(e.name()); +fn tag(reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>) -> Tag { + let (ns, name) = reader.resolver_mut().resolve_element(e.name()); let local = name.as_ref(); if is_ns(ns.clone(), OFFICE) { return match local { @@ -253,23 +254,23 @@ impl Import { return Err(xml_error("XML nested too deeply")); } if self.stack.is_empty() { - if root_seen || tag(&reader, &e) != Tag::Document { + if root_seen || tag(&mut reader, &e) != Tag::Document { return Err(xml_error("expected one office:document-content root")); } root_seen = true; } - self.start(&reader, &e)?; + self.start(&mut reader, &e)?; } Event::Empty(e) => { if self.stack.is_empty() { return Err(xml_error("expected office:document-content with a spreadsheet")); } - self.start(&reader, &e)?; + self.start(&mut reader, &e)?; self.end()?; } Event::End(_) => self.end()?, Event::Text(t) => { - let text = t.xml_content().map_err(xml_error)?; + let text = t.xml_content(XmlVersion::Explicit1_0).map_err(xml_error)?; if self.stack.is_empty() && !text.trim().is_empty() { return Err(xml_error("text outside the document")); } @@ -301,7 +302,7 @@ impl Import { Ok(()) } - fn start(&mut self, reader: &NsReader<&[u8]>, e: &BytesStart<'_>) -> Result<(), IoError> { + fn start(&mut self, reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>) -> Result<(), IoError> { let t = tag(reader, e); let depth = self.stack.len(); match t { diff --git a/crates/xlsx/src/tests/ods_tests.rs b/crates/xlsx/src/tests/ods_tests.rs index c0f2e0e..47c2586 100644 --- a/crates/xlsx/src/tests/ods_tests.rs +++ b/crates/xlsx/src/tests/ods_tests.rs @@ -23,6 +23,22 @@ fn text(s: &str) -> Value { Value::Text(s.into()) } +/// A start tag with thousands of `xmlns:` declarations is refused instead of allocated +/// (RUSTSEC-2026-0195: the resolver kept a binding per declaration, with no cap). +#[test] +fn ods_namespace_declaration_bomb_is_rejected() { + let bomb = |n: usize| -> String { (0..n).map(|i| format!(" xmlns:p{i}=\"urn:p{i}\"")).collect() }; + let doc = |decls: &str| { + format!( + r#"1"# + ) + }; + let build = |decls: &str| make_zip(&[("mimetype", MIME), ("META-INF/manifest.xml", MANIFEST), ("content.xml", doc(decls).as_bytes())]); + let ok = read_ods(&build(&bomb(8))); + assert!(ok.is_ok(), "a handful of namespaces still reads: {:?}", ok.err()); + assert!(read_ods(&build(&bomb(10_000))).is_err(), "a tag with thousands of declarations is refused"); +} + #[test] fn ods_typed_values_and_formula_caches_are_constants_and_survive_xlsx() { let bytes = row(r#" diff --git a/crates/xlsx/src/xlsb.rs b/crates/xlsx/src/xlsb.rs index 897a40b..3c84547 100644 --- a/crates/xlsx/src/xlsb.rs +++ b/crates/xlsx/src/xlsb.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use gridcraft_core::{CellError, CellRef, DateSystem, MAX_COLS, MAX_ROWS, RangeRef, Value}; use gridcraft_model::{Cell, Sheet, Visibility, Workbook}; use quick_xml::NsReader; +use quick_xml::XmlVersion; use quick_xml::events::Event; use quick_xml::name::ResolveResult; @@ -160,7 +161,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result, IoE let event = reader.read_event().map_err(|e| IoError::Xml(e.to_string()))?; match &event { Event::Start(e) | Event::Empty(e) => { - let (namespace, local) = reader.resolve_element(e.name()); + let (namespace, local) = reader.resolver_mut().resolve_element(e.name()); let valid_ns = matches!(namespace, ResolveResult::Bound(n) if n.as_ref() == ns); if depth == 0 { if seen || !valid_ns || local.as_ref() != root_name { @@ -174,7 +175,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result, IoE if !a.key.as_ref().contains(&b':') { attrs.push(( String::from_utf8_lossy(a.key.as_ref()).into_owned(), - a.unescape_value().map_err(|e| IoError::Xml(e.to_string()))?.into_owned(), + a.normalized_value(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?.into_owned(), )); } } @@ -188,7 +189,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result, IoE } } Event::End(_) => depth = depth.checked_sub(1).ok_or_else(|| invalid("unexpected metadata end"))?, - Event::Text(t) if depth == 0 && !t.xml_content().map_err(|e| IoError::Xml(e.to_string()))?.trim().is_empty() => { + Event::Text(t) if depth == 0 && !t.xml_content(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?.trim().is_empty() => { return Err(invalid("text outside package metadata")); } Event::DocType(_) => return Err(invalid("DOCTYPE in package metadata is unsupported")), diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index efa5cdc..9e03f5f 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -2,6 +2,7 @@ //! shared strings) so large parts never need a full tree, plus escaping helpers for writing. use quick_xml::Reader; +use quick_xml::XmlVersion; use quick_xml::events::{BytesStart, Event}; use crate::IoError; @@ -124,7 +125,7 @@ fn start_el(e: &BytesStart<'_>) -> El { if key == b"xmlns" || key.starts_with(b"xmlns:") { continue; } - let v = match a.unescape_value() { + let v = match a.normalized_value(XmlVersion::Explicit1_0) { Ok(v) => v.into_owned(), Err(_) => String::from_utf8_lossy(&a.value).into_owned(), }; @@ -203,7 +204,7 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res } Event::Text(t) => { if let Some(top) = stack.last_mut() { - let s = t.xml_content().map_err(|e| IoError::Xml(e.to_string()))?; + let s = t.xml_content(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?; top.text.push_str(&s); } } From fae90c8e4aece9de93547c22c2a8364a7b6e11eb Mon Sep 17 00:00:00 2001 From: webdevtodayjason <115739170+webdevtodayjason@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:37:50 +0000 Subject: [PATCH 09/42] selection.move: clamp extreme dr/dc to the sheet edge instead of overflowing --- crates/core/src/addr.rs | 23 +++++++++-- crates/engine/src/cmd/edit.rs | 6 +-- .../engine/tests/extreme_selection_moves.rs | 40 +++++++++++++++++++ 3 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 crates/engine/tests/extreme_selection_moves.rs diff --git a/crates/core/src/addr.rs b/crates/core/src/addr.rs index 8a64910..2bdfe79 100644 --- a/crates/core/src/addr.rs +++ b/crates/core/src/addr.rs @@ -30,14 +30,14 @@ impl CellRef { } /// Moved by `(dr, dc)`, `None` when the result leaves the sheet. pub fn offset(&self, dr: i64, dc: i64) -> Option { - let r = self.row as i64 + dr; - let c = self.col as i64 + dc; + let r = (self.row as i64).checked_add(dr)?; + let c = (self.col as i64).checked_add(dc)?; if (0..MAX_ROWS as i64).contains(&r) && (0..MAX_COLS as i64).contains(&c) { Some(CellRef::new(r as u32, c as u32)) } else { None } } /// Moved by `(dr, dc)` and clamped to the sheet. pub fn offset_clamped(&self, dr: i64, dc: i64) -> CellRef { - let r = (self.row as i64 + dr).clamp(0, MAX_ROWS as i64 - 1); - let c = (self.col as i64 + dc).clamp(0, MAX_COLS as i64 - 1); + let r = (self.row as i64).saturating_add(dr).clamp(0, MAX_ROWS as i64 - 1); + let c = (self.col as i64).saturating_add(dc).clamp(0, MAX_COLS as i64 - 1); CellRef::new(r as u32, c as u32) } /// Parses `A1`, `$A$1`, `xfd1048576` (case-insensitive). `None` if malformed or out of range. @@ -272,6 +272,21 @@ mod tests { assert_eq!(a.iter().count(), 9); } + #[test] + fn extreme_offsets() { + let cell = CellRef::new(1, 1); + for delta in [i64::MIN, i64::MAX] { + assert_eq!(cell.offset(delta, 0), None); + assert_eq!(cell.offset(0, delta), None); + assert_eq!(cell.offset(delta, delta), None); + } + assert_eq!(cell.offset_clamped(i64::MAX, i64::MAX), CellRef::new(MAX_ROWS - 1, MAX_COLS - 1)); + assert_eq!(cell.offset_clamped(i64::MIN, i64::MIN), CellRef::new(0, 0)); + assert_eq!(cell.offset_clamped(i64::MAX, i64::MIN), CellRef::new(MAX_ROWS - 1, 0)); + assert_eq!(cell.offset_clamped(i64::MIN, i64::MAX), CellRef::new(0, MAX_COLS - 1)); + assert_eq!(cell.offset(2, -1), Some(CellRef::new(3, 0))); + } + #[test] fn offsets() { assert_eq!(CellRef::new(0, 0).offset(-1, 0), None); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index bb54fe0..2dba66f 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1,6 +1,6 @@ //! Cell entry, selection, undo/redo, clipboard, clear, fill, find & replace. -use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value}; +use gridcraft_core::{CellRef, MAX_ROWS, RangeRef, Value}; use gridcraft_model::{Cell, Formula, StyleId}; use serde_json::{Value as Json, json}; @@ -408,9 +408,9 @@ fn selection_move(s: &mut Session, p: &Json) -> Result { if !extend && let Some(m) = sheet.merge_at(from) { // Leaving a merged cell moves from its edge. if dr > 0 { - t = CellRef::new((m.end.row + dr as u32).min(MAX_ROWS - 1), from.col); + t = CellRef::new(m.end.offset_clamped(dr, 0).row, from.col); } else if dc > 0 { - t = CellRef::new(from.row, (m.end.col + dc as u32).min(MAX_COLS - 1)); + t = CellRef::new(from.row, m.end.offset_clamped(0, dc).col); } } t diff --git a/crates/engine/tests/extreme_selection_moves.rs b/crates/engine/tests/extreme_selection_moves.rs new file mode 100644 index 0000000..f6f5737 --- /dev/null +++ b/crates/engine/tests/extreme_selection_moves.rs @@ -0,0 +1,40 @@ +use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS}; +use gridcraft_engine::Session; +use serde_json::json; + +#[test] +fn selection_move_clamps_extreme_displacements() { + let mut session = Session::new(); + session.new_workbook(); + for (dr, dc, expected) in [ + (i64::MAX, 0, CellRef::new(MAX_ROWS - 1, 1)), + (0, i64::MAX, CellRef::new(1, MAX_COLS - 1)), + (i64::MAX, i64::MAX, CellRef::new(MAX_ROWS - 1, MAX_COLS - 1)), + (i64::MIN, i64::MIN, CellRef::new(0, 0)), + (i64::MAX, i64::MIN, CellRef::new(MAX_ROWS - 1, 0)), + (i64::MIN, i64::MAX, CellRef::new(0, MAX_COLS - 1)), + (2, -1, CellRef::new(3, 0)), + ] { + session.execute("selection.set", json!({"cell": "B2"})).unwrap(); + session.execute("selection.move", json!({"dr": dr, "dc": dc})).unwrap(); + assert_eq!(session.doc().unwrap().selection.active, expected); + } +} + +#[test] +fn selection_move_from_a_merged_cell_clamps_extreme_displacements() { + let mut session = Session::new(); + session.new_workbook(); + session.execute("selection.set", json!({"range": "B2:C3"})).unwrap(); + session.execute("home.mergeCells", json!({})).unwrap(); + for (dr, dc, expected) in [ + (i64::MAX, 0, CellRef::new(MAX_ROWS - 1, 1)), + (0, i64::MAX, CellRef::new(1, MAX_COLS - 1)), + (4_294_967_297, 0, CellRef::new(MAX_ROWS - 1, 1)), + (1, 0, CellRef::new(3, 1)), + ] { + session.execute("selection.set", json!({"cell": "B2"})).unwrap(); + session.execute("selection.move", json!({"dr": dr, "dc": dc})).unwrap(); + assert_eq!(session.doc().unwrap().selection.active, expected); + } +} From aee898595f49426ffdcae86708171a3dfbc4a7e8 Mon Sep 17 00:00:00 2001 From: Sebastian Whincop Date: Sun, 11 Oct 2026 16:22:36 +1300 Subject: [PATCH 10/42] Accuracy: round an exact tie at the 16th significant digit toward zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Excel shows 15 significant digits. When a number is an exact tie at the 16th (its exact decimal expansion is 15 digits, a 5, then only zeros), Excel rounds it toward zero: TEXT(1234567890123455,"0") is 1234567890123450. GridCraft took Rust's `{:.14e}`, which rounds that tie to even, so any tie with an odd 15th digit came out one unit too high (…460). decimal_digits now formats 16 digits (correct unless the 16th is a 5), and only on a 5 checks the exact expansion (a 30-digit screen, then the full one) and truncates a true tie. Non-ties, including values just above or below a tie, round as before. One format serves almost every number. Tests: ties of every 15th-digit parity, negatives, fractional ties, the neighbours just above and below a tie, the carry after fifteen 9s (999999999999999.5 shows 999999999999999), and unchanged ordinary cases. Evidence: Windows Excel 16.0.20430 TEXT() over a 174-row probe and a 24-row carry probe agree on every row except those under a percent format and General over negatives (the two bugs named below). The suite's number-format golden goes from 183,843 to 184,900 of 187,810 passing with no row that passed before failing now. modified: crates/numfmt/src/decimal.rs modified: crates/numfmt/src/tests.rs --- crates/numfmt/src/decimal.rs | 63 +++++++++++++++++++++++++++++--- crates/numfmt/src/tests.rs | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/crates/numfmt/src/decimal.rs b/crates/numfmt/src/decimal.rs index 15cfcd3..67071d4 100644 --- a/crates/numfmt/src/decimal.rs +++ b/crates/numfmt/src/decimal.rs @@ -9,17 +9,70 @@ pub(crate) const MAX_DECIMALS: usize = 400; /// The 15 significant digits of a positive finite number and the power of ten of the first /// digit: `a ≈ d0.d1d2… × 10^exp`. +/// +/// The value is rounded to the nearest 15-digit decimal. An exact tie at the 16th digit (the exact +/// decimal expansion is 15 digits, a 5, then only zeros, as for 1234567890123455 or +/// 123456789012345.5) goes toward zero, as Excel does: `1234567890123455` shows `1234567890123450`, +/// not `…460`. Excel observed (Windows 16.0.20430, TEXT()). pub(crate) fn decimal_digits(a: f64) -> ([u8; 15], i32) { let mut digits = [0u8; 15]; if !a.is_finite() || a <= 0.0 { return (digits, 0); } - let s = format!("{:.14e}", a); - let (mant, exp) = s.split_once('e').unwrap_or((s.as_str(), "0")); - for (slot, b) in digits.iter_mut().zip(mant.bytes().filter(u8::is_ascii_digit)) { - *slot = b - b'0'; + // 16 digits, rounded. Unless the 16th is a 5 it decides the 15th exactly (4 or less rounds + // down, 6 or more up), so one format serves almost every number. + let (d16, exp) = significant_digits(a, 16); + for (slot, d) in digits.iter_mut().zip(&d16) { + *slot = *d; } - (digits, exp.parse().unwrap_or(0)) + match d16.get(15).copied().unwrap_or(0) { + 0..=4 => (digits, exp), + 5 if is_exact_tie(a) => (digits, exp), + 5 => { + // Not a tie: round the exact value (the 16-digit rounding above may be a double rounding). + let (d15, exp15) = significant_digits(a, 15); + let mut out = [0u8; 15]; + for (slot, d) in out.iter_mut().zip(&d15) { + *slot = *d; + } + (out, exp15) + } + _ => { + let mut exp = exp; + let mut carry = true; + for d in digits.iter_mut().rev() { + if *d == 9 { + *d = 0; + } else { + *d += 1; + carry = false; + break; + } + } + if carry { + digits = [0; 15]; + digits[0] = 1; + exp += 1; + } + (digits, exp) + } + } +} + +/// `n` significant digits of `a` (rounded half to even on the exact binary value) and the power of +/// ten of the first. +fn significant_digits(a: f64, n: usize) -> (Vec, i32) { + let s = format!("{:.*e}", n.saturating_sub(1), a); + let (mant, exp) = s.split_once('e').unwrap_or((s.as_str(), "0")); + (mant.bytes().filter(u8::is_ascii_digit).map(|b| b - b'0').collect(), exp.parse().unwrap_or(0)) +} + +/// True when `a`'s exact decimal expansion has a 5 as its 16th significant digit and only zeros after. +fn is_exact_tie(a: f64) -> bool { + let is_tie = |ds: &[u8]| ds.get(15) == Some(&5) && ds.get(16..).is_some_and(|t| t.iter().all(|&d| d == 0)); + // A cheap screen first (30 digits), then the exact expansion: a double has at most 767 + // significant digits and Rust prints every one of them. + is_tie(&significant_digits(a, 30).0) && is_tie(&significant_digits(a, 800).0) } /// Rounds the number `digits × 10^exp` (see [`decimal_digits`]) to `decimals` places, half away diff --git a/crates/numfmt/src/tests.rs b/crates/numfmt/src/tests.rs index ea40753..0742b26 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -421,3 +421,73 @@ fn fuzz_no_panic() { } } } + +// ---- 15 significant digits: an exact tie at the 16th digit goes toward zero ---- +// Windows Excel 16.0.20430 TEXT() over the exact doubles below. An odd 15th digit is where +// round-half-even and toward-zero differ; an even one is where they agree. + +#[test] +fn an_exact_tie_at_the_sixteenth_digit_goes_toward_zero() { + for (v, whole) in [ + (1234567890123405.0, "1234567890123400"), + (1234567890123415.0, "1234567890123410"), + (1234567890123425.0, "1234567890123420"), + (1234567890123435.0, "1234567890123430"), + (1234567890123455.0, "1234567890123450"), + (1234567890123475.0, "1234567890123470"), + (1234567890123495.0, "1234567890123490"), + (6631679258820775.0, "6631679258820770"), + (8052991635850695.0, "8052991635850690"), + ] { + assert_eq!(f("0", v), whole, "0 over {v}"); + assert_eq!(f("0.00", v), format!("{whole}.00"), "0.00 over {v}"); + assert_eq!(f("0", -v), format!("-{whole}"), "0 over -{v}"); + } + assert_eq!(f("#,##0", 1234567890123455.0), "1,234,567,890,123,450"); + assert_eq!(f("#,##0", -3267527853107975.0), "-3,267,527,853,107,970"); + // A fraction is a tie as well, when it is exact in binary. + assert_eq!(f("0", 123456789012345.5), "123456789012345"); + assert_eq!(f("0.00", 123456789012345.5), "123456789012345.00"); + // Excel observed: no carry after fifteen 9s. + assert_eq!(f("0", 999999999999999.5), "999999999999999"); + assert_eq!(f("0", -999999999999999.5), "-999999999999999"); + assert_eq!(f("0", 4999999999999995.0), "4999999999999990"); + assert_eq!(f("#,##0", -4999999999999995.0), "-4,999,999,999,999,990"); +} + +#[test] +fn only_exact_ties_go_toward_zero() { + // Just above and just below a tie round to the nearer 15-digit value (Excel observed). + assert_eq!(f("0", 1234567890123455.0 + 0.25), "1234567890123460"); + assert_eq!(f("0", 1234567890123455.5), "1234567890123460"); + assert_eq!(f("0", 1234567890123455.0 - 0.25), "1234567890123450"); + // 16 digits that are not a tie, and the ordinary decimal cases, are untouched. + assert_eq!(f("0", 1234567890123456.0), "1234567890123460"); + assert_eq!(f("0", 1234567890123454.0), "1234567890123450"); + assert_eq!(f("0.00", 2.675), "2.68"); + assert_eq!(f("0.00", 1.005), "1.01"); + assert_eq!(f("0.0", 0.285), "0.3"); + assert_eq!(f("0.0", 99999999999999.95), "100000000000000.0"); + // Ten digits and a half are not a 16th-digit tie. + assert_eq!(f("0", 0.5), "1"); + assert_eq!(f("0", 2.5), "3"); +} + +#[test] +fn decimal_digits_keeps_fifteen_digits_and_truncates_a_tie() { + use crate::decimal::decimal_digits; + let (d, e) = decimal_digits(1234567890123455.0); + assert_eq!((d, e), ([1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5], 15)); + let (d, e) = decimal_digits(1234567890123456.0); + assert_eq!((d, e), ([1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 6], 15)); + let (d, e) = decimal_digits(123456789012345.5); + assert_eq!((d, e), ([1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5], 14)); + // Excel observed: a tie after fifteen 9s does not carry (999999999999999.5 shows 999999999999999). + let (d, e) = decimal_digits(999999999999999.5); + assert_eq!((d, e), ([9; 15], 14)); + let (d, e) = decimal_digits(4999999999999995.0); + assert_eq!((d, e), ([4, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9], 15)); + // A 16th digit above 5 carries as usual. + let (d, e) = decimal_digits(999999999999999.6); + assert_eq!((d, e), ([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 15)); +} From cbf56f0707f99ae3928be6abd8ed76c760adb567 Mon Sep 17 00:00:00 2001 From: Diego Ospina Date: Sat, 10 Oct 2026 22:36:49 -0500 Subject: [PATCH 11/42] i18n: Spanish interface translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Language::Es was registered and switches formulas to Spanish (SUMA, decimal comma, semicolon arguments), but every interface label still fell back to English. This adds es.rs with all 266 labels in Excel's Spanish wording (Combinar y centrar, Inmovilizar paneles, Análisis de hipótesis, Buscar objetivo, Rastrear precedentes…) and covers Spanish in the i18n tests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012JC2QZ1Qc5KQMBswS38GKy --- crates/ui-egui/src/i18n.rs | 8 +- crates/ui-egui/src/i18n/es.rs | 275 +++++++++++++++++++++++++ crates/ui-egui/tests/formula_locale.rs | 2 +- 3 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 crates/ui-egui/src/i18n/es.rs diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 1e7b212..e44a032 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; +mod es; mod ko; mod pt; mod ru; @@ -97,7 +98,8 @@ impl Language { /// "Orientation") is keyed `context|label`: English shows only the part after the `|`. pub fn tr(self, text: &str) -> &str { let translated = match self { - Self::En | Self::Es => None, + Self::En => None, + Self::Es => es::translate(text), Self::Ja => JAPANESE.iter().find(|(english, _)| *english == text).map(|(_, translated)| *translated), Self::Zh => zh::translate(text), Self::Ko => ko::translate(text), @@ -434,12 +436,16 @@ mod tests { assert!(ko::translate(english).is_some_and(|text| !text.is_empty()), "missing Korean: {english}"); assert!(ru::translate(english).is_some_and(|text| !text.is_empty()), "missing Russian: {english}"); assert!(pt::translate(english).is_some_and(|text| !text.is_empty()), "missing Portuguese: {english}"); + assert!(es::translate(english).is_some_and(|text| !text.is_empty()), "missing Spanish: {english}"); } assert_eq!(Language::Ja.tr("Data"), "データ"); assert_eq!(Language::Zh.tr("Data"), "数据"); assert_eq!(Language::Ko.tr("Data"), "데이터"); assert_eq!(Language::Ru.tr("Data"), "Данные"); assert_eq!(Language::PtBr.tr("Data"), "Dados"); + assert_eq!(Language::Es.tr("Data"), "Datos"); + assert_eq!(Language::Es.tr("Freeze Panes"), "Inmovilizar paneles"); + assert_eq!(Language::Es.tr("Sheet1!A1"), "Sheet1!A1"); assert_eq!(Language::PtBr.tr("Date"), "Data"); assert_eq!(Language::PtBr.tr("Sheet1!A1"), "Sheet1!A1"); assert_eq!(Language::Ja.tr("Sheet1!A1"), "Sheet1!A1"); diff --git a/crates/ui-egui/src/i18n/es.rs b/crates/ui-egui/src/i18n/es.rs new file mode 100644 index 0000000..fe201cc --- /dev/null +++ b/crates/ui-egui/src/i18n/es.rs @@ -0,0 +1,275 @@ +//! Spanish translations for the interface strings in i18n.rs. Labels follow Excel's Spanish +//! wording (sentence case, «Inmovilizar paneles», «Combinar y centrar»); anything absent falls back +//! to English. Formula names and separators are localized separately (`formula_locale`). + +pub(super) fn translate(text: &str) -> Option<&'static str> { + Some(match text { + "Paste" => "Pegar", + "Paste Values" => "Pegar valores", + "Paste Formulas" => "Pegar fórmulas", + "Paste Formatting" => "Pegar formato", + "Transpose" => "Transponer", + "Paste Link" => "Pegar vínculo", + "Paste Special…" => "Pegado especial…", + "Cut (⌘X)" => "Cortar (⌘X)", + "Copy (⌘C)" => "Copiar (⌘C)", + "Format Painter (double-click to keep it on)" => "Copiar formato (doble clic para mantenerlo activo)", + "Bold (⌘B)" => "Negrita (⌘B)", + "Italic (⌘I)" => "Cursiva (⌘I)", + "Underline (⌘U)" => "Subrayado (⌘U)", + "Double Underline" => "Subrayado doble", + "Strikethrough" => "Tachado", + "Underline" => "Subrayado", + "Increase Font Size" => "Aumentar tamaño de fuente", + "Decrease Font Size" => "Disminuir tamaño de fuente", + "Wrap Text" => "Ajustar texto", + "Merge & Center" => "Combinar y centrar", + "Merge Cells" => "Combinar celdas", + "Merge Across" => "Combinar horizontalmente", + "Unmerge Cells" => "Separar celdas", + "Fill Color" => "Color de relleno", + "Font Color" => "Color de fuente", + "Borders" => "Bordes", + "Top\nAlign" => "Alinear en la\nparte superior", + "Middle\nAlign" => "Alinear en\nel medio", + "Bottom\nAlign" => "Alinear en la\nparte inferior", + "Left" => "Izquierda", + "Right" => "Derecha", + "Center" => "Centro", + "Horizontal" => "Horizontal", + "Vertical Text" => "Texto vertical", + "Decrease Indent" => "Disminuir sangría", + "Increase Indent" => "Aumentar sangría", + "Orientation" => "Orientación", + "Angle Counterclockwise" => "Ángulo ascendente", + "Angle Clockwise" => "Ángulo descendente", + "Rotate Text Up" => "Girar texto hacia arriba", + "Rotate Text Down" => "Girar texto hacia abajo", + "Accounting Number Format" => "Formato de número de contabilidad", + "Percent Style" => "Estilo porcentual", + "Comma Style" => "Estilo de millares", + "Increase Decimal" => "Aumentar decimales", + "Decrease Decimal" => "Disminuir decimales", + "More Accounting Formats…" => "Más formatos de contabilidad…", + "Conditional\nFormatting" => "Formato\ncondicional", + "Conditional Formatting" => "Formato condicional", + "Format\nas Table" => "Dar formato\ncomo tabla", + "Cell\nStyles" => "Estilos\nde celda", + "Format Cells…" => "Formato de celdas…", + "Format Cell Alignment…" => "Alineación de celdas…", + "Format\nPane" => "Panel\nde formato", + "Format" => "Formato", + "Insert" => "Insertar", + "Insert Cells…" => "Insertar celdas…", + "Delete" => "Eliminar", + "Delete Cells…" => "Eliminar celdas…", + "Insert Sheet Rows" => "Insertar filas de hoja", + "Insert Sheet Columns" => "Insertar columnas de hoja", + "Delete Sheet Rows" => "Eliminar filas de hoja", + "Delete Sheet Columns" => "Eliminar columnas de hoja", + "Row Height…" => "Alto de fila…", + "AutoFit Row Height" => "Autoajustar alto de fila", + "Column Width…" => "Ancho de columna…", + "AutoFit Column Width" => "Autoajustar ancho de columna", + "Default Width…" => "Ancho predeterminado…", + "Hide Rows" => "Ocultar filas", + "Unhide Rows" => "Mostrar filas", + "Hide Columns" => "Ocultar columnas", + "Unhide Columns" => "Mostrar columnas", + "Lock Cell" => "Bloquear celda", + "Clear" => "Borrar", + "Clear All" => "Borrar todo", + "Clear Formats" => "Borrar formatos", + "Clear Contents" => "Borrar contenido", + "Clear Comments and Notes" => "Borrar comentarios y notas", + "Clear Hyperlinks" => "Borrar hipervínculos", + "AutoSum" => "Autosuma", + "AutoSum (⌘⇧T)" => "Autosuma (⌘⇧T)", + "Flash\nFill" => "Relleno\nrápido", + "Flash Fill" => "Relleno rápido", + "Fill" => "Rellenar", + "Clear Print Area" => "Borrar área de impresión", + "Find &\nSelect" => "Buscar y\nseleccionar", + "Find…" => "Buscar…", + "Replace…" => "Reemplazar…", + "Go To…" => "Ir a…", + "Go To Special…" => "Ir a Especial…", + "PivotTable" => "Tabla dinámica", + "Recommended\nCharts" => "Gráficos\nrecomendados", + "Table" => "Tabla", + "Pictures" => "Imágenes", + "Shapes" => "Formas", + "Icons" => "Iconos", + "Text\nBox" => "Cuadro\nde texto", + "Comment" => "Comentario", + "New\nComment" => "Nuevo\ncomentario", + "New Note" => "Nueva nota", + "Show/Hide Note" => "Mostrar u ocultar nota", + "Header &\nFooter" => "Encabezado y\npie de página", + "Text to\nColumns" => "Texto en\ncolumnas", + "Link" => "Vínculo", + "Symbol" => "Símbolo", + "Sparklines" => "Minigráficos", + "Header & Footer" => "Encabezado y pie de página", + "Draw with ink" => "Dibujar con lápiz", + "Eraser" => "Borrador", + "Ink to\nShape" => "Entrada de lápiz\na forma", + "Select\nObjects" => "Seleccionar\nobjetos", + "Themes" => "Temas", + "Margins" => "Márgenes", + "Size" => "Tamaño", + "Print\nArea" => "Área de\nimpresión", + "Set Print Area" => "Establecer área de impresión", + "Breaks" => "Saltos", + "Insert Page Break" => "Insertar salto de página", + "Remove Page Break" => "Quitar salto de página", + "Reset All Page Breaks" => "Restablecer todos los saltos de página", + "Print\nTitles" => "Imprimir\ntítulos", + "Narrow" => "Estrecho", + "Wide" => "Ancho", + "Normal" => "Normal", + "Portrait" => "Vertical", + "Landscape" => "Horizontal", + "Custom Margins…" => "Márgenes personalizados…", + "Insert\nFunction" => "Insertar\nfunción", + "Name\nManager" => "Administrador\nde nombres", + "Define Name" => "Definir nombre", + "Use in Formula" => "Utilizar en la fórmula", + "Create from Selection" => "Crear desde la selección", + "Trace Precedents" => "Rastrear precedentes", + "Trace Dependents" => "Rastrear dependientes", + "Remove Arrows" => "Quitar flechas", + "Show Formulas" => "Mostrar fórmulas", + "Error Checking" => "Comprobación de errores", + "Evaluate Formula" => "Evaluar fórmula", + "Watch Window" => "Ventana Inspección", + "Calculation\nOptions" => "Opciones\nde cálculo", + "Calculate Now" => "Calcular ahora", + "Calculate Sheet" => "Calcular hoja", + "Automatic" => "Automático", + "Automatic Except for Data Tables" => "Automático excepto en las tablas de datos", + "Manual" => "Manual", + "Sort &\nFilter" => "Ordenar\ny filtrar", + "Sort" => "Ordenar", + "Sort A to Z" => "Ordenar de A a Z", + "Sort Z to A" => "Ordenar de Z a A", + "Custom Sort…" => "Orden personalizado…", + "Filter" => "Filtro", + "Reapply" => "Volver a aplicar", + "Get Data\n(Text/CSV)" => "Obtener datos\n(texto/CSV)", + "Remove\nDuplicates" => "Quitar\nduplicados", + "Data\nValidation" => "Validación\nde datos", + "Data Validation" => "Validación de datos", + "Data Validation…" => "Validación de datos…", + "Circle Invalid Data" => "Rodear con un círculo datos no válidos", + "Clear Validation" => "Borrar círculos de validación", + "Group" => "Agrupar", + "Ungroup" => "Desagrupar", + "Subtotal" => "Subtotal", + "What-If\nAnalysis" => "Análisis de\nhipótesis", + "Data Table…" => "Tabla de datos…", + "Scenario Manager…" => "Administrador de escenarios…", + "Goal Seek…" => "Buscar objetivo…", + "Spelling" => "Ortografía", + "Check\nAccessibility" => "Comprobar\naccesibilidad", + "Threaded Comments" => "Comentarios en conversación", + "Show\nComments" => "Mostrar\ncomentarios", + "Protect\nWorkbook" => "Proteger\nlibro", + "Unprotect\nSheet" => "Desproteger\nhoja", + "Protect Sheet…" => "Proteger hoja…", + "Comments" => "Comentarios", + "Notes" => "Notas", + "Page Break\nPreview" => "Vista previa de\nsalto de página", + "Page\nLayout" => "Diseño\nde página", + "Formula Bar" => "Barra de fórmulas", + "Gridlines" => "Líneas de cuadrícula", + "Headings" => "Encabezados", + "Zoom" => "Zoom", + "100%" => "100%", + "Zoom to\nSelection" => "Ampliar\nselección", + "Freeze\nPanes" => "Inmovilizar\npaneles", + "Freeze Panes" => "Inmovilizar paneles", + "Freeze Top Row" => "Inmovilizar fila superior", + "Freeze First Column" => "Inmovilizar primera columna", + "Unfreeze Panes" => "Movilizar paneles", + "Dark Mode" => "Modo oscuro", + "Interface language" => "Idioma de la interfaz", + "Command\nPalette" => "Paleta de\ncomandos", + "Agent\nControl" => "Control\ndel agente", + "Action\nJournal" => "Registro\nde acciones", + "About\nGridCraft" => "Acerca de\nGridCraft", + "Share" => "Compartir", + "Add Chart\nElement" => "Agregar elemento\nde gráfico", + "Change\nChart Type" => "Cambiar tipo\nde gráfico", + "Switch\nRow/Column" => "Cambiar entre\nfilas y columnas", + "Data Labels: None" => "Etiquetas de datos: ninguna", + "Data Labels: Show" => "Etiquetas de datos: mostrar", + "Legend: None" => "Leyenda: ninguna", + "Legend: Bottom" => "Leyenda: abajo", + "Legend: Top" => "Leyenda: arriba", + "Legend: Right" => "Leyenda: derecha", + "Gridlines: None" => "Líneas de cuadrícula: ninguna", + "Gridlines: Show" => "Líneas de cuadrícula: mostrar", + "Chart\nTitle" => "Título\ndel gráfico", + "Delete\nChart" => "Eliminar\ngráfico", + "Convert\nto Range" => "Convertir\nen rango", + "Table\nStyles" => "Estilos\nde tabla", + "Top 10 Items" => "10 superiores", + "Top 10%" => "10% superior", + "Bottom 10 Items" => "10 inferiores", + "Bottom 10%" => "10% inferior", + "Above Average" => "Por encima del promedio", + "Below Average" => "Por debajo del promedio", + "Clear Rules from Entire Sheet" => "Borrar reglas de toda la hoja", + "Clear Rules from Selected Cells" => "Borrar reglas de las celdas seleccionadas", + "Constants" => "Constantes", + "Data Bars" => "Barras de datos", + "Color Scales" => "Escalas de color", + "Icon Sets" => "Conjuntos de iconos", + "Home" => "Inicio", + "Draw" => "Dibujar", + "Page Layout" => "Disposición de página", + "Formulas" => "Fórmulas", + "Data" => "Datos", + "Review" => "Revisar", + "View" => "Vista", + "Automate" => "Automatizar", + "Table Design" => "Diseño de tabla", + "Chart Design" => "Diseño de gráfico", + "Ready" => "Listo", + "Enter" => "Introducir", + "Edit" => "Modificar", + "Point" => "Señalar", + "Calculate" => "Calcular", + "Select destination and press Enter or choose Paste" => "Seleccione el destino y presione Entrar o elija Pegar", + "General" => "General", + "Number" => "Número", + "Currency" => "Moneda", + "Accounting" => "Contabilidad", + "Date" => "Fecha", + "Time" => "Hora", + "Percentage" => "Porcentaje", + "Fraction" => "Fracción", + "Scientific" => "Científica", + "Text" => "Texto", + "Special" => "Especial", + "Custom" => "Personalizada", + "Short Date" => "Fecha corta", + "Long Date" => "Fecha larga", + "More Number Formats…" => "Más formatos de número…", + "Financial" => "Financieras", + "Logical" => "Lógicas", + "Date &\nTime" => "Fecha\ny hora", + "Lookup &\nReference" => "Búsqueda y\nreferencia", + "Math &\nTrig" => "Matemáticas y\ntrigonométricas", + "More\nFunctions" => "Más\nfunciones", + "Page Layout|Orientation" => "Orientación", + "Display theme" => "Tema de la interfaz", + "System" => "Sistema", + "Light" => "Claro", + "Dark" => "Oscuro", + "Sheet Options|View" => "Ver", + "Print" => "Imprimir", + _ => return None, + }) +} diff --git a/crates/ui-egui/tests/formula_locale.rs b/crates/ui-egui/tests/formula_locale.rs index a92ab13..9dcb65d 100644 --- a/crates/ui-egui/tests/formula_locale.rs +++ b/crates/ui-egui/tests/formula_locale.rs @@ -183,5 +183,5 @@ fn portuguese_interface_retains_its_canonical_formula_fallback() { assert!(!enter(&mut app, "=SUMA(1;2)")); let restored: UiState = serde_json::from_str(&serde_json::to_string(&app.ui).unwrap()).unwrap(); assert_eq!(restored.language, Language::PtBr); - assert_eq!(Language::Es.tr("Page Layout|Orientation"), "Orientation"); + assert_eq!(Language::Es.tr("Page Layout|Orientation"), "Orientación"); } From 2b1f851d0ae97d12f355a35732e7f6369c5a537a Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 01:07:16 -0500 Subject: [PATCH 12/42] Conditional formatting: apply every matching rule's format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only the first matching rule's format was used, so with a light red fill for >300 and a red border for >500, 510 showed only the border. Excel applies every matching rule; where two rules set the same part (fill, font colour, a border edge…), the rule with the higher priority wins, and Stop If True still ends the list. --- crates/engine/src/cf.rs | 70 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cf.rs b/crates/engine/src/cf.rs index a102863..f7d3623 100644 --- a/crates/engine/src/cf.rs +++ b/crates/engine/src/cf.rs @@ -281,10 +281,12 @@ impl CfCache { }; if let Some(st) = matched_style { any = true; - out.style = Some(match out.style.take() { - None => st.clone(), - Some(prev) => prev, - }); + // Every matching rule applies; where two set the same part, the rule with the + // higher priority (seen first) wins. + match &mut out.style { + None => out.style = Some(st.clone()), + Some(prev) => layer_under(prev, st), + } if cf.stop_if_true { break; } @@ -294,6 +296,39 @@ impl CfCache { } } +/// Adds the parts `lower` sets that `style` leaves unset (differential styles: default parts +/// are unset). +fn layer_under(style: &mut Style, lower: &Style) { + if style.fill.pattern == PatternType::None { + style.fill = lower.fill; + } + if style.font.color == Color::Auto { + style.font.color = lower.font.color; + } + style.font.bold |= lower.font.bold; + style.font.italic |= lower.font.italic; + style.font.strike |= lower.font.strike; + if style.font.underline == Underline::None { + style.font.underline = lower.font.underline; + } + let b = &mut style.borders; + for (dst, src) in [ + (&mut b.left, lower.borders.left), + (&mut b.right, lower.borders.right), + (&mut b.top, lower.borders.top), + (&mut b.bottom, lower.borders.bottom), + (&mut b.diag_down, lower.borders.diag_down), + (&mut b.diag_up, lower.borders.diag_up), + ] { + if dst.is_none() { + *dst = src; + } + } + if style.num_fmt.as_str() == "General" { + style.num_fmt = lower.num_fmt.clone(); + } +} + /// Shifts a rule formula written for the top-left cell of the range to `c`. fn relative(f: &str, first: CellRef, c: CellRef) -> String { if first == c { @@ -306,3 +341,30 @@ fn relative(f: &str, first: CellRef, c: CellRef) -> String { Err(_) => f.to_string(), } } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn every_matching_rule_applies() { + let mut s = crate::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [[200], [400], [510]]})).unwrap(); + let rule = |op: &str, value: &str, preset: &str| json!({"range": "A1:A3", "rule": {"type": "cellIs", "operator": op, "value": value, "preset": preset}}); + s.execute("home.conditionalFormat", rule("greaterThan", "300", "lightRedFill")).unwrap(); + s.execute("home.conditionalFormat", rule("greaterThan", "500", "redBorder")).unwrap(); + let d = s.doc().unwrap(); + let mut cache = CfCache::new(); + let mut look = |a: &str| cache.look(&d.wb, 0, CellRef::parse(a).unwrap()).and_then(|l| l.style); + assert!(look("A1").is_none()); + let only_fill = look("A2").unwrap(); + assert_eq!(only_fill.fill.pattern, PatternType::Solid); + assert!(only_fill.borders.top.is_none()); + let both = look("A3").unwrap(); + assert_eq!(both.fill, only_fill.fill); + assert!(!both.borders.top.is_none()); + } +} From 4e7379bd6801b935448183f052109993bc6cea06 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 03:55:17 -0500 Subject: [PATCH 13/42] Conditional formatting: cell value rules on blank and error cells Excel compares a blank cell as 0 (or empty text) in Cell Value rules, so "less than 5" or "equal to 0" also formats blank cells, and a cell holding an error never matches (the comparison is an error). GridCraft skipped blank cells and matched errors as if they sorted above every number ("greater than 5" formatted #N/A). The No Blanks rule also counted a cell holding only spaces as not blank, while the Blanks rule (like Excel's LEN(TRIM())) counted it as blank, so such a cell matched both. --- crates/engine/src/cf.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cf.rs b/crates/engine/src/cf.rs index f7d3623..6640c7a 100644 --- a/crates/engine/src/cf.rs +++ b/crates/engine/src/cf.rs @@ -144,7 +144,8 @@ impl CfCache { CfOperator::GreaterOrEqual => compare(&v, &av) != std::cmp::Ordering::Less, CfOperator::LessOrEqual => compare(&v, &av) != std::cmp::Ordering::Greater, }; - (ok && !v.is_empty()).then_some(style.as_ref()) + // As in Excel, a blank cell compares as 0 (or ""), and an error never matches. + (ok && !v.is_error()).then_some(style.as_ref()) } CfRule::Expression { formula, style } => { gridcraft_calc::evaluate(wb, sheet, c, &relative(formula, first, c)).to_bool().unwrap_or(false).then_some(style.as_ref()) @@ -154,7 +155,7 @@ impl CfCache { CfRule::BeginsWith { text: t, style } => text.to_lowercase().starts_with(&t.to_lowercase()).then_some(style.as_ref()), CfRule::EndsWith { text: t, style } => text.to_lowercase().ends_with(&t.to_lowercase()).then_some(style.as_ref()), CfRule::Blanks { style } => (v.is_empty() || text.trim().is_empty()).then_some(style.as_ref()), - CfRule::NoBlanks { style } => (!v.is_empty()).then_some(style.as_ref()), + CfRule::NoBlanks { style } => (!v.is_empty() && !text.trim().is_empty()).then_some(style.as_ref()), CfRule::Errors { style } => v.is_error().then_some(style.as_ref()), CfRule::NoErrors { style } => (!v.is_error()).then_some(style.as_ref()), CfRule::Duplicate { style } => { @@ -367,4 +368,28 @@ mod tests { assert_eq!(both.fill, only_fill.fill); assert!(!both.borders.top.is_none()); } + + #[test] + fn cell_value_rules_on_blanks_and_errors() { + let mut s = crate::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [[3, "=NA()"], [null, 7], ["=NA()", null], [" ", null], [7, null]]})).unwrap(); + let rule = |range: &str, r: serde_json::Value| json!({"range": range, "rule": r}); + s.execute("home.conditionalFormat", rule("A1:A5", json!({"type": "cellIs", "operator": "less", "value": "5", "preset": "lightRedFill"}))) + .unwrap(); + s.execute("home.conditionalFormat", rule("A1:A5", json!({"type": "noBlanks", "preset": "redBorder"}))).unwrap(); + s.execute("home.conditionalFormat", rule("B1:B2", json!({"type": "cellIs", "operator": "greater", "value": "5", "preset": "lightRedFill"}))) + .unwrap(); + let d = s.doc().unwrap(); + let mut cache = CfCache::new(); + let mut look = |a: &str| cache.look(&d.wb, 0, CellRef::parse(a).unwrap()).and_then(|l| l.style); + let fill = |st: Option + +
Rp10,000.00Name
2.50 & upx
"#; + let rows = table_cells(excel); + assert_eq!(rows.len(), 2); + assert_eq!(rows[0][0], HtmlCell { text: "Rp10,000.00".into(), num: Some(10000.0) }); + assert_eq!(rows[0][1], HtmlCell { text: "Name".into(), num: None }); + assert_eq!(rows[1].len(), 3, "colspan keeps the next cell in its column"); + assert_eq!(rows[1][0].text, "2.50 & up"); + assert_eq!(rows[1][2].text, "x"); + + let calc = r#" +
Rp10,000.00
"#; + assert_eq!(table_cells(calc), vec![vec![HtmlCell { text: "Rp10,000.00".into(), num: Some(10000.0) }]]); + + let quoted_gt = r#"
1500
"#; + assert_eq!(table_cells(quoted_gt), vec![vec![HtmlCell { text: "1500".into(), num: Some(1500.0) }]]); + } + + #[test] + fn hostile_markup_does_not_panic() { + for html in [ + "", + "<", + "", + "
a", + "�&", + "", + "
1