Skip to content

Commit

Permalink
Improve code context menu layout position esp visual stability
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@mgsloan
mgsloan committed Dec 16, 2024
1 parent a94afbc commit 454d24e
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 98 deletions.
88 changes: 57 additions & 31 deletions crates/editor/src/code_context_menus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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, MouseButton, Pixels, ScrollStrategy, SharedString, StrikethroughStyle, StyledText,
Model, MouseButton, ScrollStrategy, SharedString, StrikethroughStyle, StyledText,
UniformListScrollHandle, ViewContext, WeakView,
};
use language::Buffer;
Expand Down Expand Up @@ -34,6 +34,16 @@ pub enum CodeContextMenu {
CodeActions(CodeActionsMenu),
}

pub struct CodeContextMenuLayoutInfo {
pub origin: CodeContextMenuOrigin,
pub y_padding: gpui::Pixels,
}

pub enum CodeContextMenuOrigin {
EditorPoint(DisplayPoint),
GutterIndicator(DisplayRow),
}

impl CodeContextMenu {
pub fn select_first(
&mut self,
Expand Down Expand Up @@ -106,31 +116,29 @@ impl CodeContextMenu {
}
}

pub fn layout_info(&self, cursor_position: DisplayPoint) -> CodeContextMenuLayoutInfo {
match self {
CodeContextMenu::Completions(menu) => menu.layout_info(cursor_position),
CodeContextMenu::CodeActions(menu) => menu.layout_info(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),
}
}
}

pub enum ContextMenuOrigin {
EditorPoint(DisplayPoint),
GutterIndicator(DisplayRow),
}

#[derive(Clone, Debug)]
pub struct CompletionsMenu {
pub id: CompletionId,
Expand Down Expand Up @@ -322,13 +330,22 @@ impl CompletionsMenu {
!self.matches.is_empty()
}

fn layout_info(&self, cursor_position: DisplayPoint) -> CodeContextMenuLayoutInfo {
CodeContextMenuLayoutInfo {
origin: CodeContextMenuOrigin::EditorPoint(cursor_position),
y_padding: Popover::y_padding(),
}
}

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 @@ -779,16 +796,31 @@ impl CodeActionsMenu {
!self.actions.is_empty()
}

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

fn y_padding() -> gpui::Pixels {
px(8.)
}

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 element = uniform_list(

uniform_list(
cx.view().clone(),
"code_actions_menu",
self.actions.len(),
Expand Down Expand Up @@ -859,8 +891,10 @@ impl CodeActionsMenu {
},
)
.elevation_1(cx)
.p_1()
.max_h(max_height)
// x padding is the same as y.
.px(Self::y_padding() / 2.)
.py(Self::y_padding() / 2.)
.max_h(max_height_in_lines as f32 * cx.line_height())
.occlude()
.track_scroll(self.scroll_handle.clone())
.with_width_from_item(
Expand All @@ -876,14 +910,6 @@ impl CodeActionsMenu {
.map(|(ix, _)| ix),
)
.with_sizing_behavior(ListSizingBehavior::Infer)
.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)
.into_any_element()
}
}
57 changes: 34 additions & 23 deletions crates/editor/src/editor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ use fuzzy::StringMatchCandidate;

use code_context_menus::{
AvailableCodeAction, CodeActionContents, CodeActionsItem, CodeActionsMenu, CodeContextMenu,
CompletionsMenu, ContextMenuOrigin,
CodeContextMenuLayoutInfo, CodeContextMenuOrigin, CompletionsMenu,
};
use git::blame::GitBlame;
use gpui::{
Expand Down 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 @@ -4998,28 +4996,41 @@ 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 render_context_menu(
fn context_menu_layout_info(
&self,
cursor_position: DisplayPoint,
) -> Option<CodeContextMenuLayoutInfo> {
self.context_menu
.borrow()
.as_ref()
.map(|menu| menu.layout_info(cursor_position))
}

fn render_context_menu(
&self,
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
Loading

0 comments on commit 454d24e

Please sign in to comment.