Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve code context menu layout position esp visual stability #22102

Merged
merged 1 commit into from
Dec 17, 2024
Merged

Improve code context menu layout position esp visual stability #22102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 33 additions & 26 deletions crates/editor/src/code_context_menus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use std::{cell::Cell, cmp::Reverse, ops::Range, rc::Rc};
use fuzzy::{StringMatch, StringMatchCandidate};
use gpui::{
div, px, uniform_list, AnyElement, BackgroundExecutor, Div, FontWeight, ListSizingBehavior,
Model, Pixels, ScrollStrategy, SharedString, StrikethroughStyle, StyledText,
UniformListScrollHandle, ViewContext, WeakView,
Model, ScrollStrategy, SharedString, StrikethroughStyle, StyledText, UniformListScrollHandle,
ViewContext, WeakView,
};
use language::Buffer;
use language::{CodeLabel, Documentation};
Expand Down Expand Up @@ -106,22 +106,24 @@ impl CodeContextMenu {
}
}

pub fn origin(&self, cursor_position: DisplayPoint) -> ContextMenuOrigin {
match self {
CodeContextMenu::Completions(menu) => menu.origin(cursor_position),
CodeContextMenu::CodeActions(menu) => menu.origin(cursor_position),
}
}
pub fn render(
&self,
cursor_position: DisplayPoint,
style: &EditorStyle,
max_height: Pixels,
max_height_in_lines: u32,
workspace: Option<WeakView<Workspace>>,
cx: &mut ViewContext<Editor>,
) -> (ContextMenuOrigin, AnyElement) {
) -> AnyElement {
match self {
CodeContextMenu::Completions(menu) => (
ContextMenuOrigin::EditorPoint(cursor_position),
menu.render(style, max_height, workspace, cx),
),
CodeContextMenu::CodeActions(menu) => {
menu.render(cursor_position, style, max_height, cx)
CodeContextMenu::Completions(menu) => {
menu.render(style, max_height_in_lines, workspace, cx)
}
CodeContextMenu::CodeActions(menu) => menu.render(style, max_height_in_lines, cx),
}
}
}
Expand Down Expand Up @@ -322,13 +324,19 @@ impl CompletionsMenu {
!self.matches.is_empty()
}

fn origin(&self, cursor_position: DisplayPoint) -> ContextMenuOrigin {
ContextMenuOrigin::EditorPoint(cursor_position)
}

fn render(
&self,
style: &EditorStyle,
max_height: Pixels,
max_height_in_lines: u32,
workspace: Option<WeakView<Workspace>>,
cx: &mut ViewContext<Editor>,
) -> AnyElement {
let max_height = max_height_in_lines as f32 * cx.line_height();

let completions = self.completions.borrow_mut();
let show_completion_documentation = self.show_completion_documentation;
let widest_completion_ix = self
Expand Down Expand Up @@ -496,7 +504,7 @@ impl CompletionsMenu {
},
)
.occlude()
.max_h(max_height)
.max_h(max_height_in_lines as f32 * cx.line_height())
.track_scroll(self.scroll_handle.clone())
.with_width_from_item(widest_completion_ix)
.with_sizing_behavior(ListSizingBehavior::Infer);
Expand Down Expand Up @@ -779,13 +787,20 @@ impl CodeActionsMenu {
!self.actions.is_empty()
}

fn origin(&self, cursor_position: DisplayPoint) -> ContextMenuOrigin {
if let Some(row) = self.deployed_from_indicator {
ContextMenuOrigin::GutterIndicator(row)
} else {
ContextMenuOrigin::EditorPoint(cursor_position)
}
}

