Fix positioning of the inline context menu

Marshall Bowers and Max created

Added a new `bounds_for_item` for `ListState`.

Co-authored-by: Max <max@zed.dev>

Change summary

crates/collab_ui2/src/collab_panel.rs | 55 +++++++++++-----------------
crates/gpui2/src/elements/list.rs     | 46 ++++++++++++++++++++---
2 files changed, 60 insertions(+), 41 deletions(-)

Detailed changes

crates/collab_ui2/src/collab_panel.rs 🔗

@@ -165,7 +165,7 @@ struct ChannelMoveClipboard {
 
 const COLLABORATION_PANEL_KEY: &'static str = "CollaborationPanel";
 
-use std::{iter::once, mem, sync::Arc};
+use std::{mem, sync::Arc};
 
 use call::ActiveCall;
 use channel::{Channel, ChannelEvent, ChannelId, ChannelStore};
@@ -175,12 +175,11 @@ use editor::Editor;
 use feature_flags::{ChannelsAlpha, FeatureFlagAppExt, FeatureFlagViewExt};
 use fuzzy::{match_strings, StringMatchCandidate};
 use gpui::{
-    actions, canvas, div, fill, img, impl_actions, list, overlay, point, prelude::*, px, rems,
-    serde_json, size, Action, AnyElement, AppContext, AsyncWindowContext, Bounds, ClipboardItem,
-    DismissEvent, Div, EventEmitter, FocusHandle, Focusable, FocusableView, Hsla,
-    InteractiveElement, IntoElement, Length, ListOffset, ListState, Model, MouseDownEvent,
-    ParentElement, Pixels, Point, PromptLevel, Quad, Render, RenderOnce, ScrollHandle,
-    SharedString, Size, Stateful, Styled, Subscription, Task, View, ViewContext, VisualContext,
+    actions, canvas, div, fill, impl_actions, list, overlay, point, prelude::*, px, serde_json,
+    AnyElement, AppContext, AsyncWindowContext, Bounds, ClipboardItem, DismissEvent, Div,
+    EventEmitter, FocusHandle, Focusable, FocusableView, InteractiveElement, IntoElement,
+    ListOffset, ListState, Model, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel,
+    Render, RenderOnce, SharedString, Styled, Subscription, Task, View, ViewContext, VisualContext,
     WeakView,
 };
 use project::{Fs, Project};
@@ -189,7 +188,7 @@ use settings::{Settings, SettingsStore};
 use ui::prelude::*;
 use ui::{
     h_stack, v_stack, Avatar, Button, Color, ContextMenu, Icon, IconButton, IconElement, IconSize,
-    Label, List, ListHeader, ListItem, Tooltip,
+    Label, ListHeader, ListItem, Tooltip,
 };
 use util::{maybe, ResultExt, TryFutureExt};
 use workspace::{
@@ -1023,9 +1022,10 @@ impl CollabPanel {
                     })
                     .unwrap_or_else(|| (old_index, old_offset));
 
-                // TODO: How to handle this with `list`?
-                // self.scroll_handle
-                //     .set_logical_scroll_top(new_index, new_offset);
+                self.list_state.scroll_to(ListOffset {
+                    item_ix: new_index,
+                    offset_in_item: new_offset,
+                });
             }
         }
 
@@ -1851,15 +1851,14 @@ impl CollabPanel {
         let Some(channel) = self.selected_channel() else {
             return;
         };
-        // TODO: How to handle now that we're using `list`?
-        // let Some(bounds) = self
-        //     .selection
-        //     .and_then(|ix| self.scroll_handle.bounds_for_item(ix))
-        // else {
-        //     return;
-        // };
-        //
-        // self.deploy_channel_context_menu(bounds.center(), channel.id, self.selection.unwrap(), cx);
+        let Some(bounds) = self
+            .selection
+            .and_then(|ix| self.list_state.bounds_for_item(ix))
+        else {
+            return;
+        };
+
+        self.deploy_channel_context_menu(bounds.center(), channel.id, self.selection.unwrap(), cx);
         cx.stop_propagation();
     }
 
@@ -2047,12 +2046,7 @@ impl CollabPanel {
         )
     }
 
