From f4f2f0bb08bd3fa5426bc77f2e8af1f0124796a1 Mon Sep 17 00:00:00 2001 From: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Date: Wed, 14 Oct 2026 06:20:41 +1000 Subject: [PATCH] FIX-TABLE-SELECTION: resolve selected merged-cell owners Co-Authored-By: Codex --- crates/doc/src/table.rs | 48 +++++ crates/engine/src/cmd/table.rs | 129 ++++++------- crates/engine/src/cmd/text.rs | 11 +- crates/engine/src/tests_table.rs | 315 +++++++++++++++++++++++++++++++ crates/ui-egui/src/canvas.rs | 76 +++++++- 5 files changed, 503 insertions(+), 76 deletions(-) diff --git a/crates/doc/src/table.rs b/crates/doc/src/table.rs index 9449be93..087412bc 100644 --- a/crates/doc/src/table.rs +++ b/crates/doc/src/table.rs @@ -444,6 +444,25 @@ impl Table { v } + /// Owning cells touched by the range, once each in row-major order. Bounds are clipped + /// to the grid; resolving covered positions does not expand the selection rectangle. + pub fn selected_owners(&self, range: CellRange) -> Vec<(usize, usize)> { + let (nr, nc) = (self.nrows(), self.ncols()); + if range.r0 >= nr || range.c0 >= nc || range.r0 > range.r1 || range.c0 > range.c1 { + return Vec::new(); + } + let owners = self.owners(); + let mut selected = std::collections::BTreeSet::new(); + for r in range.r0..=range.r1.min(nr - 1) { + for c in range.c0..=range.c1.min(nc - 1) { + if let Some(&owner) = owners.get(r * nc + c) { + selected.insert(owner); + } + } + } + selected.into_iter().collect() + } + /// The owning (top-left) cell address of grid position (r, c). pub fn owner(&self, r: usize, c: usize) -> (usize, usize) { self.owners().get(r * self.ncols() + c).copied().unwrap_or((r, c)) @@ -1136,6 +1155,35 @@ mod tests { Story::with_text(StoryId(1), t, ParaFormat::default()) } + #[test] + fn selected_owners_include_covered_cells_without_expanding_range() { + for column in [true, false] { + let mut t = Table::new(1, 2, 2, 0, 0, 120.0); + let range = if column { CellRange::new(0, 1, 1, 1) } else { CellRange::new(1, 0, 1, 1) }; + assert_eq!(t.selected_owners(range), if column { vec![(0, 1), (1, 1)] } else { vec![(1, 0), (1, 1)] }); + t.merge(if column { CellRange::new(0, 0, 0, 1) } else { CellRange::new(0, 0, 1, 0) }).unwrap(); + assert_eq!(t.selected_owners(range), vec![(0, 0), (1, 1)]); + } + } + + #[test] + fn selected_owners_are_bounded_sorted_and_unique() { + let mut t = Table::new(1, 3, 3, 0, 0, 180.0); + t.merge(CellRange::new(0, 0, 1, 1)).unwrap(); + assert_eq!(t.selected_owners(CellRange::new(1, 1, usize::MAX, usize::MAX)), vec![(0, 0), (1, 2), (2, 1), (2, 2)]); + assert_eq!(t.selected_owners(CellRange::new(0, 0, 1, 1)), vec![(0, 0)], "covered positions do not duplicate an owner"); + assert!(t.selected_owners(CellRange::new(3, 0, usize::MAX, usize::MAX)).is_empty()); + assert!(t.selected_owners(CellRange::new(0, 3, usize::MAX, usize::MAX)).is_empty()); + assert!(t.selected_owners(CellRange { r0: 1, c0: 0, r1: 0, c1: 1 }).is_empty()); + t.rows.clear(); + t.cells.clear(); + assert!(t.selected_owners(CellRange::cell(0, 0)).is_empty()); + + let mut right = Table::new(2, 2, 2, 0, 0, 120.0); + right.merge(CellRange::new(0, 1, 1, 1)).unwrap(); + assert_eq!(right.selected_owners(CellRange::new(1, 0, 1, 1)), vec![(0, 1), (1, 0)], "owner order, not encounter order"); + } + #[test] fn insert_table_gets_own_paragraph() { let mut s = story("before after"); diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index ad306be8..c3df2442 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -7,6 +7,7 @@ //! whole table). use designcraft_compose as compose; +use designcraft_doc::story::floor_char_boundary; use designcraft_doc::{ AltFills, CellAddr, CellRange, CellStroke, Document, RowHeightMode, Selection, StoryId, StrokeType, Table, TableSel, TextSel, VerticalJustification, @@ -140,11 +141,9 @@ fn table_specs() -> Vec { |s, p| { let g = target(s, p, "table.textCell")?; edit_table(s, &g, "table.textCell", |t| { - for r in g.range.r0..=g.range.r1 { - for c in g.range.c0..=g.range.c1 { - if let Some(cell) = t.cell_mut(r, c) { - cell.graphic = None; - } + for (r, c) in t.selected_owners(g.range) { + if let Some(cell) = t.cell_mut(r, c) { + cell.graphic = None; } } ok() @@ -351,13 +350,17 @@ fn clamp_selection(d: &Document, sel: &mut Selection) { match story.and_then(|x| x.tables.get(&c.table)) { Some(tb) => { let (r, col) = tb.owner(c.row.min(tb.nrows() - 1), c.col.min(tb.ncols() - 1)); - let len = tb.cell(r, col).map_or(0, |x| x.text.len()); - sel.text = - Some(TextSel { anchor: t.anchor.min(len), focus: t.focus.min(len), cell: Some(CellAddr { table: c.table, row: r, col }), ..t }); + let text = tb.cell(r, col).map_or("", |x| x.text.text.as_str()); + sel.text = Some(TextSel { + anchor: floor_char_boundary(text, t.anchor), + focus: floor_char_boundary(text, t.focus), + cell: Some(CellAddr { table: c.table, row: r, col }), + ..t + }); } None => { - let len = story.map_or(0, |x| x.len()); - sel.text = Some(TextSel { anchor: t.anchor.min(len), focus: t.anchor.min(len), cell: None, ..t }); + let pos = story.map_or(0, |x| floor_char_boundary(&x.text, t.anchor)); + sel.text = Some(TextSel { anchor: pos, focus: pos, cell: None, ..t }); } } } @@ -648,7 +651,6 @@ fn set_cell(s: &mut Session, p: &Value) -> Result { let text = str_param(p, "text").map(|_| super::text_param(p, "text")); let stroke = p.get("stroke").cloned(); edit_table(s, &g, "table.setCell", |t| { - let owners = t.owners(); let nc = t.ncols(); let nr = t.nrows(); let rtl = t.options.direction == designcraft_doc::TextDirection::RightToLeft; @@ -656,50 +658,45 @@ fn set_cell(s: &mut Session, p: &Value) -> Result { // Local edits outrank previously imported edges, including explicit zero/None. let priority = if stroke.is_some() { next_cell_stroke_priority(t) } else { 0 }; let rg = g.range; - for r in rg.r0..=rg.r1 { - for c in rg.c0..=rg.c1 { - if owners[r * nc + c] != (r, c) { - continue; - } - let Some(cell) = t.cell_mut(r, c) else { continue }; - if let Some(f) = &fill { - cell.fill = f.clone(); - } - if let Some(ti) = tint { - cell.fill_tint = ti; - } - if let Some(i) = insets { - cell.insets = i; - } - if let Some(v) = vj { - cell.vj = v; - } - if let Some(tx) = &text { - let len = cell.text.len(); - cell.text.replace(0..len, tx); - } - if let Some(sv) = &stroke { - let edges = str_param(sv, "edges").unwrap_or("all"); - let at_bottom = r.saturating_add(cell.row_span.max(1) as usize); - let at_right = c.saturating_add(cell.col_span.max(1) as usize); - let perimeter = [r == 0, if rtl { at_right == nc } else { c == 0 }, at_bottom == nr, if rtl { c == 0 } else { at_right == nc }]; - for (i, on) in [ - ("top", r == rg.r0), - ("left", if rtl { at_right.saturating_sub(1) == rg.c1 } else { c == rg.c0 }), - ("bottom", at_bottom.saturating_sub(1) == rg.r1), - ("right", if rtl { c == rg.c0 } else { at_right.saturating_sub(1) == rg.c1 }), - ] - .iter() - .enumerate() - .map(|(i, (name, outer))| (i, edges == "all" || edges == *name || (edges == "outer" && *outer) || (edges == "inner" && !*outer))) - { - if on { - let base = if perimeter[i] && !cell.border_overrides[i] { &borders[i] } else { &cell.strokes[i] }; - cell.strokes[i] = parse_stroke(sv, base.clone()); - cell.stroke_defined[i] = true; - cell.stroke_priorities[i] = priority; - cell.border_overrides[i] = true; - } + for (r, c) in t.selected_owners(rg) { + let Some(cell) = t.cell_mut(r, c) else { continue }; + if let Some(f) = &fill { + cell.fill = f.clone(); + } + if let Some(ti) = tint { + cell.fill_tint = ti; + } + if let Some(i) = insets { + cell.insets = i; + } + if let Some(v) = vj { + cell.vj = v; + } + if let Some(tx) = &text { + let len = cell.text.len(); + cell.text.replace(0..len, tx); + } + if let Some(sv) = &stroke { + let edges = str_param(sv, "edges").unwrap_or("all"); + let at_bottom = r.saturating_add(cell.row_span.max(1) as usize); + let at_right = c.saturating_add(cell.col_span.max(1) as usize); + let perimeter = [r == 0, if rtl { at_right == nc } else { c == 0 }, at_bottom == nr, if rtl { c == 0 } else { at_right == nc }]; + for (i, on) in [ + ("top", r == rg.r0), + ("left", if rtl { at_right.saturating_sub(1) == rg.c1 } else { c == rg.c0 }), + ("bottom", at_bottom.saturating_sub(1) == rg.r1), + ("right", if rtl { c == rg.c0 } else { at_right.saturating_sub(1) == rg.c1 }), + ] + .iter() + .enumerate() + .map(|(i, (name, outer))| (i, edges == "all" || edges == *name || (edges == "outer" && *outer) || (edges == "inner" && !*outer))) + { + if on { + let base = if perimeter[i] && !cell.border_overrides[i] { &borders[i] } else { &cell.strokes[i] }; + cell.strokes[i] = parse_stroke(sv, base.clone()); + cell.stroke_defined[i] = true; + cell.stroke_priorities[i] = priority; + cell.border_overrides[i] = true; } } } @@ -707,6 +704,7 @@ fn set_cell(s: &mut Session, p: &Value) -> Result { // Keep both copies of unmerged shared boundaries consistent. Left/right name // physical sides, also in RTL tables. if let Some(sv) = &stroke { + let owners = t.owners(); let edges = str_param(sv, "edges").unwrap_or("all"); let selected = |side: &str| edges == "all" || edges == "outer" || edges == side; if selected("top") @@ -1095,7 +1093,10 @@ fn cell_style_create(s: &mut Session, p: &Value) -> Result { let cell = d .story(g.story) .and_then(|st| st.tables.get(&g.table)) - .and_then(|t| t.cell(g.range.r0, g.range.c0)) + .and_then(|t| { + let (r, c) = t.owner(g.range.r0, g.range.c0); + t.cell(r, c) + }) .ok_or_else(|| bad("style.cell.create", "no cell"))?; cs.fill = Some(cell.fill.clone()); cs.fill_tint = Some(cell.fill_tint); @@ -1122,17 +1123,11 @@ fn cell_style_apply(s: &mut Session, p: &Value) -> Result { let cs = designcraft_doc::resolve_cell_style(styles, &name); let g = target(s, p, "style.cell.apply")?; edit_table(s, &g, "style.cell.apply", |t| { - let owners = t.owners(); - let nc = t.ncols(); let mut n = 0; - for r in g.range.r0..=g.range.r1 { - for c in g.range.c0..=g.range.c1 { - if owners[r * nc + c] == (r, c) - && let Some(cell) = t.cell_mut(r, c) - { - cs.apply_to(cell); - n += 1; - } + for (r, c) in t.selected_owners(g.range) { + if let Some(cell) = t.cell_mut(r, c) { + cs.apply_to(cell); + n += 1; } } Ok(json!({"cells": n})) @@ -1353,7 +1348,7 @@ fn drop_cells(s: &mut Session, p: &Value) -> Result { && ts.story == sid && ts.table == tid && tid2 == tid - && ts.range.contains(fr, fc) + && s.doc()?.doc.story(sid).and_then(|st| st.tables.get(&tid)).is_some_and(|t| t.selected_owners(ts.range).binary_search(&(fr, fc)).is_ok()) { let (nr, nc) = { let t = s.doc()?.doc.story(sid).and_then(|st| st.tables.get(&tid)).ok_or_else(|| bad(ID, "no table"))?; diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index b57e9e88..a53e6e8d 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -823,14 +823,9 @@ pub(crate) fn format_targets(s: &Session) -> Vec { && let Some(t) = st.doc.story(ts.story).and_then(|x| x.tables.get(&ts.table)) { let mut v = Vec::new(); - let owners = t.owners(); - for r in ts.range.r0..=ts.range.r1.min(t.nrows().saturating_sub(1)) { - for c in ts.range.c0..=ts.range.c1.min(t.ncols().saturating_sub(1)) { - if owners[r * t.ncols() + c] == (r, c) { - let len = t.cell(r, c).map_or(0, |x| x.text.len()); - v.push(Target { story: ts.story, cell: Some(CellAddr { table: ts.table, row: r, col: c }), range: 0..len }); - } - } + for (r, c) in t.selected_owners(ts.range) { + let len = t.cell(r, c).map_or(0, |x| x.text.len()); + v.push(Target { story: ts.story, cell: Some(CellAddr { table: ts.table, row: r, col: c }), range: 0..len }); } return v; } diff --git a/crates/engine/src/tests_table.rs b/crates/engine/src/tests_table.rs index 3934aabe..c3a575d0 100644 --- a/crates/engine/src/tests_table.rs +++ b/crates/engine/src/tests_table.rs @@ -16,6 +16,321 @@ fn check(s: &Session) { s.doc().unwrap().doc.check().unwrap(); } +#[test] +fn table_text_replacement_floors_utf8_selection_and_allows_quote_typing() { + for (original, (a, b), replacement, (after_a, after_b), quoted) in [ + ("a", (1, 1), "é", (0, 0), Some("“é")), // Exact caret/quote baseline. + ("abc", (3, 3), "🙂", (0, 0), Some("“🙂")), + ("ab", (1, 2), "éX", (0, 2), None), + ("ab", (2, 1), "éX", (2, 0), None), + ("abcdef", (3, 5), "🙂éX", (0, 4), None), + ("abcdef", (5, 3), "🙂éX", (4, 0), None), + ("abc", (1, 2), "🙂", (0, 0), None), // Both endpoints floor to the same boundary. + ("abcdef", (2, 6), "é🙂", (2, 6), None), // Valid Unicode endpoints stay fixed. + ("abcdef", (1, 4), "ABCDEF", (1, 4), None), + ("abcdef", (4, 1), "ABCDEF", (4, 1), None), + ("abcdef", (6, 6), "é", (2, 2), None), // Shortening clamps to EOF. + ("abc", (1, 3), "", (0, 0), None), + ] { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let frame = s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text", "text": ""})).unwrap(); + let sid = frame["story"].as_u64().unwrap(); + let fid = frame["id"].as_u64().unwrap(); + let tid = s.execute("table.insert", &json!({"rows": 1, "cols": 1})).unwrap()["table"].as_u64().unwrap(); + s.execute("text.insert", &json!({"text": original, "raw": true})).unwrap(); + // Real commands position the child selection; text.select addresses parent stories. + for _ in a..original.len() { + s.execute("text.move", &json!({"dir": "left"})).unwrap(); + } + let (dir, steps) = if b >= a { ("right", b - a) } else { ("left", a - b) }; + for _ in 0..steps { + s.execute("text.move", &json!({"dir": dir, "extend": true})).unwrap(); + } + let mut expected = s.doc().unwrap().selection.clone(); + let selected = expected.text.as_mut().unwrap(); + assert_eq!((selected.anchor, selected.focus), (a, b)); + assert_eq!(selected.frame, Some(doc::ItemId(fid))); + assert_eq!(selected.cell, Some(doc::CellAddr { table: tid, row: 0, col: 0 })); + (selected.anchor, selected.focus) = (after_a, after_b); + let parent = s.doc().unwrap().doc.story(doc::StoryId(sid)).unwrap().text.clone(); + s.execute("table.setCell", &json!({"text": replacement})).unwrap(); + assert_eq!(table_for(&s, sid, tid).cell(0, 0).unwrap().text.text, replacement); + assert_eq!(s.doc().unwrap().selection, expected, "{original}: {a}->{b}, replacement {replacement}"); + assert!(replacement.is_char_boundary(after_a) && replacement.is_char_boundary(after_b)); + assert_eq!(s.doc().unwrap().doc.story(doc::StoryId(sid)).unwrap().text, parent); + if let Some(quoted) = quoted { + assert!(s.prefs.typographers_quotes); + s.execute("text.insert", &json!({"text": "\""})).unwrap(); + assert_eq!(table_for(&s, sid, tid).cell(0, 0).unwrap().text.text, quoted); + let selected = expected.text.as_mut().unwrap(); + (selected.anchor, selected.focus) = (3, 3); // Opening curly quote is three bytes. + assert_eq!(s.doc().unwrap().selection, expected); + } + check(&s); + } +} + +#[test] +fn table_edit_after_anchor_removal_floors_parent_caret() { + for (original, (a, b), replacement, pos, quoted, typed_pos) in [("ab", (1, 2), "é", 0, "“é\n", 3), ("abcde", (5, 1), "é🙂", 2, "é”🙂\n", 5)] + { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let other = s.execute("frame.create", &json!({"rect": [320, 36, 580, 200], "content": "text"})).unwrap(); + let other_sid = other["story"].as_u64().unwrap(); + let other_tid = s.execute("table.insert", &json!({"rows": 1, "cols": 1})).unwrap()["table"].as_u64().unwrap(); + s.execute("text.insert", &json!({"text": "keep", "raw": true})).unwrap(); + let mut expected_other = table_for(&s, other_sid, other_tid).clone(); + expected_other.cell_mut(0, 0).unwrap().insets = [7.0; 4]; + + let frame = s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text"})).unwrap(); + let sid = frame["story"].as_u64().unwrap(); + let tid = s.execute("table.insert", &json!({"rows": 1, "cols": 1})).unwrap()["table"].as_u64().unwrap(); + s.execute("text.insert", &json!({"text": original, "raw": true})).unwrap(); + for _ in a..original.len() { + s.execute("text.move", &json!({"dir": "left"})).unwrap(); + } + let (dir, steps) = if b >= a { ("right", b - a) } else { ("left", a - b) }; + for _ in 0..steps { + s.execute("text.move", &json!({"dir": dir, "extend": true})).unwrap(); + } + let mut expected = s.doc().unwrap().selection.clone(); + let selected = expected.text.unwrap(); + assert_eq!((selected.anchor, selected.focus), (a, b)); + assert_eq!(selected.frame, Some(doc::ItemId(frame["id"].as_u64().unwrap()))); + assert_eq!(selected.cell, Some(doc::CellAddr { table: tid, row: 0, col: 0 })); + // Explicit parent replacement removes its anchor, leaving the child address stale. + assert_eq!( + s.execute("find.change", &json!({"story": sid, "find": doc::TABLE_ANCHOR.to_string(), "change": replacement})).unwrap()["count"], + 1 + ); + let parent = s.doc().unwrap().doc.story(doc::StoryId(sid)).unwrap(); + assert_eq!(parent.text, format!("{replacement}\n")); + assert!(!parent.tables.contains_key(&tid)); + assert_eq!(s.doc().unwrap().selection, expected); + assert!(!parent.text.is_char_boundary(selected.anchor)); + // An explicit edit to the surviving table reaches the disappeared-table fallback. + s.execute("table.setCell", &json!({"story": other_sid, "table": other_tid, "insets": 7})).unwrap(); + let selected = expected.text.as_mut().unwrap(); + (selected.anchor, selected.focus, selected.cell) = (pos, pos, None); // Both floor the OLD anchor. + assert_eq!(s.doc().unwrap().selection, expected); + assert_eq!(s.doc().unwrap().doc.story(doc::StoryId(sid)).unwrap().text, format!("{replacement}\n")); + assert_eq!(table_for(&s, other_sid, other_tid), &expected_other); + s.execute("text.insert", &json!({"text": "\""})).unwrap(); + assert_eq!(s.doc().unwrap().doc.story(doc::StoryId(sid)).unwrap().text, quoted); + let selected = expected.text.as_mut().unwrap(); + (selected.anchor, selected.focus) = (typed_pos, typed_pos); + assert_eq!(s.doc().unwrap().selection, expected); + assert_eq!(table_for(&s, other_sid, other_tid), &expected_other); + check(&s); + } +} + +fn selected_merged_table(column: bool) -> (Session, u64, u64, u64) { + let (mut s, sid, fid) = session_with_frame(); + let tid = s.execute("table.insert", &json!({"rows": 2, "cols": 2})).unwrap()["table"].as_u64().unwrap(); + for (i, text) in ["A", "B", "C", "D"].iter().enumerate() { + s.execute("text.insert", &json!({"text": text})).unwrap(); + if i < 3 { + s.execute("table.nextCell", &json!({})).unwrap(); + } + } + let merge = if column { json!({"rows": [0, 0], "cols": [0, 1]}) } else { json!({"rows": [0, 1], "cols": [0, 0]}) }; + s.execute("table.select", &merge).unwrap(); + s.execute("table.merge", &json!({})).unwrap(); + s.execute("table.select", &json!({"rows": [1, 1], "cols": [1, 1]})).unwrap(); + s.execute(if column { "table.selectColumn" } else { "table.selectRow" }, &json!({})).unwrap(); + (s, sid, tid, fid) +} + +#[test] +fn selected_merged_owners_receive_text_formatting_and_character_styles() { + for column in [true, false] { + let (mut s, sid, tid, _) = selected_merged_table(column); + let selected = s.doc().unwrap().selection.clone(); + assert_eq!(selected.cells.unwrap().range, if column { doc::CellRange::new(0, 1, 1, 1) } else { doc::CellRange::new(1, 0, 1, 1) }); + let before = table_for(&s, sid, tid).clone(); + let neighbor = if column { (1, 0) } else { (0, 1) }; + s.execute("type.char", &json!({"attrs": {"size": 23}})).unwrap(); + for (r, c) in [(0, 0), (1, 1)] { + let cell = table_for(&s, sid, tid).cell(r, c).unwrap(); + assert!(cell.text.runs().all(|(_, f)| f.over.size == Some(23.0)), "all runs of owner {r},{c}"); + assert_eq!(cell.text.text, before.cell(r, c).unwrap().text.text); + } + assert_eq!(table_for(&s, sid, tid).cell(neighbor.0, neighbor.1), before.cell(neighbor.0, neighbor.1)); + assert_eq!(s.doc().unwrap().selection, selected); + s.execute("style.character.create", &json!({"name": "Selected owners", "chars": {"tracking": 37}})).unwrap(); + s.execute("style.character.apply", &json!({"name": "Selected owners"})).unwrap(); + for (r, c) in [(0, 0), (1, 1)] { + assert!(table_for(&s, sid, tid).cell(r, c).unwrap().text.runs().all(|(_, f)| f.style == "Selected owners")); + } + let after = table_for(&s, sid, tid); + assert_eq!(after.cell(neighbor.0, neighbor.1), before.cell(neighbor.0, neighbor.1)); + assert_eq!(after.rows, before.rows); + assert_eq!(after.columns, before.columns); + assert_eq!(s.doc().unwrap().selection, selected, "formatting preserves the original grid rectangle"); + check(&s); + } +} + +#[test] +fn selected_merged_owners_receive_cell_options_and_unique_style_counts() { + for column in [true, false] { + let (mut s, sid, tid, _) = selected_merged_table(column); + let range = s.doc().unwrap().selection.cells.unwrap().range; + let before = table_for(&s, sid, tid).clone(); + let neighbor = if column { (1, 0) } else { (0, 1) }; + let covered = if column { (0, 1) } else { (1, 0) }; + s.execute("table.setCell", &json!({"fill": "[Black]", "insets": 7, "text": "REPLACED"})).unwrap(); + for (r, c) in [(0, 0), (1, 1)] { + let cell = table_for(&s, sid, tid).cell(r, c).unwrap(); + assert_eq!(serde_json::to_value(&cell.fill).unwrap(), json!("[Black]")); + assert_eq!(cell.insets, [7.0; 4]); + assert_eq!(cell.text.text, "REPLACED"); + } + s.execute("style.cell.create", &json!({"name": "Selected cell owners", "insets": 9})).unwrap(); + assert_eq!(s.execute("style.cell.apply", &json!({"name": "Selected cell owners"})).unwrap()["cells"], 2); + let after = table_for(&s, sid, tid); + for (r, c) in [(0, 0), (1, 1)] { + assert_eq!(after.cell(r, c).unwrap().style, "Selected cell owners"); + assert_eq!(after.cell(r, c).unwrap().insets, [9.0; 4]); + } + assert_eq!(after.cell(neighbor.0, neighbor.1), before.cell(neighbor.0, neighbor.1)); + assert_eq!(after.cell(covered.0, covered.1), before.cell(covered.0, covered.1)); + assert_eq!(after.rows, before.rows); + assert_eq!(after.columns, before.columns); + assert_eq!(s.doc().unwrap().selection.cells.unwrap().range, range); + let merged = if column { json!({"rows": [0, 0], "cols": [0, 1]}) } else { json!({"rows": [0, 1], "cols": [0, 0]}) }; + s.execute("table.select", &merged).unwrap(); + assert_eq!(s.execute("style.cell.apply", &json!({"name": "Selected cell owners"})).unwrap()["cells"], 1); + check(&s); + } +} + +#[test] +fn covered_owner_strokes_keep_literal_edge_masks_and_neighbors() { + for rtl in [false, true] { + for outer in [false, true] { + let (mut s, sid, tid, _) = selected_merged_table(true); + if rtl { + s.execute("table.options", &json!({"direction": "rightToLeft"})).unwrap(); + } + s.execute("table.select", &json!({"rows": [0, 0], "cols": [1, 1]})).unwrap(); + let before = table_for(&s, sid, tid).clone(); + s.execute("table.setCell", &json!({"stroke": {"weight": 3, "edges": if outer { "outer" } else { "inner" }}})).unwrap(); + let after = table_for(&s, sid, tid); + let mask = if rtl { [true, true, true, false] } else { [true, false, true, true] }; + let expected = mask.map(|edge| if outer { edge } else { !edge }); + let owner = after.cell(0, 0).unwrap(); + assert_eq!(owner.border_overrides, expected); + assert_eq!(owner.stroke_defined, expected); + for (i, on) in expected.into_iter().enumerate() { + if on { + assert_eq!(owner.strokes[i].weight, 3.0); + assert_eq!(owner.stroke_priorities[i], 1); + } else { + assert_eq!(owner.strokes[i], before.cell(0, 0).unwrap().strokes[i]); + } + } + for (r, c) in [(0, 1), (1, 0), (1, 1)] { + assert_eq!(after.cell(r, c), before.cell(r, c), "strict merged/covered mirror guard preserves {r},{c}"); + } + assert_eq!(s.doc().unwrap().selection.cells.unwrap().range, doc::CellRange::cell(0, 1)); + check(&s); + } + } +} + +#[test] +fn cell_style_sampling_resolves_covered_point_but_retains_upper_left_policy() { + let (mut s, sid, tid, _) = selected_merged_table(true); + s.execute("table.setCell", &json!({"row": 0, "col": 0, "insets": 7})).unwrap(); + s.execute("table.select", &json!({"rows": [0, 0], "cols": [1, 1]})).unwrap(); + let selection = s.doc().unwrap().selection.clone(); + let before = table_for(&s, sid, tid).clone(); + s.execute("style.cell.create", &json!({"name": "Covered", "fromSelection": true})).unwrap(); + assert_eq!(s.doc().unwrap().doc.styles.cell.iter().find(|c| c.name == "Covered").unwrap().insets, Some([7.0; 4])); + assert_eq!(table_for(&s, sid, tid), &before); + assert_eq!(s.doc().unwrap().selection, selection); + + let (mut s, sid, _) = session_with_frame(); + let tid = s.execute("table.insert", &json!({"rows": 2, "cols": 2})).unwrap()["table"].as_u64().unwrap(); + s.execute("table.select", &json!({"rows": [0, 1], "cols": [1, 1]})).unwrap(); + s.execute("table.merge", &json!({})).unwrap(); + for (row, col, insets, size) in [(0, 1, 7, 17), (1, 0, 11, 29)] { + s.execute("table.setCell", &json!({"row": row, "col": col, "text": "Text", "insets": insets})).unwrap(); + s.execute("table.select", &json!({"rows": [row, row], "cols": [col, col]})).unwrap(); + s.execute("type.char", &json!({"attrs": {"size": size}})).unwrap(); + } + s.execute("table.selectRow", &json!({})).unwrap(); + let before = table_for(&s, sid, tid).clone(); + let selection = s.doc().unwrap().selection.clone(); + // Row-major owners are [(0,1),(1,0)], while the rectangle starts at (1,0). + assert_eq!(s.execute("type.selectionAttrs", &json!({})).unwrap()["chars"]["size"], 17.0); + s.execute("style.cell.create", &json!({"name": "Upper left", "fromSelection": true})).unwrap(); + assert_eq!(s.doc().unwrap().doc.styles.cell.iter().find(|c| c.name == "Upper left").unwrap().insets, Some([11.0; 4])); + assert_eq!(table_for(&s, sid, tid), &before); + assert_eq!(s.doc().unwrap().selection, selection); +} + +#[test] +fn selected_merged_owners_convert_graphics_back_to_text() { + let png = designcraft_render::Rendered { width: 1, height: 1, pixels: vec![255, 0, 0, 255] }.to_png(); + let image = crate::cmd::base64_encode(&png); + for column in [true, false] { + let (mut s, sid, tid, _) = selected_merged_table(column); + let neighbor = if column { (1, 0) } else { (0, 1) }; + let covered = if column { (0, 1) } else { (1, 0) }; + for (r, c) in [covered, (1, 1), neighbor] { + let placed = s.execute("table.placeGraphic", &json!({"row": r, "col": c, "base64": image, "name": "cell.png"})).unwrap(); + if (r, c) == covered { + assert_eq!((placed["row"].as_u64(), placed["col"].as_u64()), (Some(0), Some(0))); + } + } + let before = table_for(&s, sid, tid).clone(); + let selected = s.doc().unwrap().selection.cells.unwrap(); + s.execute("table.textCell", &json!({})).unwrap(); + let after = table_for(&s, sid, tid); + for (r, c) in [(0, 0), (1, 1)] { + assert!(before.cell(r, c).unwrap().graphic.is_some()); + assert!(after.cell(r, c).unwrap().graphic.is_none()); + assert_eq!(after.cell(r, c).unwrap().text, before.cell(r, c).unwrap().text); + } + assert_eq!(after.cell(neighbor.0, neighbor.1), before.cell(neighbor.0, neighbor.1)); + assert_eq!(after.cell(covered.0, covered.1), before.cell(covered.0, covered.1)); + assert_eq!(s.doc().unwrap().selection.cells, Some(selected)); + check(&s); + } +} + +#[test] +fn covered_selected_owner_drag_keeps_merged_move_restrictions_atomic() { + for column in [true, false] { + let (mut s, sid, _, fid) = selected_merged_table(column); + let before = s.doc().unwrap().doc.clone(); + let selection = s.doc().unwrap().selection.clone(); + let revision = s.doc().unwrap().revision; + let undo = s.doc().unwrap().history.undo.len(); + let redo = s.doc().unwrap().history.redo.len(); + let cs = s.cache.get(&before, doc::StoryId(sid), None); + let cells = &cs.frames[0].tables[0].cells; + let point = |row, col| { + let cell = cells.iter().find(|c| c.row == row && c.col == col).unwrap(); + before.text_xf(before.item(doc::ItemId(fid)).unwrap()) * cell.rect.center() + }; + let from = point(0, 0); + let to = if column { point(1, 0) } else { point(0, 1) }; + let err = s.execute("table.dropCells", &json!({"frame": fid, "from": [from.x, from.y], "to": [to.x, to.y]})).unwrap_err(); + assert!(err.to_string().contains(if column { "unmerge cells that span columns" } else { "unmerge cells that span rows" }), "{err}"); + let st = s.doc().unwrap(); + assert_eq!(st.doc, before); + assert_eq!(st.selection, selection); + assert_eq!(st.revision, revision); + assert_eq!((st.history.undo.len(), st.history.redo.len()), (undo, redo)); + } +} + #[test] fn insert_type_in_cells_and_tab() { let (mut s, sid, _) = session_with_frame(); diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 077ae294..cc7cee3d 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -946,11 +946,12 @@ fn draw_text_selection( let range = ts.range(); // Selected table cells: tint every selected cell. if let Some(sel) = cells.filter(|c| c.story == ts.story) { + let selected = doc.story(sel.story).and_then(|s| s.tables.get(&sel.table)).map(|t| t.selected_owners(sel.range)).unwrap_or_default(); for ft in &cs.frames { let (Some((a, _)), Some(it)) = (item_canvas_xf(doc, layout, ft.frame), doc.item(ft.frame)) else { continue }; let m = a * doc.text_xf(it); for t in ft.tables.iter().filter(|t| t.table == sel.table) { - for c in t.cells.iter().filter(|c| sel.range.contains(c.row, c.col)) { + for c in t.cells.iter().filter(|c| selected.binary_search(&(c.row, c.col)).is_ok()) { let q = [ xf.to_screen(m * Point::new(c.rect.x0, c.rect.y0)), xf.to_screen(m * Point::new(c.rect.x1, c.rect.y0)), @@ -1805,6 +1806,79 @@ mod tests { (s.horizontal_units, s.vertical_units) } + #[test] + fn selected_merged_owners_paint_the_same_row_and_column_targets() { + for column in [true, false] { + for merged in [false, true] { + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"pages": 1})).unwrap(); + app.run("frame.create", json!({"rect": [36, 36, 300, 300], "content": "text"})).unwrap(); + app.run("table.insert", json!({"rows": 2, "cols": 2})).unwrap(); + if merged { + let range = if column { json!({"rows": [0, 0], "cols": [0, 1]}) } else { json!({"rows": [0, 1], "cols": [0, 0]}) }; + app.run("table.select", range).unwrap(); + app.run("table.merge", json!({})).unwrap(); + } + app.run("table.select", json!({"rows": [1, 1], "cols": [1, 1]})).unwrap(); + app.run(if column { "table.selectColumn" } else { "table.selectRow" }, json!({})).unwrap(); + let st = app.session.doc().unwrap(); + let selection = st.selection.clone(); + let ts = selection.text.unwrap(); + let selected = selection.cells.unwrap(); + assert_eq!( + selected.range, + if column { designcraft_doc::CellRange::new(0, 1, 1, 1) } else { designcraft_doc::CellRange::new(1, 0, 1, 1) } + ); + let layout = CanvasLayout::new(&st.doc, st.editing_parents); + let cs = app.session.cache.get(&st.doc, ts.story, None); + let ft = &cs.frames[0]; + let table = ft.tables.iter().find(|t| t.table == selected.table).unwrap(); + let (a, _) = item_canvas_xf(&st.doc, &layout, ft.frame).unwrap(); + let m = a * st.doc.text_xf(st.doc.item(ft.frame).unwrap()); + let xf = Xf { min: pos2(0.0, 0.0), origin: Point::new(0.0, 0.0), zoom: 1.0, rot: 0, pivot: pos2(0.0, 0.0) }; + let owners = if merged { + [(0, 0), (1, 1)] + } else if column { + [(0, 1), (1, 1)] + } else { + [(1, 0), (1, 1)] + }; + let expected: Vec<_> = owners + .into_iter() + .map(|(r, c)| { + let rect = table.cells.iter().find(|cell| cell.row == r && cell.col == c).unwrap().rect; + vec![ + xf.to_screen(m * Point::new(rect.x0, rect.y0)), + xf.to_screen(m * Point::new(rect.x1, rect.y0)), + xf.to_screen(m * Point::new(rect.x1, rect.y1)), + xf.to_screen(m * Point::new(rect.x0, rect.y1)), + ] + }) + .collect(); + let ctx = egui::Context::default(); + let mut output = ctx.run_ui(egui::RawInput::default(), |ui| { + draw_text_selection(&app, ui.painter(), &xf, &st.doc, &layout, ts, Some(selected)); + }); + output.textures_delta.clear(); + let tint = Color32::from_rgba_unmultiplied(80, 140, 255, 90); + let painted: Vec<_> = output + .shapes + .into_iter() + .filter_map(|s| match s.shape { + egui::Shape::Path(path) if path.fill == tint => { + assert!(path.closed); + assert_eq!(path.stroke, egui::epaint::PathStroke::NONE); + Some(path.points) + } + _ => None, + }) + .collect(); + assert_eq!(painted, expected, "only the two touched owners are tinted; column={column}, merged={merged}"); + assert_eq!(app.session.doc().unwrap().selection, selection); + } + } + } + #[test] fn right_clicking_a_ruler_sets_that_rulers_units() { let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());