recent_projects: Allow bulk deleting entries without scroll jumping (#54777)

Closes #52292

This PR preserves the scroll offset of the List used in Recent Projects
while deleting items. It does that by:
- Adding the `is_scrolled_to_end` method to the GPUI list, which helps
us determine where the new selection should be, since it depends on
whether items are taking the deleted slot from below or above.
- Adding `ScrollBehavior` to `update_matches`, which lets you preserve
the scroll offset even for `List` (not `UniformList`) after a reset.

Before:


https://github.com/user-attachments/assets/e3eb7092-59ec-4b54-b57a-503555addd27

After:


https://github.com/user-attachments/assets/6929f6a0-04d7-44f9-a9b2-f9e5c077b368

Release Notes:

- Fixed the recent projects list jumping to the top after deleting a
project, so you can now bulk-delete entries by repeatedly clicking the
delete icon or pressing the keybind.

crates/gpui/src/elements/list.rs

@@ -271,6 +271,7 @@ struct ListItemSummary {
     unrendered_count: usize,
     height: Pixels,
     has_focus_handles: bool,
+    has_unknown_height: bool,
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)]
@@ -399,6 +400,25 @@ impl ListState {
         self.0.borrow().items.summary().count
     }
 
+    /// Whether the list is scrolled to the end, or `None` if the list is
+    /// not scrollable or the total content height is not yet known.
+    pub fn is_scrolled_to_end(&self) -> Option<bool> {
+        let state = self.0.borrow();
+        let bounds = state.last_layout_bounds?;
+        let summary = state.items.summary();
+        if summary.has_unknown_height {
+            return None;
+        }
+        let padding = state.last_padding.unwrap_or_default();
+        let content_height = summary.height + padding.top + padding.bottom;
+        let scroll_max = (content_height - bounds.size.height).max(px(0.));
+        if scroll_max <= px(0.) {
+            return None;
+        }
+        let scroll_top = state.scroll_top(&state.logical_scroll_top());
+        Some(scroll_top >= scroll_max)
+    }
+
     /// Inform the list state that the items in `old_range` have been replaced
     /// by `count` new items that must be recalculated.
     pub fn splice(&self, old_range: Range<usize>, count: usize) {
@@ -1385,6 +1405,7 @@ impl sum_tree::Item for ListItem {
                     px(0.)
                 },
                 has_focus_handles: focus_handle.is_some(),
+                has_unknown_height: size_hint.is_none(),
             },
             ListItem::Measured {
                 size, focus_handle, ..
@@ -1394,6 +1415,7 @@ impl sum_tree::Item for ListItem {
                 unrendered_count: 0,
                 height: size.height,
                 has_focus_handles: focus_handle.is_some(),
+                has_unknown_height: false,
             },
         }
     }
@@ -1410,6 +1432,7 @@ impl sum_tree::ContextLessSummary for ListItemSummary {
         self.unrendered_count += summary.unrendered_count;
         self.height += summary.height;
         self.has_focus_handles |= summary.has_focus_handles;
+        self.has_unknown_height |= summary.has_unknown_height;
     }
 }
 

crates/gpui/src/elements/uniform_list.rs

@@ -8,7 +8,7 @@ use crate::{
     AnyElement, App, AvailableSpace, Bounds, ContentMask, Element, ElementId, Entity,
     GlobalElementId, Hitbox, InspectorElementId, InteractiveElement, Interactivity, IntoElement,
     IsZero, LayoutId, ListSizingBehavior, Overflow, Pixels, Point, ScrollHandle, Size,
-    StyleRefinement, Styled, Window, point, size,
+    StyleRefinement, Styled, Window, point, px, size,
 };
 use smallvec::SmallVec;
 use std::{cell::RefCell, cmp, ops::Range, rc::Rc, usize};
@@ -236,6 +236,18 @@ impl UniformListScrollHandle {
         }
     }
 
