From 6fce53651b40fcb301f8576268ee1155d51b5334 Mon Sep 17 00:00:00 2001 From: tims <0xtimsb@gmail.com> Date: Tue, 7 Jan 2025 06:16:50 +0530 Subject: [PATCH] project_panel: Refine selection, copying, and deletion behavior (#22658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #22655 A more detailed write-up for this change can be found in the issue itself. Here, I'm just providing previews after the change. The preview before the change can be found in the attached issue. 1. While selecting multiple entries, the last clicked entry should be selected. [a.webm](https://github.com/user-attachments/assets/2add69c3-82a9-4e45-92e8-366aaf9b298a) 2. When holding `Ctrl`/`Cmd` on an entry, there should be clear visual feedback to indicate whether the entry is selected or marked. [b.webm](https://github.com/user-attachments/assets/2cefb8aa-e7d0-4929-9efa-89a4329f428b) 3. When only one entry is marked, but it’s different from the selection, operations should prioritize the selected entry. This let's you do quick one-off actions without disrupting the marked state. [c.webm](https://github.com/user-attachments/assets/8e7ae0c0-4387-49b9-9761-5d02a1c21a84) 4. When more than one entries are marked, operations should prioritize the marked entries. If the selection differs from the marked entries, it should not interfere with operations on the marked entries. This let's you do actions on multiple marked entries without needing to adjust the selection. [d.webm](https://github.com/user-attachments/assets/165a74be-cbe9-48ac-b558-2562485ea224) Release Notes: - Improved project panel selection, copying, and deletion behavior, to be more predictable. --- crates/project_panel/src/project_panel.rs | 159 +++++++++++++++++++--- 1 file changed, 137 insertions(+), 22 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 84a8d45026..058276b0bf 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1946,7 +1946,7 @@ impl ProjectPanel { fn copy_path(&mut self, _: &CopyPath, cx: &mut ViewContext) { let abs_file_paths = { let project = self.project.read(cx); - self.marked_entries() + self.effective_entries() .into_iter() .filter_map(|entry| { let entry_path = project.path_for_entry(entry.entry_id, cx)?.path; @@ -1970,7 +1970,7 @@ impl ProjectPanel { fn copy_relative_path(&mut self, _: &CopyRelativePath, cx: &mut ViewContext) { let file_paths = { let project = self.project.read(cx); - self.marked_entries() + self.effective_entries() .into_iter() .filter_map(|entry| { Some( @@ -2154,7 +2154,7 @@ impl ProjectPanel { } fn disjoint_entries(&self, cx: &AppContext) -> BTreeSet { - let marked_entries = self.marked_entries(); + let marked_entries = self.effective_entries(); let mut sanitized_entries = BTreeSet::new(); if marked_entries.is_empty() { return sanitized_entries; @@ -2201,25 +2201,35 @@ impl ProjectPanel { sanitized_entries } - // Returns the union of the currently selected entry and all marked entries. - fn marked_entries(&self) -> BTreeSet { - let mut entries = self - .marked_entries + fn effective_entries(&self) -> BTreeSet { + if let Some(selection) = self.selection { + let selection = SelectedEntry { + entry_id: self.resolve_entry(selection.entry_id), + worktree_id: selection.worktree_id, + }; + + // Default to using just the selected item when nothing is marked. + if self.marked_entries.is_empty() { + return BTreeSet::from([selection]); + } + + // Allow operating on the selected item even when something else is marked, + // making it easier to perform one-off actions without clearing a mark. + if self.marked_entries.len() == 1 && !self.marked_entries.contains(&selection) { + return BTreeSet::from([selection]); + } + } + + // Return only marked entries since we've already handled special cases where + // only selection should take precedence. At this point, marked entries may or + // may not include the current selection, which is intentional. + self.marked_entries .iter() .map(|entry| SelectedEntry { entry_id: self.resolve_entry(entry.entry_id), worktree_id: entry.worktree_id, }) - .collect::>(); - - if let Some(selection) = self.selection { - entries.insert(SelectedEntry { - entry_id: self.resolve_entry(selection.entry_id), - worktree_id: selection.worktree_id, - }); - } - - entries + .collect::>() } /// Finds the currently selected subentry for a given leaf entry id. If a given entry @@ -3262,16 +3272,18 @@ impl ProjectPanel { marked_selections: selections, }; - let default_color = if is_marked || is_active { + let default_color = if is_marked { item_colors.marked_active } else { item_colors.default }; - let bg_hover_color = if self.mouse_down || is_marked || is_active { + let bg_hover_color = if self.mouse_down || is_marked { item_colors.marked_active - } else { + } else if !is_active { item_colors.hover + } else { + item_colors.default }; let border_color = @@ -3414,8 +3426,11 @@ impl ProjectPanel { } else if event.down.modifiers.secondary() { if event.down.click_count > 1 { this.split_entry(entry_id, cx); - } else if !this.marked_entries.insert(selection) { - this.marked_entries.remove(&selection); + } else { + this.selection = Some(selection); + if !this.marked_entries.insert(selection) { + this.marked_entries.remove(&selection); + } } } else if kind.is_dir() { this.marked_entries.clear(); @@ -7850,6 +7865,106 @@ mod tests { ); } + #[gpui::test] + async fn test_selection_vs_marked_entries_priority(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + json!({ + "dir1": { + "file1.txt": "", + "file2.txt": "", + "file3.txt": "", + }, + "dir2": { + "file4.txt": "", + "file5.txt": "", + }, + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + toggle_expand_dir(&panel, "root/dir1", cx); + toggle_expand_dir(&panel, "root/dir2", cx); + + cx.simulate_modifiers_change(gpui::Modifiers { + control: true, + ..Default::default() + }); + + select_path_with_mark(&panel, "root/dir1/file2.txt", cx); + select_path(&panel, "root/dir1/file1.txt", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " file1.txt <== selected", + " file2.txt <== marked", + " file3.txt", + " v dir2", + " file4.txt", + " file5.txt", + ], + "Initial state with one marked entry and different selection" + ); + + // Delete should operate on the selected entry (file1.txt) + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " file2.txt <== selected <== marked", + " file3.txt", + " v dir2", + " file4.txt", + " file5.txt", + ], + "Should delete selected file, not marked file" + ); + + select_path_with_mark(&panel, "root/dir1/file3.txt", cx); + select_path_with_mark(&panel, "root/dir2/file4.txt", cx); + select_path(&panel, "root/dir2/file5.txt", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " file2.txt <== marked", + " file3.txt <== marked", + " v dir2", + " file4.txt <== marked", + " file5.txt <== selected", + ], + "Initial state with multiple marked entries and different selection" + ); + + // Delete should operate on all marked entries, ignoring the selection + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " v dir2", + " file5.txt <== selected", + ], + "Should delete all marked files, leaving only the selected file" + ); + } + #[gpui::test] async fn test_selection_fallback_to_next_highest_worktree(cx: &mut gpui::TestAppContext) { init_test_with_editor(cx);