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] 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 72d33e2dc..d890f17d2 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 fb8a9d7f7..47da52d1f 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 264479a8c..ede1accc3 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 18649e66a..736989228 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 000000000..5aa07e080 --- /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 000000000..565a3d15e --- /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 05779299e..edfc95e8b 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 |