From 4855da53dfa62791d9dd102b9db9cf7ddb08d153 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 17 Jun 2024 03:43:52 -0400 Subject: [PATCH] Don't hide inline assist when editor loses focus (#12990) Release Notes: - Now when an editor loses focus (e.g. from switching tabs) and then gains focus again, it doesn't close the inline assist. Instead, it only closes when you move the cursor outside of it, e.g. by clicking somewhere else in its parent editor. --------- Co-authored-by: Antonio Scandurra --- crates/assistant/src/assistant_panel.rs | 4 +- crates/assistant/src/inline_assistant.rs | 87 +++++++++-------------- crates/diagnostics/src/diagnostics.rs | 2 +- crates/editor/src/editor.rs | 29 +++++--- crates/gpui/src/window.rs | 34 ++++++--- crates/terminal_view/src/terminal_view.rs | 2 +- crates/workspace/src/modal_layer.rs | 2 +- crates/workspace/src/pane.rs | 9 +-- crates/zed/src/zed.rs | 2 +- 9 files changed, 90 insertions(+), 81 deletions(-) diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index c4bf50eba1..f28c7fb1f6 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -29,7 +29,7 @@ use futures::future::Shared; use futures::{FutureExt, StreamExt}; use gpui::{ div, point, rems, Action, AnyElement, AnyView, AppContext, AsyncAppContext, AsyncWindowContext, - ClipboardItem, Context as _, Empty, EventEmitter, FocusHandle, FocusableView, + ClipboardItem, Context as _, Empty, EventEmitter, FocusHandle, FocusOutEvent, FocusableView, InteractiveElement, IntoElement, Model, ModelContext, ParentElement, Pixels, Render, SharedString, StatefulInteractiveElement, Styled, Subscription, Task, UpdateGlobal, View, ViewContext, VisualContext, WeakView, WindowContext, @@ -296,7 +296,7 @@ impl AssistantPanel { } } - fn focus_out(&mut self, cx: &mut ViewContext) { + fn focus_out(&mut self, _event: FocusOutEvent, cx: &mut ViewContext) { self.toolbar .update(cx, |toolbar, cx| toolbar.focus_changed(false, cx)); cx.notify(); diff --git a/crates/assistant/src/inline_assistant.rs b/crates/assistant/src/inline_assistant.rs index 463c8a6262..0b528c0d41 100644 --- a/crates/assistant/src/inline_assistant.rs +++ b/crates/assistant/src/inline_assistant.rs @@ -277,19 +277,19 @@ impl InlineAssistant { ) { let assist_id = inline_assist_editor.read(cx).id; match event { - InlineAssistEditorEvent::Started => { + InlineAssistEditorEvent::StartRequested => { self.start_inline_assist(assist_id, cx); } - InlineAssistEditorEvent::Stopped => { + InlineAssistEditorEvent::StopRequested => { self.stop_inline_assist(assist_id, cx); } - InlineAssistEditorEvent::Confirmed => { + InlineAssistEditorEvent::ConfirmRequested => { self.finish_inline_assist(assist_id, false, cx); } - InlineAssistEditorEvent::Canceled => { + InlineAssistEditorEvent::CancelRequested => { self.finish_inline_assist(assist_id, true, cx); } - InlineAssistEditorEvent::Dismissed => { + InlineAssistEditorEvent::DismissRequested => { self.dismiss_inline_assist(assist_id, cx); } InlineAssistEditorEvent::Resized { height_in_lines } => { @@ -345,14 +345,8 @@ impl InlineAssistant { match event { EditorEvent::SelectionsChanged { local } if *local => { - if let Some(decorations) = assist.editor_decorations.as_ref() { - if decorations - .prompt_editor - .focus_handle(cx) - .contains_focused(cx) - { - cx.focus_view(&editor); - } + if let CodegenStatus::Idle = &assist.codegen.read(cx).status { + self.finish_inline_assist(assist_id, true, cx); } } EditorEvent::Saved => { @@ -813,11 +807,11 @@ impl InlineAssistId { } enum InlineAssistEditorEvent { - Started, - Stopped, - Confirmed, - Canceled, - Dismissed, + StartRequested, + StopRequested, + ConfirmRequested, + CancelRequested, + DismissRequested, Resized { height_in_lines: u8 }, } @@ -850,15 +844,17 @@ impl Render for InlineAssistEditor { .icon_size(IconSize::XSmall) .tooltip(|cx| Tooltip::for_action("Transform", &menu::Confirm, cx)) .on_click( - cx.listener(|_, _, cx| cx.emit(InlineAssistEditorEvent::Started)), + cx.listener(|_, _, cx| { + cx.emit(InlineAssistEditorEvent::StartRequested) + }), ), IconButton::new("cancel", IconName::Close) .icon_color(Color::Muted) .size(ButtonSize::None) .tooltip(|cx| Tooltip::for_action("Cancel Assist", &menu::Cancel, cx)) - .on_click( - cx.listener(|_, _, cx| cx.emit(InlineAssistEditorEvent::Canceled)), - ), + .on_click(cx.listener(|_, _, cx| { + cx.emit(InlineAssistEditorEvent::CancelRequested) + })), ] } CodegenStatus::Pending => { @@ -876,15 +872,15 @@ impl Render for InlineAssistEditor { ) }) .on_click( - cx.listener(|_, _, cx| cx.emit(InlineAssistEditorEvent::Stopped)), + cx.listener(|_, _, cx| cx.emit(InlineAssistEditorEvent::StopRequested)), ), IconButton::new("cancel", IconName::Close) .icon_color(Color::Muted) .size(ButtonSize::None) .tooltip(|cx| Tooltip::text("Cancel Assist", cx)) - .on_click( - cx.listener(|_, _, cx| cx.emit(InlineAssistEditorEvent::Canceled)), - ), + .on_click(cx.listener(|_, _, cx| { + cx.emit(InlineAssistEditorEvent::CancelRequested) + })), ] } CodegenStatus::Error(_) | CodegenStatus::Done => { @@ -903,7 +899,7 @@ impl Render for InlineAssistEditor { ) }) .on_click(cx.listener(|_, _, cx| { - cx.emit(InlineAssistEditorEvent::Started); + cx.emit(InlineAssistEditorEvent::StartRequested); })) } else { IconButton::new("confirm", IconName::Check) @@ -911,16 +907,16 @@ impl Render for InlineAssistEditor { .size(ButtonSize::None) .tooltip(|cx| Tooltip::for_action("Confirm Assist", &menu::Confirm, cx)) .on_click(cx.listener(|_, _, cx| { - cx.emit(InlineAssistEditorEvent::Confirmed); + cx.emit(InlineAssistEditorEvent::ConfirmRequested); })) }, IconButton::new("cancel", IconName::Close) .icon_color(Color::Muted) .size(ButtonSize::None) .tooltip(|cx| Tooltip::for_action("Cancel Assist", &menu::Cancel, cx)) - .on_click( - cx.listener(|_, _, cx| cx.emit(InlineAssistEditorEvent::Canceled)), - ), + .on_click(cx.listener(|_, _, cx| { + cx.emit(InlineAssistEditorEvent::CancelRequested) + })), ] } }; @@ -1100,23 +1096,6 @@ impl InlineAssistEditor { self.edited_since_done = true; cx.notify(); } - EditorEvent::Blurred => { - if let CodegenStatus::Idle = &self.codegen.read(cx).status { - let assistant_panel_is_focused = self - .workspace - .as_ref() - .and_then(|workspace| { - let panel = - workspace.upgrade()?.read(cx).panel::(cx)?; - Some(panel.focus_handle(cx).contains_focused(cx)) - }) - .unwrap_or(false); - - if !assistant_panel_is_focused { - cx.emit(InlineAssistEditorEvent::Canceled); - } - } - } _ => {} } } @@ -1142,10 +1121,10 @@ impl InlineAssistEditor { fn cancel(&mut self, _: &editor::actions::Cancel, cx: &mut ViewContext) { match &self.codegen.read(cx).status { CodegenStatus::Idle | CodegenStatus::Done | CodegenStatus::Error(_) => { - cx.emit(InlineAssistEditorEvent::Canceled); + cx.emit(InlineAssistEditorEvent::CancelRequested); } CodegenStatus::Pending => { - cx.emit(InlineAssistEditorEvent::Stopped); + cx.emit(InlineAssistEditorEvent::StopRequested); } } } @@ -1153,16 +1132,16 @@ impl InlineAssistEditor { fn confirm(&mut self, _: &menu::Confirm, cx: &mut ViewContext) { match &self.codegen.read(cx).status { CodegenStatus::Idle => { - cx.emit(InlineAssistEditorEvent::Started); + cx.emit(InlineAssistEditorEvent::StartRequested); } CodegenStatus::Pending => { - cx.emit(InlineAssistEditorEvent::Dismissed); + cx.emit(InlineAssistEditorEvent::DismissRequested); } CodegenStatus::Done | CodegenStatus::Error(_) => { if self.edited_since_done { - cx.emit(InlineAssistEditorEvent::Started); + cx.emit(InlineAssistEditorEvent::StartRequested); } else { - cx.emit(InlineAssistEditorEvent::Confirmed); + cx.emit(InlineAssistEditorEvent::ConfirmRequested); } } } diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 4b8c943dd0..b2eea68bb6 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -150,7 +150,7 @@ impl ProjectDiagnosticsEditor { let focus_handle = cx.focus_handle(); cx.on_focus_in(&focus_handle, |this, cx| this.focus_in(cx)) .detach(); - cx.on_focus_out(&focus_handle, |this, cx| this.focus_out(cx)) + cx.on_focus_out(&focus_handle, |this, _event, cx| this.focus_out(cx)) .detach(); let excerpts = cx.new_model(|cx| { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index da4cb30215..6ad0ca7fec 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -66,11 +66,12 @@ use git::diff_hunk_to_display; use gpui::{ div, impl_actions, point, prelude::*, px, relative, size, uniform_list, Action, AnyElement, AppContext, AsyncWindowContext, AvailableSpace, BackgroundExecutor, Bounds, ClipboardItem, - Context, DispatchPhase, ElementId, EventEmitter, FocusHandle, FocusableView, FontId, FontStyle, - FontWeight, HighlightStyle, Hsla, InteractiveText, KeyContext, ListSizingBehavior, Model, - MouseButton, PaintQuad, ParentElement, Pixels, Render, SharedString, Size, StrikethroughStyle, - Styled, StyledText, Subscription, Task, TextStyle, UnderlineStyle, UniformListScrollHandle, - View, ViewContext, ViewInputHandler, VisualContext, WeakView, WhiteSpace, WindowContext, + Context, DispatchPhase, ElementId, EventEmitter, FocusHandle, FocusOutEvent, FocusableView, + FontId, FontStyle, FontWeight, HighlightStyle, Hsla, InteractiveText, KeyContext, + ListSizingBehavior, Model, MouseButton, PaintQuad, ParentElement, Pixels, Render, SharedString, + Size, StrikethroughStyle, Styled, StyledText, Subscription, Task, TextStyle, UnderlineStyle, + UniformListScrollHandle, View, ViewContext, ViewInputHandler, VisualContext, WeakFocusHandle, + WeakView, WhiteSpace, WindowContext, }; use highlight_matching_bracket::refresh_matching_bracket_highlights; use hover_popover::{hide_hover, HoverState}; @@ -448,6 +449,7 @@ struct BufferOffset(usize); /// See the [module level documentation](self) for more information. pub struct Editor { focus_handle: FocusHandle, + last_focused: Option, /// The text buffer being edited buffer: Model, /// Map of how text in the buffer should be displayed. @@ -1735,6 +1737,8 @@ impl Editor { ); let focus_handle = cx.focus_handle(); cx.on_focus(&focus_handle, Self::handle_focus).detach(); + cx.on_focus_out(&focus_handle, Self::handle_focus_out) + .detach(); cx.on_blur(&focus_handle, Self::handle_blur).detach(); let show_indent_guides = if mode == EditorMode::SingleLine { @@ -1745,6 +1749,7 @@ impl Editor { let mut this = Self { focus_handle, + last_focused: None, buffer: buffer.clone(), display_map: display_map.clone(), selections, @@ -11315,9 +11320,13 @@ impl Editor { fn handle_focus(&mut self, cx: &mut ViewContext) { cx.emit(EditorEvent::Focused); - if let Some(rename) = self.pending_rename.as_ref() { - let rename_editor_focus_handle = rename.editor.read(cx).focus_handle.clone(); - cx.focus(&rename_editor_focus_handle); + + if let Some(last_focused) = self + .last_focused + .take() + .and_then(|last_focused| last_focused.upgrade()) + { + cx.focus(&last_focused); } else { if let Some(blame) = self.blame.as_ref() { blame.update(cx, GitBlame::focus) @@ -11339,6 +11348,10 @@ impl Editor { } } + fn handle_focus_out(&mut self, event: FocusOutEvent, _cx: &mut ViewContext) { + self.last_focused = Some(event.blurred); + } + pub fn handle_blur(&mut self, cx: &mut ViewContext) { self.blink_manager.update(cx, BlinkManager::disable); self.buffer diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 13dfc543f5..562497c1da 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -85,13 +85,20 @@ impl DispatchPhase { type AnyObserver = Box bool + 'static>; -type AnyWindowFocusListener = Box bool + 'static>; +type AnyWindowFocusListener = + Box bool + 'static>; -struct FocusEvent { +struct WindowFocusEvent { previous_focus_path: SmallVec<[FocusId; 8]>, current_focus_path: SmallVec<[FocusId; 8]>, } +/// This is provided when subscribing for `ViewContext::on_focus_out` events. +pub struct FocusOutEvent { + /// A weak focus handle representing what was blurred. + pub blurred: WeakFocusHandle, +} + slotmap::new_key_type! { /// A globally unique identifier for a focusable element. pub struct FocusId; @@ -1397,7 +1404,7 @@ impl<'a> WindowContext<'a> { .retain(&(), |listener| listener(self)); } - let event = FocusEvent { + let event = WindowFocusEvent { previous_focus_path: if previous_window_active { previous_focus_path } else { @@ -4055,6 +4062,7 @@ impl<'a, V: 'static> ViewContext<'a, V> { } /// Register a listener to be called when the given focus handle or one of its descendants receives focus. + /// This does not fire if the given focus handle - or one of its descendants - was previously focused. /// Returns a subscription and persists until the subscription is dropped. pub fn on_focus_in( &mut self, @@ -4124,17 +4132,25 @@ impl<'a, V: 'static> ViewContext<'a, V> { pub fn on_focus_out( &mut self, handle: &FocusHandle, - mut listener: impl FnMut(&mut V, &mut ViewContext) + 'static, + mut listener: impl FnMut(&mut V, FocusOutEvent, &mut ViewContext) + 'static, ) -> Subscription { let view = self.view.downgrade(); let focus_id = handle.id; let (subscription, activate) = self.window.new_focus_listener(Box::new(move |event, cx| { view.update(cx, |view, cx| { - if event.previous_focus_path.contains(&focus_id) - && !event.current_focus_path.contains(&focus_id) - { - listener(view, cx) + if let Some(blurred_id) = event.previous_focus_path.last().copied() { + if event.previous_focus_path.contains(&focus_id) + && !event.current_focus_path.contains(&focus_id) + { + let event = FocusOutEvent { + blurred: WeakFocusHandle { + id: blurred_id, + handles: Arc::downgrade(&cx.window.focus_handles), + }, + }; + listener(view, event, cx) + } } }) .is_ok() @@ -4193,7 +4209,7 @@ impl<'a, V: 'static> ViewContext<'a, V> { }); } - /// Emit an event to be handled any other views that have subscribed via [ViewContext::subscribe]. + /// Emit an event to be handled by any other views that have subscribed via [ViewContext::subscribe]. pub fn emit(&mut self, event: Evt) where Evt: 'static, diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index fda2a8b59e..d785e21f6b 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -152,7 +152,7 @@ impl TerminalView { let focus_in = cx.on_focus_in(&focus_handle, |terminal_view, cx| { terminal_view.focus_in(cx); }); - let focus_out = cx.on_focus_out(&focus_handle, |terminal_view, cx| { + let focus_out = cx.on_focus_out(&focus_handle, |terminal_view, _event, cx| { terminal_view.focus_out(cx); }); diff --git a/crates/workspace/src/modal_layer.rs b/crates/workspace/src/modal_layer.rs index 44923d783e..b735b4c710 100644 --- a/crates/workspace/src/modal_layer.rs +++ b/crates/workspace/src/modal_layer.rs @@ -87,7 +87,7 @@ impl ModalLayer { cx.subscribe(&new_modal, |this, _, _: &DismissEvent, cx| { this.hide_modal(cx); }), - cx.on_focus_out(&focus_handle, |this, cx| { + cx.on_focus_out(&focus_handle, |this, _event, cx| { if this.dismiss_on_focus_lost { this.hide_modal(cx); } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 0b6d05414e..e690a07d72 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -14,9 +14,10 @@ use futures::{stream::FuturesUnordered, StreamExt}; use gpui::{ actions, anchored, deferred, impl_actions, prelude::*, Action, AnchorCorner, AnyElement, AppContext, AsyncWindowContext, ClickEvent, DismissEvent, Div, DragMoveEvent, EntityId, - EventEmitter, ExternalPaths, FocusHandle, FocusableView, KeyContext, Model, MouseButton, - MouseDownEvent, NavigationDirection, Pixels, Point, PromptLevel, Render, ScrollHandle, - Subscription, Task, View, ViewContext, VisualContext, WeakFocusHandle, WeakView, WindowContext, + EventEmitter, ExternalPaths, FocusHandle, FocusOutEvent, FocusableView, KeyContext, Model, + MouseButton, MouseDownEvent, NavigationDirection, Pixels, Point, PromptLevel, Render, + ScrollHandle, Subscription, Task, View, ViewContext, VisualContext, WeakFocusHandle, WeakView, + WindowContext, }; use itertools::Itertools; use parking_lot::Mutex; @@ -517,7 +518,7 @@ impl Pane { .map_or(false, |menu| menu.focus_handle(cx).is_focused(cx)) } - fn focus_out(&mut self, cx: &mut ViewContext) { + fn focus_out(&mut self, _event: FocusOutEvent, cx: &mut ViewContext) { self.was_focused = false; self.toolbar.update(cx, |toolbar, cx| { toolbar.focus_changed(false, cx); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index a127a2f984..f7f1ac0d1f 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -2214,7 +2214,7 @@ mod tests { cx.background_executor.run_until_parked(); window - .read_with(cx, |workspace, cx| { + .update(cx, |workspace, cx| { assert_eq!(workspace.panes().len(), 1); assert!(workspace.active_item(cx).is_none()); })