+    /// Whether the list is scrolled to the end, or `None` if the list is
+    /// not scrollable.
+    pub fn is_scrolled_to_end(&self) -> Option<bool> {
+        let state = self.0.borrow();
+        let max_offset = state.base_handle.max_offset();
+        if max_offset.y <= px(0.) {
+            return None;
+        }
+        let offset = state.base_handle.offset();
+        Some(-offset.y >= max_offset.y)
+    }
+
     /// Scroll to the bottom of the list.
     pub fn scroll_to_bottom(&self) {
         self.scroll_to_item(usize::MAX, ScrollStrategy::Bottom);

crates/picker/src/picker.rs

@@ -35,6 +35,12 @@ pub enum Direction {
     Down,
 }
 
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum ScrollBehavior {
+    RevealSelected,
+    PreserveOffset,
+}
+
 actions!(
     picker,
     [
@@ -687,9 +693,19 @@ impl<D: PickerDelegate> Picker<D> {
     }
 
     pub fn update_matches(&mut self, query: String, window: &mut Window, cx: &mut Context<Self>) {
+        self.update_matches_with_options(query, ScrollBehavior::RevealSelected, window, cx);
+    }
+
+    pub fn update_matches_with_options(
+        &mut self,
+        query: String,
+        scroll_behavior: ScrollBehavior,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
         let delegate_pending_update_matches = self.delegate.update_matches(query, window, cx);
 
-        self.matches_updated(window, cx);
+        self.matches_updated(scroll_behavior, window, cx);
         // This struct ensures that we can synchronously drop the task returned by the
         // delegate's `update_matches` method and the task that the picker is spawning.
         // If we simply capture the delegate's task into the picker's task, when the picker's
@@ -709,19 +725,40 @@ impl<D: PickerDelegate> Picker<D> {
                 })?;
                 delegate_pending_update_matches.await;
                 this.update_in(cx, |this, window, cx| {
-                    this.matches_updated(window, cx);
+                    this.matches_updated(scroll_behavior, window, cx);
                 })
             }),
         });
     }
 
