diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index dafd99c37..a379cb47b 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2399,9 +2399,10 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & d.selection.anchor = sel.anchor; } } else if mods.command { - // Ctrl+click on a hyperlink's text follows it instead of adding the cell to the selection. + // Ctrl+click (without Alt) on a hyperlink's text follows it instead of adding the cell to the selection. if let Some(h) = sh.hyperlinks.get(&c).cloned() && primary_clicked + && !mods.alt && p.x < geo.cell_rect(sh, c).left() + geo.cell_rect(sh, c).width() * 0.8 { follow_link(app, &h.target); @@ -3410,7 +3411,8 @@ fn fill_options_tag(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo, sh: &Sheet let size = vec2(18.0, 18.0); let corner = geo.range_rect(sh, tag.target).right_bottom(); let button = geo.mirror_rect(Rect::from_min_size(corner + vec2(2.0, 2.0), size)); - if !geo.cells.contains_rect(button) { + // `button` is in screen space, so compare it with the mirrored cells area. + if !geo.mirror_rect(geo.cells).contains_rect(button) { return; } let lang = crate::i18n::current(ui.ctx()); diff --git a/crates/ui-egui/tests/it/fill_options_button.rs b/crates/ui-egui/tests/it/fill_options_button.rs index 2cb209fa9..d735f846a 100644 --- a/crates/ui-egui/tests/it/fill_options_button.rs +++ b/crates/ui-egui/tests/it/fill_options_button.rs @@ -209,3 +209,65 @@ fn the_button_stays_inside_the_grid_on_a_right_to_left_sheet() { let filled = geo.screen_cell_rect(sheet, CellRef::parse("B6").unwrap()); assert!(button.center().x < filled.left() && button.top() >= filled.bottom() - 1.0, "below-left of the mirrored range's corner"); } + +/// Right-to-left sheet with column A made `chars` wide (so column B's right edge, where the fill +/// range B1:B6 ends, sits far enough right to be scrolled to either edge of the cells area). +fn rtl_wide_a(chars: u32) -> Harness { + harness(|s| { + one_two(s); + s.execute("home.columnWidth", json!({"cols": "A:A", "chars": chars})).unwrap(); + s.execute("view.rightToLeft", json!({"on": true})).unwrap(); + }) +} + +/// Logical right edge of the filled column B (scroll-dependent) and the current geometry. +fn b_right(h: &Harness) -> (f32, Geo) { + 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.cell_rect(sheet, CellRef::parse("B6").unwrap()).right(), geo) +} + +fn scroll_b_right_to(h: &mut Harness, logical_x: f32) { + let (now, geo) = b_right(h); + h.state_mut().view_mut().unwrap().scroll.x += (now - logical_x) / geo.z; + h.run_steps(3); + let got = b_right(h).0; + assert!((got - logical_x).abs() < 1.0, "setup: column B ends at logical x {got}, wanted {logical_x}"); +} + +/// Logical x of the range's corner is `cells.right() - 25`: the 18 px button (at corner + 2 .. +/// corner + 20) fits in the cells area, so on screen it sits at the left edge of the mirrored cells +/// area, left of the filled range. `fill_options_tag` tested the mirrored button against the +/// unmirrored cells, i.e. against the row-heading strip it does not own, and hid it. +#[test] +fn the_button_shows_near_the_left_screen_edge_on_a_right_to_left_sheet() { + let mut h = rtl_wide_a(220); + let (_, geo) = b_right(&h); + scroll_b_right_to(&mut h, geo.cells.right() - 25.0); + drag_handle(&mut h, "B2", "B6", PointerButton::Primary); + + let (_, geo) = b_right(&h); + let cells = h.state().grid.cells_rect.unwrap(); + assert!(geo.header_w > 20.0 && geo.cells.right() - 25.0 + 20.0 > geo.cells.right() - geo.header_w, "setup: in the zone the old check hides"); + let button = h.query_by_label("Auto Fill Options").expect("the button fits inside the cells area, so it shows").rect(); + assert!(cells.contains_rect(button), "{button:?} outside {cells:?}"); +} + +/// The corner is 10 px left of `cells.left()` (scrolled partly behind the row headings, which are +/// on the right on screen). The button would start 8 px behind the headings, so it must not be +/// drawn there. The old check let it through because the mirrored button fits the unmirrored cells. +#[test] +fn the_button_stays_off_the_row_headings_on_a_right_to_left_sheet() { + let mut h = rtl_wide_a(20); + drag_handle(&mut h, "B2", "B6", PointerButton::Primary); + assert!(h.query_by_label("Auto Fill Options").is_some(), "setup: the button shows before scrolling"); + let (_, geo) = b_right(&h); + scroll_b_right_to(&mut h, geo.cells.left() - 10.0); + + let cells = h.state().grid.cells_rect.unwrap(); + if let Some(b) = h.query_by_label("Auto Fill Options") { + let button = b.rect(); + assert!(cells.contains_rect(button), "{button:?} overlaps the row headings, cells area is {cells:?}"); + } +} diff --git a/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs b/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs index 2e557e12e..a61350d3d 100644 --- a/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs +++ b/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs @@ -83,3 +83,15 @@ fn ctrl_click_beside_the_link_text_still_extends_the_selection() { assert_eq!(ranges, ["B2", "B4"], "the hyperlink cell joins the selection"); assert!(h.state().dialog.is_none(), "no link was followed"); } + +#[test] +fn ctrl_alt_click_on_the_link_text_still_extends_the_selection() { + let mut h = harness(); + click_at(&mut h, "B2", 0.9, Modifiers::NONE); + click_at(&mut h, "B4", 0.3, Modifiers { alt: true, ..Modifiers::COMMAND }); + let d = h.state().session.active().unwrap(); + let ranges: Vec = d.selection.ranges.iter().map(|r| r.a1()).collect(); + assert_eq!(ranges, ["B2", "B4"], "Ctrl+Alt+click adds the hyperlink cell to the selection"); + assert_eq!(d.selection.active.a1(), "B4", "the link was not followed"); + assert!(h.state().dialog.is_none(), "no link dialog"); +}