git_ui: Fix select next/previous entry selects non-visible entry when tree view is enabled (#45030)

Remco Smits created 1 week ago

Before this commit, we would select a non-visible entry when a directory
is collapsed. Now we correctly select the visible entry that is visually
the previous/next entry in the list.

**Note**: I removed the `cx.notify()` call as it's already part of the
`self.scroll_to_selected_entry(cx)` call. So we don't notify twice :).

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

**Before**


https://github.com/user-attachments/assets/da0b8084-0081-4d98-ad8a-c11c3b95a1b7

**After**


https://github.com/user-attachments/assets/8a16afb0-fdde-4317-b419-13143d5d608e

Release Notes:

- git_ui: Fix select next/previous entry selects non-visible entry when
tree view is enabled

Change summary

crates/git_ui/src/git_panel.rs | 91 +++++++++++++++++++++++------------
1 file changed, 59 insertions(+), 32 deletions(-)

Detailed changes

crates/git_ui/src/git_panel.rs

@@ -961,28 +961,44 @@ impl GitPanel {
             return;
         }
 
-        if let Some(selected_entry) = self.selected_entry {
-            let new_selected_entry = if selected_entry > 0 {
-                selected_entry - 1
-            } else {
-                selected_entry
-            };
+        let Some(selected_entry) = self.selected_entry else {
+            return;
+        };
 
-            if matches!(
-                self.entries.get(new_selected_entry),
-                Some(GitListEntry::Header(..))
-            ) {
-                if new_selected_entry > 0 {
-                    self.selected_entry = Some(new_selected_entry - 1)
-                }
-            } else {
-                self.selected_entry = Some(new_selected_entry);
+        let new_index = match &self.view_mode {
+            GitPanelViewMode::Flat => selected_entry.saturating_sub(1),
+            GitPanelViewMode::Tree(state) => {
+                let Some(current_logical_index) = state
+                    .logical_indices
+                    .iter()
+                    .position(|&i| i == selected_entry)
+                else {
+                    return;
+                };
+
+                state.logical_indices[current_logical_index.saturating_sub(1)]
             }
+        };
 
-            self.scroll_to_selected_entry(cx);
+        if selected_entry == 0 && new_index == 0 {
+            return;
         }
 
-        cx.notify();
+        if matches!(
+            self.entries.get(new_index.saturating_sub(1)),
+            Some(GitListEntry::Header(..))
+        ) && new_index == 0
+        {
+            return;
+        }
+
+        if matches!(self.entries.get(new_index), Some(GitListEntry::Header(..))) {
+            self.selected_entry = Some(new_index.saturating_sub(1));
+        } else {
+            self.selected_entry = Some(new_index);
+        }
+
+        self.scroll_to_selected_entry(cx);
     }
 
     fn select_next(&mut self, _: &SelectNext, _window: &mut Window, cx: &mut Context<Self>) {
@@ -991,25 +1007,36 @@ impl GitPanel {
             return;
         }
 
-        if let Some(selected_entry) = self.selected_entry {
-            let new_selected_entry = if selected_entry < item_count - 1 {
-                selected_entry + 1
-            } else {
-                selected_entry
-            };
-            if matches!(
-                self.entries.get(new_selected_entry),
-                Some(GitListEntry::Header(..))
-            ) {
-                self.selected_entry = Some(new_selected_entry + 1);
-            } else {
-                self.selected_entry = Some(new_selected_entry);
+        let Some(selected_entry) = self.selected_entry else {
+            return;
+        };
+
+        if selected_entry == item_count - 1 {
+            return;
+        }
+
+        let new_index = match &self.view_mode {
+            GitPanelViewMode::Flat => selected_entry.saturating_add(1),
+            GitPanelViewMode::Tree(state) => {
+                let Some(current_logical_index) = state
+                    .logical_indices
+                    .iter()
+                    .position(|&i| i == selected_entry)
+                else {
+                    return;
+                };
+
+                state.logical_indices[current_logical_index.saturating_add(1)]
             }
+        };
 
-            self.scroll_to_selected_entry(cx);
+        if matches!(self.entries.get(new_index), Some(GitListEntry::Header(..))) {
+            self.selected_entry = Some(new_index.saturating_add(1));
+        } else {
+            self.selected_entry = Some(new_index);
         }
 
-        cx.notify();
+        self.scroll_to_selected_entry(cx);
     }
 
     fn select_last(&mut self, _: &SelectLast, _window: &mut Window, cx: &mut Context<Self>) {