-    fn matches_updated(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-        if let ElementContainer::List(state) = &mut self.element_container {
-            state.reset(self.delegate.match_count());
+    fn matches_updated(
+        &mut self,
+        scroll_behavior: ScrollBehavior,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        let match_count = self.delegate.match_count();
+        match &mut self.element_container {
+            ElementContainer::List(state) => match scroll_behavior {
+                ScrollBehavior::RevealSelected => {
+                    state.reset(match_count);
+                    let index = self.delegate.selected_index();
+                    self.scroll_to_item_index(index);
+                }
+                ScrollBehavior::PreserveOffset => {
+                    let offset = state.logical_scroll_top();
+                    state.reset(match_count);
+                    state.scroll_to(offset);
+                }
+            },
+            ElementContainer::UniformList(_) => match scroll_behavior {
+                ScrollBehavior::RevealSelected => {
+                    let index = self.delegate.selected_index();
+                    self.scroll_to_item_index(index);
+                }
+                ScrollBehavior::PreserveOffset => {}
+            },
         }
-
-        let index = self.delegate.selected_index();
-        self.scroll_to_item_index(index);
         self.pending_update_matches = None;
         if let Some(secondary) = self.confirm_on_update.take() {
             self.do_confirm(secondary, window, cx);
@@ -752,6 +789,13 @@ impl<D: PickerDelegate> Picker<D> {
         }
     }
 
+    pub fn is_scrolled_to_end(&self) -> Option<bool> {
+        match &self.element_container {
+            ElementContainer::List(state) => state.is_scrolled_to_end(),
+            ElementContainer::UniformList(scroll_handle) => scroll_handle.is_scrolled_to_end(),
+        }
+    }
+
     fn render_element(
         &self,
         window: &mut Window,

crates/recent_projects/Cargo.toml

@@ -64,6 +64,7 @@ fs.workspace = true
 gpui = { workspace = true, features = ["test-support"] }
 http_client.workspace = true
 language = { workspace = true, features = ["test-support"] }
+picker = { workspace = true, features = ["test-support"] }
 project = { workspace = true, features = ["test-support"] }
 release_channel.workspace = true
 remote = { workspace = true, features = ["test-support"] }

crates/recent_projects/src/recent_projects.rs

@@ -29,7 +29,7 @@ use gpui::{
 };
 
 use picker::{
-    Picker, PickerDelegate,
+    Picker, PickerDelegate, ScrollBehavior,
     highlighted_match_with_paths::{HighlightedMatch, HighlightedMatchWithPaths},
 };
 use project::{Worktree, git_store::Repository};
@@ -83,6 +83,15 @@ enum ProjectPickerEntry {
     RecentProject(StringMatch),
 }
 
+fn is_selectable_entry(entry: &ProjectPickerEntry) -> bool {
+    matches!(
+        entry,
+        ProjectPickerEntry::OpenFolder { .. }
+            | ProjectPickerEntry::ProjectGroup(_)
+            | ProjectPickerEntry::RecentProject(_)
+    )
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum ProjectPickerStyle {
     Modal,
@@ -814,8 +823,7 @@ pub struct RecentProjectsDelegate {
     selected_index: usize,
     render_paths: bool,
     create_new_window: bool,
-    // Flag to reset index when there is a new query vs not reset index when user delete an item
-    reset_selected_match_index: bool,
+    snap_selection_to_first_non_header_match: bool,
     has_any_non_local_projects: bool,
     project_connection_options: Option<RemoteConnectionOptions>,
     focus_handle: FocusHandle,
@@ -843,7 +851,7 @@ impl RecentProjectsDelegate {
             selected_index: 0,
             create_new_window,
             render_paths,
-            reset_selected_match_index: true,
+            snap_selection_to_first_non_header_match: true,
             has_any_non_local_projects: project_connection_options.is_some(),
             project_connection_options,
             focus_handle,
@@ -1067,14 +1075,14 @@ impl PickerDelegate for RecentProjectsDelegate {
 
         self.filtered_entries = entries;
 
-        if self.reset_selected_match_index {
+        if self.snap_selection_to_first_non_header_match {
             self.selected_index = self
                 .filtered_entries
                 .iter()
                 .position(|e| !matches!(e, ProjectPickerEntry::Header(_)))
                 .unwrap_or(0);
         }
-        self.reset_selected_match_index = true;
+        self.snap_selection_to_first_non_header_match = true;
         Task::ready(())
     }
 
@@ -2106,6 +2114,69 @@ impl RecentProjectsDelegate {
         .detach();
     }
 
+    /// Returns the new selection index after the entry at `deleted_index`
+    /// is removed.
+    ///
+    /// - Prefers the nearest entry matching `prefer_section` so the user
+    ///   stays in the same section they were navigating.
+    /// - Falls back to any other selectable entry so the picker doesn't
+    ///   land on a header.
+    fn replacement_index_after_deletion(
+        &self,
+        deleted_index: usize,
+        prefer_previous: bool,
+        prefer_section: fn(&ProjectPickerEntry) -> bool,
+    ) -> Option<usize> {
+        let replacement_index = |matches_entry: fn(&ProjectPickerEntry) -> bool| {
+            let next_index = self
+                .filtered_entries
+                .iter()
+                .enumerate()
+                .skip(deleted_index)
+                .find_map(|(index, entry)| matches_entry(entry).then_some(index));
+            let previous_index = self
+                .filtered_entries
+                .iter()
+                .enumerate()
+                .take(deleted_index.min(self.filtered_entries.len()))
+                .rev()
+                .find_map(|(index, entry)| matches_entry(entry).then_some(index));
+
+            if prefer_previous {
+                previous_index.or(next_index)
+            } else {
+                next_index.or(previous_index)
+            }
+        };
+
+        replacement_index(prefer_section).or_else(|| replacement_index(is_selectable_entry))
+    }
+
+    fn update_picker_after_recent_project_deletion(
+        picker: &mut Picker<Self>,
+        deleted_index: usize,
+        workspaces: Vec<RecentWorkspace>,
+        window: &mut Window,
+        cx: &mut Context<Picker<Self>>,
+    ) {
+        let prefer_previous = picker.is_scrolled_to_end() == Some(true);
+        picker.delegate.set_workspaces(workspaces);
+        picker.delegate.snap_selection_to_first_non_header_match = false;
+        picker.update_matches_with_options(
+            picker.query(cx),
+            ScrollBehavior::PreserveOffset,
+            window,
+            cx,
+        );
+        if let Some(replacement_index) = picker.delegate.replacement_index_after_deletion(
+            deleted_index,
+            prefer_previous,
+            |entry| matches!(entry, ProjectPickerEntry::RecentProject(_)),
+        ) {
+            picker.set_selected_index(replacement_index, None, false, window, cx);
+        }
+    }
+
     fn delete_recent_project(
         &self,
         ix: usize,
@@ -2115,7 +2186,10 @@ impl RecentProjectsDelegate {
         if let Some(ProjectPickerEntry::RecentProject(selected_match)) =
             self.filtered_entries.get(ix)
         {
-            let recent_workspace = self.workspaces[selected_match.candidate_id].clone();
+            let Some(recent_workspace) = self.workspaces.get(selected_match.candidate_id).cloned()
+            else {
+                return;
+            };
             let fs = self
                 .workspace
                 .upgrade()
@@ -2133,12 +2207,9 @@ impl RecentProjectsDelegate {
                     .await
                     .unwrap_or_default();
                 this.update_in(cx, move |picker, window, cx| {
-                    picker.delegate.set_workspaces(workspaces);
-                    picker
-                        .delegate
-                        .set_selected_index(ix.saturating_sub(1), window, cx);
-                    picker.delegate.reset_selected_match_index = false;
-                    picker.update_matches(picker.query(cx), window, cx);
+                    Self::update_picker_after_recent_project_deletion(
+                        picker, ix, workspaces, window, cx,
+                    );
                     // After deleting a project, we want to update the history manager to reflect the change.
                     // But we do not emit a update event when user opens a project, because it's handled in `workspace::load_workspace`.
                     if let Some(history_manager) = HistoryManager::global(cx) {
@@ -2234,7 +2305,7 @@ impl RecentProjectsDelegate {
 
 #[cfg(test)]
 mod tests {
-    use gpui::{TestAppContext, UpdateGlobal};
+    use gpui::{TestAppContext, UpdateGlobal, VisualTestContext};
 
     use serde_json::json;
     use settings::SettingsStore;
@@ -2243,6 +2314,220 @@ mod tests {
 
     use super::*;
 
+    // Test picker for the empty query:
+    //
+    //   [0] Header("Current Folders")
+    //   [1] OpenFolder(0)
+    //   [2] OpenFolder(1)
+    //   [3] Header("This Window")
+    //   [4] ProjectGroup(0)
+    //   [5] ProjectGroup(1)
+    //   [6] Header("Recent Projects")
+    //   [7..=26] RecentProject(0..=19)
+    //
+    const RECENT_PROJECT_COUNT: usize = 20;
+    const FIRST_RECENT_PROJECT: usize = 7;
+    const LAST_RECENT_PROJECT: usize = FIRST_RECENT_PROJECT + RECENT_PROJECT_COUNT - 1;
+
+    fn open_folder(index: usize) -> OpenFolderEntry {
+        OpenFolderEntry {
+            worktree_id: WorktreeId::from_usize(index),
+            name: format!("project-folder-{index}").into(),
+            path: PathBuf::from(format!("/current/project-folder-{index}")),
+            branch: None,
+            is_active: false,
+        }
+    }
+
+    fn project_group(index: usize) -> ProjectGroupKey {
+        ProjectGroupKey::new(
+            None,
+            PathList::new(&[PathBuf::from(format!("/this-window/project-{index}"))]),
+        )
+    }
+
+    fn recent_workspace(index: usize) -> RecentWorkspace {
+        let paths = PathList::new(&[PathBuf::from(format!("/recent/project-{index:02}"))]);
+        RecentWorkspace {
+            workspace_id: WorkspaceId::from_i64(index as i64),
+            location: SerializedWorkspaceLocation::Local,
+            paths: paths.clone(),
+            identity_paths: paths,
+            timestamp: Utc::now(),
+        }
+    }
+
+    fn recent_workspaces() -> Vec<RecentWorkspace> {
+        (0..RECENT_PROJECT_COUNT).map(recent_workspace).collect()
+    }
+
+    fn draw(cx: &mut VisualTestContext) {
+        cx.update(|window, cx| window.draw(cx).clear());
+    }
+
+    fn build_picker(
+        cx: &mut TestAppContext,
+    ) -> (
+        Entity<Picker<RecentProjectsDelegate>>,
+        &mut VisualTestContext,
+    ) {
+        init_test(cx);
+        let (picker, cx) = cx.add_window_view(|window, cx| {
+            let mut delegate = RecentProjectsDelegate::new(
+                WeakEntity::new_invalid(),
+                false,
+                cx.focus_handle(),
+                vec![open_folder(0), open_folder(1)],
+                vec![project_group(0), project_group(1)],
+                None,
+                ProjectPickerStyle::Modal,
+            );
+            delegate.set_workspaces(recent_workspaces());
+            Picker::list(delegate, window, cx)
+                .list_measure_all()
+                .show_scrollbar(true)
+                .max_height(Some(px(240.).into()))
+        });
+        draw(cx);
+        (picker, cx)
+    }
+
+    fn scroll_to_and_select(
+        picker: &Entity<Picker<RecentProjectsDelegate>>,
+        cx: &mut VisualTestContext,
+        index: usize,
+    ) -> usize {
+        picker.update_in(cx, |picker, window, cx| {
+            picker.set_selected_index(index, None, true, window, cx);
+        });
+        draw(cx);
+        picker.update(cx, |picker, _| picker.logical_scroll_top_index())
+    }
+
+    fn delete_recent_project_in_picker(
+        picker: &Entity<Picker<RecentProjectsDelegate>>,
+        cx: &mut VisualTestContext,
+        index: usize,
+    ) {
+        picker.update_in(cx, |picker, window, cx| {
+            let Some(ProjectPickerEntry::RecentProject(hit)) =
+                picker.delegate.filtered_entries.get(index)
+            else {
+                panic!("expected entry at {index} to be a recent project");
+            };
+            let mut workspaces = picker.delegate.workspaces.clone();
+            workspaces.remove(hit.candidate_id);
+            RecentProjectsDelegate::update_picker_after_recent_project_deletion(
+                picker, index, workspaces, window, cx,
+            );
+        });
+    }
+
+    #[track_caller]
+    fn assert_scroll_top_is(
+        picker: &Entity<Picker<RecentProjectsDelegate>>,
+        cx: &mut VisualTestContext,
+        expected: usize,
+        phase: &str,
+    ) {
+        picker.update(cx, |picker, _| {
+            assert_eq!(
+                picker.logical_scroll_top_index(),
+                expected,
+                "scroll top should remain at {expected} ({phase})"
+            );
+            assert_selected_entry_is_recent_project(picker);
+        });
+    }
+
+    #[track_caller]
+    fn assert_pinned_to_bottom(
+        picker: &Entity<Picker<RecentProjectsDelegate>>,
+        cx: &mut VisualTestContext,
+        phase: &str,
+    ) {
+        picker.update(cx, |picker, _| {
+            assert_eq!(
+                picker.is_scrolled_to_end(),
+                Some(true),
+                "picker should remain pinned to the bottom ({phase})"
+            );
+            assert!(
+                picker.logical_scroll_top_index() > 0,
+                "picker should not jump to the top while pinned to the bottom ({phase})"
+            );
+            assert_selected_entry_is_recent_project(picker);
+        });
+    }
+
+    #[track_caller]
+    fn assert_selected_entry_is_recent_project(picker: &Picker<RecentProjectsDelegate>) {
+        assert!(matches!(
+            picker
+                .delegate
+                .filtered_entries
+                .get(picker.delegate.selected_index),
+            Some(ProjectPickerEntry::RecentProject(_))
+        ));
+    }
+
+    #[gpui::test]
+    fn deleting_top_recent_project_preserves_scroll_position(cx: &mut TestAppContext) {
+        let target = FIRST_RECENT_PROJECT;
+        let (picker, cx) = build_picker(cx);
+        let scroll_top = scroll_to_and_select(&picker, cx, target);
+        assert!(
+            scroll_top > 0,
+            "test should start scrolled away from the top"
+        );
+
+        delete_recent_project_in_picker(&picker, cx, target);
+        assert_scroll_top_is(&picker, cx, scroll_top, "after delete");
+
+        // The picker re-runs layout on the next frame; the scroll position
+        // must still be preserved after that redraw.
+        draw(cx);
+        assert_scroll_top_is(&picker, cx, scroll_top, "after redraw");
+    }
+
+    #[gpui::test]
+    fn deleting_middle_recent_project_preserves_scroll_position(cx: &mut TestAppContext) {
+        let target = FIRST_RECENT_PROJECT + RECENT_PROJECT_COUNT / 2;
+        let (picker, cx) = build_picker(cx);
+        let scroll_top = scroll_to_and_select(&picker, cx, target);
+        assert!(
+            scroll_top > 0,
+            "test should start scrolled away from the top"
+        );
+
+        delete_recent_project_in_picker(&picker, cx, target);
+        assert_scroll_top_is(&picker, cx, scroll_top, "after delete");
+
+        draw(cx);
+        assert_scroll_top_is(&picker, cx, scroll_top, "after redraw");
+    }
+
+    #[gpui::test]
+    fn deleting_last_recent_project_preserves_scroll_position(cx: &mut TestAppContext) {
+        let target = LAST_RECENT_PROJECT;
+        let (picker, cx) = build_picker(cx);
+        scroll_to_and_select(&picker, cx, target);
+
+        picker.update(cx, |picker, _| {
+            assert_eq!(
+                picker.is_scrolled_to_end(),
+                Some(true),
+                "selecting the last entry should leave the picker pinned to the bottom"
+            );
+        });
+
+        delete_recent_project_in_picker(&picker, cx, target);
+        assert_pinned_to_bottom(&picker, cx, "after delete");
+
+        draw(cx);
+        assert_pinned_to_bottom(&picker, cx, "after redraw");
+    }
+
     #[gpui::test]
     async fn test_open_dev_container_action_with_single_config(cx: &mut TestAppContext) {
         let app_state = init_test(cx);