From 137104e00eb64c56cbd119dd70116bd0a6cdc9d6 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 11 Dec 2023 17:59:42 +0200 Subject: [PATCH 1/6] Fix dock panels not focusing their contents on toggle Co-authored-by: Antonio --- crates/workspace2/src/dock.rs | 1 + crates/workspace2/src/pane.rs | 25 +++++++++++-------------- crates/workspace2/src/workspace2.rs | 1 + 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/crates/workspace2/src/dock.rs b/crates/workspace2/src/dock.rs index e8840fb172..b656a79a8d 100644 --- a/crates/workspace2/src/dock.rs +++ b/crates/workspace2/src/dock.rs @@ -346,6 +346,7 @@ impl Dock { }) .ok(); } + // todo!() we do not use this event in the production code (even in zed1), remove it PanelEvent::Activate => { if let Some(ix) = this .panel_entries diff --git a/crates/workspace2/src/pane.rs b/crates/workspace2/src/pane.rs index 5c5801960e..1d1f6b6b55 100644 --- a/crates/workspace2/src/pane.rs +++ b/crates/workspace2/src/pane.rs @@ -10,7 +10,7 @@ use gpui::{ actions, impl_actions, overlay, prelude::*, rems, Action, AnchorCorner, AnyWeakView, AppContext, AsyncWindowContext, DismissEvent, Div, EntityId, EventEmitter, FocusHandle, Focusable, FocusableView, Model, MouseButton, NavigationDirection, Pixels, Point, PromptLevel, - Render, Task, View, ViewContext, VisualContext, WeakView, WindowContext, + Render, Subscription, Task, View, ViewContext, VisualContext, WeakView, WindowContext, }; use parking_lot::Mutex; use project::{Project, ProjectEntryId, ProjectPath}; @@ -174,6 +174,7 @@ pub struct Pane { // can_drop: Rc, &WindowContext) -> bool>, can_split: bool, // render_tab_bar_buttons: Rc) -> AnyElement>, + subscriptions: Vec, } pub struct ItemNavHistory { @@ -312,10 +313,17 @@ impl Pane { // context_menu.update(cx, |menu, _| { // menu.set_position_mode(OverlayPositionMode::Local) // }); + // + let focus_handle = cx.focus_handle(); + + let subscriptions = vec![ + cx.on_focus_in(&focus_handle, move |this, cx| this.focus_in(cx)), + cx.on_focus_out(&focus_handle, move |this, cx| this.focus_out(cx)), + ]; let handle = cx.view().downgrade(); Self { - focus_handle: cx.focus_handle(), + focus_handle, items: Vec::new(), activation_history: Vec::new(), was_focused: false, @@ -402,6 +410,7 @@ impl Pane { // }) // .into_any() // }), + subscriptions, } } @@ -2062,18 +2071,6 @@ impl Render for Pane { .track_focus(&self.focus_handle) .size_full() .overflow_hidden() - .on_focus_in({ - let this = this.clone(); - move |event, cx| { - this.update(cx, |this, cx| this.focus_in(cx)).ok(); - } - }) - .on_focus_out({ - let this = this.clone(); - move |event, cx| { - this.update(cx, |this, cx| this.focus_out(cx)).ok(); - } - }) .on_action(cx.listener(|pane, _: &SplitLeft, cx| pane.split(SplitDirection::Left, cx))) .on_action(cx.listener(|pane, _: &SplitUp, cx| pane.split(SplitDirection::Up, cx))) .on_action( diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index 43aa13847d..a352b0a67a 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -1633,6 +1633,7 @@ impl Workspace { panel.focus_handle(cx).focus(cx); reveal_dock = true; } else { + // todo!() // if panel.is_zoomed(cx) { // dock.set_open(false, cx); // } From 43b8d65fee10de974993acb5fb37037ce56f43c6 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 7 Dec 2023 15:35:30 +0100 Subject: [PATCH 2/6] Transfer focus to the workspace when window focus is lost --- crates/gpui2/src/app.rs | 2 -- crates/gpui2/src/window.rs | 23 +++++++++++++++++++++++ crates/workspace2/src/workspace2.rs | 5 +++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index 88c5b8211d..fa9960c1fb 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -646,8 +646,6 @@ impl AppContext { } /// Repeatedly called during `flush_effects` to handle a focused handle being dropped. - /// For now, we simply blur the window if this happens, but we may want to support invoking - /// a window blur handler to restore focus to some logical element. fn release_dropped_focus_handles(&mut self) { for window_handle in self.windows() { window_handle diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 16de7ef62a..d8ca026f71 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -2419,6 +2419,29 @@ impl<'a, V: 'static> ViewContext<'a, V> { subscription } + /// Register a listener to be called when the window loses focus. + /// Unlike [on_focus_changed], returns a subscription and persists until the subscription + /// is dropped. + pub fn on_window_focus_lost( + &mut self, + mut listener: impl FnMut(&mut V, &mut ViewContext) + 'static, + ) -> Subscription { + let view = self.view.downgrade(); + let (subscription, activate) = self.window.focus_listeners.insert( + (), + Box::new(move |event, cx| { + view.update(cx, |view, cx| { + if event.blurred.is_none() && event.focused.is_none() { + listener(view, cx) + } + }) + .is_ok() + }), + ); + self.app.defer(move |_| activate()); + subscription + } + /// Register a listener to be called when the given focus handle or one of its descendants loses focus. /// Unlike [on_focus_changed], returns a subscription and persists until the subscription /// is dropped. diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index a352b0a67a..2bcf6ec13e 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -532,6 +532,11 @@ impl Workspace { cx.notify() }) .detach(); + cx.on_window_focus_lost(|this, cx| { + let focus_handle = this.focus_handle(cx); + cx.focus(&focus_handle); + }) + .detach(); let weak_handle = cx.view().downgrade(); let pane_history_timestamp = Arc::new(AtomicUsize::new(0)); From 717b2885f89a4ba77d72cce026d375a437547005 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 12 Dec 2023 15:06:39 +0200 Subject: [PATCH 3/6] Attempt to remove the dangeous element focus API --- crates/diagnostics2/src/diagnostics.rs | 24 +++++++---- crates/gpui2/src/elements/div.rs | 46 +--------------------- crates/gpui2/src/window.rs | 1 + crates/storybook2/src/stories/focus.rs | 15 ++++--- crates/terminal_view2/src/terminal_view.rs | 33 ++++++---------- 5 files changed, 39 insertions(+), 80 deletions(-) diff --git a/crates/diagnostics2/src/diagnostics.rs b/crates/diagnostics2/src/diagnostics.rs index 386f5338b4..1001ed880e 100644 --- a/crates/diagnostics2/src/diagnostics.rs +++ b/crates/diagnostics2/src/diagnostics.rs @@ -13,10 +13,10 @@ use editor::{ }; use futures::future::try_join_all; use gpui::{ - actions, div, AnyElement, AnyView, AppContext, Context, Div, EventEmitter, FocusEvent, - FocusHandle, Focusable, FocusableElement, FocusableView, InteractiveElement, IntoElement, - Model, ParentElement, Render, SharedString, Styled, Subscription, Task, View, ViewContext, - VisualContext, WeakView, WindowContext, + actions, div, AnyElement, AnyView, AppContext, Context, Div, EventEmitter, FocusHandle, + Focusable, FocusableView, InteractiveElement, IntoElement, Model, ParentElement, Render, + SharedString, Styled, Subscription, Task, View, ViewContext, VisualContext, WeakView, + WindowContext, }; use language::{ Anchor, Bias, Buffer, Diagnostic, DiagnosticEntry, DiagnosticSeverity, Point, Selection, @@ -109,7 +109,6 @@ impl Render for ProjectDiagnosticsEditor { div() .track_focus(&self.focus_handle) .size_full() - .on_focus_in(cx.listener(Self::focus_in)) .on_action(cx.listener(Self::toggle_warnings)) .child(child) } @@ -149,6 +148,11 @@ impl ProjectDiagnosticsEditor { _ => {} }); + let focus_handle = cx.focus_handle(); + + let focus_in_subscription = + cx.on_focus_in(&focus_handle, |diagnostics, cx| diagnostics.focus_in(cx)); + let excerpts = cx.build_model(|cx| MultiBuffer::new(project_handle.read(cx).replica_id())); let editor = cx.build_view(|cx| { let mut editor = @@ -171,13 +175,17 @@ impl ProjectDiagnosticsEditor { summary, workspace, excerpts, - focus_handle: cx.focus_handle(), + focus_handle, editor, path_states: Default::default(), paths_to_update: HashMap::default(), include_warnings: ProjectDiagnosticsSettings::get_global(cx).include_warnings, current_diagnostics: HashMap::default(), - _subscriptions: vec![project_event_subscription, editor_event_subscription], + _subscriptions: vec![ + project_event_subscription, + editor_event_subscription, + focus_in_subscription, + ], }; this.update_excerpts(None, cx); this @@ -202,7 +210,7 @@ impl ProjectDiagnosticsEditor { cx.notify(); } - fn focus_in(&mut self, _: &FocusEvent, cx: &mut ViewContext) { + fn focus_in(&mut self, cx: &mut ViewContext) { if self.focus_handle.is_focused(cx) && !self.path_states.is_empty() { self.editor.focus_handle(cx).focus(cx) } diff --git a/crates/gpui2/src/elements/div.rs b/crates/gpui2/src/elements/div.rs index 7c365d8734..82ecbb27d1 100644 --- a/crates/gpui2/src/elements/div.rs +++ b/crates/gpui2/src/elements/div.rs @@ -441,6 +441,7 @@ pub trait StatefulInteractiveElement: InteractiveElement { } } +// TODO kb do we leave those on an element? pub trait FocusableElement: InteractiveElement { fn focus(mut self, f: impl FnOnce(StyleRefinement) -> StyleRefinement) -> Self where @@ -485,51 +486,6 @@ pub trait FocusableElement: InteractiveElement { })); self } - - fn on_focus_in(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self - where - Self: Sized, - { - self.interactivity() - .focus_listeners - .push(Box::new(move |focus_handle, event, cx| { - let descendant_blurred = event - .blurred - .as_ref() - .map_or(false, |blurred| focus_handle.contains(blurred, cx)); - let descendant_focused = event - .focused - .as_ref() - .map_or(false, |focused| focus_handle.contains(focused, cx)); - - if !descendant_blurred && descendant_focused { - listener(event, cx) - } - })); - self - } - - fn on_focus_out(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self - where - Self: Sized, - { - self.interactivity() - .focus_listeners - .push(Box::new(move |focus_handle, event, cx| { - let descendant_blurred = event - .blurred - .as_ref() - .map_or(false, |blurred| focus_handle.contains(blurred, cx)); - let descendant_focused = event - .focused - .as_ref() - .map_or(false, |focused| focus_handle.contains(focused, cx)); - if descendant_blurred && !descendant_focused { - listener(event, cx) - } - })); - self - } } pub type FocusListeners = SmallVec<[FocusListener; 2]>; diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index d8ca026f71..0b8a7f1263 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -168,6 +168,7 @@ impl FocusHandle { self.id.within_focused(cx) } + // TODO kb now never used? /// Obtains whether this handle contains the given handle in the most recently rendered frame. pub(crate) fn contains(&self, other: &Self, cx: &WindowContext) -> bool { self.id.contains(other.id, cx) diff --git a/crates/storybook2/src/stories/focus.rs b/crates/storybook2/src/stories/focus.rs index 787de727ea..a4fae72b54 100644 --- a/crates/storybook2/src/stories/focus.rs +++ b/crates/storybook2/src/stories/focus.rs @@ -49,8 +49,9 @@ impl Render for FocusStory { })) .on_focus(cx.listener(|_, _, _| println!("Parent focused"))) .on_blur(cx.listener(|_, _, _| println!("Parent blurred"))) - .on_focus_in(cx.listener(|_, _, _| println!("Parent focus_in"))) - .on_focus_out(cx.listener(|_, _, _| println!("Parent focus_out"))) + // TODO kb todo!() remove? + // .on_focus_in(cx.listener(|_, _, _| println!("Parent focus_in"))) + // .on_focus_out(cx.listener(|_, _, _| println!("Parent focus_out"))) .on_key_down(cx.listener(|_, event, _| println!("Key down on parent {:?}", event))) .on_key_up(cx.listener(|_, event, _| println!("Key up on parent {:?}", event))) .size_full() @@ -70,8 +71,9 @@ impl Render for FocusStory { .in_focus(|style| style.bg(color_6)) .on_focus(cx.listener(|_, _, _| println!("Child 1 focused"))) .on_blur(cx.listener(|_, _, _| println!("Child 1 blurred"))) - .on_focus_in(cx.listener(|_, _, _| println!("Child 1 focus_in"))) - .on_focus_out(cx.listener(|_, _, _| println!("Child 1 focus_out"))) + // TODO kb todo!() remove? + // .on_focus_in(cx.listener(|_, _, _| println!("Child 1 focus_in"))) + // .on_focus_out(cx.listener(|_, _, _| println!("Child 1 focus_out"))) .on_key_down( cx.listener(|_, event, _| println!("Key down on child 1 {:?}", event)), ) @@ -90,8 +92,9 @@ impl Render for FocusStory { .bg(color_4) .on_focus(cx.listener(|_, _, _| println!("Child 2 focused"))) .on_blur(cx.listener(|_, _, _| println!("Child 2 blurred"))) - .on_focus_in(cx.listener(|_, _, _| println!("Child 2 focus_in"))) - .on_focus_out(cx.listener(|_, _, _| println!("Child 2 focus_out"))) + // TODO kb todo!() remove? + // .on_focus_in(cx.listener(|_, _, _| println!("Child 2 focus_in"))) + // .on_focus_out(cx.listener(|_, _, _| println!("Child 2 focus_out"))) .on_key_down( cx.listener(|_, event, _| println!("Key down on child 2 {:?}", event)), ) diff --git a/crates/terminal_view2/src/terminal_view.rs b/crates/terminal_view2/src/terminal_view.rs index 8a0e3aeb6a..1ac5ba8ab5 100644 --- a/crates/terminal_view2/src/terminal_view.rs +++ b/crates/terminal_view2/src/terminal_view.rs @@ -10,9 +10,8 @@ pub mod terminal_panel; use editor::{scroll::autoscroll::Autoscroll, Editor}; use gpui::{ div, impl_actions, overlay, AnyElement, AppContext, DismissEvent, Div, EventEmitter, - FocusEvent, FocusHandle, Focusable, FocusableElement, FocusableView, KeyContext, KeyDownEvent, - Keystroke, Model, MouseButton, MouseDownEvent, Pixels, Render, Styled, Subscription, Task, - View, VisualContext, WeakView, + FocusHandle, Focusable, FocusableView, KeyContext, KeyDownEvent, Keystroke, Model, MouseButton, + MouseDownEvent, Pixels, Render, Styled, Subscription, Task, View, VisualContext, WeakView, }; use language::Bias; use persistence::TERMINAL_DB; @@ -263,19 +262,13 @@ impl TerminalView { }) .detach(); - let focus = cx.focus_handle(); - // let focus_in = cx.on_focus_in(&focus, |this, cx| { - // this.has_new_content = false; - // this.terminal.read(cx).focus_in(); - // this.blink_cursors(this.blink_epoch, cx); - // cx.notify(); - // }); - // let focus_out = cx.on_focus_out(&focus, |this, cx| { - // this.terminal.update(cx, |terminal, _| { - // terminal.focus_out(); - // }); - // cx.notify(); - // }); + let focus_handle = cx.focus_handle(); + 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| { + terminal_view.focus_out(cx); + }); Self { terminal, @@ -289,7 +282,7 @@ impl TerminalView { blink_epoch: 0, can_navigate_to_selected_word: false, workspace_id, - _subscriptions: vec![/*focus_in, focus_out*/], + _subscriptions: vec![focus_in, focus_out], } } @@ -614,14 +607,14 @@ impl TerminalView { }); } - fn focus_in(&mut self, event: &FocusEvent, cx: &mut ViewContext) { + fn focus_in(&mut self, cx: &mut ViewContext) { self.has_new_content = false; self.terminal.read(cx).focus_in(); self.blink_cursors(self.blink_epoch, cx); cx.notify(); } - fn focus_out(&mut self, event: &FocusEvent, cx: &mut ViewContext) { + fn focus_out(&mut self, cx: &mut ViewContext) { self.terminal.update(cx, |terminal, _| { terminal.focus_out(); }); @@ -650,8 +643,6 @@ impl Render for TerminalView { .on_action(cx.listener(TerminalView::clear)) .on_action(cx.listener(TerminalView::show_character_palette)) .on_action(cx.listener(TerminalView::select_all)) - .on_focus_in(cx.listener(Self::focus_in)) - .on_focus_out(cx.listener(Self::focus_out)) .on_key_down(cx.listener(Self::key_down)) .on_mouse_down( MouseButton::Right, From 2cde1a2e15122e887152c7bd111f5b18a39968c9 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 12 Dec 2023 15:36:20 +0200 Subject: [PATCH 4/6] Re-focus window on workspace on corresponding window blur Co-authored-by: Antonio --- crates/gpui2/src/app.rs | 100 ++++++++++++++-------------- crates/gpui2/src/window.rs | 44 +++++++++--- crates/workspace2/src/workspace2.rs | 2 +- 3 files changed, 85 insertions(+), 61 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index fa9960c1fb..7d61f4d847 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -569,59 +569,61 @@ impl AppContext { /// such as notifying observers, emitting events, etc. Effects can themselves /// cause effects, so we continue looping until all effects are processed. fn flush_effects(&mut self) { - loop { - self.release_dropped_entities(); - self.release_dropped_focus_handles(); - if let Some(effect) = self.pending_effects.pop_front() { - match effect { - Effect::Notify { emitter } => { - self.apply_notify_effect(emitter); + while !self.pending_effects.is_empty() { + loop { + self.release_dropped_entities(); + self.release_dropped_focus_handles(); + if let Some(effect) = self.pending_effects.pop_front() { + match effect { + Effect::Notify { emitter } => { + self.apply_notify_effect(emitter); + } + Effect::Emit { + emitter, + event_type, + event, + } => self.apply_emit_effect(emitter, event_type, event), + Effect::FocusChanged { + window_handle, + focused, + } => { + self.apply_focus_changed_effect(window_handle, focused); + } + Effect::Refresh => { + self.apply_refresh_effect(); + } + Effect::NotifyGlobalObservers { global_type } => { + self.apply_notify_global_observers_effect(global_type); + } + Effect::Defer { callback } => { + self.apply_defer_effect(callback); + } } - Effect::Emit { - emitter, - event_type, - event, - } => self.apply_emit_effect(emitter, event_type, event), - Effect::FocusChanged { - window_handle, - focused, - } => { - self.apply_focus_changed_effect(window_handle, focused); - } - Effect::Refresh => { - self.apply_refresh_effect(); - } - Effect::NotifyGlobalObservers { global_type } => { - self.apply_notify_global_observers_effect(global_type); - } - Effect::Defer { callback } => { - self.apply_defer_effect(callback); - } - } - } else { - break; - } - } - - for window in self.windows.values() { - if let Some(window) = window.as_ref() { - if window.dirty { - window.platform_window.invalidate(); + } else { + break; } } - } - #[cfg(any(test, feature = "test-support"))] - for window in self - .windows - .values() - .filter_map(|window| { - let window = window.as_ref()?; - window.dirty.then_some(window.handle) - }) - .collect::>() - { - self.update_window(window, |_, cx| cx.draw()).unwrap(); + for window in self.windows.values() { + if let Some(window) = window.as_ref() { + if window.dirty { + window.platform_window.invalidate(); + } + } + } + + #[cfg(any(test, feature = "test-support"))] + for window in self + .windows + .values() + .filter_map(|window| { + let window = window.as_ref()?; + window.dirty.then_some(window.handle) + }) + .collect::>() + { + self.update_window(window, |_, cx| cx.draw()).unwrap(); + } } } diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 0b8a7f1263..c94da71d85 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -229,6 +229,7 @@ pub struct Window { pub(crate) next_frame: Frame, pub(crate) focus_handles: Arc>>, pub(crate) focus_listeners: SubscriberSet<(), AnyWindowFocusListener>, + pub(crate) blur_listeners: SubscriberSet<(), AnyObserver>, default_prevented: bool, mouse_position: Point, requested_cursor_style: Option, @@ -362,6 +363,7 @@ impl Window { next_frame: Frame::new(DispatchTree::new(cx.keymap.clone(), cx.actions.clone())), focus_handles: Arc::new(RwLock::new(SlotMap::with_key())), focus_listeners: SubscriberSet::new(), + blur_listeners: SubscriberSet::new(), default_prevented: true, mouse_position, requested_cursor_style: None, @@ -1235,6 +1237,16 @@ impl<'a> WindowContext<'a> { /// Draw pixels to the display for this window based on the contents of its scene. pub(crate) fn draw(&mut self) -> Scene { + let window_was_focused = self + .window + .focus + .and_then(|focus_id| { + self.window + .rendered_frame + .dispatch_tree + .focusable_node_id(focus_id) + }) + .is_some(); self.text_system().start_frame(); self.window.platform_window.clear_input_handler(); self.window.layout_engine.as_mut().unwrap().clear(); @@ -1273,6 +1285,23 @@ impl<'a> WindowContext<'a> { }); } + let window_is_focused = self + .window + .focus + .and_then(|focus_id| { + self.window + .next_frame + .dispatch_tree + .focusable_node_id(focus_id) + }) + .is_some(); + if window_was_focused && !window_is_focused { + self.window + .blur_listeners + .clone() + .retain(&(), |listener| listener(self)); + } + self.window .next_frame .dispatch_tree @@ -2423,23 +2452,16 @@ impl<'a, V: 'static> ViewContext<'a, V> { /// Register a listener to be called when the window loses focus. /// Unlike [on_focus_changed], returns a subscription and persists until the subscription /// is dropped. - pub fn on_window_focus_lost( + pub fn on_blur_window( &mut self, mut listener: impl FnMut(&mut V, &mut ViewContext) + 'static, ) -> Subscription { let view = self.view.downgrade(); - let (subscription, activate) = self.window.focus_listeners.insert( + let (subscription, activate) = self.window.blur_listeners.insert( (), - Box::new(move |event, cx| { - view.update(cx, |view, cx| { - if event.blurred.is_none() && event.focused.is_none() { - listener(view, cx) - } - }) - .is_ok() - }), + Box::new(move |cx| view.update(cx, |view, cx| listener(view, cx)).is_ok()), ); - self.app.defer(move |_| activate()); + activate(); subscription } diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index 2bcf6ec13e..520eee0d2f 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -532,7 +532,7 @@ impl Workspace { cx.notify() }) .detach(); - cx.on_window_focus_lost(|this, cx| { + cx.on_blur_window(|this, cx| { let focus_handle = this.focus_handle(cx); cx.focus(&focus_handle); }) From ca8e8d10658a20b95e7993b477701c0e2334620c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 12 Dec 2023 16:10:56 +0200 Subject: [PATCH 5/6] Finish removing all dangerous focus APIs --- crates/gpui2/src/elements/div.rs | 29 ------------ crates/gpui2/src/window.rs | 3 +- crates/storybook2/src/stories/focus.rs | 57 ++++++++++++++--------- crates/ui2/src/components/context_menu.rs | 12 +++-- 4 files changed, 45 insertions(+), 56 deletions(-) diff --git a/crates/gpui2/src/elements/div.rs b/crates/gpui2/src/elements/div.rs index 82ecbb27d1..d9be9d0122 100644 --- a/crates/gpui2/src/elements/div.rs +++ b/crates/gpui2/src/elements/div.rs @@ -441,7 +441,6 @@ pub trait StatefulInteractiveElement: InteractiveElement { } } -// TODO kb do we leave those on an element? pub trait FocusableElement: InteractiveElement { fn focus(mut self, f: impl FnOnce(StyleRefinement) -> StyleRefinement) -> Self where @@ -458,34 +457,6 @@ pub trait FocusableElement: InteractiveElement { self.interactivity().in_focus_style = f(StyleRefinement::default()); self } - - fn on_focus(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self - where - Self: Sized, - { - self.interactivity() - .focus_listeners - .push(Box::new(move |focus_handle, event, cx| { - if event.focused.as_ref() == Some(focus_handle) { - listener(event, cx) - } - })); - self - } - - fn on_blur(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self - where - Self: Sized, - { - self.interactivity() - .focus_listeners - .push(Box::new(move |focus_handle, event, cx| { - if event.blurred.as_ref() == Some(focus_handle) { - listener(event, cx) - } - })); - self - } } pub type FocusListeners = SmallVec<[FocusListener; 2]>; diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index c94da71d85..4f9aaa56a2 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -168,9 +168,8 @@ impl FocusHandle { self.id.within_focused(cx) } - // TODO kb now never used? /// Obtains whether this handle contains the given handle in the most recently rendered frame. - pub(crate) fn contains(&self, other: &Self, cx: &WindowContext) -> bool { + pub fn contains(&self, other: &Self, cx: &WindowContext) -> bool { self.id.contains(other.id, cx) } } diff --git a/crates/storybook2/src/stories/focus.rs b/crates/storybook2/src/stories/focus.rs index a4fae72b54..6dc8187690 100644 --- a/crates/storybook2/src/stories/focus.rs +++ b/crates/storybook2/src/stories/focus.rs @@ -1,14 +1,16 @@ use gpui::{ - actions, div, prelude::*, Div, FocusHandle, Focusable, KeyBinding, Render, Stateful, View, - WindowContext, + actions, div, prelude::*, Div, FocusHandle, Focusable, KeyBinding, Render, Stateful, + Subscription, View, WindowContext, }; use ui::prelude::*; actions!(focus, [ActionA, ActionB, ActionC]); pub struct FocusStory { + parent_focus: FocusHandle, child_1_focus: FocusHandle, child_2_focus: FocusHandle, + _focus_subscriptions: Vec, } impl FocusStory { @@ -19,9 +21,37 @@ impl FocusStory { KeyBinding::new("cmd-c", ActionC, None), ]); - cx.build_view(move |cx| Self { - child_1_focus: cx.focus_handle(), - child_2_focus: cx.focus_handle(), + cx.build_view(move |cx| { + let parent_focus = cx.focus_handle(); + let child_1_focus = cx.focus_handle(); + let child_2_focus = cx.focus_handle(); + let _focus_subscriptions = vec![ + cx.on_focus(&parent_focus, |_, _| { + println!("Parent focused"); + }), + cx.on_blur(&parent_focus, |_, _| { + println!("Parent blurred"); + }), + cx.on_focus(&child_1_focus, |_, _| { + println!("Child 1 focused"); + }), + cx.on_blur(&child_1_focus, |_, _| { + println!("Child 1 blurred"); + }), + cx.on_focus(&child_2_focus, |_, _| { + println!("Child 2 focused"); + }), + cx.on_blur(&child_2_focus, |_, _| { + println!("Child 2 blurred"); + }), + ]; + + Self { + parent_focus, + child_1_focus, + child_2_focus, + _focus_subscriptions, + } }) } } @@ -39,7 +69,7 @@ impl Render for FocusStory { div() .id("parent") - .focusable() + .track_focus(&self.parent_focus) .key_context("parent") .on_action(cx.listener(|_, _action: &ActionA, _cx| { println!("Action A dispatched on parent"); @@ -47,11 +77,6 @@ impl Render for FocusStory { .on_action(cx.listener(|_, _action: &ActionB, _cx| { println!("Action B dispatched on parent"); })) - .on_focus(cx.listener(|_, _, _| println!("Parent focused"))) - .on_blur(cx.listener(|_, _, _| println!("Parent blurred"))) - // TODO kb todo!() remove? - // .on_focus_in(cx.listener(|_, _, _| println!("Parent focus_in"))) - // .on_focus_out(cx.listener(|_, _, _| println!("Parent focus_out"))) .on_key_down(cx.listener(|_, event, _| println!("Key down on parent {:?}", event))) .on_key_up(cx.listener(|_, event, _| println!("Key up on parent {:?}", event))) .size_full() @@ -69,11 +94,6 @@ impl Render for FocusStory { .bg(color_4) .focus(|style| style.bg(color_5)) .in_focus(|style| style.bg(color_6)) - .on_focus(cx.listener(|_, _, _| println!("Child 1 focused"))) - .on_blur(cx.listener(|_, _, _| println!("Child 1 blurred"))) - // TODO kb todo!() remove? - // .on_focus_in(cx.listener(|_, _, _| println!("Child 1 focus_in"))) - // .on_focus_out(cx.listener(|_, _, _| println!("Child 1 focus_out"))) .on_key_down( cx.listener(|_, event, _| println!("Key down on child 1 {:?}", event)), ) @@ -90,11 +110,6 @@ impl Render for FocusStory { .w_full() .h_6() .bg(color_4) - .on_focus(cx.listener(|_, _, _| println!("Child 2 focused"))) - .on_blur(cx.listener(|_, _, _| println!("Child 2 blurred"))) - // TODO kb todo!() remove? - // .on_focus_in(cx.listener(|_, _, _| println!("Child 2 focus_in"))) - // .on_focus_out(cx.listener(|_, _, _| println!("Child 2 focus_out"))) .on_key_down( cx.listener(|_, event, _| println!("Key down on child 2 {:?}", event)), ) diff --git a/crates/ui2/src/components/context_menu.rs b/crates/ui2/src/components/context_menu.rs index c2dc0abe1a..3e54298514 100644 --- a/crates/ui2/src/components/context_menu.rs +++ b/crates/ui2/src/components/context_menu.rs @@ -4,7 +4,7 @@ use crate::{ }; use gpui::{ px, Action, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView, - IntoElement, Render, View, VisualContext, + IntoElement, Render, Subscription, View, VisualContext, }; use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev}; use std::{rc::Rc, time::Duration}; @@ -25,6 +25,7 @@ pub struct ContextMenu { focus_handle: FocusHandle, selected_index: Option, delayed: bool, + _on_blur_subscription: Subscription, } impl FocusableView for ContextMenu { @@ -40,14 +41,18 @@ impl ContextMenu { cx: &mut WindowContext, f: impl FnOnce(Self, &mut WindowContext) -> Self, ) -> View { - // let handle = cx.view().downgrade(); cx.build_view(|cx| { + let focus_handle = cx.focus_handle(); + let _on_blur_subscription = cx.on_blur(&focus_handle, |this: &mut ContextMenu, cx| { + this.cancel(&menu::Cancel, cx) + }); f( Self { items: Default::default(), - focus_handle: cx.focus_handle(), + focus_handle, selected_index: None, delayed: false, + _on_blur_subscription, }, cx, ) @@ -223,7 +228,6 @@ impl Render for ContextMenu { } el }) - .on_blur(cx.listener(|this, _, cx| this.cancel(&Default::default(), cx))) .flex_none() .child( List::new().children(self.items.iter().enumerate().map( From 0140bb862e85a5adaae741b8e15e08d579e0a5df Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 12 Dec 2023 17:15:54 +0200 Subject: [PATCH 6/6] Fix the tests Co-authored-by: Antonio --- crates/gpui2/src/app.rs | 100 ++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index 7d61f4d847..2d619c7733 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -569,61 +569,61 @@ impl AppContext { /// such as notifying observers, emitting events, etc. Effects can themselves /// cause effects, so we continue looping until all effects are processed. fn flush_effects(&mut self) { - while !self.pending_effects.is_empty() { - loop { - self.release_dropped_entities(); - self.release_dropped_focus_handles(); - if let Some(effect) = self.pending_effects.pop_front() { - match effect { - Effect::Notify { emitter } => { - self.apply_notify_effect(emitter); - } - Effect::Emit { - emitter, - event_type, - event, - } => self.apply_emit_effect(emitter, event_type, event), - Effect::FocusChanged { - window_handle, - focused, - } => { - self.apply_focus_changed_effect(window_handle, focused); - } - Effect::Refresh => { - self.apply_refresh_effect(); - } - Effect::NotifyGlobalObservers { global_type } => { - self.apply_notify_global_observers_effect(global_type); - } - Effect::Defer { callback } => { - self.apply_defer_effect(callback); + loop { + self.release_dropped_entities(); + self.release_dropped_focus_handles(); + if let Some(effect) = self.pending_effects.pop_front() { + match effect { + Effect::Notify { emitter } => { + self.apply_notify_effect(emitter); + } + Effect::Emit { + emitter, + event_type, + event, + } => self.apply_emit_effect(emitter, event_type, event), + Effect::FocusChanged { + window_handle, + focused, + } => { + self.apply_focus_changed_effect(window_handle, focused); + } + Effect::Refresh => { + self.apply_refresh_effect(); + } + Effect::NotifyGlobalObservers { global_type } => { + self.apply_notify_global_observers_effect(global_type); + } + Effect::Defer { callback } => { + self.apply_defer_effect(callback); + } + } + } else { + for window in self.windows.values() { + if let Some(window) = window.as_ref() { + if window.dirty { + window.platform_window.invalidate(); } } - } else { + } + + #[cfg(any(test, feature = "test-support"))] + for window in self + .windows + .values() + .filter_map(|window| { + let window = window.as_ref()?; + window.dirty.then_some(window.handle) + }) + .collect::>() + { + self.update_window(window, |_, cx| cx.draw()).unwrap(); + } + + if self.pending_effects.is_empty() { break; } } - - for window in self.windows.values() { - if let Some(window) = window.as_ref() { - if window.dirty { - window.platform_window.invalidate(); - } - } - } - - #[cfg(any(test, feature = "test-support"))] - for window in self - .windows - .values() - .filter_map(|window| { - let window = window.as_ref()?; - window.dirty.then_some(window.handle) - }) - .collect::>() - { - self.update_window(window, |_, cx| cx.draw()).unwrap(); - } } }