Merge pull request #218 from MarekSpodBiedry/issue-190

Resize charts from the corner handle and the Size fields, keep typed chart and table names (#190)

crates/ui-egui/src/grid.rs

@@ -1103,6 +1103,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() {
@@ -1134,7 +1137,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);
@@ -1197,7 +1205,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);
@@ -1213,7 +1223,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 };
                 }

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));
         }
     });
 }

crates/ui-egui/src/ribbon.rs

@@ -1739,9 +1739,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}));
         }
     });

crates/ui-egui/src/widgets.rs

@@ -125,6 +125,25 @@ pub fn split_button(ui: &mut Ui, icon: Icon, accent: Option<Color32>, 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<f32>) -> Option<String> {
+    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::<String>(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::<String>(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::<String>(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<String> {

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}");
+}

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");
+}

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 |