From 8186af99a347dfa9f9fd5af88da419b97b9727fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yara=20=F0=9F=8F=B3=EF=B8=8F=E2=80=8D=E2=9A=A7=EF=B8=8F?= Date: Tue, 30 Jun 2026 12:09:26 +0200 Subject: [PATCH] Restore last picker (#59912) Adds ability to re-open any closed picker. Release Notes: - Added: workspace::ReopenLastPicker Co-authored-by: Richard Feldman --- assets/keymaps/default-linux.json | 1 + assets/keymaps/default-macos.json | 1 + assets/keymaps/default-windows.json | 1 + crates/command_palette/src/command_palette.rs | 3 +- crates/picker/src/picker.rs | 24 +++ crates/workspace/src/modal_layer.rs | 101 +++++++++-- crates/workspace/src/workspace.rs | 168 ++++++++++++++++++ 7 files changed, 282 insertions(+), 17 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index bf9f84ad21d..56b853384f9 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -678,6 +678,7 @@ "ctrl-shift-f": "pane::DeploySearch", "ctrl-shift-h": ["pane::DeploySearch", { "replace_enabled": true }], "ctrl-shift-t": "pane::ReopenClosedItem", + "ctrl-k ctrl-p": "workspace::ReopenLastPicker", "ctrl-k ctrl-s": "zed::OpenKeymap", "ctrl-k ctrl-t": "theme_selector::Toggle", "ctrl-k ctrl-shift-t": "theme::ToggleMode", diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 6896193e50c..009490e4278 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -740,6 +740,7 @@ "cmd-shift-f": "pane::DeploySearch", "cmd-shift-h": ["pane::DeploySearch", { "replace_enabled": true }], "cmd-shift-t": "pane::ReopenClosedItem", + "cmd-k cmd-p": "workspace::ReopenLastPicker", "cmd-k cmd-s": "zed::OpenKeymap", "cmd-k cmd-t": "theme_selector::Toggle", "cmd-k cmd-shift-t": "theme::ToggleMode", diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index f27fe19a7fa..035e4704458 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -666,6 +666,7 @@ "ctrl-shift-f": "pane::DeploySearch", "ctrl-shift-h": ["pane::DeploySearch", { "replace_enabled": true }], "ctrl-shift-t": "pane::ReopenClosedItem", + "ctrl-k ctrl-p": "workspace::ReopenLastPicker", "ctrl-k ctrl-s": "zed::OpenKeymap", "ctrl-k ctrl-t": "theme_selector::Toggle", "ctrl-k ctrl-shift-t": "theme::ToggleMode", diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 2b8f5a9fa94..3b8a6e770e4 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -125,7 +125,8 @@ impl CommandPalette { ); let picker = cx.new(|cx| { - let picker = Picker::uniform_list(delegate, window, cx); + // One-shot action; there's nothing to reopen. + let picker = Picker::uniform_list(delegate, window, cx).reopenable(false, cx); picker.set_query(query, window, cx); picker }); diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index dd0af8ef048..2f9d0ad3c4f 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -140,6 +140,7 @@ pub struct Picker { /// Handle for the default footer's Actions popover menu. Used to keep the /// picker open while that menu has focus. actions_menu_handle: PopoverMenuHandle, + reopenable: bool, } #[derive(Debug, Default, Clone, Copy, PartialEq)] @@ -544,7 +545,12 @@ impl Picker { item_bounds: Rc::new(RefCell::new(HashMap::default())), size_bounds, actions_menu_handle: PopoverMenuHandle::default(), + reopenable: true, }; + if this.reopenable { + let focus_handle = this.focus_handle(cx); + workspace::register_reopenable_picker(&focus_handle, cx); + } this.update_matches("".to_string(), window, cx); // give the delegate 4ms to render the first set of suggestions. this.delegate @@ -622,6 +628,24 @@ impl Picker { self } + /// Controls whether a modal hosting this picker can be revealed again with + /// `workspace::ReopenLastPicker` after it's dismissed. Defaults to `true`; + /// pass `false` to exclude this picker. As a builder, this only takes effect + /// while constructing the picker, before it's opened. + pub fn reopenable(mut self, reopenable: bool, cx: &mut App) -> Self { + if reopenable == self.reopenable { + return self; + } + self.reopenable = reopenable; + let focus_handle = self.focus_handle(cx); + if reopenable { + workspace::register_reopenable_picker(&focus_handle, cx); + } else { + workspace::deregister_reopenable_picker(&focus_handle, cx); + } + self + } + /// Presents the picker as embedded inside a larger container that provides /// its own chrome and dismissal, rather than the default self-contained /// modal. Use [`popover`](Self::popover) instead for menu-attached pickers. diff --git a/crates/workspace/src/modal_layer.rs b/crates/workspace/src/modal_layer.rs index 5949c0b1fff..4964d82159f 100644 --- a/crates/workspace/src/modal_layer.rs +++ b/crates/workspace/src/modal_layer.rs @@ -1,6 +1,6 @@ use gpui::{ - AnyView, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable as _, ManagedView, - MouseButton, Subscription, + AnyView, App, DismissEvent, Entity, EventEmitter, FocusHandle, Global, ManagedView, + MouseButton, Subscription, WeakFocusHandle, }; use ui::prelude::*; @@ -10,6 +10,42 @@ pub enum DismissDecision { Pending, } +// A modal that hosts a picker forwards its own focus handle to the inner picker, +// so the modal layer recognizes a reopenable picker by focus identity rather than +// requiring each modal wrapper to implement its own opt-in. +#[derive(Default)] +struct ReopenablePickerRegistry { + handles: Vec, +} + +impl Global for ReopenablePickerRegistry {} + +pub fn register_reopenable_picker(focus_handle: &FocusHandle, cx: &mut App) { + let registry = cx.default_global::(); + registry.handles.retain(|handle| handle.upgrade().is_some()); + if !registry.handles.iter().any(|handle| handle == focus_handle) { + registry.handles.push(focus_handle.downgrade()); + } +} + +pub fn deregister_reopenable_picker(focus_handle: &FocusHandle, cx: &mut App) { + let registry = cx.default_global::(); + registry + .handles + .retain(|handle| handle.upgrade().is_some() && handle != focus_handle); +} + +fn focus_handle_is_reopenable(focus_handle: &FocusHandle, cx: &App) -> bool { + cx.try_global::() + .is_some_and(|registry| { + registry + .handles + .iter() + .filter_map(|handle| handle.upgrade()) + .any(|handle| &handle == focus_handle) + }) +} + pub trait ModalView: ManagedView { fn on_before_dismiss( &mut self, @@ -31,6 +67,8 @@ pub trait ModalView: ManagedView { trait ModalViewHandle { fn on_before_dismiss(&mut self, window: &mut Window, cx: &mut App) -> DismissDecision; fn view(&self) -> AnyView; + fn focus_handle(&self, cx: &App) -> FocusHandle; + fn subscribe_dismiss(&self, window: &mut Window, cx: &mut Context) -> Subscription; fn fade_out_background(&self, cx: &mut App) -> bool; fn render_bare(&self, cx: &mut App) -> bool; } @@ -44,6 +82,16 @@ impl ModalViewHandle for Entity { self.clone().into() } + fn focus_handle(&self, cx: &App) -> FocusHandle { + self.read(cx).focus_handle(cx) + } + + fn subscribe_dismiss(&self, window: &mut Window, cx: &mut Context) -> Subscription { + cx.subscribe_in(self, window, |this, _, _: &DismissEvent, window, cx| { + this.hide_modal(window, cx); + }) + } + fn fade_out_background(&self, cx: &mut App) -> bool { self.read(cx).fade_out_background() } @@ -62,6 +110,10 @@ pub struct ActiveModal { pub struct ModalLayer { active_modal: Option, + // Kept alive (hidden) rather than dropped on dismissal so `ReopenLastPicker` + // can reveal it again with its exact prior state. Left intact across + // non-reopenable modals (e.g. the command palette), which may trigger the reopen. + stashed_modal: Option>, dismiss_on_focus_lost: bool, } @@ -79,6 +131,7 @@ impl ModalLayer { pub fn new() -> Self { Self { active_modal: None, + stashed_modal: None, dismiss_on_focus_lost: false, } } @@ -103,27 +156,25 @@ impl ModalLayer { } } let new_modal = cx.new(|cx| build_view(window, cx)); - self.show_modal(new_modal, window, cx); + self.show_modal(Box::new(new_modal), window, cx); cx.emit(ModalOpenedEvent); } /// Shows a modal and sets up subscriptions for dismiss events and focus tracking. /// The modal is automatically focused after being shown. - fn show_modal(&mut self, new_modal: Entity, window: &mut Window, cx: &mut Context) - where - V: ModalView, - { + fn show_modal( + &mut self, + modal: Box, + window: &mut Window, + cx: &mut Context, + ) { let focus_handle = cx.focus_handle(); + let modal_focus_handle = modal.focus_handle(cx); + let dismiss_subscription = modal.subscribe_dismiss(window, cx); self.active_modal = Some(ActiveModal { - modal: Box::new(new_modal.clone()), + modal, _subscriptions: [ - cx.subscribe_in( - &new_modal, - window, - |this, _, _: &DismissEvent, window, cx| { - this.hide_modal(window, cx); - }, - ), + dismiss_subscription, cx.on_focus_out(&focus_handle, window, |this, _event, window, cx| { if this.dismiss_on_focus_lost { this.hide_modal(window, cx); @@ -134,11 +185,25 @@ impl ModalLayer { focus_handle, }); cx.defer_in(window, move |_, window, cx| { - window.focus(&new_modal.focus_handle(cx), cx); + window.focus(&modal_focus_handle, cx); }); cx.notify(); } + /// Reveals the most recently stashed reopenable modal, if any. Returns whether + /// a modal was revealed. + pub fn reveal_stashed_modal(&mut self, window: &mut Window, cx: &mut Context) -> bool { + if self.active_modal.is_some() { + return false; + } + let Some(modal) = self.stashed_modal.take() else { + return false; + }; + self.show_modal(modal, window, cx); + cx.emit(ModalOpenedEvent); + true + } + /// Attempts to hide the currently active modal. /// /// The modal's `on_before_dismiss` method is called to determine if dismissal should proceed. @@ -166,11 +231,15 @@ impl ModalLayer { } if let Some(active_modal) = self.active_modal.take() { + let reopenable = focus_handle_is_reopenable(&active_modal.modal.focus_handle(cx), cx); if let Some(previous_focus) = active_modal.previous_focus_handle && active_modal.focus_handle.contains_focused(window, cx) { previous_focus.focus(window, cx); } + if reopenable { + self.stashed_modal = Some(active_modal.modal); + } cx.notify(); } self.dismiss_on_focus_lost = false; diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index f1c9984334a..cef5ece31b0 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -299,6 +299,8 @@ actions!( OpenComponentPreview, /// Reloads the active item. ReloadActiveItem, + /// Reopens the most recently dismissed picker in the current window. + ReopenLastPicker, /// Resets the active dock to its default size. ResetActiveDockSize, /// Resets all open docks to their default sizes. @@ -7446,6 +7448,7 @@ impl Workspace { .on_action(cx.listener(Self::activate_pane_at_index)) .on_action(cx.listener(Self::move_item_to_pane_at_index)) .on_action(cx.listener(Self::move_focused_panel_to_next_position)) + .on_action(cx.listener(Self::reopen_last_picker)) .on_action(cx.listener(Self::toggle_edit_predictions_all_files)) .on_action(cx.listener(Self::toggle_theme_mode)) .on_action(cx.listener(|workspace, _: &Unfollow, window, cx| { @@ -7915,6 +7918,22 @@ impl Workspace { .update(cx, |modal_layer, cx| modal_layer.hide_modal(window, cx)) } + fn reopen_last_picker( + &mut self, + _: &ReopenLastPicker, + window: &mut Window, + cx: &mut Context, + ) { + // When triggered from within another modal (e.g. the command palette), that + // modal's dismissal is asynchronous, so defer the reveal until it has closed; + // otherwise a modal would still be active and the reveal would be a no-op. + cx.defer_in(window, |workspace, window, cx| { + workspace.modal_layer.update(cx, |modal_layer, cx| { + modal_layer.reveal_stashed_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)) @@ -14091,6 +14110,155 @@ mod tests { } } + // Registers its focus handle as a reopenable picker on construction, like a real + // `Picker` does, so the modal layer recognizes it by focus identity. + struct ReopenableTestModal(FocusHandle); + + impl ReopenableTestModal { + fn new(_: &mut Window, cx: &mut Context) -> Self { + let focus_handle = cx.focus_handle(); + register_reopenable_picker(&focus_handle, cx); + Self(focus_handle) + } + } + + impl EventEmitter for ReopenableTestModal {} + + impl Focusable for ReopenableTestModal { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.0.clone() + } + } + + impl ModalView for ReopenableTestModal {} + + impl Render for ReopenableTestModal { + fn render( + &mut self, + _window: &mut Window, + _cx: &mut Context, + ) -> impl IntoElement { + div().track_focus(&self.0) + } + } + + #[gpui::test] + async fn test_reopen_last_picker(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + // A non-reopenable modal is dropped on dismissal and cannot be revealed. + workspace.update_in(cx, |workspace, window, cx| { + workspace.toggle_modal(window, cx, TestModal::new); + }); + cx.executor().run_until_parked(); + assert!(workspace.read_with(cx, |workspace, cx| { + workspace.active_modal::(cx).is_some() + })); + workspace.update_in(cx, |workspace, window, cx| { + let revealed = workspace.modal_layer.update(cx, |modal_layer, cx| { + modal_layer.hide_modal(window, cx); + modal_layer.reveal_stashed_modal(window, cx) + }); + assert!(!revealed, "a non-reopenable modal should not be revealable"); + }); + + // A reopenable modal is stashed on dismissal and revealed as the *same* + // entity, so its prior state is preserved exactly. + workspace.update_in(cx, |workspace, window, cx| { + workspace.toggle_modal(window, cx, ReopenableTestModal::new); + }); + cx.executor().run_until_parked(); + let original_id = workspace.read_with(cx, |workspace, cx| { + workspace + .active_modal::(cx) + .unwrap() + .entity_id() + }); + workspace.update_in(cx, |workspace, window, cx| { + workspace + .modal_layer + .update(cx, |modal_layer, cx| modal_layer.hide_modal(window, cx)); + }); + cx.executor().run_until_parked(); + assert!(workspace.read_with(cx, |workspace, cx| { + workspace.active_modal::(cx).is_none() + })); + workspace.update_in(cx, |workspace, window, cx| { + let revealed = workspace.modal_layer.update(cx, |modal_layer, cx| { + modal_layer.reveal_stashed_modal(window, cx) + }); + assert!(revealed, "a reopenable modal should be revealable"); + }); + cx.executor().run_until_parked(); + let revealed_id = workspace.read_with(cx, |workspace, cx| { + workspace + .active_modal::(cx) + .unwrap() + .entity_id() + }); + assert_eq!( + original_id, revealed_id, + "reveal should restore the same modal entity rather than building a new one" + ); + + workspace.update_in(cx, |workspace, window, cx| { + let revealed = workspace.modal_layer.update(cx, |modal_layer, cx| { + modal_layer.reveal_stashed_modal(window, cx) + }); + assert!(!revealed, "reveal should be a no-op while a modal is open"); + }); + + // A non-reopenable modal must not discard the stash, which is what lets the + // command palette be used to trigger the reopen. + workspace.update_in(cx, |workspace, window, cx| { + workspace + .modal_layer + .update(cx, |modal_layer, cx| modal_layer.hide_modal(window, cx)); + workspace.toggle_modal(window, cx, TestModal::new); + }); + cx.executor().run_until_parked(); + workspace.update_in(cx, |workspace, window, cx| { + let revealed = workspace.modal_layer.update(cx, |modal_layer, cx| { + modal_layer.hide_modal(window, cx); + modal_layer.reveal_stashed_modal(window, cx) + }); + assert!( + revealed, + "a non-reopenable modal must not discard the stash" + ); + }); + cx.executor().run_until_parked(); + + // Reopen triggered from within a modal that dismisses asynchronously and + // dispatches the action in the same cycle, as the command palette does. + workspace.update_in(cx, |workspace, window, cx| { + workspace + .modal_layer + .update(cx, |modal_layer, cx| modal_layer.hide_modal(window, cx)); + workspace.toggle_modal(window, cx, TestModal::new); + }); + cx.executor().run_until_parked(); + workspace.update_in(cx, |workspace, window, cx| { + // Mirror the command palette's confirm: emit DismissEvent on itself and + // dispatch the reopen action within the same update. + let palette = workspace.active_modal::(cx).unwrap(); + palette.update(cx, |_, cx| cx.emit(DismissEvent)); + workspace.reopen_last_picker(&ReopenLastPicker, window, cx); + }); + cx.executor().run_until_parked(); + assert!( + workspace.read_with(cx, |workspace, cx| workspace + .active_modal::(cx) + .is_some()), + "reopen triggered from within a dismissing modal should reveal the stash" + ); + } + #[gpui::test] async fn test_panels(cx: &mut gpui::TestAppContext) { init_test(cx);