fix(selection): Shift+arrows and Enter/Tab treat a merged cell as one cell (#3619)

Shift+Left/Up from a selected merged cell now widen the selection past
its edges, Shift+arrows shrink an area back over a merged cell, and
Enter/Tab leave a selected merged cell instead of walking the cells it
covers. Inside a larger selection Enter visits a merged cell once.

Fixes #3619

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/edit/selection.rs

@@ -63,6 +63,11 @@ pub(super) fn selection_move(s: &mut Session, p: &Json) -> Result<Json> {
     let d = s.doc_mut()?;
     let Some(sheet) = d.wb.active() else { return Err(EngineError::NoDocument) };
     let sel = &mut d.selection;
+    if extend && !jmp {
+        // Shift+arrows move the edge the anchor doesn't hold, keeping merged cells whole.
+        sel.extend_by(sheet, dr, dc);
+        return Ok(json!({"selection": sel.a1(), "active": sel.active.a1()}));
+    }
     // Extension moves the far corner, not the active cell.
     let from = if extend { sel.moving_end() } else { sel.active };
     // Skip hidden rows/columns and step over merged areas.
@@ -77,6 +82,14 @@ pub(super) fn selection_move(s: &mut Session, p: &Json) -> Result<Json> {
         jump(sheet, from, dr.signum(), dc.signum())
     } else {
         let mut t = from.offset_clamped(dr, dc);
+        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.offset_clamped(dr, 0).row, from.col);
+            } else if dc > 0 {
+                t = CellRef::new(from.row, m.end.offset_clamped(0, dc).col);
+            }
+        }
         let mut guard = 0;
         while (sheet.is_row_hidden(t.row) || sheet.is_col_hidden(t.col)) && guard < 100_000 {
             let n = t.offset_clamped(dr.signum(), dc.signum());
@@ -86,14 +99,6 @@ pub(super) fn selection_move(s: &mut Session, p: &Json) -> Result<Json> {
             t = n;
             guard += 1;
         }
-        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.offset_clamped(dr, 0).row, from.col);
-            } else if dc > 0 {
-                t = CellRef::new(from.row, m.end.offset_clamped(0, dc).col);
-            }
-        }
         t
     };
     if let Some(m) = sheet.merge_at(to)
@@ -140,7 +145,29 @@ pub(super) fn selection_next(s: &mut Session, p: &Json) -> Result<Json> {
     let fwd = bool_param(p, "forward").unwrap_or(true);
     let by_row = bool_param(p, "byRow").unwrap_or(false);
     let d = s.doc_mut()?;
-    d.selection.cycle(fwd, by_row);
+    let Some(sheet) = d.wb.active() else { return Err(EngineError::NoDocument) };
+    let sel = &d.selection;
+    if sel.is_one_merged_cell(sheet) {
+        // A selection that is just one merged cell is one cell, as in Excel: Enter and Tab leave
+        // it for the next cell instead of walking the cells it covers.
+        let step = if fwd { 1 } else { -1 };
+        let (dr, dc) = if by_row { (0, step) } else { (step, 0) };
+        selection_move(s, &json!({"dr": dr, "dc": dc}))?;
+        return Ok(json!({"active": s.doc()?.selection.active.a1()}));
+    }
+    // Inside a larger selection the cells a merged cell covers are skipped: it is visited once,
+    // at its top-left cell.
+    let area = sel.current();
+    let cells =
+        (u64::from(area.end.row.saturating_sub(area.start.row)) + 1).saturating_mul(u64::from(area.end.col.saturating_sub(area.start.col)) + 1);
+    let mut next = sel.clone();
+    for _ in 0..cells.min(1 << 20) {
+        next.cycle(fwd, by_row);
+        if sheet.merge_at(next.active).is_none_or(|m| m.start == next.active) {
+            break;
+        }
+    }
+    d.selection = next;
     Ok(json!({"active": d.selection.active.a1()}))
 }
 

crates/engine/src/selection.rs

