From 35595fe3c2ea70578350753fdfbd30a95fe91962 Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Wed, 15 Oct 2025 18:50:49 +0200 Subject: [PATCH] search: Dismiss modal view when running search action (#39446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, using cmd-f or cmd-shift-f to search while a modal is active (e.g. after cmd-t or cmd-p) doesn't do anything — you need to first close the modal manually before initiating a search. This PR allows these actions to run regardless of whether a modal is active. Some context: VSCode lets you do this too, and for me it's quite common to do a symbol search with cmd-t immediately followed by a regular search with cmd-shift-f if I don't find what I'm looking for, so having to close the modal first is slightly disruptive. cmd-t followed by cmd-p does dismiss the project symbols modal in order to display the file search modal, so it makes sense to me to also allow search actions to dismiss an active modal. Maybe this blunt fix has unintended consequences? If some types of modals shouldn't be dismissed when running cmd-f, or some actions shouldn't dismiss a currently active modal, then we'll have to go about it differently. Release Notes: - Added the ability to run search actions when a modal is currently active --- crates/search/src/buffer_search/registrar.rs | 2 +- crates/search/src/project_search.rs | 69 ++++++++++++++++++-- crates/workspace/src/workspace.rs | 5 ++ 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/crates/search/src/buffer_search/registrar.rs b/crates/search/src/buffer_search/registrar.rs index 0e227cbb7c3a465892a2eed867a001aa48b80ff9..2c640e67cee98fb5e56eda1484ecf6e2fab41976 100644 --- a/crates/search/src/buffer_search/registrar.rs +++ b/crates/search/src/buffer_search/registrar.rs @@ -62,7 +62,7 @@ impl SearchActionsRegistrar for DivRegistrar<'_, '_, T> { impl SearchActionsRegistrar for Workspace { fn register_handler(&mut self, callback: impl ActionExecutor) { self.register_action(move |workspace, action: &A, window, cx| { - if workspace.has_active_modal(window, cx) { + if workspace.has_active_modal(window, cx) && !workspace.hide_modal(window, cx) { cx.propagate(); return; } diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 9014f1764d7a6097908624e0d93bc6174e998445..4197e57e4d4db0f3ca6d3827d5b32ba19fab6295 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -153,7 +153,7 @@ pub fn init(cx: &mut App) { // Both on present and dismissed search, we need to unconditionally handle those actions to focus from the editor. workspace.register_action(move |workspace, action: &DeploySearch, window, cx| { - if workspace.has_active_modal(window, cx) { + if workspace.has_active_modal(window, cx) && !workspace.hide_modal(window, cx) { cx.propagate(); return; } @@ -161,7 +161,7 @@ pub fn init(cx: &mut App) { cx.notify(); }); workspace.register_action(move |workspace, action: &NewSearch, window, cx| { - if workspace.has_active_modal(window, cx) { + if workspace.has_active_modal(window, cx) && !workspace.hide_modal(window, cx) { cx.propagate(); return; } @@ -2281,7 +2281,7 @@ fn register_workspace_action( callback: fn(&mut ProjectSearchBar, &A, &mut Window, &mut Context), ) { workspace.register_action(move |workspace, action: &A, window, cx| { - if workspace.has_active_modal(window, cx) { + if workspace.has_active_modal(window, cx) && !workspace.hide_modal(window, cx) { cx.propagate(); return; } @@ -2308,7 +2308,7 @@ fn register_workspace_action_for_present_search( callback: fn(&mut Workspace, &A, &mut Window, &mut Context), ) { workspace.register_action(move |workspace, action: &A, window, cx| { - if workspace.has_active_modal(window, cx) { + if workspace.has_active_modal(window, cx) && !workspace.hide_modal(window, cx) { cx.propagate(); return; } @@ -4148,6 +4148,67 @@ pub mod tests { }); } + #[gpui::test] + async fn test_search_dismisses_modal(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.background_executor.clone()); + fs.insert_tree( + path!("/dir"), + json!({ + "one.rs": "const ONE: usize = 1;", + }), + ) + .await; + let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; + let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx)); + + struct EmptyModalView { + focus_handle: gpui::FocusHandle, + } + impl EventEmitter for EmptyModalView {} + impl Render for EmptyModalView { + fn render(&mut self, _: &mut Window, _: &mut Context<'_, Self>) -> impl IntoElement { + div() + } + } + impl Focusable for EmptyModalView { + fn focus_handle(&self, _cx: &App) -> gpui::FocusHandle { + self.focus_handle.clone() + } + } + impl workspace::ModalView for EmptyModalView {} + + window + .update(cx, |workspace, window, cx| { + workspace.toggle_modal(window, cx, |_, cx| EmptyModalView { + focus_handle: cx.focus_handle(), + }); + assert!(workspace.has_active_modal(window, cx)); + }) + .unwrap(); + + cx.dispatch_action(window.into(), Deploy::find()); + + window + .update(cx, |workspace, window, cx| { + assert!(!workspace.has_active_modal(window, cx)); + workspace.toggle_modal(window, cx, |_, cx| EmptyModalView { + focus_handle: cx.focus_handle(), + }); + assert!(workspace.has_active_modal(window, cx)); + }) + .unwrap(); + + cx.dispatch_action(window.into(), DeploySearch::find()); + + window + .update(cx, |workspace, window, cx| { + assert!(!workspace.has_active_modal(window, cx)); + }) + .unwrap(); + } + fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { let settings = SettingsStore::test(cx); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 81247b89593074e8f44565d9a8daabaf6b8307f5..086554335f2733a7e6c8d7976d65acbaeb044050 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -5864,6 +5864,11 @@ impl Workspace { }) } + pub fn hide_modal(&mut self, window: &mut Window, cx: &mut App) -> bool { + self.modal_layer + .update(cx, |modal_layer, cx| modal_layer.hide_modal(window, cx)) + } + pub fn toggle_status_toast(&mut self, entity: Entity, cx: &mut App) { self.toast_layer .update(cx, |toast_layer, cx| toast_layer.toggle_toast(cx, entity))