When completions menu is displayed above cursor, reverse order (#23446)

Michael Sloan created

Considered doing this when previously working on completions menu
layout, as it brings the default selection position next to the cursor
position, and is generally more symmetrical. With #23445 there is now a
more compelling reason, as the "translucent, cropped bottom" display
doesn't make sense when displayed above.

Release Notes:

- N/A

Change summary

crates/editor/src/code_context_menus.rs  | 103 ++++++++++++++++++++-----
crates/editor/src/editor.rs              |   3 
crates/editor/src/element.rs             |  32 ++++----
crates/gpui/src/elements/uniform_list.rs |  57 +++++++++++++-
crates/gpui/src/geometry.rs              |   2 
5 files changed, 152 insertions(+), 45 deletions(-)

Detailed changes

crates/editor/src/code_context_menus.rs 🔗

@@ -125,11 +125,16 @@ impl CodeContextMenu {
         &self,
         style: &EditorStyle,
         max_height_in_lines: u32,
+        y_flipped: bool,
         cx: &mut ViewContext<Editor>,
     ) -> AnyElement {
         match self {
-            CodeContextMenu::Completions(menu) => menu.render(style, max_height_in_lines, cx),
-            CodeContextMenu::CodeActions(menu) => menu.render(style, max_height_in_lines, cx),
+            CodeContextMenu::Completions(menu) => {
+                menu.render(style, max_height_in_lines, y_flipped, cx)
+            }
+            CodeContextMenu::CodeActions(menu) => {
+                menu.render(style, max_height_in_lines, y_flipped, cx)
+            }
         }
     }
 
@@ -268,31 +273,50 @@ impl CompletionsMenu {
         provider: Option<&dyn CompletionProvider>,
         cx: &mut ViewContext<Editor>,
     ) {
-        self.update_selection_index(0, provider, cx);
+        let index = if self.scroll_handle.y_flipped() {
+            self.entries.borrow().len() - 1
+        } else {
+            0
+        };
+        self.update_selection_index(index, provider, cx);
     }
 
-    fn select_prev(
+    fn select_last(
         &mut self,
         provider: Option<&dyn CompletionProvider>,
         cx: &mut ViewContext<Editor>,
     ) {
-        self.update_selection_index(self.prev_match_index(), provider, cx);
+        let index = if self.scroll_handle.y_flipped() {
+            0
+        } else {
+            self.entries.borrow().len() - 1
+        };
+        self.update_selection_index(index, provider, cx);
     }
 
-    fn select_next(
+    fn select_prev(
         &mut self,
         provider: Option<&dyn CompletionProvider>,
         cx: &mut ViewContext<Editor>,
     ) {
-        self.update_selection_index(self.next_match_index(), provider, cx);
+        let index = if self.scroll_handle.y_flipped() {
+            self.next_match_index()
+        } else {
+            self.prev_match_index()
+        };
+        self.update_selection_index(index, provider, cx);
     }
 
-    fn select_last(
+    fn select_next(
         &mut self,
         provider: Option<&dyn CompletionProvider>,
         cx: &mut ViewContext<Editor>,
     ) {
-        let index = self.entries.borrow().len() - 1;
+        let index = if self.scroll_handle.y_flipped() {
+            self.prev_match_index()
+        } else {
+            self.next_match_index()
+        };
         self.update_selection_index(index, provider, cx);
     }
 
@@ -336,6 +360,11 @@ impl CompletionsMenu {
             }
             _ => {
                 entries.insert(0, hint);
+                // When `y_flipped`, need to scroll to bring it into view.
+                if self.selected_item == 0 {
+                    self.scroll_handle
+                        .scroll_to_item(self.selected_item, ScrollStrategy::Top);
+                }
             }
         }
     }
@@ -439,6 +468,7 @@ impl CompletionsMenu {
         &self,
         style: &EditorStyle,
         max_height_in_lines: u32,
+        y_flipped: bool,
         cx: &mut ViewContext<Editor>,
     ) -> AnyElement {
         let completions = self.completions.borrow_mut();
@@ -664,6 +694,7 @@ impl CompletionsMenu {
         .occlude()
         .max_h(max_height_in_lines as f32 * cx.line_height())
         .track_scroll(self.scroll_handle.clone())
+        .y_flipped(y_flipped)
         .with_width_from_item(widest_completion_ix)
         .with_sizing_behavior(ListSizingBehavior::Infer);
 
@@ -981,39 +1012,63 @@ pub struct CodeActionsMenu {
 
 impl CodeActionsMenu {
     fn select_first(&mut self, cx: &mut ViewContext<Editor>) {
-        self.selected_item = 0;
+        self.selected_item = if self.scroll_handle.y_flipped() {
+            self.actions.len() - 1
+        } else {
+            0
+        };
+        self.scroll_handle
+            .scroll_to_item(self.selected_item, ScrollStrategy::Top);
+        cx.notify()
+    }
+
+    fn select_last(&mut self, cx: &mut ViewContext<Editor>) {
+        self.selected_item = if self.scroll_handle.y_flipped() {
+            0
+        } else {
+            self.actions.len() - 1
+        };
         self.scroll_handle
             .scroll_to_item(self.selected_item, ScrollStrategy::Top);
         cx.notify()
     }
 
     fn select_prev(&mut self, cx: &mut ViewContext<Editor>) {
-        if self.selected_item > 0 {
-            self.selected_item -= 1;
+        self.selected_item = if self.scroll_handle.y_flipped() {
+            self.next_match_index()
         } else {
-            self.selected_item = self.actions.len() - 1;
-        }
+            self.prev_match_index()
+        };
         self.scroll_handle
             .scroll_to_item(self.selected_item, ScrollStrategy::Top);
         cx.notify();
     }
 
     fn select_next(&mut self, cx: &mut ViewContext<Editor>) {
-        if self.selected_item + 1 < self.actions.len() {
-            self.selected_item += 1;
+        self.selected_item = if self.scroll_handle.y_flipped() {
+            self.prev_match_index()
         } else {
-            self.selected_item = 0;
-        }
+            self.next_match_index()
+        };
         self.scroll_handle
             .scroll_to_item(self.selected_item, ScrollStrategy::Top);
         cx.notify();
     }
 
-    fn select_last(&mut self, cx: &mut ViewContext<Editor>) {
-        self.selected_item = self.actions.len() - 1;
-        self.scroll_handle
-            .scroll_to_item(self.selected_item, ScrollStrategy::Top);
-        cx.notify()
+    fn prev_match_index(&self) -> usize {
+        if self.selected_item > 0 {
+            self.selected_item - 1
+        } else {
+            self.actions.len() - 1
+        }
+    }
+
+    fn next_match_index(&self) -> usize {
+        if self.selected_item + 1 < self.actions.len() {
+            self.selected_item + 1
+        } else {
+            0
+        }
     }
 
     fn visible(&self) -> bool {
@@ -1032,6 +1087,7 @@ impl CodeActionsMenu {
         &self,
         _style: &EditorStyle,
         max_height_in_lines: u32,
+        y_flipped: bool,
         cx: &mut ViewContext<Editor>,
     ) -> AnyElement {
         let actions = self.actions.clone();
@@ -1107,6 +1163,7 @@ impl CodeActionsMenu {
         .occlude()
         .max_h(max_height_in_lines as f32 * cx.line_height())
         .track_scroll(self.scroll_handle.clone())
+        .y_flipped(y_flipped)
         .with_width_from_item(
             self.actions
                 .iter()

crates/editor/src/editor.rs 🔗

@@ -5246,11 +5246,12 @@ impl Editor {
         &self,
         style: &EditorStyle,
         max_height_in_lines: u32,
+        y_flipped: bool,
         cx: &mut ViewContext<Editor>,
     ) -> Option<AnyElement> {
         self.context_menu.borrow().as_ref().and_then(|menu| {
             if menu.visible() {
-                Some(menu.render(style, max_height_in_lines, cx))
+                Some(menu.render(style, max_height_in_lines, y_flipped, cx))
             } else {
                 None
             }

crates/editor/src/element.rs 🔗

@@ -3136,10 +3136,10 @@ impl EditorElement {
         let available_above = bottom_y_when_flipped - text_hitbox.top();
         let available_below = text_hitbox.bottom() - target_position.y;
         let y_overflows_below = unconstrained_max_height > available_below;
-        let mut y_is_flipped = y_overflows_below && available_above > available_below;
+        let mut y_flipped = y_overflows_below && available_above > available_below;
         let mut height = cmp::min(
             unconstrained_max_height,
-            if y_is_flipped {
+            if y_flipped {
                 available_above
             } else {
                 available_below
@@ -3151,16 +3151,16 @@ impl EditorElement {
             let available_above = bottom_y_when_flipped;
             let available_below = viewport_bounds.bottom() - target_position.y;
             if available_below > 3. * line_height {
-                y_is_flipped = false;
+                y_flipped = false;
                 height = min_height;
             } else if available_above > 3. * line_height {
-                y_is_flipped = true;
+                y_flipped = true;
                 height = min_height;
             } else if available_above > available_below {
-                y_is_flipped = true;
+                y_flipped = true;
                 height = available_above;
             } else {
-                y_is_flipped = false;
+                y_flipped = false;
                 height = available_below;
             }
         }
@@ -3169,7 +3169,7 @@ impl EditorElement {
 
         // TODO(mgsloan): use viewport_bounds.width as a max width when rendering menu.
         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, y_flipped, cx)
         }) else {
             return;
         };
@@ -3181,7 +3181,7 @@ impl EditorElement {
             x: target_position
                 .x
                 .min((viewport_bounds.right() - menu_size.width).max(Pixels::ZERO)),
-            y: if y_is_flipped {
+            y: if y_flipped {
                 bottom_y_when_flipped - menu_size.height
             } else {
                 target_position.y
@@ -3192,7 +3192,7 @@ impl EditorElement {
         // Layout documentation aside
         let menu_bounds = Bounds::new(menu_position, menu_size);
         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_flipped {
             Bounds::new(
                 point(
                     menu_position.x,
@@ -3205,7 +3205,7 @@ impl EditorElement {
         };
         self.layout_context_menu_aside(
             text_hitbox,
-            y_is_flipped,
+            y_flipped,
             menu_position,
             menu_bounds,
             max_menu_bounds,
@@ -3220,7 +3220,7 @@ impl EditorElement {
     fn layout_context_menu_aside(
         &self,
         text_hitbox: &Hitbox,
-        y_is_flipped: bool,
+        y_flipped: bool,
         menu_position: gpui::Point<Pixels>,
         menu_bounds: Bounds<Pixels>,
         max_menu_bounds: Bounds<Pixels>,
@@ -3231,7 +3231,7 @@ impl EditorElement {
     ) {
         let mut extend_amount = Edges::all(MENU_GAP);
         // Extend to include the cursored line to avoid overlapping it.
-        if y_is_flipped {
+        if y_flipped {
             extend_amount.bottom = line_height;
         } else {
             extend_amount.top = line_height;
@@ -3280,13 +3280,13 @@ impl EditorElement {
             let fit_within = |available: Edges<Pixels>, wanted: Size<Pixels>| {
                 // Prefer to fit on the same side of the line as the menu, then on the other side of
                 // the line.
-                if !y_is_flipped && wanted.height < available.bottom {
+                if !y_flipped && wanted.height < available.bottom {
                     Some(bottom_position)
-                } else if !y_is_flipped && wanted.height < available.top {
+                } else if !y_flipped && wanted.height < available.top {
                     Some(top_position)
-                } else if y_is_flipped && wanted.height < available.top {
+                } else if y_flipped && wanted.height < available.top {
                     Some(top_position)
-                } else if y_is_flipped && wanted.height < available.bottom {
+                } else if y_flipped && wanted.height < available.bottom {
                     Some(bottom_position)
                 } else {
                     None

crates/gpui/src/elements/uniform_list.rs 🔗

@@ -106,6 +106,8 @@ pub struct UniformListScrollState {
     pub deferred_scroll_to_item: Option<(usize, ScrollStrategy)>,
     /// Size of the item, captured during last layout.
     pub last_item_size: Option<ItemSize>,
+    /// Whether the list was vertically flipped during last layout.
+    pub y_flipped: bool,
 }
 
 #[derive(Copy, Clone, Debug, Default)]
@@ -125,6 +127,7 @@ impl UniformListScrollHandle {
             base_handle: ScrollHandle::new(),
             deferred_scroll_to_item: None,
             last_item_size: None,
+            y_flipped: false,
         })))
     }
 
@@ -133,6 +136,11 @@ impl UniformListScrollHandle {
         self.0.borrow_mut().deferred_scroll_to_item = Some((ix, strategy));
     }
 
+    /// Check if the list is flipped vertically.
+    pub fn y_flipped(&self) -> bool {
+        self.0.borrow().y_flipped
+    }
+
     /// Get the index of the topmost visible child.
     #[cfg(any(test, feature = "test-support"))]
     pub fn logical_scroll_top_index(&self) -> usize {
@@ -264,9 +272,13 @@ impl Element for UniformList {
                     bounds.bottom_right() - point(border.right + padding.right, border.bottom),
                 );
 
-                if let Some(handle) = self.scroll_handle.as_mut() {
-                    handle.0.borrow_mut().base_handle.set_bounds(bounds);
-                }
+                let y_flipped = if let Some(scroll_handle) = self.scroll_handle.as_mut() {
+                    let mut scroll_state = scroll_handle.0.borrow_mut();
+                    scroll_state.base_handle.set_bounds(bounds);
+                    scroll_state.y_flipped
+                } else {
+                    false
+                };
 
                 if self.item_count > 0 {
                     let content_height =
@@ -286,7 +298,10 @@ impl Element for UniformList {
                         scroll_offset.x = Pixels::ZERO;
                     }
 
-                    if let Some((ix, scroll_strategy)) = shared_scroll_to_item {
+                    if let Some((mut ix, scroll_strategy)) = shared_scroll_to_item {
+                        if y_flipped {
+                            ix = self.item_count.saturating_sub(ix + 1);
+                        }
                         let list_height = padded_bounds.size.height;
                         let mut updated_scroll_offset = shared_scroll_offset.borrow_mut();
                         let item_top = item_height * ix + padding.top;
@@ -330,7 +345,15 @@ impl Element for UniformList {
                     let visible_range = first_visible_element_ix
                         ..cmp::min(last_visible_element_ix, self.item_count);
 
-                    let mut items = (self.render_items)(visible_range.clone(), cx);
+                    let items = if y_flipped {
+                        let flipped_range = self.item_count.saturating_sub(visible_range.end)
+                            ..self.item_count.saturating_sub(visible_range.start);
+                        let mut items = (self.render_items)(flipped_range, cx);
+                        items.reverse();
+                        items
+                    } else {
+                        (self.render_items)(visible_range.clone(), cx)
+                    };
 
                     let content_mask = ContentMask { bounds };
                     cx.with_content_mask(Some(content_mask), |cx| {
@@ -500,6 +523,30 @@ impl UniformList {
         self.scroll_handle = Some(handle);
         self
     }
+
+    /// Sets whether the list is flipped vertically, such that item 0 appears at the bottom.
+    pub fn y_flipped(mut self, y_flipped: bool) -> Self {
+        if let Some(ref scroll_handle) = self.scroll_handle {
+            let mut scroll_state = scroll_handle.0.borrow_mut();
+            let mut base_handle = &scroll_state.base_handle;
+            let offset = base_handle.offset();
+            match scroll_state.last_item_size {
+                Some(last_size) if scroll_state.y_flipped != y_flipped => {
+                    let new_y_offset =
+                        -(offset.y + last_size.contents.height - last_size.item.height);
+                    base_handle.set_offset(point(offset.x, new_y_offset));
+                    scroll_state.y_flipped = y_flipped;
+                }
+                // Handle case where list is initially flipped.
+                None if y_flipped => {
+                    base_handle.set_offset(point(offset.x, Pixels::MIN));
+                    scroll_state.y_flipped = y_flipped;
+                }
+                _ => {}
+            }
+        }
+        self
+    }
 }
 
 impl InteractiveElement for UniformList {

crates/gpui/src/geometry.rs 🔗

@@ -2500,6 +2500,8 @@ impl Pixels {
     pub const ZERO: Pixels = Pixels(0.0);
     /// The maximum value that can be represented by `Pixels`.
     pub const MAX: Pixels = Pixels(f32::MAX);
+    /// The minimum value that can be represented by `Pixels`.
+    pub const MIN: Pixels = Pixels(f32::MIN);
 
     /// Floors the `Pixels` value to the nearest whole number.
     ///