diff --git a/src/terminal/view.rs b/src/terminal/view.rs index d1fb47ab..ccc7cd50 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -336,6 +336,10 @@ pub struct TerminalView { scroll_anim: Option, scroll_anim_epoch: u64, gesture_until: Option, + /// Whether the trackpad gesture in flight is zooming, latched at its first + /// event. `None` between gestures, and never set for a wheel, which has no + /// gesture to belong to and decides notch by notch. + gesture_zoom: Option, pub title: String, /// A title the pane has been told about but has not adopted yet — see /// `set_title_when_settled`. `None` means the tab is showing the newest @@ -1579,6 +1583,7 @@ impl TerminalView { scroll_anim: None, scroll_anim_epoch: 0, gesture_until: None, + gesture_zoom: None, title: DEFAULT_TITLE.to_string(), pending_title: None, default_title: DEFAULT_TITLE.to_string(), @@ -5472,12 +5477,38 @@ impl TerminalView { } fn on_scroll(&mut self, ev: &ScrollWheelEvent, window: &mut Window, cx: &mut Context) { + let gesturing = self.track_scroll_gesture(ev.touch_phase); // One modifier turns the wheel into a zoom, the way it does in a // browser. Which one is the user's to say, because the default is the // platform modifier and on macOS that is a key half the world is // already holding for something else (#668). - if zoom_wheel(cx.global::().mouse_zoom_modifier, &ev.modifiers) { - self.zoom_scroll(ev, window, cx); + let wants_zoom = zoom_wheel(cx.global::().mouse_zoom_modifier, &ev.modifiers); + // A trackpad gesture answers "scroll or zoom?" once, on its first + // event, and keeps that answer until the stream dies — momentum tail + // included. The tail is why: those events are the system's, not the + // hand's, yet each one is stamped with whatever modifiers happen to be + // down as it is delivered. Reaching for ⌘ during a flick's coast — + // ⌘-Tab, ⌘-C, anything — would otherwise turn hundreds of coasting + // lines into zoom steps and leave the font at its minimum, from a + // gesture that was never a zoom (#912). + // The answer is latched per gesture, not per stream: fingers going + // back down ask it again. Carrying it over would be the same bug + // wearing the other coat — one ⌘-zoom, and every later flick zooms + // with nothing held at all, because `Started` keeps the gesture live + // and would find the old answer still sitting there. + if !gesturing || matches!(ev.touch_phase, gpui::TouchPhase::Started) { + self.gesture_zoom = None; + // Leftover travel belongs to the gesture that earned it; a new one + // must not start already part-way to a step. + self.zoom_debt = 0.; + } + let zoom = if gesturing { + *self.gesture_zoom.get_or_insert(wants_zoom) + } else { + wants_zoom + }; + if zoom { + self.zoom_scroll(ev, gesturing, window, cx); return; } let mult = cx.global::().mouse_scroll_multiplier; @@ -5486,7 +5517,6 @@ impl TerminalView { ScrollDelta::Pixels(p) => p.y.as_f32() / self.line_height.as_f32(), }; let delta = raw * mult; - let gesturing = self.track_scroll_gesture(ev.touch_phase); let quantized = !ev.modifiers.shift && { let mode = *self.terminal.term.lock().mode(); @@ -5522,7 +5552,13 @@ impl TerminalView { /// asked for the wheel. Steps go out as the same actions the keyboard and /// the View menu send, so the min/max clamp and the saved setting live in /// one place — [`Tty7App::change_font_size`](crate::ui::app::Tty7App). - fn zoom_scroll(&mut self, ev: &ScrollWheelEvent, window: &mut Window, cx: &mut Context) { + fn zoom_scroll( + &mut self, + ev: &ScrollWheelEvent, + gesturing: bool, + window: &mut Window, + cx: &mut Context, + ) { // Whatever the scrollback still had in flight is dropped: it was // travelling in lines of a font that is about to change size. self.cancel_scroll_anim(); @@ -5530,7 +5566,6 @@ impl TerminalView { ScrollDelta::Lines(p) => p.y, ScrollDelta::Pixels(p) => p.y.as_f32() / self.line_height.as_f32(), }; - let gesturing = self.track_scroll_gesture(ev.touch_phase); let (steps, debt) = zoom_scroll_steps(lines, self.zoom_debt, gesturing); self.zoom_debt = debt; for _ in 0..steps.unsigned_abs() { @@ -13315,6 +13350,73 @@ mod gpui_tests { .unwrap(); } + /// The bug this guards: a two-finger flick coasts long after the fingers + /// are gone, and every coasting event carries whatever modifiers are down + /// when it lands. Grabbing ⌘ for something else mid-coast used to read as + /// a zoom and run the font down to its minimum in a blink (#912). + #[gpui::test] + fn a_modifier_pressed_mid_flick_does_not_hijack_it_into_a_zoom(cx: &mut TestAppContext) { + let (window, _daemon) = harness(cx); + window + .update(cx, |view, w, cx| { + scroll_into_history(view, 10); + view.on_scroll(&wheel(view, -0.5, gpui::TouchPhase::Started), w, cx); + let before = display_offset(view); + // The tail, now stamped with ⌘ the hand reached for. + let mut tail = wheel(view, -3., gpui::TouchPhase::Moved); + tail.modifiers = Modifiers::secondary_key(); + view.on_scroll(&tail, w, cx); + assert!( + display_offset(view) != before || view.scroll_anim.is_some(), + "the tail stayed a scroll, the way the gesture started" + ); + assert_eq!(view.zoom_debt, 0., "and never paid into the zoom"); + }) + .unwrap(); + } + + /// The same latch the other way round: a zoom gesture that outlives the + /// modifier keeps zooming rather than dumping its tail into the buffer. + #[gpui::test] + fn a_zoom_gesture_keeps_zooming_after_the_modifier_is_released(cx: &mut TestAppContext) { + let (window, _daemon) = harness(cx); + window + .update(cx, |view, w, cx| { + scroll_into_history(view, 10); + let mut start = wheel(view, -0.5, gpui::TouchPhase::Started); + start.modifiers = Modifiers::secondary_key(); + view.on_scroll(&start, w, cx); + view.on_scroll(&wheel(view, -0.5, gpui::TouchPhase::Moved), w, cx); + assert_eq!(display_offset(view), 10, "the grid never moved"); + assert!(view.scroll_anim.is_none(), "and nothing was queued for it"); + }) + .unwrap(); + } + + /// The latch is per gesture, not per stream: fingers going down again + /// start a fresh question. Carrying the last answer over would mean one + /// ⌘-zoom left every later flick zooming with no modifier held at all. + #[gpui::test] + fn a_new_gesture_is_not_bound_by_what_the_last_one_answered(cx: &mut TestAppContext) { + let (window, _daemon) = harness(cx); + window + .update(cx, |view, w, cx| { + scroll_into_history(view, 10); + let mut zoom = wheel(view, -0.5, gpui::TouchPhase::Started); + zoom.modifiers = Modifiers::secondary_key(); + view.on_scroll(&zoom, w, cx); + assert_eq!(display_offset(view), 10, "that one was a zoom"); + // Fingers down again, nothing held: a plain scroll. + view.on_scroll(&wheel(view, -3., gpui::TouchPhase::Started), w, cx); + assert_ne!( + display_offset(view), + 10, + "the new gesture asked the question again" + ); + }) + .unwrap(); + } + /// A detent is one step however many lines the platform bills it as — /// macOS calls a single notch five. #[test]