diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 102c669..0b467e0 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -417,14 +417,7 @@ fn selection_move(s: &mut Session, p: &Json) -> Result { let Some(sheet) = d.wb.active() else { return Err(EngineError::NoDocument) }; let sel = &mut d.selection; // Extension moves the far corner, not the active cell. - let from = if extend { - let r = sel.current(); - let far_row = if sel.anchor.row == r.start.row { r.end.row } else { r.start.row }; - let far_col = if sel.anchor.col == r.start.col { r.end.col } else { r.start.col }; - CellRef::new(far_row, far_col) - } else { - sel.active - }; + let from = if extend { sel.moving_end() } else { sel.active }; // Skip hidden rows/columns and step over merged areas. let mut to = if jmp { jump(sheet, from, dr.signum(), dc.signum()) diff --git a/crates/engine/src/selection.rs b/crates/engine/src/selection.rs index 98f0c6c..27418a2 100644 --- a/crates/engine/src/selection.rs +++ b/crates/engine/src/selection.rs @@ -43,6 +43,13 @@ impl Selection { pub fn is_single_cell(&self) -> bool { self.ranges.len() == 1 && self.current().is_single() } + /// The corner of the current area opposite the anchor: the end that Shift+arrows move. + pub fn moving_end(&self) -> CellRef { + let r = self.current(); + let row = if self.anchor.row == r.start.row { r.end.row } else { r.start.row }; + let col = if self.anchor.col == r.start.col { r.end.col } else { r.start.col }; + CellRef::new(row, col) + } /// Extends the current area from the anchor to `to` (Shift+click / Shift+arrows). pub fn extend_to(&mut self, to: CellRef) { let r = RangeRef::new(self.anchor, to); diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 7739233..97ff0ca 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -68,6 +68,9 @@ pub enum Drag { pub struct GridState { pub drag: Drag, pub ensure_visible: bool, + /// With `ensure_visible`: scroll to the moving end of the selection instead of the active cell + /// (Shift/Ctrl+Shift+arrows keep the active cell on the anchor while the far corner moves). + pub reveal_moving_end: bool, pub last_title: Option, pub drag_select: bool, pub rect: Option, @@ -533,10 +536,11 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { view.scroll.x = view.scroll.x.min(max_x); view.scroll.y = view.scroll.y.min(max_y); let mut geo = make_geo(app, view.scroll); - // Keep the active cell visible after keyboard moves. + // Keep the active cell (or the moving end of an extension) visible after keyboard moves. if app.grid.ensure_visible { app.grid.ensure_visible = false; - let target = app.editor.as_ref().and_then(|e| e.point_cell).unwrap_or(sel.active); + let cell = if std::mem::take(&mut app.grid.reveal_moving_end) { sel.moving_end() } else { sel.active }; + let target = app.editor.as_ref().and_then(|e| e.point_cell).unwrap_or(cell); if paper { crate::paper_view::ensure_covers(app, &wb, si, target); geo = make_geo(app, view.scroll); @@ -2391,6 +2395,7 @@ fn keyboard(app: &mut SheetApp, ctx: &egui::Context, resp: &egui::Response, geo: let mv = |app: &mut SheetApp, dr: i64, dc: i64| { let _ = app.session.run("selection.move", json!({"dr": dr, "dc": dc, "extend": shift, "jump": cmd})); app.grid.ensure_visible = true; + app.grid.reveal_moving_end = shift; }; match key { Key::ArrowUp => mv(app, -1, 0), diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 3372e1d..a150c06 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -214,6 +214,7 @@ pub(crate) fn dispatch(app: &mut SheetApp, ctx: &egui::Context, key: Key, physic let dc = if shortcut == Shortcut::ScreenRight { cols } else { -cols }; let _ = app.session.run("selection.move", json!({"dr": 0, "dc": dc, "extend": m.shift})); app.grid.ensure_visible = true; + app.grid.reveal_moving_end = m.shift; } Shortcut::Edit => app.begin_edit(None, false), Shortcut::ContextMenu => app.grid.context_menu = Some(geo.cell_rect(sh, active).center()), @@ -245,4 +246,5 @@ fn extend(app: &mut SheetApp, anchor: CellRef, active: CellRef, to: CellRef) { d.selection.anchor = anchor; } app.grid.ensure_visible = true; + app.grid.reveal_moving_end = true; } diff --git a/crates/ui-egui/tests/selection_scroll.rs b/crates/ui-egui/tests/selection_scroll.rs new file mode 100644 index 0000000..06e0d18 --- /dev/null +++ b/crates/ui-egui/tests/selection_scroll.rs @@ -0,0 +1,106 @@ +//! Extending the selection from the keyboard scrolls its moving end into view, not the anchor. + +use egui::{Event, Key, Modifiers}; +use gridcraft_engine::Session; +use gridcraft_engine::core::{CellRef, RangeRef}; +use gridcraft_ui_egui::{SheetApp, grid::Geo}; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +/// Ctrl on Windows and Linux sets both `ctrl` and `command`. +const CTRL: Modifiers = Modifiers { alt: false, ctrl: true, shift: false, mac_cmd: false, command: true }; +const CTRL_SHIFT: Modifiers = Modifiers { shift: true, ..CTRL }; + +/// Column A filled down to A200 and row 1 filled across to CV1, far past one screen each way. +fn harness() -> Harness { + let mut s = Session::new(); + s.new_workbook(); + let down: Vec> = (1..=200).map(|r| vec![r]).collect(); + s.execute("range.setValues", json!({"range": "A1", "values": down})).unwrap(); + let across: Vec = (1..=100).collect(); + s.execute("range.setValues", json!({"range": "A1", "values": [across]})).unwrap(); + s.execute("selection.set", json!({"cell": "A1"})).unwrap(); + let mut h = Harness::builder().with_size(egui::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(s, Default::default()), + ); + h.run_steps(4); + h +} + +fn key(h: &mut Harness, k: Key, m: Modifiers) { + h.input_mut().events.push(Event::Key { key: k, physical_key: Some(k), pressed: true, repeat: false, modifiers: m }); + h.input_mut().events.push(Event::Key { key: k, physical_key: Some(k), pressed: false, repeat: false, modifiers: m }); + h.run_steps(2); +} + +/// Whether the cell's centre is on screen (a whole cell can sit a few pixels under the edge when +/// scrolling far down widens the row headings). +fn visible(h: &Harness, a1: &str) -> bool { + 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); + geo.cells.contains(geo.cell_rect(sheet, CellRef::parse(a1).unwrap()).center()) +} + +fn assert_extended(h: &Harness, anchor: &str, moving_end: &str) { + let selection = &h.state().session.active().unwrap().selection; + let (anchor_cell, end_cell) = (CellRef::parse(anchor).unwrap(), CellRef::parse(moving_end).unwrap()); + assert_eq!(selection.current(), RangeRef::new(anchor_cell, end_cell)); + assert_eq!(selection.anchor, anchor_cell); + assert_eq!(selection.active, anchor_cell, "extending keeps the active cell on the anchor"); + assert!(visible(h, moving_end), "the moving end {moving_end} is scrolled into view"); +} + +#[test] +fn ctrl_shift_arrows_scroll_the_moving_end_into_view() { + // Down, then back Up past the top once the view has scrolled. + let mut h = harness(); + key(&mut h, Key::ArrowDown, CTRL_SHIFT); + assert_extended(&h, "A1", "A200"); + assert!(!visible(&h, "A1"), "the view follows the moving end, away from the anchor"); + let mut h = harness(); + key(&mut h, Key::ArrowDown, CTRL); + assert!(visible(&h, "A200") && !visible(&h, "A1")); + key(&mut h, Key::ArrowUp, CTRL_SHIFT); + assert_extended(&h, "A200", "A1"); + // Right, then back Left. + let mut h = harness(); + key(&mut h, Key::ArrowRight, CTRL_SHIFT); + assert_extended(&h, "A1", "CV1"); + let mut h = harness(); + key(&mut h, Key::ArrowRight, CTRL); + assert!(visible(&h, "CV1") && !visible(&h, "A1")); + key(&mut h, Key::ArrowLeft, CTRL_SHIFT); + assert_extended(&h, "CV1", "A1"); +} + +#[test] +fn shift_arrows_scroll_the_moving_end_into_view() { + let mut h = harness(); + for _ in 0..60 { + key(&mut h, Key::ArrowDown, Modifiers::SHIFT); + } + assert_extended(&h, "A1", "A61"); + for _ in 0..30 { + key(&mut h, Key::ArrowRight, Modifiers::SHIFT); + } + assert_extended(&h, "A1", "AE61"); +} + +#[test] +fn ctrl_shift_home_and_end_scroll_the_moving_end_into_view() { + let mut h = harness(); + key(&mut h, Key::End, CTRL_SHIFT); + assert_extended(&h, "A1", "CV200"); + let mut h = harness(); + key(&mut h, Key::End, CTRL); + assert!(!visible(&h, "A1")); + key(&mut h, Key::Home, CTRL_SHIFT); + assert_extended(&h, "CV200", "A1"); +}