Skip to content

Commit

Permalink
Improve code context menu layout position esp visual stability (zed-…
Browse files Browse the repository at this point in the history 
…industries#22102)

* Now decides whether the menu is above or below the target position
before rendering it. This causes its position to no longer vary
depending on the length of completions

* When the text area is height constrained (< 12) lines, now chooses the
side which has the most space. Before it would always display above if
height constrained below.

* Misc code cleanups

Release Notes:

- Improved completions menu layout to be more stable and use available
space better.
  • Loading branch information
@mgsloan
mgsloan authored Dec 17, 2024
1 parent fc5a810 commit a062c0f
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 93 deletions.
59 changes: 33 additions & 26 deletions crates/editor/src/code_context_menus.rs
View file
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
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
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

0 comments on commit a062c0f

Please sign in to comment.