fn render(
&self,
cursor_position: DisplayPoint,
_style: &EditorStyle,
max_height: Pixels,
max_height_in_lines: u32,
cx: &mut ViewContext<Editor>,
) -> (ContextMenuOrigin, AnyElement) {
) -> AnyElement {
let actions = self.actions.clone();
let selected_item = self.selected_item;
let list = uniform_list(
Expand Down Expand Up @@ -857,7 +872,7 @@ impl CodeActionsMenu {
},
)
.occlude()
.max_h(max_height)
.max_h(max_height_in_lines as f32 * cx.line_height())
.track_scroll(self.scroll_handle.clone())
.with_width_from_item(
self.actions
Expand All @@ -873,14 +888,6 @@ impl CodeActionsMenu {
)
.with_sizing_behavior(ListSizingBehavior::Infer);

let element = Popover::new().child(list).into_any_element();

let cursor_position = if let Some(row) = self.deployed_from_indicator {
ContextMenuOrigin::GutterIndicator(row)
} else {
ContextMenuOrigin::EditorPoint(cursor_position)
};

(cursor_position, element)
Popover::new().child(list).into_any_element()
}
}
52 changes: 30 additions & 22 deletions crates/editor/src/editor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,18 +1382,16 @@ impl Editor {
if self.pending_rename.is_some() {
key_context.add("renaming");
}
if self.context_menu_visible() {
match self.context_menu.borrow().as_ref() {
Some(CodeContextMenu::Completions(_)) => {
key_context.add("menu");
key_context.add("showing_completions")
}
Some(CodeContextMenu::CodeActions(_)) => {
key_context.add("menu");
key_context.add("showing_code_actions")
}
None => {}
match self.context_menu.borrow().as_ref() {
Some(CodeContextMenu::Completions(_)) => {
key_context.add("menu");
key_context.add("showing_completions")
}
Some(CodeContextMenu::CodeActions(_)) => {
key_context.add("menu");
key_context.add("showing_code_actions")
}
None => {}
}

// Disable vim contexts when a sub-editor (e.g. rename/inline assistant) is focused.
Expand Down Expand Up @@ -4999,28 +4997,38 @@ impl Editor {
}))
}

#[cfg(feature = "test-support")]
pub fn context_menu_visible(&self) -> bool {
self.context_menu
.borrow()
.as_ref()
.map_or(false, |menu| menu.visible())
}

fn context_menu_origin(&self, cursor_position: DisplayPoint) -> Option<ContextMenuOrigin> {
self.context_menu
.borrow()
.as_ref()
.map(|menu| menu.origin(cursor_position))
}

