Merge pull request #1810 from zed-industries/contacts-scroll-position

Antonio Scandurra created

Maintain scroll position in contacts list

Change summary

.gitignore                                      | 10 ++
Cargo.toml                                      | 58 ++++++++++++++++++
crates/collab_ui/src/contact_list.rs            | 60 +++++++++++++++++-
crates/gpui/src/elements/list.rs                | 45 +++++++++----
crates/live_kit_client/LiveKitBridge/.gitignore |  9 --
5 files changed, 153 insertions(+), 29 deletions(-)

Detailed changes

.gitignore 🔗

@@ -9,4 +9,12 @@
 /assets/themes/*.json
 /assets/themes/Internal/*.json
 /assets/themes/Experiments/*.json
-**/venv
+**/venv
+.build
+Packages
+*.xcodeproj
+xcuserdata/
+DerivedData/
+.swiftpm/config/registries.json
+.swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata
+.netrc

Cargo.toml 🔗

@@ -1,5 +1,61 @@
 [workspace]
-members = ["crates/*"]
+members = [
+    "crates/activity_indicator",
+    "crates/assets",
+    "crates/auto_update",
+    "crates/breadcrumbs",
+    "crates/call",
+    "crates/cli",
+    "crates/client",
+    "crates/clock",
+    "crates/collab",
+    "crates/collab_ui",
+    "crates/collections",
+    "crates/command_palette",
+    "crates/context_menu",
+    "crates/db",
+    "crates/diagnostics",
+    "crates/drag_and_drop",
+    "crates/editor",
+    "crates/file_finder",
+    "crates/fs",
+    "crates/fsevent",
+    "crates/fuzzy",
+    "crates/git",
+    "crates/go_to_line",
+    "crates/gpui",
+    "crates/gpui_macros",
+    "crates/journal",
+    "crates/language",
+    "crates/live_kit_client",
+    "crates/live_kit_server",
+    "crates/lsp",
+    "crates/media",
+    "crates/menu",
+    "crates/outline",
+    "crates/picker",
+    "crates/plugin",
+    "crates/plugin_macros",
+    "crates/plugin_runtime",
+    "crates/project",
+    "crates/project_panel",
+    "crates/project_symbols",
+    "crates/rope",
+    "crates/rpc",
+    "crates/search",
+    "crates/settings",
+    "crates/snippet",
+    "crates/sum_tree",
+    "crates/terminal",
+    "crates/text",
+    "crates/theme",
+    "crates/theme_selector",
+    "crates/theme_testbench",
+    "crates/util",
+    "crates/vim",
+    "crates/workspace",
+    "crates/zed",
+]
 default-members = ["crates/zed"]
 resolver = "2"
 

crates/collab_ui/src/contact_list.rs 🔗

@@ -1,4 +1,4 @@
-use std::sync::Arc;
+use std::{mem, sync::Arc};
 
 use crate::contacts_popover;
 use call::ActiveCall;
@@ -334,8 +334,14 @@ impl ContactList {
         } else if !self.entries.is_empty() {
             self.selection = Some(0);
         }
-        cx.notify();
         self.list_state.reset(self.entries.len());
+        if let Some(ix) = self.selection {
+            self.list_state.scroll_to(ListOffset {
+                item_ix: ix,
+                offset_in_item: 0.,
+            });
+        }
+        cx.notify();
     }
 
     fn select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext<Self>) {
@@ -346,8 +352,14 @@ impl ContactList {
                 self.selection = None;
             }
         }
-        cx.notify();
         self.list_state.reset(self.entries.len());
