project_panel: Fix autoscroll and filename editor focus race condition (#42739) (cherry-pick to stable) (#43130)

zed-zippy[bot] and Smit Barmase created 1 month ago

Cherry-pick of #42739 to stable

----
Closes https://github.com/zed-industries/zed/issues/40867

Since the recent changes in

[https://github.com/zed-industries/zed/pull/38881](https://github.com/zed-industries/zed/pull/38881),
the filename editor is sometimes not focused after duplicating a file or
creating a new one, and similarly, autoscroll sometimes didn’t work. It
turns out that multiple calls to `update_visible_entries_task` cancel
the existing task, which might contain information about whether we need
to focus the filename editor and autoscroll after the task ends. To fix
this, we now carry that information forward to the next task that
overwrites it, so that when the latest task ends, we can use that
information to do the right thing.

Release Notes:

- Fixed an issue in the Project Panel where duplicating or creating an
entry sometimes didn’t focus the rename editing field.

Co-authored-by: Smit Barmase <[e-mail]>

Change summary

crates/project_panel/src/project_panel.rs       | 43 +++++++++++++++---
crates/project_panel/src/project_panel_tests.rs | 11 +++
2 files changed, 44 insertions(+), 10 deletions(-)

Detailed changes

crates/project_panel/src/project_panel.rs

@@ -136,10 +136,26 @@ pub struct ProjectPanel {
     previous_drag_position: Option<Point<Pixels>>,
     sticky_items_count: usize,
     last_reported_update: Instant,
-    update_visible_entries_task: Task<()>,
+    update_visible_entries_task: UpdateVisibleEntriesTask,
     state: State,
 }
 
+struct UpdateVisibleEntriesTask {
+    _visible_entries_task: Task<()>,
+    focus_filename_editor: bool,
+    autoscroll: bool,
+}
+
+impl Default for UpdateVisibleEntriesTask {
+    fn default() -> Self {
+        UpdateVisibleEntriesTask {
+            _visible_entries_task: Task::ready(()),
+            focus_filename_editor: Default::default(),
+            autoscroll: Default::default(),
+        }
+    }
+}
+
 enum DragTarget {
     /// Dragging on an entry
     Entry {
@@ -733,7 +749,7 @@ impl ProjectPanel {
                     expanded_dir_ids: Default::default(),
                     unfolded_dir_ids: Default::default(),
                 },
-                update_visible_entries_task: Task::ready(()),
+                update_visible_entries_task: Default::default(),
             };
             this.update_visible_entries(None, false, false, window, cx);
 
@@ -1823,6 +1839,9 @@ impl ProjectPanel {
             depth: 0,
             validation_state: ValidationState::None,
         });
+        self.filename_editor.update(cx, |editor, cx| {
+            editor.clear(window, cx);
+        });
         self.update_visible_entries(Some((worktree_id, NEW_ENTRY_ID)), true, true, window, cx);
         cx.notify();
     }
@@ -1889,9 +1908,8 @@ impl ProjectPanel {
                     editor.change_selections(Default::default(), window, cx, |s| {
                         s.select_ranges([selection])
                     });
-                    window.focus(&editor.focus_handle(cx));
                 });
-                self.update_visible_entries(None, false, true, window, cx);
+                self.update_visible_entries(None, true, true, window, cx);
                 cx.notify();
             }
         }
@@ -3229,7 +3247,8 @@ impl ProjectPanel {
             .collect();
         let hide_root = settings.hide_root && visible_worktrees.len() == 1;
         let hide_hidden = settings.hide_hidden;
-        self.update_visible_entries_task = cx.spawn_in(window, async move |this, cx| {
+
+        let visible_entries_task = cx.spawn_in(window, async move |this, cx| {
             let new_state = cx
                 .background_spawn(async move {
                     for worktree_snapshot in visible_worktrees {
@@ -3475,19 +3494,27 @@ impl ProjectPanel {
                             .sum::<usize>(),
                     )
                 }
-                if focus_filename_editor {
+                if this.update_visible_entries_task.focus_filename_editor {
+                    this.update_visible_entries_task.focus_filename_editor = false;
                     this.filename_editor.update(cx, |editor, cx| {
-                        editor.clear(window, cx);
                         window.focus(&editor.focus_handle(cx));
                     });
                 }
-                if autoscroll {
+                if this.update_visible_entries_task.autoscroll {
+                    this.update_visible_entries_task.autoscroll = false;
                     this.autoscroll(cx);
                 }
                 cx.notify();
             })
             .ok();
         });
+
+        self.update_visible_entries_task = UpdateVisibleEntriesTask {
+            _visible_entries_task: visible_entries_task,
+            focus_filename_editor: focus_filename_editor
+                || self.update_visible_entries_task.focus_filename_editor,
+            autoscroll: autoscroll || self.update_visible_entries_task.autoscroll,
+        };
     }
 
     fn expand_entry(

crates/project_panel/src/project_panel_tests.rs

@@ -807,6 +807,7 @@ async fn test_editing_files(cx: &mut gpui::TestAppContext) {
     panel.update_in(cx, |panel, window, cx| {
         panel.rename(&Default::default(), window, cx)
     });
+    cx.run_until_parked();
     assert_eq!(
         visible_entries_as_strings(&panel, 0..10, cx),
         &[
@@ -1200,7 +1201,9 @@ async fn test_copy_paste(cx: &mut gpui::TestAppContext) {
         panel.paste(&Default::default(), window, cx);
     });
     cx.executor().run_until_parked();
-
+    panel.update_in(cx, |panel, window, cx| {
+        assert!(panel.filename_editor.read(cx).is_focused(window));
+    });
     assert_eq!(
         visible_entries_as_strings(&panel, 0..50, cx),
         &[
@@ -1239,7 +1242,9 @@ async fn test_copy_paste(cx: &mut gpui::TestAppContext) {
         panel.paste(&Default::default(), window, cx);
     });
     cx.executor().run_until_parked();
-
+    panel.update_in(cx, |panel, window, cx| {
+        assert!(panel.filename_editor.read(cx).is_focused(window));
+    });
     assert_eq!(
         visible_entries_as_strings(&panel, 0..50, cx),
         &[
@@ -2398,6 +2403,7 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) {
         ],
     );
     panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx));
+    cx.executor().run_until_parked();
     panel.update_in(cx, |panel, window, cx| {
         assert!(panel.filename_editor.read(cx).is_focused(window));
     });
@@ -2603,6 +2609,7 @@ async fn test_create_duplicate_items_and_check_history(cx: &mut gpui::TestAppCon
         ],
     );
     panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx));
+    cx.executor().run_until_parked();
     panel.update_in(cx, |panel, window, cx| {
         assert!(panel.filename_editor.read(cx).is_focused(window));
     });