edit predictions: Always position jump/accept popovers inside viewport (#25348)

Agus Zubiaga and Danilo created

https://github.com/user-attachments/assets/345961c5-9bcb-4ee5-80f2-03d5fd0741d3

Release Notes:

- edit prediction: Fixed jump/accept popover position for long lines

---------

Co-authored-by: Danilo <danilo@zed.dev>

Change summary

crates/editor/src/element.rs | 142 +++++++++++++++++++++++++++----------
1 file changed, 101 insertions(+), 41 deletions(-)

Detailed changes

crates/editor/src/element.rs 🔗

@@ -3711,6 +3711,9 @@ impl EditorElement {
         }
     }
 
+    const EDIT_PREDICTION_POPOVER_PADDING_X: Pixels = Pixels(24.);
+    const EDIT_PREDICTION_POPOVER_PADDING_Y: Pixels = Pixels(2.);
+
     #[allow(clippy::too_many_arguments)]
     fn layout_edit_prediction_popover(
         &self,
@@ -3729,9 +3732,6 @@ impl EditorElement {
         window: &mut Window,
         cx: &mut App,
     ) -> Option<(AnyElement, gpui::Point<Pixels>)> {
-        const PADDING_X: Pixels = Pixels(24.);
-        const PADDING_Y: Pixels = Pixels(2.);
-
         let editor = self.editor.read(cx);
         let active_inline_completion = editor.active_inline_completion.as_ref()?;
 
@@ -3864,7 +3864,10 @@ impl EditorElement {
                         .into_any();
 
                     let size = element.layout_as_root(AvailableSpace::min_size(), window, cx);
-                    let offset = point((text_bounds.size.width - size.width) / 2., PADDING_Y);
+                    let offset = point(
+                        (text_bounds.size.width - size.width) / 2.,
+                        Self::EDIT_PREDICTION_POPOVER_PADDING_Y,
+                    );
 
                     let origin = text_bounds.origin + offset;
                     element.prepaint_at(origin, window, cx);
@@ -3882,27 +3885,27 @@ impl EditorElement {
                     let size = element.layout_as_root(AvailableSpace::min_size(), window, cx);
                     let offset = point(
                         (text_bounds.size.width - size.width) / 2.,
-                        text_bounds.size.height - size.height - PADDING_Y,
+                        text_bounds.size.height
+                            - size.height
+                            - Self::EDIT_PREDICTION_POPOVER_PADDING_Y,
                     );
 
                     let origin = text_bounds.origin + offset;
                     element.prepaint_at(origin, window, cx);
                     Some((element, origin))
                 } else {
-                    let mut element = editor
-                        .render_edit_prediction_line_popover("Jump to Edit", None, window, cx)?
-                        .into_any();
-                    let target_line_end = DisplayPoint::new(
-                        target_display_point.row(),
-                        editor_snapshot.line_len(target_display_point.row()),
+                    return self.layout_edit_prediction_end_of_line_popover(
+                        "Jump to Edit",
+                        editor_snapshot,
+                        visible_row_range,
+                        target_display_point,
+                        line_height,
+                        scroll_pixel_position,
+                        content_origin,
+                        editor_width,
+                        window,
+                        cx,
                     );
-                    let origin = self.editor.update(cx, |editor, _cx| {
-                        editor.display_to_pixel_point(target_line_end, editor_snapshot, window)
-                    })?;
-
-                    let origin = clamp_start(start_point + origin + point(PADDING_X, px(0.)));
-                    element.prepaint_as_root(origin, AvailableSpace::min_size(), window, cx);
-                    Some((element, origin))
                 }
             }
             InlineCompletion::Edit {
@@ -3939,28 +3942,18 @@ impl EditorElement {
                         let range = &edits.first()?.0;
                         let target_display_point = range.end.to_display_point(editor_snapshot);
 
-                        let target_line_end = DisplayPoint::new(
-                            target_display_point.row(),
-                            editor_snapshot.line_len(target_display_point.row()),
+                        return self.layout_edit_prediction_end_of_line_popover(
+                            "Accept",
+                            editor_snapshot,
+                            visible_row_range,
+                            target_display_point,
+                            line_height,
+                            scroll_pixel_position,
+                            content_origin,
+                            editor_width,
+                            window,
+                            cx,
                         );
-                        let (mut element, origin) = self.editor.update(cx, |editor, cx| {
-                            Some((
-                                editor
-                                    .render_edit_prediction_line_popover(
-                                        "Accept", None, window, cx,
-                                    )?
-                                    .into_any(),
-                                editor.display_to_pixel_point(
-                                    target_line_end,
-                                    editor_snapshot,
-                                    window,
-                                )?,
-                            ))
-                        })?;
-
-                        let origin = clamp_start(start_point + origin + point(PADDING_X, px(0.)));
-                        element.prepaint_as_root(origin, AvailableSpace::min_size(), window, cx);
-                        return Some((element, origin));
                     }
                     EditDisplayMode::Inline => return None,
                     EditDisplayMode::DiffPopover => {}
@@ -4036,8 +4029,10 @@ impl EditorElement {
                         ..Default::default()
                     });
 
-                let x_after_longest =
-                    text_bounds.origin.x + longest_line_width + PADDING_X - scroll_pixel_position.x;
+                let x_after_longest = text_bounds.origin.x
+                    + longest_line_width
+                    + Self::EDIT_PREDICTION_POPOVER_PADDING_X
+                    - scroll_pixel_position.x;
 
                 let element_bounds = element.layout_as_root(AvailableSpace::min_size(), window, cx);
 
@@ -4096,6 +4091,71 @@ impl EditorElement {
         }
     }
 
+    #[allow(clippy::too_many_arguments)]
+    fn layout_edit_prediction_end_of_line_popover(
+        &self,
+        label: &'static str,
+        editor_snapshot: &EditorSnapshot,
+        visible_row_range: Range<DisplayRow>,
+        target_display_point: DisplayPoint,
+        line_height: Pixels,
+        scroll_pixel_position: gpui::Point<Pixels>,
+        content_origin: gpui::Point<Pixels>,
+        editor_width: Pixels,
+        window: &mut Window,
+        cx: &mut App,
+    ) -> Option<(AnyElement, gpui::Point<Pixels>)> {
+        let target_line_end = DisplayPoint::new(
+            target_display_point.row(),
+            editor_snapshot.line_len(target_display_point.row()),
+        );
+
+        let mut element = self
+            .editor
+            .read(cx)
+            .render_edit_prediction_line_popover(label, None, window, cx)?
+            .into_any();
+
+        let size = element.layout_as_root(AvailableSpace::min_size(), window, cx);
+
+        let line_origin = self.editor.update(cx, |editor, _cx| {
+            editor.display_to_pixel_point(target_line_end, editor_snapshot, window)
+        })?;
+
+        let start_point = content_origin - point(scroll_pixel_position.x, Pixels::ZERO);
+        let mut origin = start_point
+            + line_origin
+            + point(Self::EDIT_PREDICTION_POPOVER_PADDING_X, Pixels::ZERO);
+        origin.x = origin.x.max(content_origin.x);
+
+        let max_x = content_origin.x + editor_width - size.width;
+
+        if origin.x > max_x {
+            let offset = line_height + Self::EDIT_PREDICTION_POPOVER_PADDING_Y;
+
+            let icon = if visible_row_range.contains(&(target_display_point.row() + 2)) {
+                origin.y += offset;
+                IconName::ArrowUp
+            } else {
+                origin.y -= offset;
+                IconName::ArrowDown
+            };
+
+            element = self
+                .editor
+                .read(cx)
+                .render_edit_prediction_line_popover(label, Some(icon), window, cx)?
+                .into_any();
+
+            let size = element.layout_as_root(AvailableSpace::min_size(), window, cx);
+
+            origin.x = content_origin.x + editor_width - size.width - px(2.);
+        }
+
+        element.prepaint_at(origin, window, cx);
+        Some((element, origin))
+    }
+
     fn layout_mouse_context_menu(
         &self,
         editor_snapshot: &EditorSnapshot,