Skip to content

Commit

Permalink
edit predictions: Always position jump/accept popovers inside viewport (
Browse files Browse the repository at this point in the history 
#25348)

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 <[email protected]>
  • Loading branch information
@agu-z @danilo-leal
agu-z and danilo-leal authored Feb 24, 2025
1 parent f020291 commit 72a9429
Showing 1 changed file with 101 additions and 41 deletions.
142 changes: 101 additions & 41 deletions crates/editor/src/element.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()?;

Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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 => {}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 72a9429

Please sign in to comment.