Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Improve layout of completions doc popover (#22337)

Browse source 
* Now more often displayed to the right. Resizes docs width if more
space than min width is available.

* If constrained in horizontal space and so displayed above/below resize
docs height to fit.

* Makes space for scrollbar and gap.

Layout is imperfect for viewport sizes smaller than the context menu,
left TODOs in the code for handling this. Wanted to get this change out
for feedback first.

Release Notes:

- N/A
This commit is contained in:
Michael Sloan 2024-12-21 14:22:15 -07:00 • committed by GitHub
parent 6b92e0b5da
commit dbb76100e5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 143 additions and 111 deletions
Download patch file Download diff file Expand all files Collapse all files

24
crates/editor/src/code_context_menus.rs
View file

@ -1,8 +1,8 @@
use fuzzy::{StringMatch, StringMatchCandidate}; use fuzzy::{StringMatch, StringMatchCandidate};
use gpui::{ use gpui::{
div, px, uniform_list, AnyElement, BackgroundExecutor, Div, FontWeight, ListSizingBehavior, div, px, uniform_list, AnyElement, BackgroundExecutor, Div, FontWeight, ListSizingBehavior,
Model, ScrollStrategy, SharedString, StrikethroughStyle, StyledText, UniformListScrollHandle, Model, ScrollStrategy, SharedString, Size, StrikethroughStyle, StyledText,
ViewContext, WeakView, UniformListScrollHandle, ViewContext, WeakView,
}; };
use language::Buffer; use language::Buffer;
use language::{CodeLabel, Documentation}; use language::{CodeLabel, Documentation};
@ -30,7 +30,10 @@ use crate::{
}; };
use crate::{AcceptInlineCompletion, InlineCompletionMenuHint, InlineCompletionText}; use crate::{AcceptInlineCompletion, InlineCompletionMenuHint, InlineCompletionText};
pub const MAX_COMPLETIONS_ASIDE_WIDTH: Pixels = px(500.); pub const MENU_GAP: Pixels = px(4.);
pub const MENU_ASIDE_X_PADDING: Pixels = px(16.);
pub const MENU_ASIDE_MIN_WIDTH: Pixels = px(260.);
pub const MENU_ASIDE_MAX_WIDTH: Pixels = px(500.);
pub enum CodeContextMenu { pub enum CodeContextMenu {
Completions(CompletionsMenu), Completions(CompletionsMenu),
@ -131,14 +134,12 @@ impl CodeContextMenu {
pub fn render_aside( pub fn render_aside(
&self, &self,
style: &EditorStyle, style: &EditorStyle,
max_height: Pixels, max_size: Size<Pixels>,
workspace: Option<WeakView<Workspace>>, workspace: Option<WeakView<Workspace>>,
cx: &mut ViewContext<Editor>, cx: &mut ViewContext<Editor>,
) -> Option<AnyElement> { ) -> Option<AnyElement> {
match self { match self {
CodeContextMenu::Completions(menu) => { CodeContextMenu::Completions(menu) => menu.render_aside(style, max_size, workspace, cx),
menu.render_aside(style, max_height, workspace, cx)
}
CodeContextMenu::CodeActions(_) => None, CodeContextMenu::CodeActions(_) => None,
} }
} }
@ -613,7 +614,7 @@ impl CompletionsMenu {
fn render_aside( fn render_aside(
&self, &self,
style: &EditorStyle, style: &EditorStyle,
max_height: Pixels, max_size: Size<Pixels>,
workspace: Option<WeakView<Workspace>>, workspace: Option<WeakView<Workspace>>,
cx: &mut ViewContext<Editor>, cx: &mut ViewContext<Editor>,
) -> Option<AnyElement> { ) -> Option<AnyElement> {
@ -663,10 +664,9 @@ impl CompletionsMenu {
.child( .child(
multiline_docs multiline_docs
.id("multiline_docs") .id("multiline_docs")
.max_h(max_height) .px(MENU_ASIDE_X_PADDING / 2.)
.px_2() .max_w(max_size.width)
.min_w(px(260.)) .max_h(max_size.height)
.max_w(MAX_COMPLETIONS_ASIDE_WIDTH)
.overflow_y_scroll() .overflow_y_scroll()
.occlude(), .occlude(),
) )

4
crates/editor/src/editor.rs
View file

@ -5133,14 +5133,14 @@ impl Editor {
fn render_context_menu_aside( fn render_context_menu_aside(
&self, &self,
style: &EditorStyle, style: &EditorStyle,
max_height: Pixels, max_size: Size<Pixels>,
cx: &mut ViewContext<Editor>, cx: &mut ViewContext<Editor>,
) -> Option<AnyElement> { ) -> Option<AnyElement> {
self.context_menu.borrow().as_ref().and_then(|menu| { self.context_menu.borrow().as_ref().and_then(|menu| {
if menu.visible() { if menu.visible() {
menu.render_aside( menu.render_aside(
style, style,
max_height, max_size,
self.workspace.as_ref().map(|(w, _)| w.clone()), self.workspace.as_ref().map(|(w, _)| w.clone()),
cx, cx,
) )

156
crates/editor/src/element.rs
View file

@ -1,6 +1,6 @@
use crate::{ use crate::{
blame_entry_tooltip::{blame_entry_relative_timestamp, BlameEntryTooltip}, blame_entry_tooltip::{blame_entry_relative_timestamp, BlameEntryTooltip},
code_context_menus::{CodeActionsMenu, MAX_COMPLETIONS_ASIDE_WIDTH}, code_context_menus::{CodeActionsMenu, MENU_ASIDE_MAX_WIDTH, MENU_ASIDE_MIN_WIDTH, MENU_GAP},
display_map::{ display_map::{
Block, BlockContext, BlockStyle, DisplaySnapshot, HighlightedChunk, ToDisplayPoint, Block, BlockContext, BlockStyle, DisplaySnapshot, HighlightedChunk, ToDisplayPoint,
}, },
@ -2931,6 +2931,11 @@ impl EditorElement {
} }
}; };
let viewport_bounds = Bounds::new(Default::default(), cx.viewport_size()).extend(Edges {
right: -Self::SCROLLBAR_WIDTH - MENU_GAP,
..Default::default()
});
// If the context menu's max height won't fit below, then flip it above the line and display // 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. // it in reverse order. If the available space above is less than below.
let unconstrained_max_height = line_height * 12. + POPOVER_Y_PADDING; let unconstrained_max_height = line_height * 12. + POPOVER_Y_PADDING;
@ -2952,7 +2957,7 @@ impl EditorElement {
// If less than 3 lines fit within the text bounds, instead fit within the window. // If less than 3 lines fit within the text bounds, instead fit within the window.
if height < min_height { if height < min_height {
let available_above = bottom_y_when_flipped; let available_above = bottom_y_when_flipped;
let available_below = cx.viewport_size().height - target_position.y; let available_below = viewport_bounds.bottom() - target_position.y;
if available_below > 3. * line_height { if available_below > 3. * line_height {
y_is_flipped = false; y_is_flipped = false;
height = min_height; height = min_height;
@ -2970,6 +2975,7 @@ impl EditorElement {
let max_height_in_lines = ((height - POPOVER_Y_PADDING) / line_height).floor() as u32; let max_height_in_lines = ((height - POPOVER_Y_PADDING) / line_height).floor() as u32;
// TODO(mgsloan): use viewport_bounds.width as a max width when rendering menu.
let Some(mut menu_element) = self.editor.update(cx, |editor, cx| { let Some(mut menu_element) = self.editor.update(cx, |editor, cx| {
editor.render_context_menu(&self.style, max_height_in_lines, cx) editor.render_context_menu(&self.style, max_height_in_lines, cx)
}) else { }) else {
@ -2978,13 +2984,11 @@ impl EditorElement {
let menu_size = menu_element.layout_as_root(AvailableSpace::min_size(), cx); let menu_size = menu_element.layout_as_root(AvailableSpace::min_size(), cx);
let menu_position = gpui::Point { 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 // Snap the right edge of the list to the right edge of the window if its horizontal bounds
// overflow. // overflow. Include space for the scrollbar.
(cx.viewport_size().width - menu_size.width).max(Pixels::ZERO) x: target_position
} else { .x
target_position.x .min((viewport_bounds.right() - menu_size.width).max(Pixels::ZERO)),
},
y: if y_is_flipped { y: if y_is_flipped {
bottom_y_when_flipped - menu_size.height bottom_y_when_flipped - menu_size.height
} else { } else {
@ -2993,10 +2997,7 @@ impl EditorElement {
}; };
cx.defer_draw(menu_element, menu_position, 1); cx.defer_draw(menu_element, menu_position, 1);
let aside_element = self.editor.update(cx, |editor, cx| { // Layout documentation aside
editor.render_context_menu_aside(&self.style, unconstrained_max_height, cx)
});
if let Some(aside_element) = aside_element {
let menu_bounds = Bounds::new(menu_position, menu_size); let menu_bounds = Bounds::new(menu_position, menu_size);
let max_menu_size = size(menu_size.width, unconstrained_max_height); let max_menu_size = size(menu_size.width, unconstrained_max_height);
let max_menu_bounds = if y_is_flipped { let max_menu_bounds = if y_is_flipped {
@ -3010,27 +3011,18 @@ impl EditorElement {
} else { } else {
Bounds::new(target_position, max_menu_size) Bounds::new(target_position, max_menu_size)
}; };
// Pad the target by 4 pixels to create a gap.
let mut extend_amount = Edges::all(px(4.));
// Extend to include the cursored line to avoid overlapping it.
if y_is_flipped {
extend_amount.bottom = line_height;
} else {
extend_amount.top = line_height;
}
self.layout_context_menu_aside( self.layout_context_menu_aside(
text_hitbox, text_hitbox,
y_is_flipped, y_is_flipped,
menu_position, menu_position,
menu_bounds.extend(extend_amount), menu_bounds,
max_menu_bounds.extend(extend_amount), max_menu_bounds,
unconstrained_max_height, unconstrained_max_height,
aside_element, line_height,
viewport_bounds,
cx, cx,
); );
} }
}
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
fn layout_context_menu_aside( fn layout_context_menu_aside(
@ -3038,35 +3030,65 @@ impl EditorElement {
text_hitbox: &Hitbox, text_hitbox: &Hitbox,
y_is_flipped: bool, y_is_flipped: bool,
menu_position: gpui::Point<Pixels>, menu_position: gpui::Point<Pixels>,
target_bounds: Bounds<Pixels>, menu_bounds: Bounds<Pixels>,
max_target_bounds: Bounds<Pixels>, max_menu_bounds: Bounds<Pixels>,
max_height: Pixels, max_height: Pixels,
aside: AnyElement, line_height: Pixels,
viewport_bounds: Bounds<Pixels>,
cx: &mut WindowContext, cx: &mut WindowContext,
) { ) {
let mut aside = aside; let mut extend_amount = Edges::all(MENU_GAP);
// Extend to include the cursored line to avoid overlapping it.
if y_is_flipped {
extend_amount.bottom = line_height;
} else {
extend_amount.top = line_height;
}
let target_bounds = menu_bounds.extend(extend_amount);
let max_target_bounds = max_menu_bounds.extend(extend_amount);
let available_within_viewport = target_bounds.space_within(&viewport_bounds);
let positioned_aside = if available_within_viewport.right >= MENU_ASIDE_MIN_WIDTH {
let max_width = cmp::min(
available_within_viewport.right - px(1.),
MENU_ASIDE_MAX_WIDTH,
);
let Some(mut aside) =
self.render_context_menu_aside(size(max_width, max_height - POPOVER_Y_PADDING), cx)
else {
return;
};
aside.layout_as_root(AvailableSpace::min_size(), cx);
let right_position = point(target_bounds.right(), menu_position.y);
Some((aside, right_position))
} else {
let max_size = size(
// TODO(mgsloan): Once the menu is bounded by viewport width the bound on viewport
// won't be needed here.
cmp::min(
cmp::max(menu_bounds.size.width - px(2.), MENU_ASIDE_MIN_WIDTH),
viewport_bounds.right(),
),
cmp::min(
max_height,
cmp::max(
available_within_viewport.top,
available_within_viewport.bottom,
),
) - POPOVER_Y_PADDING,
);
let Some(mut aside) = self.render_context_menu_aside(max_size, cx) else {
return;
};
let actual_size = aside.layout_as_root(AvailableSpace::min_size(), cx); let actual_size = aside.layout_as_root(AvailableSpace::min_size(), cx);
// Snap to right side of window if it would overflow. let top_position = point(menu_position.x, target_bounds.top() - actual_size.height);
let aside_x = cmp::min( let bottom_position = point(menu_position.x, target_bounds.bottom());
menu_position.x,
cx.viewport_size().width - actual_size.width,
);
if aside_x < px(0.) {
// Not enough space, skip drawing.
return;
}
let top_position = point(aside_x, target_bounds.top() - actual_size.height); let fit_within = |available: Edges<Pixels>, wanted: Size<Pixels>| {
let bottom_position = point(aside_x, target_bounds.bottom()); // Prefer to fit on the same side of the line as the menu, then on the other side of
let right_position = point(target_bounds.right(), menu_position.y); // the line.
if !y_is_flipped && wanted.height < available.bottom {
let fit_horizontally_within = |available: Edges<Pixels>, wanted: Size<Pixels>| {
// Prefer to fit to the right, then on the same side of the line as the menu, then on
// the other side of the line.
if wanted.width < available.right {
Some(right_position)
} else if !y_is_flipped && wanted.height < available.bottom {
Some(bottom_position) Some(bottom_position)
} else if !y_is_flipped && wanted.height < available.top { } else if !y_is_flipped && wanted.height < available.top {
Some(top_position) Some(top_position)
@ -3080,24 +3102,34 @@ impl EditorElement {
}; };
// Prefer choosing a direction using max sizes rather than actual size for stability. // Prefer choosing a direction using max sizes rather than actual size for stability.
let mut available = max_target_bounds.space_within(&text_hitbox.bounds); let available_within_text = max_target_bounds.space_within(&text_hitbox.bounds);
let mut wanted = size(MAX_COMPLETIONS_ASIDE_WIDTH, max_height); let wanted = size(MENU_ASIDE_MAX_WIDTH, max_height);
let aside_position = fit_horizontally_within(available, wanted) let aside_position = fit_within(available_within_text, wanted)
.or_else(|| {
// Fallback: fit max size in window. // Fallback: fit max size in window.
available = max_target_bounds .or_else(|| fit_within(max_target_bounds.space_within(&viewport_bounds), wanted))
.space_within(&Bounds::new(Default::default(), cx.viewport_size()));
fit_horizontally_within(available, wanted)
})
.or_else(|| {
// Fallback: fit actual size in window. // Fallback: fit actual size in window.
wanted = actual_size; .or_else(|| fit_within(available_within_viewport, actual_size));
fit_horizontally_within(available, wanted)
}); aside_position.map(|position| (aside, position))
};
// Skip drawing if it doesn't fit anywhere. // Skip drawing if it doesn't fit anywhere.
if let Some(aside_position) = aside_position { if let Some((aside, position)) = positioned_aside {
cx.defer_draw(aside, aside_position, 1); cx.defer_draw(aside, position, 1);
}
}
fn render_context_menu_aside(
&self,
max_size: Size<Pixels>,
cx: &mut WindowContext,
) -> Option<AnyElement> {
if max_size.width < px(100.) || max_size.height < px(12.) {
None
} else {
self.editor.update(cx, |editor, cx| {
editor.render_context_menu_aside(&self.style, max_size, cx)
})
} }
} }