Avoid scrolling when clicking near editor top/bottom

Also makes go-to navigation autoscroll guarantee some amount of
margin regardless of user cursor scroll margin

Cargo.lock

@@ -12748,7 +12748,7 @@ dependencies = [
 
 [[package]]
 name = "zed"
-version = "0.127.0"
+version = "0.128.0"
 dependencies = [
  "activity_indicator",
  "anyhow",

assets/settings/default.json

@@ -157,7 +157,7 @@
     // Whether to show fold buttons in the gutter.
     "folds": true
   },
-  // The number of lines to keep above/below the cursor when scrolling.
+  // The number of lines to keep above/below the cursor when moving the cursor.
   "vertical_scroll_margin": 3,
   "relative_line_numbers": false,
   // When to populate a new search's query based on the text under the cursor.

crates/editor/src/editor.rs

@@ -2129,20 +2129,17 @@ impl Editor {
         let start;
         let end;
         let mode;
-        let auto_scroll;
         match click_count {
             1 => {
                 start = buffer.anchor_before(position.to_point(&display_map));
                 end = start;
                 mode = SelectMode::Character;
-                auto_scroll = true;
             }
             2 => {
                 let range = movement::surrounding_word(&display_map, position);
                 start = buffer.anchor_before(range.start.to_point(&display_map));
                 end = buffer.anchor_before(range.end.to_point(&display_map));
                 mode = SelectMode::Word(start..end);
-                auto_scroll = true;
             }
             3 => {
                 let position = display_map
@@ -2156,17 +2153,15 @@ impl Editor {
                 start = buffer.anchor_before(line_start);
                 end = buffer.anchor_before(next_line_start);
                 mode = SelectMode::Line(start..end);
-                auto_scroll = true;
             }
             _ => {
                 start = buffer.anchor_before(0);
                 end = buffer.anchor_before(buffer.len());
                 mode = SelectMode::All;
-                auto_scroll = false;
             }
         }
 
-        self.change_selections(auto_scroll.then(|| Autoscroll::newest()), cx, |s| {
+        self.change_selections(None, cx, |s| {
             if !add {
                 s.clear_disjoint();
             } else if click_count > 1 {
@@ -6872,7 +6867,7 @@ impl Editor {
         self.select_next_match_internal(
             &display_map,
             action.replace_newest,
-            Some(Autoscroll::newest()),
+            Some(Autoscroll::newest_margin()),
             cx,
         )?;
         Ok(())
@@ -6924,7 +6919,7 @@ impl Editor {
 
                 if let Some(next_selected_range) = next_selected_range {
                     self.unfold_ranges([next_selected_range.clone()], false, true, cx);
-                    self.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                    self.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                         if action.replace_newest {
                             s.delete(s.newest_anchor().id);
                         }
@@ -7005,7 +7000,7 @@ impl Editor {
                     true,
                     cx,
                 );
-                self.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                self.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                     s.select(selections);
                 });
             } else if let Some(selected_text) = selected_text {
@@ -7444,7 +7439,7 @@ impl Editor {
             if let Some(popover) = self.hover_state.diagnostic_popover.as_ref() {
                 let (group_id, jump_to) = popover.activation_info();
                 if self.activate_diagnostics(group_id, cx) {
-                    self.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                    self.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                         let mut new_selection = s.newest_anchor().clone();
                         new_selection.collapse_to(jump_to, SelectionGoal::None);
                         s.select_anchors(vec![new_selection.clone()]);
@@ -7491,7 +7486,7 @@ impl Editor {
 
             if let Some((primary_range, group_id)) = group {
                 if self.activate_diagnostics(group_id, cx) {
-                    self.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                    self.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                         s.select(vec![Selection {
                             id: selection.id,
                             start: primary_range.start,
@@ -7598,7 +7593,7 @@ impl Editor {
             .dedup();
 
         if let Some(hunk) = hunks.next() {
-            self.change_selections(Some(Autoscroll::fit()), cx, |s| {
+            self.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                 let row = hunk.start_display_row();
                 let point = DisplayPoint::new(row, 0);
                 s.select_display_ranges([point..point]);
@@ -7750,7 +7745,7 @@ impl Editor {
                         let range = target.range.to_offset(target.buffer.read(cx));
                         let range = editor.range_for_match(&range);
                         if Some(&target.buffer) == editor.buffer.read(cx).as_singleton().as_ref() {
-                            editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                            editor.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                                 s.select_ranges([range]);
                             });
                         } else {
@@ -7770,7 +7765,7 @@ impl Editor {
                                     // to avoid creating a history entry at the previous cursor location.
                                     pane.update(cx, |pane, _| pane.disable_history());
                                     target_editor.change_selections(
-                                        Some(Autoscroll::fit()),
+                                        Some(Autoscroll::fit_margin()),
                                         cx,
                                         |s| {
                                             s.select_ranges([range]);
@@ -7979,7 +7974,7 @@ impl Editor {
                     let range = editor.range_for_match(&range);
 
                     if Some(&target.buffer) == editor.buffer().read(cx).as_singleton().as_ref() {
-                        editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                        editor.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                             s.select_ranges([range]);
                         });
                     } else {
@@ -7993,9 +7988,13 @@ impl Editor {
                                     )
                                 });
                             target_editor.update(cx, |target_editor, cx| {
-                                target_editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
-                                    s.select_ranges([range]);
-                                })
+                                target_editor.change_selections(
+                                    Some(Autoscroll::fit_margin()),
+                                    cx,
+                                    |s| {
+                                        s.select_ranges([range]);
+                                    },
+                                )
                             })
                         })
                     }
@@ -8242,7 +8241,7 @@ impl Editor {
                             }),
                             disposition: BlockDisposition::Below,
                         }],
-                        Some(Autoscroll::fit()),
+                        Some(Autoscroll::fit_margin()),
                         cx,
                     )[0];
                     this.pending_rename = Some(RenameState {
@@ -8321,7 +8320,7 @@ impl Editor {
 
         self.remove_blocks(
             [rename.block_id].into_iter().collect(),
-            Some(Autoscroll::fit()),
+            Some(Autoscroll::fit_margin()),
             cx,
         );
         self.clear_highlights::<Rename>(cx);
@@ -8682,7 +8681,7 @@ impl Editor {
             self.display_map.update(cx, |map, cx| map.fold(ranges, cx));
 
             if auto_scroll {
-                self.request_autoscroll(Autoscroll::fit(), cx);
+                self.request_autoscroll(Autoscroll::fit_margin(), cx);
             }
 
             cx.notify();
@@ -8701,7 +8700,7 @@ impl Editor {
             self.display_map
                 .update(cx, |map, cx| map.unfold(ranges, inclusive, cx));
             if auto_scroll {
-                self.request_autoscroll(Autoscroll::fit(), cx);
+                self.request_autoscroll(Autoscroll::fit_margin(), cx);
             }
 
             cx.notify();
@@ -9528,7 +9527,7 @@ impl Editor {
                 for (buffer, ranges) in new_selections_by_buffer.into_iter() {
                     let editor = workspace.open_project_item::<Self>(pane.clone(), buffer, cx);
                     editor.update(cx, |editor, cx| {
-                        editor.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                        editor.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                             s.select_ranges(ranges);
                         });
                     });
@@ -9571,7 +9570,7 @@ impl Editor {
                 };
 
                 let nav_history = editor.nav_history.take();
-                editor.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                editor.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                     s.select_ranges([cursor..cursor]);
                 });
                 editor.nav_history = nav_history;

crates/editor/src/element.rs

@@ -1650,6 +1650,12 @@ impl EditorElement {
         em_width: Pixels,
         cx: &mut ElementContext,
     ) {
+        struct MeasuredHoverPopover {
+            element: AnyElement,
+            size: Size<Pixels>,
+            horizontal_offset: Pixels,
+        }
+
         let max_size = size(
             (120. * em_width) // Default size
                 .min(hitbox.size.width / 2.) // Shrink to half of the editor width
@@ -1669,7 +1675,7 @@ impl EditorElement {
                 cx,
             )
         });
-        let Some((position, mut hover_popovers)) = hover_popovers else {
+        let Some((position, hover_popovers)) = hover_popovers else {
             return;
         };
 
@@ -1679,48 +1685,70 @@ impl EditorElement {
         let hovered_row_layout =
             &line_layouts[(position.row() - visible_display_row_range.start) as usize].line;
 
-        // Minimum required size: Take the first popover, and add 1.5 times the minimum popover
-        // height. This is the size we will use to decide whether to render popovers above or below
-        // the hovered line.
-        let first_size = hover_popovers[0].measure(available_space, cx);
-        let height_to_reserve = first_size.height + 1.5 * MIN_POPOVER_LINE_HEIGHT * line_height;
-
         // Compute Hovered Point
         let x =
             hovered_row_layout.x_for_index(position.column() as usize) - scroll_pixel_position.x;
         let y = position.row() as f32 * line_height - scroll_pixel_position.y;
         let hovered_point = content_origin + point(x, y);
 
-        if hovered_point.y - height_to_reserve > Pixels::ZERO {
+        let mut overall_height = Pixels::ZERO;
+        let mut measured_hover_popovers = Vec::new();
+        for mut hover_popover in hover_popovers {
+            let size = hover_popover.measure(available_space, cx);
+            let horizontal_offset =
+                (text_hitbox.upper_right().x - (hovered_point.x + size.width)).min(Pixels::ZERO);
+
+            overall_height += HOVER_POPOVER_GAP + size.height;
+
+            measured_hover_popovers.push(MeasuredHoverPopover {
+                element: hover_popover,
+                size,
+                horizontal_offset,
+            });
+        }
+        overall_height += HOVER_POPOVER_GAP;
+
+        fn draw_occluder(width: Pixels, origin: gpui::Point<Pixels>, cx: &mut ElementContext) {
+            let mut occlusion = div()
+                .size_full()
+                .occlude()
+                .on_mouse_move(|_, cx| cx.stop_propagation())
+                .into_any_element();
+            occlusion.measure(size(width, HOVER_POPOVER_GAP).into(), cx);
+            cx.defer_draw(occlusion, origin, 2);
+        }
+
+        if hovered_point.y > overall_height {
             // There is enough space above. Render popovers above the hovered point
             let mut current_y = hovered_point.y;
-            for mut hover_popover in hover_popovers {
-                let size = hover_popover.measure(available_space, cx);
-                let mut popover_origin = point(hovered_point.x, current_y - size.height);
+            for (position, popover) in measured_hover_popovers.into_iter().with_position() {
+                let size = popover.size;
+                let popover_origin = point(
+                    hovered_point.x + popover.horizontal_offset,
+                    current_y - size.height,
+                );
 
-                let x_out_of_bounds = text_hitbox.upper_right().x - (popover_origin.x + size.width);
-                if x_out_of_bounds < Pixels::ZERO {
-                    popover_origin.x = popover_origin.x + x_out_of_bounds;
+                cx.defer_draw(popover.element, popover_origin, 2);
+                if position != itertools::Position::Last {
+                    let origin = point(popover_origin.x, popover_origin.y - HOVER_POPOVER_GAP);
+                    draw_occluder(size.width, origin, cx);
                 }
 
-                cx.defer_draw(hover_popover, popover_origin, 2);
-
                 current_y = popover_origin.y - HOVER_POPOVER_GAP;
             }
         } else {
             // There is not enough space above. Render popovers below the hovered point
             let mut current_y = hovered_point.y + line_height;
-            for mut hover_popover in hover_popovers {
-                let size = hover_popover.measure(available_space, cx);
-                let mut popover_origin = point(hovered_point.x, current_y);
+            for (position, popover) in measured_hover_popovers.into_iter().with_position() {
+                let size = popover.size;
+                let popover_origin = point(hovered_point.x + popover.horizontal_offset, current_y);
 
-                let x_out_of_bounds = text_hitbox.upper_right().x - (popover_origin.x + size.width);
-                if x_out_of_bounds < Pixels::ZERO {
-                    popover_origin.x = popover_origin.x + x_out_of_bounds;
+                cx.defer_draw(popover.element, popover_origin, 2);
+                if position != itertools::Position::Last {
+                    let origin = point(popover_origin.x, popover_origin.y + size.height);
+                    draw_occluder(size.width, origin, cx);
                 }
 
-                cx.defer_draw(hover_popover, popover_origin, 2);
-
                 current_y = popover_origin.y + size.height + HOVER_POPOVER_GAP;
             }
         }

crates/editor/src/scroll/autoscroll.rs

@@ -27,11 +27,25 @@ impl Autoscroll {
         Self::Strategy(AutoscrollStrategy::Center)
     }
 
-    /// scrolls so the neweset cursor is near the top
+    /// scrolls so the newest cursor is near the top
     /// (offset by vertical_scroll_margin)
     pub fn focused() -> Self {
         Self::Strategy(AutoscrollStrategy::Focused)
     }
+
+    /// scrolls the minimal amount to (try) and fit all cursors onscreen
+    /// while guaranteeing an amount of margin from the top and bottom
+    /// of the screen, regardless of user configured scroll margin
+    pub fn fit_margin() -> Self {
+        Self::Strategy(AutoscrollStrategy::FitMargin)
+    }
+
+    /// scrolls the minimal amount to fit the newest cursor onscreen
+    /// while guaranteeing an amount of margin from the top and bottom
+    /// of the screen, regardless of user configured scroll margin
+    pub fn newest_margin() -> Self {
+        Self::Strategy(AutoscrollStrategy::NewestMargin)
+    }
 }
 
 #[derive(PartialEq, Eq, Default, Clone, Copy)]
@@ -41,6 +55,8 @@ pub enum AutoscrollStrategy {
     #[default]
     Center,
     Focused,
+    FitMargin,
+    NewestMargin,
     Top,
     Bottom,
 }
@@ -119,7 +135,7 @@ impl Editor {
         let margin = if matches!(self.mode, EditorMode::AutoHeight { .. }) {
             0.
         } else {
-            ((visible_lines - (target_bottom - target_top)) / 2.0).floor()
+            ((visible_lines - (target_bottom - target_top)) / 6.0).floor()
         };
 
         let strategy = match autoscroll {
@@ -142,8 +158,20 @@ impl Editor {
         };
 
         match strategy {
-            AutoscrollStrategy::Fit | AutoscrollStrategy::Newest => {
-                let margin = margin.min(self.scroll_manager.vertical_scroll_margin);
+            AutoscrollStrategy::Fit
+            | AutoscrollStrategy::Newest
+            | AutoscrollStrategy::FitMargin
+            | AutoscrollStrategy::NewestMargin => {
+                let margin = if matches!(
+                    strategy,
+                    AutoscrollStrategy::FitMargin | AutoscrollStrategy::NewestMargin
+                ) && !matches!(self.mode, EditorMode::AutoHeight { .. })
+                {
+                    margin.max(self.scroll_manager.vertical_scroll_margin)
+                } else {
+                    margin.min(self.scroll_manager.vertical_scroll_margin)
+                };
+
                 let target_top = (target_top - margin).max(0.0);
                 let target_bottom = target_bottom + margin;
                 let start_row = scroll_position.y;

crates/gpui/src/window.rs

@@ -956,12 +956,6 @@ impl<'a> WindowContext<'a> {
         self.window.next_frame.focus = self.window.focus;
         self.window.next_frame.window_active = self.window.active.get();
 
-        // Set the cursor only if we're the active window.
-        if self.is_window_active() {
-            let cursor_style = self.compute_cursor_style().unwrap_or(CursorStyle::Arrow);
-            self.platform.set_cursor_style(cursor_style);
-        }
-
         // Register requested input handler with the platform window.
         if let Some(input_handler) = self.window.next_frame.input_handlers.pop() {
             self.window
@@ -1017,6 +1011,8 @@ impl<'a> WindowContext<'a> {
                 .clone()
                 .retain(&(), |listener| listener(&event, self));
         }
+
+        self.reset_cursor_style();
         self.window.refreshing = false;
         self.window.draw_phase = DrawPhase::None;
         self.window.needs_present.set(true);
@@ -1031,16 +1027,20 @@ impl<'a> WindowContext<'a> {
         profiling::finish_frame!();
     }
 
-    fn compute_cursor_style(&mut self) -> Option<CursorStyle> {
-        // TODO: maybe we should have a HashMap keyed by HitboxId.
-        let request = self
-            .window
-            .next_frame
-            .cursor_styles
-            .iter()
-            .rev()
-            .find(|request| request.hitbox_id.is_hovered(self))?;
-        Some(request.style)
+    fn reset_cursor_style(&self) {
+        // Set the cursor only if we're the active window.
+        if self.is_window_active() {
+            let style = self
+                .window
+                .rendered_frame
+                .cursor_styles
+                .iter()
+                .rev()
+                .find(|request| request.hitbox_id.is_hovered(self))
+                .map(|request| request.style)
+                .unwrap_or(CursorStyle::Arrow);
+            self.platform.set_cursor_style(style);
+        }
     }
 
     /// Dispatch a given keystroke as though the user had typed it.
@@ -1175,7 +1175,11 @@ impl<'a> WindowContext<'a> {
     }
 
     fn dispatch_mouse_event(&mut self, event: &dyn Any) {
-        self.window.mouse_hit_test = self.window.rendered_frame.hit_test(self.mouse_position());
+        let hit_test = self.window.rendered_frame.hit_test(self.mouse_position());
+        if hit_test != self.window.mouse_hit_test {
+            self.window.mouse_hit_test = hit_test;
+            self.reset_cursor_style();
+        }
 
         let mut mouse_listeners = mem::take(&mut self.window.rendered_frame.mouse_listeners);
         self.with_element_context(|cx| {

crates/gpui/src/window/element_cx.rs

@@ -79,7 +79,7 @@ impl Hitbox {
     }
 }
 
-#[derive(Default)]
+#[derive(Default, Eq, PartialEq)]
 pub(crate) struct HitTest(SmallVec<[HitboxId; 8]>);
 
 pub(crate) struct DeferredDraw {

crates/workspace/src/pane_group.rs

@@ -941,8 +941,6 @@ mod element {
                         let flexes = self.flexes.clone();
                         let child_bounds = child.bounds;
                         let axis = self.axis;
-                        let handle_hitbox = handle.hitbox.clone();
-                        let was_hovered = handle_hitbox.is_hovered(cx);
                         move |e: &MouseMoveEvent, phase, cx| {
                             let dragged_handle = dragged_handle.borrow();
                             if phase.bubble() {
@@ -957,8 +955,6 @@ mod element {
                                         workspace.clone(),
                                         cx,
                                     )
-                                } else if was_hovered != handle_hitbox.is_hovered(cx) {
-                                    cx.refresh();
                                 }
                             }
                         }

crates/zed/Cargo.toml

@@ -2,7 +2,7 @@
 description = "The fast, collaborative code editor."
 edition = "2021"
 name = "zed"
-version = "0.127.0"
+version = "0.128.0"
 publish = false
 license = "GPL-3.0-or-later"
 authors = ["Zed Team <hi@zed.dev>"]