+        if let Some(ix) = self.selection {
+            self.list_state.scroll_to(ListOffset {
+                item_ix: ix,
+                offset_in_item: 0.,
+            });
+        }
+        cx.notify();
     }
 
     fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext<Self>) {
@@ -404,7 +416,7 @@ impl ContactList {
         let executor = cx.background().clone();
 
         let prev_selected_entry = self.selection.and_then(|ix| self.entries.get(ix).cloned());
-        self.entries.clear();
+        let old_entries = mem::take(&mut self.entries);
 
         if let Some(room) = ActiveCall::global(cx).read(cx).room() {
             let room = room.read(cx);
@@ -653,7 +665,47 @@ impl ContactList {
             }
         }
 
+        let old_scroll_top = self.list_state.logical_scroll_top();
         self.list_state.reset(self.entries.len());
+
+        // Attempt to maintain the same scroll position.
+        if let Some(old_top_entry) = old_entries.get(old_scroll_top.item_ix) {
+            let new_scroll_top = self
+                .entries
+                .iter()
+                .position(|entry| entry == old_top_entry)
+                .map(|item_ix| ListOffset {
+                    item_ix,
+                    offset_in_item: old_scroll_top.offset_in_item,
+                })
+                .or_else(|| {
+                    let entry_after_old_top = old_entries.get(old_scroll_top.item_ix + 1)?;
+                    let item_ix = self
+                        .entries
+                        .iter()
+                        .position(|entry| entry == entry_after_old_top)?;
+                    Some(ListOffset {
+                        item_ix,
+                        offset_in_item: 0.,
+                    })
+                })
+                .or_else(|| {
+                    let entry_before_old_top =
+                        old_entries.get(old_scroll_top.item_ix.saturating_sub(1))?;
+                    let item_ix = self
+                        .entries
+                        .iter()
+                        .position(|entry| entry == entry_before_old_top)?;
+                    Some(ListOffset {
+                        item_ix,
+                        offset_in_item: 0.,
+                    })
+                });
+
+            self.list_state
+                .scroll_to(new_scroll_top.unwrap_or(old_scroll_top));
+        }
+
         cx.notify();
     }
 

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

@@ -38,8 +38,8 @@ struct StateInner {
 
 #[derive(Clone, Copy, Debug, Default, PartialEq)]
 pub struct ListOffset {
-    item_ix: usize,
-    offset_in_item: f32,
+    pub item_ix: usize,
+    pub offset_in_item: f32,
 }
 
 #[derive(Clone)]
@@ -112,18 +112,7 @@ impl Element for List {
         let mut new_items = SumTree::new();
         let mut rendered_items = VecDeque::new();
         let mut rendered_height = 0.;
-        let mut scroll_top = state
-            .logical_scroll_top
-            .unwrap_or_else(|| match state.orientation {
-                Orientation::Top => ListOffset {
-                    item_ix: 0,
-                    offset_in_item: 0.,
-                },
-                Orientation::Bottom => ListOffset {
-                    item_ix: state.items.summary().count,
-                    offset_in_item: 0.,
-                },
-            });
+        let mut scroll_top = state.logical_scroll_top();
 
         // Render items after the scroll top, including those in the trailing overdraw.
         let mut cursor = old_items.cursor::<Count>();
@@ -421,6 +410,20 @@ impl ListState {
     ) {
         self.0.borrow_mut().scroll_handler = Some(Box::new(handler))
     }
+
+    pub fn logical_scroll_top(&self) -> ListOffset {
+        self.0.borrow().logical_scroll_top()
+    }
+
+    pub fn scroll_to(&self, mut scroll_top: ListOffset) {
+        let state = &mut *self.0.borrow_mut();
+        let item_count = state.items.summary().count;
+        if scroll_top.item_ix >= item_count {
+            scroll_top.item_ix = item_count;
+            scroll_top.offset_in_item = 0.;
+        }
+        state.logical_scroll_top = Some(scroll_top);
+    }
 }
 
 impl StateInner {
@@ -514,6 +517,20 @@ impl StateInner {
         cx.notify();
     }
 
+    fn logical_scroll_top(&self) -> ListOffset {
+        self.logical_scroll_top
+            .unwrap_or_else(|| match self.orientation {
+                Orientation::Top => ListOffset {
+                    item_ix: 0,
+                    offset_in_item: 0.,
+                },
+                Orientation::Bottom => ListOffset {
+                    item_ix: self.items.summary().count,
+                    offset_in_item: 0.,
+                },
+            })
+    }
+
     fn scroll_top(&self, logical_scroll_top: &ListOffset) -> f32 {
         let mut cursor = self.items.cursor::<ListItemSummary>();
         cursor.seek(&Count(logical_scroll_top.item_ix), Bias::Right, &());