project_panel: Only show sticky item shadow when list is scrolled (#34050)

Smit Barmase created

Follow up: https://github.com/zed-industries/zed/pull/34042

- Removes `top_slot_items` from `uniform_list` in favor of using
existing `decorations`
- Add condition to only show shadow for sticky item when list is
scrolled and scrollable

Release Notes:

- N/A

Change summary

crates/gpui/src/elements/uniform_list.rs  |  49 +----
crates/project_panel/src/project_panel.rs | 123 +++++++---------
crates/ui/src/components/indent_guides.rs |   1 
crates/ui/src/components/sticky_items.rs  | 187 +++++++++++++++---------
4 files changed, 181 insertions(+), 179 deletions(-)

Detailed changes

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

@@ -42,7 +42,6 @@ where
         item_count,
         item_to_measure_index: 0,
         render_items: Box::new(render_range),
-        top_slot: None,
         decorations: Vec::new(),
         interactivity: Interactivity {
             element_id: Some(id),
@@ -62,7 +61,6 @@ pub struct UniformList {
     render_items: Box<
         dyn for<'a> Fn(Range<usize>, &'a mut Window, &'a mut App) -> SmallVec<[AnyElement; 64]>,
     >,
-    top_slot: Option<Box<dyn UniformListTopSlot>>,
     decorations: Vec<Box<dyn UniformListDecoration>>,
     interactivity: Interactivity,
     scroll_handle: Option<UniformListScrollHandle>,
@@ -73,7 +71,6 @@ pub struct UniformList {
 /// Frame state used by the [UniformList].
 pub struct UniformListFrameState {
     items: SmallVec<[AnyElement; 32]>,
-    top_slot_items: SmallVec<[AnyElement; 8]>,
     decorations: SmallVec<[AnyElement; 1]>,
 }
 
@@ -145,6 +142,15 @@ impl UniformListScrollHandle {
             .map(|(ix, _)| ix)
             .unwrap_or_else(|| this.base_handle.logical_scroll_top().0)
     }
+
+    /// Checks if the list can be scrolled vertically.
+    pub fn is_scrollable(&self) -> bool {
+        if let Some(size) = self.0.borrow().last_item_size {
+            size.contents.height > size.item.height
+        } else {
+            false
+        }
+    }
 }
 
 impl Styled for UniformList {
@@ -217,7 +223,6 @@ impl Element for UniformList {
             UniformListFrameState {
                 items: SmallVec::new(),
                 decorations: SmallVec::new(),
-                top_slot_items: SmallVec::new(),
             },
         )
     }
@@ -369,17 +374,8 @@ impl Element for UniformList {
                     let last_visible_element_ix = ((-scroll_offset.y + padded_bounds.size.height)
                         / item_height)
                         .ceil() as usize;
-                    let initial_range = first_visible_element_ix
-                        ..cmp::min(last_visible_element_ix, self.item_count);
 
-                    let mut top_slot_elements = if let Some(ref mut top_slot) = self.top_slot {
-                        top_slot.compute(initial_range, window, cx)
-                    } else {
-                        SmallVec::new()
-                    };
-                    let top_slot_offset = top_slot_elements.len();
-
-                    let visible_range = (top_slot_offset + first_visible_element_ix)
+                    let visible_range = first_visible_element_ix
                         ..cmp::min(last_visible_element_ix, self.item_count);
 
                     let items = if y_flipped {
@@ -418,20 +414,6 @@ impl Element for UniformList {
                             frame_state.items.push(item);
                         }
 
-                        if let Some(ref top_slot) = self.top_slot {
-                            top_slot.prepaint(
-                                &mut top_slot_elements,
-                                padded_bounds,
-                                item_height,
-                                scroll_offset,
-                                padding,
-                                can_scroll_horizontally,
-                                window,
-                                cx,
-                            );
-                        }
-                        frame_state.top_slot_items = top_slot_elements;
-
                         let bounds = Bounds::new(
                             padded_bounds.origin
                                 + point(
@@ -448,6 +430,7 @@ impl Element for UniformList {
                             let mut decoration = decoration.as_ref().compute(
                                 visible_range.clone(),
                                 bounds,
+                                scroll_offset,
                                 item_height,
                                 self.item_count,
                                 window,
@@ -493,9 +476,6 @@ impl Element for UniformList {
                 for decoration in &mut request_layout.decorations {
                     decoration.paint(window, cx);
                 }
-                if let Some(ref top_slot) = self.top_slot {
-                    top_slot.paint(&mut request_layout.top_slot_items, window, cx);
-                }
             },
         )
     }
@@ -518,6 +498,7 @@ pub trait UniformListDecoration {
         &self,
         visible_range: Range<usize>,
         bounds: Bounds<Pixels>,
+        scroll_offset: Point<Pixels>,
         item_height: Pixels,
         item_count: usize,
         window: &mut Window,
@@ -592,12 +573,6 @@ impl UniformList {
         self
     }
 
-    /// Sets a top slot for the list.
-    pub fn with_top_slot(mut self, top_slot: impl UniformListTopSlot + 'static) -> Self {
-        self.top_slot = Some(Box::new(top_slot));
-        self
-    }
-
     fn measure_item(
         &self,
         list_width: Option<Pixels>,

crates/project_panel/src/project_panel.rs 🔗

@@ -56,8 +56,8 @@ use std::{
 use theme::ThemeSettings;
 use ui::{
     Color, ContextMenu, DecoratedIcon, Icon, IconDecoration, IconDecorationKind, IndentGuideColors,
-    IndentGuideLayout, KeyBinding, Label, LabelSize, ListItem, ListItemSpacing, Scrollbar,
-    ScrollbarState, StickyCandidate, Tooltip, prelude::*, v_flex,
+    IndentGuideLayout, KeyBinding, Label, LabelSize, ListItem, ListItemSpacing, ScrollableHandle,
+    Scrollbar, ScrollbarState, StickyCandidate, Tooltip, prelude::*, v_flex,
 };
 use util::{ResultExt, TakeUntilExt, TryFutureExt, maybe, paths::compare_paths};
 use workspace::{
@@ -3327,7 +3327,13 @@ impl ProjectPanel {
         range: Range<usize>,
         window: &mut Window,
         cx: &mut Context<ProjectPanel>,
-        mut callback: impl FnMut(&Entry, &HashSet<Arc<Path>>, &mut Window, &mut Context<ProjectPanel>),
+        mut callback: impl FnMut(
+            &Entry,
+            usize,
+            &HashSet<Arc<Path>>,
+            &mut Window,
+            &mut Context<ProjectPanel>,
+        ),
     ) {
         let mut ix = 0;
         for (_, visible_worktree_entries, entries_paths) in &self.visible_entries {
@@ -3348,8 +3354,10 @@ impl ProjectPanel {
                     .map(|e| (e.path.clone()))
                     .collect()
             });
-            for entry in visible_worktree_entries[entry_range].iter() {
-                callback(&entry, entries, window, cx);
+            let base_index = ix + entry_range.start;
+            for (i, entry) in visible_worktree_entries[entry_range].iter().enumerate() {
+                let global_index = base_index + i;
+                callback(&entry, global_index, entries, window, cx);
             }
             ix = end_ix;
         }
@@ -3930,7 +3938,15 @@ impl ProjectPanel {
             }
         };
 
-        let last_sticky_item = details.sticky.as_ref().map_or(false, |item| item.is_last);
+        let show_sticky_shadow = details.sticky.as_ref().map_or(false, |item| {
+            if item.is_last {
+                let is_scrollable = self.scroll_handle.is_scrollable();
+                let is_scrolled = self.scroll_handle.offset().y < px(0.);
+                is_scrollable && is_scrolled
+            } else {
+                false
+            }
+        });
         let shadow_color_top = hsla(0.0, 0.0, 0.0, 0.15);
         let shadow_color_bottom = hsla(0.0, 0.0, 0.0, 0.);
         let sticky_shadow = div()
@@ -3956,7 +3972,7 @@ impl ProjectPanel {
             .border_r_2()
             .border_color(border_color)
             .hover(|style| style.bg(bg_hover_color).border_color(border_hover_color))
-            .when(is_sticky && last_sticky_item, |this| this.child(sticky_shadow))
+            .when(show_sticky_shadow, |this| this.child(sticky_shadow))
             .when(!is_sticky, |this| {
                 this
                 .when(is_highlighted && folded_directory_drag_target.is_none(), |this| this.border_color(transparent_white()).bg(item_colors.drag_over))
@@ -4828,54 +4844,6 @@ impl ProjectPanel {
         None
     }
 
-    fn candidate_entries_in_range_for_sticky(
-        &self,
-        range: Range<usize>,
-        _window: &mut Window,
-        _cx: &mut Context<Self>,
-    ) -> Vec<StickyProjectPanelCandidate> {
-        let mut result = Vec::new();
-        let mut current_offset = 0;
-
-        for (_, visible_worktree_entries, entries_paths) in &self.visible_entries {
-            let worktree_len = visible_worktree_entries.len();
-            let worktree_end_offset = current_offset + worktree_len;
-
-            if current_offset >= range.end {
-                break;
-            }
-
-            if worktree_end_offset > range.start {
-                let local_start = range.start.saturating_sub(current_offset);
-                let local_end = range.end.saturating_sub(current_offset).min(worktree_len);
-
-                let paths = entries_paths.get_or_init(|| {
-                    visible_worktree_entries
-                        .iter()
-                        .map(|e| e.path.clone())
-                        .collect()
-                });
-
-                let entries_from_this_worktree = visible_worktree_entries[local_start..local_end]
-                    .iter()
-                    .enumerate()
-                    .map(|(i, entry)| {
-                        let (depth, _) = Self::calculate_depth_and_difference(&entry.entry, paths);
-                        StickyProjectPanelCandidate {
-                            index: current_offset + local_start + i,
-                            depth,
-                        }
-                    });
-
-                result.extend(entries_from_this_worktree);
-            }
-
-            current_offset = worktree_end_offset;
-        }
-
-        result
-    }
-
     fn render_sticky_entries(
         &self,
         child: StickyProjectPanelCandidate,
@@ -4926,6 +4894,10 @@ impl ProjectPanel {
             break 'outer;
         }
 
+        if sticky_parents.is_empty() {
+            return SmallVec::new();
+        }
+
         sticky_parents.reverse();
 
         let git_status_enabled = ProjectPanelSettings::get_global(cx).git_status;
@@ -4940,6 +4912,8 @@ impl ProjectPanel {
             Default::default()
         };
 
+        // already checked if non empty above
+        let last_item_index = sticky_parents.len() - 1;
         sticky_parents
             .iter()
             .enumerate()
@@ -4950,7 +4924,7 @@ impl ProjectPanel {
                     .unwrap_or_default();
                 let sticky_details = Some(StickyDetails {
                     sticky_index: index,
-                    is_last: index == sticky_parents.len() - 1,
+                    is_last: index == last_item_index,
                 });
                 let details = self.details_for_entry(
                     entry,
@@ -5191,17 +5165,6 @@ impl Render for ProjectPanel {
                             items
                         })
                     })
-                    .when(show_sticky_scroll, |list| {
-                        list.with_top_slot(ui::sticky_items(
-                            cx.entity().clone(),
-                            |this, range, window, cx| {
-                                this.candidate_entries_in_range_for_sticky(range, window, cx)
-                            },
-                            |this, marker_entry, window, cx| {
-                                this.render_sticky_entries(marker_entry, window, cx)
-                            },
-                        ))
-                    })
                     .when(show_indent_guides, |list| {
                         list.with_decoration(
                             ui::indent_guides(
@@ -5215,7 +5178,7 @@ impl Render for ProjectPanel {
                                         range,
                                         window,
                                         cx,
-                                        |entry, entries, _, _| {
+                                        |entry, _, entries, _, _| {
                                             let (depth, _) = Self::calculate_depth_and_difference(
                                                 entry, entries,
                                             );
@@ -5301,6 +5264,30 @@ impl Render for ProjectPanel {
                             ),
                         )
                     })
+                    .when(show_sticky_scroll, |list| {
+                        list.with_decoration(ui::sticky_items(
+                            cx.entity().clone(),
+                            |this, range, window, cx| {
+                                let mut items = SmallVec::with_capacity(range.end - range.start);
+                                this.iter_visible_entries(
+                                    range,
+                                    window,
+                                    cx,
+                                    |entry, index, entries, _, _| {
+                                        let (depth, _) =
+                                            Self::calculate_depth_and_difference(entry, entries);
+                                        let candidate =
+                                            StickyProjectPanelCandidate { index, depth };
+                                        items.push(candidate);
+                                    },
+                                );
+                                items
+                            },
+                            |this, marker_entry, window, cx| {
+                                this.render_sticky_entries(marker_entry, window, cx)
+                            },
+                        ))
+                    })
                     .size_full()
                     .with_sizing_behavior(ListSizingBehavior::Infer)
                     .with_horizontal_sizing_behavior(ListHorizontalSizingBehavior::Unconstrained)

crates/ui/src/components/indent_guides.rs 🔗

@@ -147,6 +147,7 @@ mod uniform_list {
             &self,
             visible_range: Range<usize>,
             bounds: Bounds<Pixels>,
+            _scroll_offset: Point<Pixels>,
             item_height: Pixels,
             item_count: usize,
             window: &mut Window,

crates/ui/src/components/sticky_items.rs 🔗

@@ -1,7 +1,8 @@
-use std::ops::Range;
+use std::{ops::Range, rc::Rc};
 
 use gpui::{
-    AnyElement, App, AvailableSpace, Bounds, Context, Entity, Pixels, Render, UniformListTopSlot,
+    AnyElement, App, AvailableSpace, Bounds, Context, Element, ElementId, Entity, GlobalElementId,
+    InspectorElementId, IntoElement, LayoutId, Pixels, Point, Render, Style, UniformListDecoration,
     Window, point, size,
 };
 use smallvec::SmallVec;
@@ -10,16 +11,16 @@ pub trait StickyCandidate {
     fn depth(&self) -> usize;
 }
 
+#[derive(Clone)]
 pub struct StickyItems<T> {
-    compute_fn: Box<dyn Fn(Range<usize>, &mut Window, &mut App) -> Vec<T>>,
-    render_fn: Box<dyn Fn(T, &mut Window, &mut App) -> SmallVec<[AnyElement; 8]>>,
-    last_item_is_drifting: bool,
-    anchor_index: Option<usize>,
+    compute_fn: Rc<dyn Fn(Range<usize>, &mut Window, &mut App) -> SmallVec<[T; 8]>>,
+    render_fn: Rc<dyn Fn(T, &mut Window, &mut App) -> SmallVec<[AnyElement; 8]>>,
 }
 
 pub fn sticky_items<V, T>(
     entity: Entity<V>,
-    compute_fn: impl Fn(&mut V, Range<usize>, &mut Window, &mut Context<V>) -> Vec<T> + 'static,
+    compute_fn: impl Fn(&mut V, Range<usize>, &mut Window, &mut Context<V>) -> SmallVec<[T; 8]>
+    + 'static,
     render_fn: impl Fn(&mut V, T, &mut Window, &mut Context<V>) -> SmallVec<[AnyElement; 8]> + 'static,
 ) -> StickyItems<T>
 where
@@ -29,37 +30,106 @@ where
     let entity_compute = entity.clone();
     let entity_render = entity.clone();
 
-    let compute_fn = Box::new(
-        move |range: Range<usize>, window: &mut Window, cx: &mut App| -> Vec<T> {
+    let compute_fn = Rc::new(
+        move |range: Range<usize>, window: &mut Window, cx: &mut App| -> SmallVec<[T; 8]> {
             entity_compute.update(cx, |view, cx| compute_fn(view, range, window, cx))
         },
     );
-    let render_fn = Box::new(
+    let render_fn = Rc::new(
         move |entry: T, window: &mut Window, cx: &mut App| -> SmallVec<[AnyElement; 8]> {
             entity_render.update(cx, |view, cx| render_fn(view, entry, window, cx))
         },
     );
+
     StickyItems {
         compute_fn,
         render_fn,
-        last_item_is_drifting: false,
-        anchor_index: None,
     }
 }
 
-impl<T> UniformListTopSlot for StickyItems<T>
+struct StickyItemsElement {
+    elements: SmallVec<[AnyElement; 8]>,
+}
+
+impl IntoElement for StickyItemsElement {
+    type Element = Self;
+
+    fn into_element(self) -> Self::Element {
+        self
+    }
+}
+
+impl Element for StickyItemsElement {
+    type RequestLayoutState = ();
+    type PrepaintState = ();
+
+    fn id(&self) -> Option<ElementId> {
+        None
+    }
+
+    fn source_location(&self) -> Option<&'static core::panic::Location<'static>> {
+        None
+    }
+
+    fn request_layout(
+        &mut self,
+        _id: Option<&GlobalElementId>,
+        _inspector_id: Option<&InspectorElementId>,
+        window: &mut Window,
+        cx: &mut App,
+    ) -> (LayoutId, Self::RequestLayoutState) {
+        (window.request_layout(Style::default(), [], cx), ())
+    }
+
+    fn prepaint(
+        &mut self,
+        _id: Option<&GlobalElementId>,
+        _inspector_id: Option<&InspectorElementId>,
+        _bounds: Bounds<Pixels>,
+        _request_layout: &mut Self::RequestLayoutState,
+        _window: &mut Window,
+        _cx: &mut App,
+    ) -> Self::PrepaintState {
+        ()
+    }
+
+    fn paint(
+        &mut self,
+        _id: Option<&GlobalElementId>,
+        _inspector_id: Option<&InspectorElementId>,
+        _bounds: Bounds<Pixels>,
+        _request_layout: &mut Self::RequestLayoutState,
+        _prepaint: &mut Self::PrepaintState,
+        window: &mut Window,
+        cx: &mut App,
+    ) {
+        // reverse so that last item is bottom most among sticky items
+        for item in self.elements.iter_mut().rev() {
+            item.paint(window, cx);
+        }
+    }
+}
+
+impl<T> UniformListDecoration for StickyItems<T>
 where
     T: StickyCandidate + Clone + 'static,
 {
     fn compute(
-        &mut self,
+        &self,
         visible_range: Range<usize>,
+        bounds: Bounds<Pixels>,
+        scroll_offset: Point<Pixels>,
+        item_height: Pixels,
+        _item_count: usize,
         window: &mut Window,
         cx: &mut App,
-    ) -> SmallVec<[AnyElement; 8]> {
+    ) -> AnyElement {
         let entries = (self.compute_fn)(visible_range.clone(), window, cx);
+        let mut elements = SmallVec::new();
 
         let mut anchor_entry = None;
+        let mut last_item_is_drifting = false;
+        let mut anchor_index = None;
 
         let mut iter = entries.iter().enumerate().peekable();
         while let Some((ix, current_entry)) = iter.next() {
@@ -75,8 +145,8 @@ where
                 let next_depth = next_entry.depth();
 
                 if next_depth < current_depth && next_depth < index_in_range {
-                    self.last_item_is_drifting = true;
-                    self.anchor_index = Some(visible_range.start + ix);
+                    last_item_is_drifting = true;
+                    anchor_index = Some(visible_range.start + ix);
                     anchor_entry = Some(current_entry.clone());
                     break;
                 }
@@ -84,67 +154,36 @@ where
         }
 
         if let Some(anchor_entry) = anchor_entry {
-            (self.render_fn)(anchor_entry, window, cx)
-        } else {
-            SmallVec::new()
-        }
-    }
+            elements = (self.render_fn)(anchor_entry, window, cx);
+            let items_count = elements.len();
+
+            for (ix, element) in elements.iter_mut().enumerate() {
+                let mut item_y_offset = None;
+                if ix == items_count - 1 && last_item_is_drifting {
+                    if let Some(anchor_index) = anchor_index {
+                        let scroll_top = -scroll_offset.y;
+                        let anchor_top = item_height * anchor_index;
+                        let sticky_area_height = item_height * items_count;
+                        item_y_offset =
+                            Some((anchor_top - scroll_top - sticky_area_height).min(Pixels::ZERO));
+                    };
+                }
 
-    fn prepaint(
-        &self,
-        items: &mut SmallVec<[AnyElement; 8]>,
-        bounds: Bounds<Pixels>,
-        item_height: Pixels,
-        scroll_offset: gpui::Point<Pixels>,
-        padding: gpui::Edges<Pixels>,
-        can_scroll_horizontally: bool,
-        window: &mut Window,
-        cx: &mut App,
-    ) {
-        let items_count = items.len();
-
-        for (ix, item) in items.iter_mut().enumerate() {
-            let mut item_y_offset = None;
-            if ix == items_count - 1 && self.last_item_is_drifting {
-                if let Some(anchor_index) = self.anchor_index {
-                    let scroll_top = -scroll_offset.y;
-                    let anchor_top = item_height * anchor_index;
-                    let sticky_area_height = item_height * items_count;
-                    item_y_offset =
-                        Some((anchor_top - scroll_top - sticky_area_height).min(Pixels::ZERO));
-                };
-            }
+                let sticky_origin = bounds.origin
+                    + point(
+                        -scroll_offset.x,
+                        -scroll_offset.y + item_height * ix + item_y_offset.unwrap_or(Pixels::ZERO),
+                    );
 
-            let sticky_origin = bounds.origin
-                + point(
-                    if can_scroll_horizontally {
-                        scroll_offset.x + padding.left
-                    } else {
-                        scroll_offset.x
-                    },
-                    item_height * ix + padding.top + item_y_offset.unwrap_or(Pixels::ZERO),
+                let available_space = size(
+                    AvailableSpace::Definite(bounds.size.width),
+                    AvailableSpace::Definite(item_height),
                 );
-
-            let available_width = if can_scroll_horizontally {
-                bounds.size.width + scroll_offset.x.abs()
-            } else {
-                bounds.size.width
-            };
-
-            let available_space = size(
-                AvailableSpace::Definite(available_width),
-                AvailableSpace::Definite(item_height),
-            );
-
-            item.layout_as_root(available_space, window, cx);
-            item.prepaint_at(sticky_origin, window, cx);
+                element.layout_as_root(available_space, window, cx);
+                element.prepaint_at(sticky_origin, window, cx);
+            }
         }
-    }
 
-    fn paint(&self, items: &mut SmallVec<[AnyElement; 8]>, window: &mut Window, cx: &mut App) {
-        // reverse so that last item is bottom most among sticky items
-        for item in items.iter_mut().rev() {
-            item.paint(window, cx);
-        }
+        StickyItemsElement { elements }.into_any_element()
     }
 }