fn render_context_menu(
&self,
cursor_position: DisplayPoint,
style: &EditorStyle,
max_height: Pixels,
max_height_in_lines: u32,
cx: &mut ViewContext<Editor>,
) -> Option<(ContextMenuOrigin, AnyElement)> {
self.context_menu.borrow().as_ref().map(|menu| {
menu.render(
cursor_position,
style,
max_height,
self.workspace.as_ref().map(|(w, _)| w.clone()),
cx,
)
) -> Option<AnyElement> {
self.context_menu.borrow().as_ref().and_then(|menu| {
if menu.visible() {
Some(menu.render(
style,
max_height_in_lines,
self.workspace.as_ref().map(|(w, _)| w.clone()),
cx,
))
} else {
None
}
})
}

Expand Down
125 changes: 82 additions & 43 deletions crates/editor/src/element.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ use std::{
};
use sum_tree::Bias;
use theme::{ActiveTheme, Appearance, PlayerColor};
use ui::prelude::*;
use ui::{h_flex, ButtonLike, ButtonStyle, ContextMenu, Tooltip};
use ui::{prelude::*, POPOVER_Y_PADDING};
use unicode_segmentation::UnicodeSegmentation;
use util::RangeExt;
use util::ResultExt;
Expand Down Expand Up @@ -2771,7 +2771,6 @@ impl EditorElement {
fn layout_context_menu(
&self,
line_height: Pixels,
hitbox: &Hitbox,
text_hitbox: &Hitbox,
content_origin: gpui::Point<Pixels>,
start_row: DisplayRow,
Expand All @@ -2780,56 +2779,98 @@ impl EditorElement {
newest_selection_head: DisplayPoint,
gutter_overshoot: Pixels,
cx: &mut WindowContext,
) -> bool {
let max_height = cmp::min(
12. * line_height,
cmp::max(3. * line_height, (hitbox.size.height - line_height) / 2.),
);
let Some((position, mut context_menu)) = self.editor.update(cx, |editor, cx| {
if editor.context_menu_visible() {
editor.render_context_menu(newest_selection_head, &self.style, max_height, cx)
} else {
None
}
}) else {
return false;
) {
let Some(context_menu_origin) = self
.editor
.read(cx)
.context_menu_origin(newest_selection_head)
else {
return;
};

let context_menu_size = context_menu.layout_as_root(AvailableSpace::min_size(), cx);

let (x, y) = match position {
crate::ContextMenuOrigin::EditorPoint(point) => {
let cursor_row_layout = &line_layouts[point.row().minus(start_row) as usize];
let x = cursor_row_layout.x_for_index(point.column() as usize)
- scroll_pixel_position.x;
let y = point.row().next_row().as_f32() * line_height - scroll_pixel_position.y;
(x, y)
let target_offset = match context_menu_origin {
crate::ContextMenuOrigin::EditorPoint(display_point) => {
let cursor_row_layout =
&line_layouts[display_point.row().minus(start_row) as usize];
gpui::Point {
x: cursor_row_layout.x_for_index(display_point.column() as usize)
- scroll_pixel_position.x,
y: display_point.row().next_row().as_f32() * line_height
- scroll_pixel_position.y,
}
}
crate::ContextMenuOrigin::GutterIndicator(row) => {
// Context menu was spawned via a click on a gutter. Ensure it's a bit closer to the indicator than just a plain first column of the
// text field.
let x = -gutter_overshoot;
let y = row.next_row().as_f32() * line_height - scroll_pixel_position.y;
(x, y)
gpui::Point {
x: -gutter_overshoot,
y: row.next_row().as_f32() * line_height - scroll_pixel_position.y,
}
}
};

let mut list_origin = content_origin + point(x, y);
let list_width = context_menu_size.width;
let list_height = context_menu_size.height;
// If the context menu's max height won't fit below, then flip it above the line and display
// it in reverse order. If the available space above is less than below.
let unconstrained_max_height = line_height * 12. + POPOVER_Y_PADDING;
let min_height = line_height * 3. + POPOVER_Y_PADDING;
let target_position = content_origin + target_offset;
let y_overflows_below = target_position.y + unconstrained_max_height > text_hitbox.bottom();
let bottom_y_when_flipped = target_position.y - line_height;
let available_above = bottom_y_when_flipped - text_hitbox.top();
let available_below = text_hitbox.bottom() - target_position.y;
let mut y_is_flipped = y_overflows_below && available_above > available_below;
let mut max_height = cmp::min(
unconstrained_max_height,
if y_is_flipped {
available_above
} else {
available_below
},
);

// Snap the right edge of the list to the right edge of the window if
// its horizontal bounds overflow.
if list_origin.x + list_width > cx.viewport_size().width {
list_origin.x = (cx.viewport_size().width - list_width).max(Pixels::ZERO);
// If less than 3 lines fit within the text bounds, instead fit within the window.
if max_height < 3. * line_height {
let available_above = bottom_y_when_flipped;
let available_below = cx.viewport_size().height - target_position.y;
if available_below > 3. * line_height {
y_is_flipped = false;
max_height = min_height;
} else if available_above > 3. * line_height {
y_is_flipped = true;
max_height = min_height;
} else if available_above > available_below {
y_is_flipped = true;
max_height = available_above;
} else {
y_is_flipped = false;
max_height = available_below;
}
}

if list_origin.y + list_height > text_hitbox.bottom_right().y {
list_origin.y -= line_height + list_height;
}
let max_height_in_lines = ((max_height - POPOVER_Y_PADDING) / line_height).floor() as u32;

let Some(mut menu) = self.editor.update(cx, |editor, cx| {
editor.render_context_menu(&self.style, max_height_in_lines, cx)
}) else {
return;
};

let menu_size = menu.layout_as_root(AvailableSpace::min_size(), cx);
let menu_position = gpui::Point {
x: if target_position.x + menu_size.width > cx.viewport_size().width {
// Snap the right edge of the list to the right edge of the window if its horizontal bounds
// overflow.
(cx.viewport_size().width - menu_size.width).max(Pixels::ZERO)
} else {
target_position.x
},
y: if y_is_flipped {
bottom_y_when_flipped - menu_size.height
} else {
target_position.y
},
};

cx.defer_draw(context_menu, list_origin, 1);
true
cx.defer_draw(menu, menu_position, 1);
}

#[allow(clippy::too_many_arguments)]
Expand Down Expand Up @@ -5893,13 +5934,11 @@ impl Element for EditorElement {
rows_with_hunk_bounds
},
);
let mut _context_menu_visible = false;
let mut code_actions_indicator = None;
if let Some(newest_selection_head) = newest_selection_head {
if (start_row..end_row).contains(&newest_selection_head.row()) {
_context_menu_visible = self.layout_context_menu(
self.layout_context_menu(
line_height,
&hitbox,
&text_hitbox,
content_origin,
start_row,
Expand Down
Loading
Loading