From 9518b041d55113f62ea484a467d6122b4068b1ed Mon Sep 17 00:00:00 2001 From: AlissonFA Date: Sun, 11 Oct 2026 01:57:11 -0300 Subject: [PATCH] M2: fix cell vertical alignment positioning and add keytips Closes #241 --- crates/engine/src/tests.rs | 34 +++++++++++++++ crates/ui-egui/src/grid.rs | 81 +++++++++++++++++++++++++++++++----- crates/ui-egui/src/ribbon.rs | 11 ++++- 3 files changed, 115 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6bd03d55e..6489ea238 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1633,3 +1633,37 @@ fn notes_comments_and_links_survive_a_json_save() { assert_eq!(after.hyperlinks, before.hyperlinks); assert_eq!(after.cells, before.cells); } + +#[test] +fn vertical_alignment_commands_and_roundtrip() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "top"})).unwrap(); + s.execute("cell.set", json!({"cell": "A2", "input": "middle"})).unwrap(); + s.execute("cell.set", json!({"cell": "A3", "input": "bottom"})).unwrap(); + + s.execute("selection.set", json!({"range": "A1"})).unwrap(); + s.execute("home.alignTop", json!({})).unwrap(); + + s.execute("selection.set", json!({"range": "A2"})).unwrap(); + s.execute("home.alignMiddle", json!({})).unwrap(); + + s.execute("selection.set", json!({"range": "A3"})).unwrap(); + s.execute("home.alignBottom", json!({})).unwrap(); + + let style_of = |s: &Session, cell: &str| { + let d = s.doc().unwrap(); + d.wb.styles.get(d.wb.active().unwrap().style_id(CellRef::parse(cell).unwrap())).clone() + }; + + assert_eq!(style_of(&s, "A1").align.v, gridcraft_model::VAlign::Top); + assert_eq!(style_of(&s, "A2").align.v, gridcraft_model::VAlign::Center); + assert_eq!(style_of(&s, "A3").align.v, gridcraft_model::VAlign::Bottom); + + let d = s.doc().unwrap(); + let bytes = gridcraft_xlsx::write_xlsx(&d.wb).unwrap(); + let (reopened, _) = gridcraft_xlsx::read_xlsx(&bytes).unwrap(); + let reopen_style = |cell: &str| reopened.styles.get(reopened.active().unwrap().style_id(CellRef::parse(cell).unwrap())).clone(); + assert_eq!(reopen_style("A1").align.v, gridcraft_model::VAlign::Top); + assert_eq!(reopen_style("A2").align.v, gridcraft_model::VAlign::Center); + assert_eq!(reopen_style("A3").align.v, gridcraft_model::VAlign::Bottom); +} diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 72d33e2dc..a513de4ca 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -482,6 +482,28 @@ fn vertical_scrollbar(app: &mut SheetApp, ui: &mut egui::Ui, sh: &Sheet, geo: &G } } +pub(crate) fn wrapped_cell_text_y(valign: VAlign, rect: Rect, gh: f32, z: f32) -> f32 { + let pad_v = 1.5 * z; + let y_top = rect.top() + pad_v; + let y_bottom = (rect.bottom() - gh - pad_v).max(y_top); + match valign { + VAlign::Top => y_top, + VAlign::Center | VAlign::Justify | VAlign::Distributed => (y_top + y_bottom) / 2.0, + VAlign::Bottom => y_bottom, + } +} + +pub(crate) fn cell_text_y(valign: VAlign, rect: Rect, gh: f32, z: f32) -> f32 { + let top_inset = 1.0 * z; + let y_top = rect.top() - top_inset; + let y_bottom = (rect.bottom() - gh - 0.5 * z).max(y_top); + match valign { + VAlign::Top => y_top, + VAlign::Center | VAlign::Justify | VAlign::Distributed => (y_top + y_bottom) / 2.0, + VAlign::Bottom => y_bottom, + } +} + #[allow(clippy::too_many_arguments)] fn paint_quadrant(p: &Painter, geo: &Geo, wb: &Workbook, si: usize, sh: &Sheet, rows: (u32, u32), cols: (u32, u32), cf: &mut CfCache, t: &Tokens) { let z = geo.z; @@ -643,11 +665,7 @@ fn paint_quadrant(p: &Painter, geo: &Geo, wb: &Workbook, si: usize, sh: &Sheet, HAlign::Right => rect.right() - pad, _ => text_left + pad + indent, }; - let y = match st.align.v { - VAlign::Top => rect.top() + 1.0, - VAlign::Center => rect.center().y - galley.size().y / 2.0, - _ => rect.bottom() - galley.size().y - 1.0, - }; + let y = wrapped_cell_text_y(st.align.v, *rect, galley.size().y, z); p.with_clip_rect(rect.intersect(p.clip_rect())).galley(pos2(x, y), galley, color); continue; } @@ -713,11 +731,7 @@ fn paint_quadrant(p: &Painter, geo: &Geo, wb: &Workbook, si: usize, sh: &Sheet, _ => text_left + pad + indent, }; let gh = galley.size().y; - let y = match st.align.v { - VAlign::Top => rect.top() + 1.0, - VAlign::Center | VAlign::Justify | VAlign::Distributed => rect.center().y - gh / 2.0, - VAlign::Bottom => rect.bottom() - gh - 0.5 * z, - }; + let y = cell_text_y(st.align.v, *rect, gh, z); let cp = p.with_clip_rect(clip.intersect(p.clip_rect())); if rotated { let angle = -(st.align.rotation as f32).to_radians(); @@ -2223,3 +2237,50 @@ fn paint_pages(app: &mut SheetApp, p: &Painter, geo: &Geo, sh: &Sheet) { } p.rect_stroke(pr, 0.0, Stroke::new(3.0, blue), StrokeKind::Outside); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cell_text_y_moves_monotonically_in_standard_and_tall_rows() { + let rect_std = Rect::from_min_max(pos2(0.0, 0.0), pos2(100.0, 20.0)); + let gh = 18.5; + let z = 1.0; + + let y_top_std = cell_text_y(VAlign::Top, rect_std, gh, z); + let y_center_std = cell_text_y(VAlign::Center, rect_std, gh, z); + let y_bottom_std = cell_text_y(VAlign::Bottom, rect_std, gh, z); + + assert!(y_top_std < y_center_std, "Top ({y_top_std}) must be higher than Center ({y_center_std})"); + assert!(y_center_std < y_bottom_std, "Center ({y_center_std}) must be higher than Bottom ({y_bottom_std})"); + + // Verify tall row (e.g. 40px) + let rect_tall = Rect::from_min_max(pos2(0.0, 0.0), pos2(100.0, 40.0)); + let y_top_tall = cell_text_y(VAlign::Top, rect_tall, gh, z); + let y_center_tall = cell_text_y(VAlign::Center, rect_tall, gh, z); + let y_bottom_tall = cell_text_y(VAlign::Bottom, rect_tall, gh, z); + + assert!(y_top_tall < y_center_tall, "Top ({y_top_tall}) must be higher than Center ({y_center_tall})"); + assert!(y_center_tall < y_bottom_tall, "Center ({y_center_tall}) must be higher than Bottom ({y_bottom_tall})"); + } + + #[test] + fn wrapped_cell_text_y_positions_correctly_and_safeguards_overflow() { + let rect = Rect::from_min_max(pos2(0.0, 0.0), pos2(100.0, 40.0)); + let gh = 30.0; + let z = 1.0; + + let y_top = wrapped_cell_text_y(VAlign::Top, rect, gh, z); + let y_center = wrapped_cell_text_y(VAlign::Center, rect, gh, z); + let y_bottom = wrapped_cell_text_y(VAlign::Bottom, rect, gh, z); + + assert!(y_top < y_center); + assert!(y_center < y_bottom); + + // Overflow case: gh exceeds rect height + let gh_overflow = 60.0; + let y_bottom_ovf = wrapped_cell_text_y(VAlign::Bottom, rect, gh_overflow, z); + assert!(y_bottom_ovf >= rect.top()); + } +} diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 264479a8c..60064a46c 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -495,7 +495,16 @@ fn home(app: &mut SheetApp, ui: &mut Ui) { (Icon::AlignMiddle, VAlign::Center, "home.alignMiddle", "Middle Align"), (Icon::AlignBottom, VAlign::Bottom, "home.alignBottom", "Bottom Align"), ] { - if toggle_button(ui, icon, st.align.v == v, tip).clicked() { + let response = toggle_button(ui, icon, st.align.v == v, tip); + if matches!(app.keytips.prefix(), Some("H" | "HA")) { + let sequence = match v { + VAlign::Top => "HAT", + VAlign::Center => "HAM", + _ => "HAB", + }; + keytip(app, ui, sequence, response.rect.center_bottom()); + } + if response.clicked() { act(app, id, json!({})); } }