-    fn render_list_entry(
-        &mut self,
-        // entry: &ListEntry,
-        ix: usize,
-        cx: &mut ViewContext<Self>,
-    ) -> AnyElement {
+    fn render_list_entry(&mut self, ix: usize, cx: &mut ViewContext<Self>) -> AnyElement {
         let entry = &self.entries[ix];
 
         let is_selected = self.selection == Some(ix);
@@ -2120,14 +2114,7 @@ impl CollabPanel {
     fn render_signed_in(&mut self, cx: &mut ViewContext<Self>) -> Div {
         v_stack()
             .size_full()
-            .child(
-                v_stack()
-                    .size_full()
-                    // .id("scroll")
-                    // .overflow_y_scroll()
-                    // .track_scroll(&self.scroll_handle)
-                    .child(list(self.list_state.clone()).full().into_any_element()),
-            )
+            .child(list(self.list_state.clone()).full())
             .child(
                 div().p_2().child(
                     div()

crates/gpui2/src/elements/list.rs 🔗

@@ -1,6 +1,7 @@
 use crate::{
-    px, AnyElement, AvailableSpace, BorrowAppContext, DispatchPhase, Element, IntoElement, Pixels,
-    Point, ScrollWheelEvent, Size, Style, StyleRefinement, Styled, WindowContext,
+    point, px, AnyElement, AvailableSpace, BorrowAppContext, Bounds, DispatchPhase, Element,
+    IntoElement, Pixels, Point, ScrollWheelEvent, Size, Style, StyleRefinement, Styled,
+    WindowContext,
 };
 use collections::VecDeque;
 use refineable::Refineable as _;
@@ -23,7 +24,7 @@ pub struct List {
 pub struct ListState(Rc<RefCell<StateInner>>);
 
 struct StateInner {
-    last_layout_width: Option<Pixels>,
+    last_layout_bounds: Option<Bounds<Pixels>>,
     render_item: Box<dyn FnMut(usize, &mut WindowContext) -> AnyElement>,
     items: SumTree<ListItem>,
     logical_scroll_top: Option<ListOffset>,
@@ -83,7 +84,7 @@ impl ListState {
         let mut items = SumTree::new();
         items.extend((0..element_count).map(|_| ListItem::Unrendered), &());
         Self(Rc::new(RefCell::new(StateInner {
-            last_layout_width: None,
+            last_layout_bounds: None,
             render_item: Box::new(render_item),
             items,
             logical_scroll_top: None,
@@ -152,6 +153,35 @@ impl ListState {
         }
         state.logical_scroll_top = Some(scroll_top);
     }
+
+    /// Get the bounds for the given item in window coordinates.
+    pub fn bounds_for_item(&self, ix: usize) -> Option<Bounds<Pixels>> {
+        let state = &*self.0.borrow();
+        let bounds = state.last_layout_bounds.unwrap_or_default();
+        let scroll_top = state.logical_scroll_top.unwrap_or_default();
+
+        if ix < scroll_top.item_ix {
+            return None;
+        }
+
+        let mut cursor = state.items.cursor::<(Count, Height)>();
+        cursor.seek(&Count(scroll_top.item_ix), Bias::Right, &());
+
+        let scroll_top = cursor.start().1 .0 + scroll_top.offset_in_item;
+
+        cursor.seek_forward(&Count(ix), Bias::Right, &());
+        if let Some(&ListItem::Rendered { height }) = cursor.item() {
+            let &(Count(count), Height(top)) = cursor.start();
+            if count == ix {
+                let top = bounds.top() + top - scroll_top;
+                return Some(Bounds::from_corners(
+                    point(bounds.left(), top),
+                    point(bounds.right(), top + height),
+                ));
+            }
+        }
+        None
+    }
 }
 
 impl StateInner {
@@ -234,7 +264,7 @@ impl std::fmt::Debug for ListItem {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Default)]
 pub struct ListOffset {
     pub item_ix: usize,
     pub offset_in_item: Pixels,
@@ -265,7 +295,9 @@ impl Element for List {
         let state = &mut *self.state.0.borrow_mut();
 
         // If the width of the list has changed, invalidate all cached item heights
-        if state.last_layout_width != Some(bounds.size.width) {
+        if state.last_layout_bounds.map_or(true, |last_bounds| {
+            last_bounds.size.width != bounds.size.width
+        }) {
             state.items = SumTree::from_iter(
                 (0..state.items.summary().count).map(|_| ListItem::Unrendered),
                 &(),
@@ -392,7 +424,7 @@ impl Element for List {
         }
 
         state.items = new_items;
-        state.last_layout_width = Some(bounds.size.width);
+        state.last_layout_bounds = Some(bounds);
 
         let list_state = self.state.clone();
         let height = bounds.size.height;