@@ -43,6 +43,10 @@ impl Selection {
     pub fn is_single_cell(&self) -> bool {
         self.ranges.len() == 1 && self.current().is_single()
     }
+    /// Whether the selection is exactly one merged cell, which Enter and Tab treat as one cell.
+    pub fn is_one_merged_cell(&self, sheet: &Sheet) -> bool {
+        self.ranges.len() == 1 && sheet.merge_at(self.active).is_some_and(|m| m == self.current())
+    }
     /// 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();
@@ -67,23 +71,28 @@ impl Selection {
     /// they are: clicking column B's header selects just B even when B1:C1 is merged, as in Excel.
     pub fn expand_merges(&mut self, sheet: &Sheet) {
         for r in self.ranges.iter_mut() {
-            if r.is_full_cols() || r.is_full_rows() {
-                continue;
-            }
-            let mut cur = *r;
-            loop {
-                let mut grown = cur;
-                for m in &sheet.merges {
-                    if m.intersects(&grown) {
-                        grown = grown.union(m);
-                    }
-                }
-                if grown == cur {
-                    break;
-                }
-                cur = grown;
-            }
-            *r = cur;
+            *r = with_merges(sheet, *r);
+        }
+    }
+    /// Shift+arrows: moves the edge of the current area that the anchor doesn't hold by `dr`
+    /// rows and `dc` columns, keeping merged cells whole, as Excel does. The anchor is the whole
+    /// merged cell it is in, so from a selected merged cell Shift+Left and Shift+Up widen the
+    /// selection past its left and top edges. A step that would shrink the area onto the inside
+    /// of a merged cell (where keeping it whole would undo the step) goes on to the next line
+    /// that does shrink it, and when the area can't shrink any more it grows in that direction.
+    pub fn extend_by(&mut self, sheet: &Sheet, dr: i64, dc: i64) {
+        let anchor = sheet.merge_at(self.anchor).unwrap_or(RangeRef::cell(self.anchor));
+        let mut r = self.current();
+        if dc != 0 {
+            r = step_edge(sheet, r, anchor, dc, true);
+        }
+        if dr != 0 {
+            r = step_edge(sheet, r, anchor, dr, false);
+        }
+        if let Some(last) = self.ranges.last_mut() {
+            *last = r;
+        } else {
+            self.ranges.push(r);
         }
     }
     /// Moves the active cell within the selection (Enter/Tab inside a multi-cell selection).
@@ -128,6 +137,91 @@ impl Selection {
     }
 }
 
+/// `r` grown until every merged cell it touches is inside it. Whole columns and whole rows stay
+/// as they are.
+fn with_merges(sheet: &Sheet, r: RangeRef) -> RangeRef {
+    if r.is_full_cols() || r.is_full_rows() {
+        return r;
+    }
+    let mut cur = r;
+    loop {
+        let mut grown = cur;
+        for m in &sheet.merges {
+            if m.intersects(&grown) {
+                grown = grown.union(m);
+            }
+        }
+        if grown == cur {
+            return cur;
+        }
+        cur = grown;
+    }
+}
+
+/// One Shift+arrow step of [`Selection::extend_by`] along one axis (`cols`: columns, else rows).
+fn step_edge(sheet: &Sheet, r: RangeRef, anchor: RangeRef, d: i64, cols: bool) -> RangeRef {
+    let max = i64::from(if cols { MAX_COLS } else { MAX_ROWS }) - 1;
+    let hidden = |i: i64| if cols { sheet.is_col_hidden(i as u32) } else { sheet.is_row_hidden(i as u32) };
+    // `from` moved by `by`, then on past hidden lines in the same direction (not off the sheet).
+    let land = |from: i64, by: i64| {
+        let mut t = from.saturating_add(by).clamp(0, max);
+        // Bounded like the arrow keys' walk over hidden lines.
+        for _ in 0..100_000 {
+            if !hidden(t) {
+                break;
+            }
+            let n = (t + by.signum()).clamp(0, max);
+            if n == t {
+                break;
+            }
+            t = n;
+        }
+        t
+    };
+    let span = |x: &RangeRef| {
+        if cols { (i64::from(x.start.col), i64::from(x.end.col)) } else { (i64::from(x.start.row), i64::from(x.end.row)) }
+    };
+    let with = |lo: i64, hi: i64| {
+        let (lo, hi) = (lo.clamp(0, max) as u32, hi.clamp(0, max) as u32);
+        let (s, e) = if cols {
+            (CellRef::new(r.start.row, lo), CellRef::new(r.end.row, hi))
+        } else {
+            (CellRef::new(lo, r.start.col), CellRef::new(hi, r.end.col))
+        };
+        with_merges(sheet, RangeRef::new(s, e))
+    };
+    let (lo, hi) = span(&r);
+    let (alo, ahi) = span(&anchor);
+    // The anchor holds the low edge unless the area reaches past it on that side.
+    let moving_hi = lo >= alo;
+    let shrinking = if moving_hi { d < 0 && hi > ahi } else { d > 0 && lo < alo };
+    if shrinking {
+        let mut t = if moving_hi { hi } else { lo };
+        let mut by = d;
+        loop {
+            t = if moving_hi { land(t, by).max(ahi) } else { land(t, by).min(alo) };
+            let cand = if moving_hi { with(lo, t) } else { with(t, hi) };
+            if cand != r {
+                return cand;
+            }
+            if t == if moving_hi { ahi } else { alo } {
+                break;
+            }
+            // The step was undone because a merged cell crosses the new edge, and so is every
+            // edge up to where that merged cell starts (ends, for the low edge): go on from there
+            // so a tall or wide merged cell costs one step, not one per line it covers.
+            let across = sheet.merges.iter().filter(|m| m.intersects(&r)).filter_map(|m| {
+                let (ms, me) = span(m);
+                if moving_hi { (ms <= t && me > t).then_some(ms) } else { (ms < t && me >= t).then_some(me) }
+            });
+            t = if moving_hi { across.min().map_or(t, |ms| ms.min(t)) } else { across.max().map_or(t, |me| me.max(t)) };
+            by = d.signum();
+        }
+    }
+    // Grow the area on the side the arrow points to.
+    if d > 0 { with(lo, land(hi, d)) } else { with(land(lo, d), hi) }
+}
+
 impl Selection {
     /// Ctrl+. (Next Corner of Selection): the active cell moves clockwise to the next corner of
     /// the current area (top-left, top-right, bottom-right, bottom-left) and becomes the anchor,
@@ -287,6 +381,36 @@ mod tests {
         assert_eq!(current_region(&s, CellRef::new(5, 0)).a1(), "A1:B20000");
     }
 
+    #[test]
+    fn shift_arrows_from_a_merged_cell() {
+        let mut s = Sheet::new("S");
+        s.merges.push(RangeRef::parse("B2:C3").unwrap());
+        let at = |s: &Sheet, a1: &str| {
+            let mut sel = Selection::at(CellRef::parse(a1).unwrap());
+            sel.expand_merges(s);
+            sel
+        };
+        for ((dr, dc), want) in [((0, -1), "A2:C3"), ((-1, 0), "B1:C3"), ((0, 1), "B2:D3"), ((1, 0), "B2:C4")] {
+            let mut sel = at(&s, "B2");
+            sel.extend_by(&s, dr, dc);
+            assert_eq!(sel.a1(), want);
+        }
+        // Shrinking onto the inside of the merged cell goes on past it.
+        let mut sel = at(&s, "A1");
+        sel.extend_by(&s, 1, 1);
+        assert_eq!(sel.a1(), "A1:C3");
+        sel.extend_by(&s, 0, -1);
+        assert_eq!(sel.a1(), "A1:A3");
+        // A hidden column is stepped over, and the sheet edge stops the area.
+        s.cols.entry(3).or_default().hidden = true;
+        let mut sel = at(&s, "B2");
+        sel.extend_by(&s, 0, 1);
+        assert_eq!(sel.a1(), "B2:E3");
+        let mut sel = at(&s, "A1");
+        sel.extend_by(&s, -1, -1);
+        assert_eq!(sel.a1(), "A1");
+    }
+
     #[test]
     fn cycling() {
         let mut sel = Selection::range(RangeRef::parse("A1:B2").unwrap());

crates/engine/tests/it/main.rs

@@ -18,6 +18,7 @@ mod hidden_sheet_navigation;
 mod ink_roundtrip;
 mod line_count_overflow;
 mod long_operator_chains;
+mod merged_selection_keys;
 mod offset_overflow;
 mod percent_entry;
 mod pivot_unknown_func;

crates/engine/tests/it/merged_selection_keys.rs

@@ -0,0 +1,162 @@
+//! Shift+arrows and Enter/Tab on merged cells, as Excel does them.
+
+use gridcraft_engine::Session;
+use serde_json::json;
+
+/// A new workbook with B2:C3 merged and nothing selected but A1.
+fn merged_book() -> Session {
+    let mut s = Session::new();
+    s.new_workbook();
+    s.execute("selection.set", json!({"range": "B2:C3"})).unwrap();
+    s.execute("home.mergeCells", json!({})).unwrap();
+    s.execute("selection.set", json!({"cell": "A1"})).unwrap();
+    s
+}
+
+fn shift(s: &mut Session, dr: i64, dc: i64) -> String {
+    s.execute("selection.move", json!({"dr": dr, "dc": dc, "extend": true})).unwrap();
+    s.doc().unwrap().selection.a1()
+}
+
+fn enter(s: &mut Session, forward: bool, by_row: bool) -> (String, String) {
+    s.execute("selection.next", json!({"forward": forward, "byRow": by_row})).unwrap();
+    let sel = &s.doc().unwrap().selection;
+    (sel.active.a1(), sel.a1())
+}
+
+#[test]
+fn shift_arrows_from_a_merged_cell_widen_past_each_edge() {
+    let mut s = merged_book();
+    for ((dr, dc), want) in [((0, -1), "A2:C3"), ((-1, 0), "B1:C3"), ((0, 1), "B2:D3"), ((1, 0), "B2:C4")] {
+        s.execute("selection.set", json!({"cell": "B2"})).unwrap();
+        assert_eq!(s.doc().unwrap().selection.a1(), "B2:C3");
+        assert_eq!(shift(&mut s, dr, dc), want, "Shift+({dr},{dc}) from the merged cell B2:C3");
+        assert_eq!(s.doc().unwrap().selection.active.a1(), "B2", "the active cell stays");
+    }
+}
+
+#[test]
+fn shift_arrows_keep_going_and_come_back_to_the_merged_cell() {
+    let mut s = merged_book();
+    s.execute("selection.set", json!({"cell": "B2"})).unwrap();
+    assert_eq!(shift(&mut s, 1, 0), "B2:C4");
+    assert_eq!(shift(&mut s, 1, 0), "B2:C5");
+    assert_eq!(shift(&mut s, 1, 0), "B2:C6");
+    assert_eq!(shift(&mut s, -1, 0), "B2:C5");
+    s.execute("selection.set", json!({"cell": "B2"})).unwrap();
+    assert_eq!(shift(&mut s, 0, -1), "A2:C3");
+    assert_eq!(shift(&mut s, 0, 1), "B2:C3", "back to just the merged cell");
+    assert_eq!(shift(&mut s, 0, 1), "B2:D3");
+    assert_eq!(shift(&mut s, -1, 0), "B1:D3");
+    assert_eq!(shift(&mut s, 1, 0), "B2:D3");
+}
+
+#[test]
+fn shift_arrows_shrink_past_a_merged_cell_they_pulled_in() {
+    let mut s = merged_book();
+    assert_eq!(shift(&mut s, 0, 1), "A1:B1");
+    assert_eq!(shift(&mut s, 1, 0), "A1:C3", "B2 pulls in the whole merged cell");
+    assert_eq!(shift(&mut s, 0, -1), "A1:A3", "one step back leaves the merged cell out");
+    assert_eq!(shift(&mut s, 0, 1), "A1:C3");
+    assert_eq!(shift(&mut s, -1, 0), "A1:C1");
+    assert_eq!(shift(&mut s, 1, 0), "A1:C3");
+    assert_eq!(shift(&mut s, 1, 0), "A1:C4");
+}
+
+#[test]
+fn enter_and_tab_leave_a_selected_merged_cell() {
+    let mut s = merged_book();
+    for (forward, by_row, want) in [(true, false, "B4"), (false, false, "B1"), (true, true, "D2"), (false, true, "A2")] {
+        s.execute("selection.set", json!({"cell": "B2"})).unwrap();
+        assert_eq!(enter(&mut s, forward, by_row), (want.to_string(), want.to_string()), "forward {forward} byRow {by_row}");
+    }
+}
+
+#[test]
+fn enter_and_tab_out_of_a_merged_cell_step_over_hidden_lines() {
+    let mut s = merged_book();
+    s.execute("selection.set", json!({"range": "A4"})).unwrap();
+    s.execute("home.hideRows", json!({})).unwrap();
+    s.execute("selection.set", json!({"range": "D1"})).unwrap();
+    s.execute("home.hideColumns", json!({})).unwrap();
+    s.execute("selection.set", json!({"cell": "B2"})).unwrap();
+    assert_eq!(enter(&mut s, true, false).0, "B5", "row 4 is hidden");
+    s.execute("selection.set", json!({"cell": "B2"})).unwrap();
+    assert_eq!(enter(&mut s, true, true).0, "E2", "column D is hidden");
+    s.execute("selection.set", json!({"cell": "B2"})).unwrap();
+    s.execute("selection.move", json!({"dr": 1})).unwrap();
+    assert_eq!(s.doc().unwrap().selection.a1(), "B5", "the Down arrow too");
+}
+
+#[test]
+fn enter_inside_a_selection_visits_a_merged_cell_once() {
+    let mut s = merged_book();
+    s.execute("selection.set", json!({"range": "A1:C3", "active": "A1"})).unwrap();
+    let seen: Vec<String> = (0..6).map(|_| enter(&mut s, true, false).0).collect();
+    assert_eq!(seen, ["A2", "A3", "B1", "B2", "C1", "A1"]);
+    assert_eq!(s.doc().unwrap().selection.a1(), "A1:C3", "the selection stays");
+    let back: Vec<String> = (0..2).map(|_| enter(&mut s, false, false).0).collect();
+    assert_eq!(back, ["C1", "B2"]);
+}
+
+#[test]
+fn shift_arrow_that_cannot_shrink_the_area_grows_it() {
+    // A2:B3 merged, anchor B1: Shift+Down pulls the area out to column A, past the anchor.
+    // Shift+Right can't shrink it from the left (the merged cell would come back), so it grows.
+    let mut s = Session::new();
+    s.new_workbook();
+    s.execute("selection.set", json!({"range": "A2:B3"})).unwrap();
+    s.execute("home.mergeCells", json!({})).unwrap();
+    s.execute("selection.set", json!({"cell": "B1"})).unwrap();
+    assert_eq!(shift(&mut s, 1, 0), "A1:B3");
+    assert_eq!(shift(&mut s, 0, 1), "A1:C3");
+    assert_eq!(s.doc().unwrap().selection.active.a1(), "B1");
+}
+
+#[test]
+fn shift_left_shrinks_past_a_chain_of_merged_cells_in_one_step() {
+    // B2:C3 and D3:E4 are both inside A1:E4. Shift+Left from E can't stop in column D (D3:E4
+    // would come back) and stops in C, where B2:C3 is whole.
+    let mut s = merged_book();
+    s.execute("selection.set", json!({"range": "D3:E4"})).unwrap();
+    s.execute("home.mergeCells", json!({})).unwrap();
+    s.execute("selection.set", json!({"cell": "A1"})).unwrap();
+    assert_eq!(shift(&mut s, 3, 4), "A1:E4");
+    assert_eq!(shift(&mut s, 0, -1), "A1:C4");
+    assert_eq!(shift(&mut s, -1, 0), "A1:C3", "row 4 holds nothing merged once column D is out");
+    assert_eq!(shift(&mut s, 0, -1), "A1:A3", "C can't stop inside B2:C3");
+}
+
+#[test]
+fn shift_up_over_a_tall_merged_cell_takes_one_step() {
+    let mut s = Session::new();
+    s.new_workbook();
+    s.execute("selection.set", json!({"range": "B2:C500000"})).unwrap();
+    s.execute("home.mergeCells", json!({})).unwrap();
+    s.execute("selection.set", json!({"cell": "A1"})).unwrap();
+    assert_eq!(shift(&mut s, 0, 1), "A1:B1");
+    assert_eq!(shift(&mut s, 1, 0), "A1:C500000");
+    assert_eq!(shift(&mut s, -1, 0), "A1:C1", "row 1 is the last row above the merged cell");
+    assert_eq!(shift(&mut s, 1, 0), "A1:C500000");
+    assert_eq!(shift(&mut s, 0, -1), "A1:A500000");
+}
+
+#[test]
+fn shift_arrows_change_only_the_last_area() {
+    let mut s = merged_book();
+    s.execute("selection.set", json!({"range": "E5:F6,B2"})).unwrap();
+    assert_eq!(s.doc().unwrap().selection.a1(), "E5:F6,B2:C3");
+    assert_eq!(shift(&mut s, 0, -1), "E5:F6,A2:C3");
+}
+
+#[test]
+fn enter_skips_a_merged_cell_whose_start_is_outside_the_selection() {
+    // A whole column isn't widened to the merged cells it touches. B1 is covered by A1:B1, whose
+    // top-left cell is outside column B, so Enter skips it.
+    let mut s = Session::new();
+    s.new_workbook();
+    s.execute("selection.set", json!({"range": "A1:B1"})).unwrap();
+    s.execute("home.mergeCells", json!({})).unwrap();
+    s.execute("selection.set", json!({"range": "B:B", "active": "B2"})).unwrap();
+    assert_eq!(enter(&mut s, false, false).0, "B1048576", "B1 is skipped, going up wraps to the bottom");
+}

crates/ui-egui/src/app/editing.rs

@@ -404,7 +404,11 @@ impl SheetApp {
 
     /// Enter/Tab move inside a multi-cell selection, otherwise to the next cell.
     pub fn move_after_enter(&mut self, dr: i64, dc: i64) {
-        let multi = self.session.active().is_some_and(|d| !d.selection.is_single_cell());
+        // A selection that is just one merged cell moves like one cell (Enter leaves it).
+        let multi = self
+            .session
+            .active()
+            .is_some_and(|d| !d.selection.is_single_cell() && !d.wb.active().is_some_and(|sh| d.selection.is_one_merged_cell(sh)));
         // Any other move (arrows, clicks, another sheet) leaves the selection off the cell the
         // last Tab landed on, which ends the run.
         let here = self.session.active().map(|d| (d.uid, d.wb.active_sheet(), d.selection.active));
@@ -503,3 +507,35 @@ mod protected_cell_tests {
         assert!(app.run("file.saveBytes", json!({"format": "xlsx"})).is_ok());
     }
 }
+
+#[cfg(test)]
+mod merged_cell_enter_tests {
+    use super::*;
+    use gridcraft_engine::Session;
+
+    #[test]
+    fn enter_and_tab_leave_a_selected_merged_cell_and_keep_the_tab_run() {
+        let mut session = Session::new();
+        session.new_workbook();
+        let mut app = SheetApp::new(session, Default::default());
+        app.run("selection.set", json!({"range": "B2:C3"})).unwrap();
+        app.run("home.mergeCells", json!({})).unwrap();
+        let sel = |app: &SheetApp| app.session.active().map(|d| d.selection.a1()).unwrap_or_default();
+        for ((dr, dc), want) in [((1, 0), "B4"), ((-1, 0), "B1"), ((0, 1), "D2"), ((0, -1), "A2")] {
+            app.run("selection.set", json!({"cell": "B2"})).unwrap();
+            assert_eq!(sel(&app), "B2:C3");
+            app.move_after_enter(dr, dc);
+            assert_eq!(sel(&app), want, "Enter/Tab ({dr},{dc}) from the merged cell B2:C3");
+        }
+        // A Tab run through the merged cell goes on past it: Enter then goes back to the column
+        // the run started in, on the next row.
+        app.tab_run = None; // the Shift+Tab above ended on A2 and started a run there
+        app.run("selection.set", json!({"cell": "A2"})).unwrap();
+        app.move_after_enter(0, 1);
+        assert_eq!(sel(&app), "B2:C3");
+        app.move_after_enter(0, 1);
+        assert_eq!(sel(&app), "D2");
+        app.move_after_enter(1, 0);
+        assert_eq!(sel(&app), "A3");
+    }
+}