Merge pull request #3383 from vangel76/fix/3350-ctrl-tab-arrows

Tabs: Ctrl-click on the sheet arrows jumps to the first or last sheet

crates/ui-egui/src/tabs.rs

@@ -22,6 +22,12 @@ fn navigator_items(sheets: &[Arc<Sheet>]) -> Vec<(String, String)> {
     sheets.iter().enumerate().filter(|(_, s)| s.visibility == Visibility::Visible).map(|(i, s)| (i.to_string(), s.name.clone())).collect()
 }
 
+/// Workbook index of the first (or, with `last`, the last) visible sheet: where Ctrl-clicking a tab arrow lands.
+fn edge_sheet(sheets: &[Arc<Sheet>], last: bool) -> Option<usize> {
+    let mut visible = sheets.iter().enumerate().filter(|(_, s)| s.visibility == Visibility::Visible).map(|(i, _)| i);
+    if last { visible.next_back() } else { visible.next() }
+}
+
 /// Switch to sheet `i` from the tab strip or the navigator. In point mode the formula being typed keeps going on the
 /// other sheet; otherwise the edit is committed first.
 fn activate_sheet(app: &mut SheetApp, i: usize) {
@@ -55,6 +61,18 @@ fn tab_drop(i: usize, steps: i64, copy: bool) -> Option<serde_json::Value> {
     Some(json!({"sheet": i, "to": to}))
 }
 
+/// Where a request to scroll the active tab into view is kept until the tabs are drawn.
+fn reveal_request_id(scope: egui::Id) -> egui::Id {
+    scope.with("worksheet_tabs_reveal_request")
+}
+
+/// Asks the tab strip to bring the active tab into view in this frame, even when the active sheet
+/// did not change (a Ctrl-click on an arrow whose edge sheet is already active).
+fn request_reveal(ui: &Ui) {
+    let id = reveal_request_id(ui.scope_id());
+    ui.ctx().data_mut(|data| data.insert_temp(id, true));
+}
+
 /// The New sheet (+) button and the gap before it.
 const NEW_SHEET_WIDTH: f32 = 32.0;
 /// The handle between the tabs and the horizontal scrollbar.
@@ -117,10 +135,28 @@ pub fn sheet_tabs(app: &mut SheetApp, ui: &mut Ui) {
             ui.add_space(8.0);
             if view.show_sheet_tabs {
                 if crate::widgets::icon_button(ui, Icon::Left, t.text_dim, app.ui.language.tr("Previous sheet"), vec2(24.0, 24.0)).clicked() {
-                    app.run_or_alert("sheet.previous", json!({}));
+                    // Ctrl-click jumps to the first visible sheet, like Excel's tab scroll buttons.
+                    let edge =
+                        if ui.input(|i| i.modifiers.command) { app.session.active().and_then(|d| edge_sheet(&d.wb.sheets, false)) } else { None };
+                    match edge {
+                        Some(i) => {
+                            activate_sheet(app, i);
+                            request_reveal(ui);
+                        }
+                        None => app.run_or_alert("sheet.previous", json!({})),
+                    }
                 }
                 if crate::widgets::icon_button(ui, Icon::Right, t.text_dim, app.ui.language.tr("Next sheet"), vec2(24.0, 24.0)).clicked() {
-                    app.run_or_alert("sheet.next", json!({}));
+                    // Ctrl-click jumps to the last visible sheet.
+                    let edge =
+                        if ui.input(|i| i.modifiers.command) { app.session.active().and_then(|d| edge_sheet(&d.wb.sheets, true)) } else { None };
+                    match edge {
+                        Some(i) => {
+                            activate_sheet(app, i);
+                            request_reveal(ui);
+                        }
+                        None => app.run_or_alert("sheet.next", json!({})),
+                    }
                 }
                 let all = crate::widgets::icon_button(ui, Icon::SheetList, t.text_dim, app.ui.language.tr("All sheets"), vec2(24.0, 24.0));
                 all.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, app.ui.language.tr("All sheets")));
@@ -163,10 +199,13 @@ pub fn sheet_tabs(app: &mut SheetApp, ui: &mut Ui) {
             // allow browsing other tabs without snapping back on every frame.
             let reveal_key = egui::Id::unique((d.uid, d.wb.active_sheet(), &sheets, tabs_width.to_bits()));
             let reveal_id = ui.scope_id().with("worksheet_tabs_reveal");
+            let request_id = reveal_request_id(ui.scope_id());
             let reveal_active = ui.ctx().data_mut(|data| {
                 let changed = data.get_temp::<egui::Id>(reveal_id) != Some(reveal_key);
                 data.insert_temp(reveal_id, reveal_key);
-                changed
+                // A Ctrl-click on an arrow whose edge sheet is already active changes nothing above,
+                // yet asks to bring that tab into view.
+                changed | data.remove_temp::<bool>(request_id).unwrap_or(false)
             });
             if view.show_sheet_tabs {
                 ui.allocate_ui_with_layout(vec2(tabs_width, 30.0), egui::Layout::left_to_right(egui::Align::Center), |ui| {
@@ -522,7 +561,8 @@ fn status_bar_menu(app: &mut SheetApp, ui: &mut Ui) {
 #[cfg(test)]
 mod tests {
     use super::{
-        MIN_TRACK, NEW_SHEET_WIDTH, SPLIT_WIDTH, activate_sheet, dragged_share, drop_copies_sheet, navigator_items, rename_needs_focus, tab_drop,
+        MIN_TRACK, NEW_SHEET_WIDTH, SPLIT_WIDTH, activate_sheet, dragged_share, drop_copies_sheet, edge_sheet, navigator_items, rename_needs_focus,
+        tab_drop,
     };
     use crate::SheetApp;
     use egui::Modifiers;
@@ -614,6 +654,16 @@ mod tests {
         assert_eq!(navigator_items(&sheets), vec![("0".to_string(), "Data".to_string()), ("3".to_string(), "Summary".to_string())]);
     }
 
+    #[test]
+    fn ctrl_click_on_the_tab_arrows_lands_on_the_first_and_last_visible_sheet() {
+        let mut hidden = Sheet::new("Hidden");
+        hidden.visibility = Visibility::Hidden;
+        let sheets = [hidden.clone(), Sheet::new("A"), Sheet::new("B"), Sheet::new("C"), hidden].map(Arc::new);
+        assert_eq!(edge_sheet(&sheets, false), Some(1));
+        assert_eq!(edge_sheet(&sheets, true), Some(3));
+        assert_eq!(edge_sheet(&[], true), None);
+    }
+
     #[test]
     fn a_rejected_commit_keeps_the_sheet_and_the_edit() {
         let mut session = gridcraft_engine::Session::new();

crates/ui-egui/tests/it/sheet_tabs.rs

@@ -58,6 +58,20 @@ fn click(h: &mut Harness, at: egui::Pos2) {
     h.run_steps(20);
 }
 
+fn ctrl_click(h: &mut Harness, at: egui::Pos2) {
+    let m = Modifiers::COMMAND;
+    h.input_mut().events.extend([
+        Event::ModifiersChanged(m),
+        Event::PointerMoved(at),
+        Event::PointerButton { pos: at, button: PointerButton::Primary, pressed: true, modifiers: m },
+    ]);
+    h.step();
+    h.input_mut().events.push(Event::PointerButton { pos: at, button: PointerButton::Primary, pressed: false, modifiers: m });
+    h.run_steps(20);
+    h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE));
+    h.run_steps(2);
+}
+
 #[test]
 fn opening_and_activating_late_sheets_reveals_their_tabs() {
     for width in [500.0, 1000.0] {
@@ -85,6 +99,65 @@ fn navigation_arrows_reveal_the_selected_tab() {
     assert_visible(&h, "Tab29");
 }
 
+/// A Ctrl/Cmd-click on the Next arrow jumps to the last sheet; repeated after the strip was scrolled
+/// away from it, the already active edge tab is brought back into view.
+#[test]
+fn repeated_ctrl_click_on_the_arrow_reveals_the_active_edge_tab() {
+    let mut h = harness(700.0, 0);
+    let (_, clip) = tab(&h, "Tab00").unwrap();
+    let next = pos2(clip.left() - 44.0, clip.center().y);
+    ctrl_click(&mut h, next);
+    assert_eq!(h.state().session.doc().unwrap().wb.active_sheet(), 29);
+    assert_visible(&h, "Tab29");
+    // Browse the strip back to the start without changing the selection.
+    h.input_mut().events.extend([Event::PointerMoved(clip.center())]);
+    for _ in 0..3 {
+        h.input_mut().events.push(Event::MouseWheel {
+            unit: egui::MouseWheelUnit::Point,
+            source: egui::MouseWheelSource::Unknown,
+            phase: egui::TouchPhase::Move,
+            delta: vec2(0.0, 600.0),
+            modifiers: Modifiers::NONE,
+        });
+        h.run_steps(10);
+    }
+    h.run_steps(40);
+    assert!(!tab(&h, "Tab29").is_some_and(|(r, c)| c.contains_rect(r)), "the strip was scrolled away from the active tab");
+    assert_eq!(h.state().session.doc().unwrap().wb.active_sheet(), 29);
+    ctrl_click(&mut h, next);
+    assert_eq!(h.state().session.doc().unwrap().wb.active_sheet(), 29);
+    assert_visible(&h, "Tab29");
+}
+
+/// The same for the Previous arrow and the first sheet.
+#[test]
+fn repeated_ctrl_click_on_the_previous_arrow_reveals_the_first_tab() {
+    let mut h = harness(700.0, 29);
+    let (_, clip) = tab(&h, "Tab29").unwrap();
+    let prev = pos2(clip.left() - 68.0, clip.center().y);
+    ctrl_click(&mut h, prev);
+    assert_eq!(h.state().session.doc().unwrap().wb.active_sheet(), 0);
+    assert_visible(&h, "Tab00");
+    // Browse the strip to its end without changing the selection.
+    h.input_mut().events.extend([Event::PointerMoved(clip.center())]);
+    for _ in 0..3 {
+        h.input_mut().events.push(Event::MouseWheel {
+            unit: egui::MouseWheelUnit::Point,
+            source: egui::MouseWheelSource::Unknown,
+            phase: egui::TouchPhase::Move,
+            delta: vec2(0.0, -600.0),
+            modifiers: Modifiers::NONE,
+        });
+        h.run_steps(10);
+    }
+    h.run_steps(40);
+    assert!(!tab(&h, "Tab00").is_some_and(|(r, c)| c.contains_rect(r)), "the strip was scrolled away from the active tab");
+    assert_eq!(h.state().session.doc().unwrap().wb.active_sheet(), 0);
+    ctrl_click(&mut h, prev);
+    assert_eq!(h.state().session.doc().unwrap().wb.active_sheet(), 0);
+    assert_visible(&h, "Tab00");
+}
+
 #[test]
 /// Horizontal wheel/trackpad browsing leaves the workbook unchanged, does not snap back on idle frames, and activating a sheet still reveals its tab.
 fn manual_wheel_and_trackpad_browsing_does_not_change_selection_or_snap_back() {