Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1740 from zed-industries/fix-dock-focus-issues

Browse source 
Fix Dock infinite loop
This commit is contained in:
Kay Simmons 2022-10-12 16:19:09 -07:00 • committed by GitHub
commit d42bf8eebe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 641 additions and 462 deletions
Download patch file Download diff file Expand all files Collapse all files

955
Cargo.lock generated
View file

File diff suppressed because it is too large Load diff

35
crates/gpui/src/app.rs
View file

@ -618,7 +618,6 @@ pub struct MutableAppContext {
HashMap<usize, (Rc<RefCell<Presenter>>, Box<dyn platform::Window>)>, HashMap<usize, (Rc<RefCell<Presenter>>, Box<dyn platform::Window>)>,
foreground: Rc<executor::Foreground>, foreground: Rc<executor::Foreground>,
pending_effects: VecDeque<Effect>, pending_effects: VecDeque<Effect>,
pending_focus_index: Option<usize>,
pending_notifications: HashSet<usize>, pending_notifications: HashSet<usize>,
pending_global_notifications: HashSet<TypeId>, pending_global_notifications: HashSet<TypeId>,
pending_flushes: usize, pending_flushes: usize,
@ -673,7 +672,6 @@ impl MutableAppContext {
presenters_and_platform_windows: Default::default(), presenters_and_platform_windows: Default::default(),
foreground, foreground,
pending_effects: VecDeque::new(), pending_effects: VecDeque::new(),
pending_focus_index: None,
pending_notifications: Default::default(), pending_notifications: Default::default(),
pending_global_notifications: Default::default(), pending_global_notifications: Default::default(),
pending_flushes: 0, pending_flushes: 0,
@ -1876,9 +1874,6 @@ impl MutableAppContext {
let mut refreshing = false; let mut refreshing = false;
loop { loop {
if let Some(effect) = self.pending_effects.pop_front() { if let Some(effect) = self.pending_effects.pop_front() {
if let Some(pending_focus_index) = self.pending_focus_index.as_mut() {
*pending_focus_index = pending_focus_index.saturating_sub(1);
}
match effect { match effect {
Effect::Subscription { Effect::Subscription {
entity_id, entity_id,
@ -2259,8 +2254,6 @@ impl MutableAppContext {
} }
fn handle_focus_effect(&mut self, window_id: usize, focused_id: Option<usize>) { fn handle_focus_effect(&mut self, window_id: usize, focused_id: Option<usize>) {
self.pending_focus_index.take();
if self if self
.cx .cx
.windows .windows
@ -2383,10 +2376,6 @@ impl MutableAppContext {
} }
pub fn focus(&mut self, window_id: usize, view_id: Option<usize>) { pub fn focus(&mut self, window_id: usize, view_id: Option<usize>) {
if let Some(pending_focus_index) = self.pending_focus_index {
self.pending_effects.remove(pending_focus_index);
}
self.pending_focus_index = Some(self.pending_effects.len());
self.pending_effects self.pending_effects
.push_back(Effect::Focus { window_id, view_id }); .push_back(Effect::Focus { window_id, view_id });
} }
@ -3465,6 +3454,15 @@ impl<'a, T: View> ViewContext<'a, T> {
self.app.focused_view_id(self.window_id) == Some(self.view_id) self.app.focused_view_id(self.window_id) == Some(self.view_id)
} }
pub fn is_child(&self, view: impl Into<AnyViewHandle>) -> bool {
let view = view.into();
if self.window_id != view.window_id {
return false;
}
self.parents(view.window_id, view.view_id)
.any(|parent| parent == self.view_id)
}
pub fn blur(&mut self) { pub fn blur(&mut self) {
self.app.focus(self.window_id, None); self.app.focus(self.window_id, None);
} }
@ -6378,18 +6376,29 @@ mod tests {
assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new()); assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new());
view_1.update(cx, |_, cx| { view_1.update(cx, |_, cx| {
// Ensure only the latest focus is honored. // Ensure focus events are sent for all intermediate focuses
cx.focus(&view_2); cx.focus(&view_2);
cx.focus(&view_1); cx.focus(&view_1);
cx.focus(&view_2); cx.focus(&view_2);
}); });
assert_eq!( assert_eq!(
mem::take(&mut *view_events.lock()), mem::take(&mut *view_events.lock()),
["view 1 blurred", "view 2 focused"], [
"view 1 blurred",
"view 2 focused",
"view 2 blurred",
"view 1 focused",
"view 1 blurred",
"view 2 focused"
],
); );
assert_eq!( assert_eq!(
mem::take(&mut *observed_events.lock()), mem::take(&mut *observed_events.lock()),
[ [
"view 2 observed view 1's blur",
"view 1 observed view 2's focus",
"view 1 observed view 2's blur",
"view 2 observed view 1's focus",
"view 2 observed view 1's blur", "view 2 observed view 1's blur",
"view 1 observed view 2's focus" "view 1 observed view 2's focus"
] ]

4
crates/search/src/project_search.rs
View file

@ -200,6 +200,10 @@ impl View for ProjectSearchView {
.0 .0
.insert(self.model.read(cx).project.downgrade(), handle) .insert(self.model.read(cx).project.downgrade(), handle)
}); });
if cx.is_self_focused() {
self.focus_query_editor(cx);
}
} }
} }

10
crates/workspace/src/dock.rs
View file

@ -170,7 +170,11 @@ impl Dock {
} else { } else {
cx.focus(pane); cx.focus(pane);
} }
} else if let Some(last_active_center_pane) = workspace.last_active_center_pane.clone() { } else if let Some(last_active_center_pane) = workspace
.last_active_center_pane
.as_ref()
.and_then(|pane| pane.upgrade(cx))
{
cx.focus(last_active_center_pane); cx.focus(last_active_center_pane);
} }
cx.emit(crate::Event::DockAnchorChanged); cx.emit(crate::Event::DockAnchorChanged);
@ -583,10 +587,11 @@ mod tests {
} }
pub fn center_pane_handle(&self) -> ViewHandle<Pane> { pub fn center_pane_handle(&self) -> ViewHandle<Pane> {
self.workspace(|workspace, _| { self.workspace(|workspace, cx| {
workspace workspace
.last_active_center_pane .last_active_center_pane
.clone() .clone()
.and_then(|pane| pane.upgrade(cx))
.unwrap_or_else(|| workspace.center.panes()[0].clone()) .unwrap_or_else(|| workspace.center.panes()[0].clone())
}) })
} }
@ -597,6 +602,7 @@ mod tests {
let pane = workspace let pane = workspace
.last_active_center_pane .last_active_center_pane
.clone() .clone()
.and_then(|pane| pane.upgrade(cx))
.unwrap_or_else(|| workspace.center.panes()[0].clone()); .unwrap_or_else(|| workspace.center.panes()[0].clone());
Pane::add_item( Pane::add_item(
workspace, workspace,

77
crates/workspace/src/pane.rs
View file

@ -112,10 +112,10 @@ pub fn init(cx: &mut MutableAppContext) {
pane.activate_item(pane.items.len() - 1, true, true, cx); pane.activate_item(pane.items.len() - 1, true, true, cx);
}); });
cx.add_action(|pane: &mut Pane, _: &ActivatePrevItem, cx| { cx.add_action(|pane: &mut Pane, _: &ActivatePrevItem, cx| {
pane.activate_prev_item(cx); pane.activate_prev_item(true, cx);
}); });
cx.add_action(|pane: &mut Pane, _: &ActivateNextItem, cx| { cx.add_action(|pane: &mut Pane, _: &ActivateNextItem, cx| {
pane.activate_next_item(cx); pane.activate_next_item(true, cx);
}); });
cx.add_async_action(Pane::close_active_item); cx.add_async_action(Pane::close_active_item);
cx.add_async_action(Pane::close_inactive_items); cx.add_async_action(Pane::close_inactive_items);
@ -189,7 +189,6 @@ pub fn init(cx: &mut MutableAppContext) {
#[derive(Debug)] #[derive(Debug)]
pub enum Event { pub enum Event {
Focused,
ActivateItem { local: bool }, ActivateItem { local: bool },
Remove, Remove,
RemoveItem { item_id: usize }, RemoveItem { item_id: usize },
@ -201,7 +200,7 @@ pub struct Pane {
items: Vec<Box<dyn ItemHandle>>, items: Vec<Box<dyn ItemHandle>>,
is_active: bool, is_active: bool,
active_item_index: usize, active_item_index: usize,
last_focused_view: Option<AnyWeakViewHandle>, last_focused_view_by_item: HashMap<usize, AnyWeakViewHandle>,
autoscroll: bool, autoscroll: bool,
nav_history: Rc<RefCell<NavHistory>>, nav_history: Rc<RefCell<NavHistory>>,
toolbar: ViewHandle<Toolbar>, toolbar: ViewHandle<Toolbar>,
@ -263,7 +262,7 @@ impl Pane {
items: Vec::new(), items: Vec::new(),
is_active: true, is_active: true,
active_item_index: 0, active_item_index: 0,
last_focused_view: None, last_focused_view_by_item: Default::default(),
autoscroll: false, autoscroll: false,
nav_history: Rc::new(RefCell::new(NavHistory { nav_history: Rc::new(RefCell::new(NavHistory {
mode: NavigationMode::Normal, mode: NavigationMode::Normal,
@ -632,32 +631,29 @@ impl Pane {
if focus_item { if focus_item {
self.focus_active_item(cx); self.focus_active_item(cx);
} }
if activate_pane {
cx.emit(Event::Focused);
}
self.autoscroll = true; self.autoscroll = true;
cx.notify(); cx.notify();
} }
} }
pub fn activate_prev_item(&mut self, cx: &mut ViewContext<Self>) { pub fn activate_prev_item(&mut self, activate_pane: bool, cx: &mut ViewContext<Self>) {
let mut index = self.active_item_index; let mut index = self.active_item_index;
if index > 0 { if index > 0 {
index -= 1; index -= 1;
} else if !self.items.is_empty() { } else if !self.items.is_empty() {
index = self.items.len() - 1; index = self.items.len() - 1;
} }
self.activate_item(index, true, true, cx); self.activate_item(index, activate_pane, activate_pane, cx);
} }
pub fn activate_next_item(&mut self, cx: &mut ViewContext<Self>) { pub fn activate_next_item(&mut self, activate_pane: bool, cx: &mut ViewContext<Self>) {
let mut index = self.active_item_index; let mut index = self.active_item_index;
if index + 1 < self.items.len() { if index + 1 < self.items.len() {
index += 1; index += 1;
} else { } else {
index = 0; index = 0;
} }
self.activate_item(index, true, true, cx); self.activate_item(index, activate_pane, activate_pane, cx);
} }
pub fn close_active_item( pub fn close_active_item(
@ -784,7 +780,7 @@ impl Pane {
// Remove the item from the pane. // Remove the item from the pane.
pane.update(&mut cx, |pane, cx| { pane.update(&mut cx, |pane, cx| {
if let Some(item_ix) = pane.items.iter().position(|i| i.id() == item.id()) { if let Some(item_ix) = pane.items.iter().position(|i| i.id() == item.id()) {
pane.remove_item(item_ix, cx); pane.remove_item(item_ix, false, cx);
} }
}); });
} }
@ -794,15 +790,15 @@ impl Pane {
}) })
} }
fn remove_item(&mut self, item_ix: usize, cx: &mut ViewContext<Self>) { fn remove_item(&mut self, item_ix: usize, activate_pane: bool, cx: &mut ViewContext<Self>) {
if item_ix == self.active_item_index { if item_ix == self.active_item_index {
// Activate the previous item if possible. // Activate the previous item if possible.
// This returns the user to the previously opened tab if they closed // This returns the user to the previously opened tab if they closed
// a new item they just navigated to. // a new item they just navigated to.
if item_ix > 0 { if item_ix > 0 {
self.activate_prev_item(cx); self.activate_prev_item(activate_pane, cx);
} else if item_ix + 1 < self.items.len() { } else if item_ix + 1 < self.items.len() {
self.activate_next_item(cx); self.activate_next_item(activate_pane, cx);
} }
} }
@ -965,26 +961,27 @@ impl Pane {
log::warn!("Tried to move item handle which was not in `from` pane. Maybe tab was closed during drop"); log::warn!("Tried to move item handle which was not in `from` pane. Maybe tab was closed during drop");
return; return;
} }
let (item_ix, item_handle) = item_to_move.unwrap(); let (item_ix, item_handle) = item_to_move.unwrap();
let item_handle = item_handle.clone();
if from != to {
// Close item from previous pane
from.update(cx, |from, cx| {
from.remove_item(item_ix, false, cx);
});
}
// This automatically removes duplicate items in the pane // This automatically removes duplicate items in the pane
Pane::add_item( Pane::add_item(
workspace, workspace,
&to, &to,
item_handle.clone(), item_handle,
true, true,
true, true,
Some(destination_index), Some(destination_index),
cx, cx,
); );
if from != to {
// Close item from previous pane
from.update(cx, |from, cx| {
from.remove_item(item_ix, cx);
});
}
cx.focus(to); cx.focus(to);
} }
@ -1488,21 +1485,27 @@ impl View for Pane {
} }
fn on_focus_in(&mut self, focused: AnyViewHandle, cx: &mut ViewContext<Self>) { fn on_focus_in(&mut self, focused: AnyViewHandle, cx: &mut ViewContext<Self>) {
if cx.is_self_focused() { if let Some(active_item) = self.active_item() {
if let Some(last_focused_view) = self if cx.is_self_focused() {
.last_focused_view // Pane was focused directly. We need to either focus a view inside the active item,
.as_ref() // or focus the active item itself
.and_then(|handle| handle.upgrade(cx)) if let Some(weak_last_focused_view) =
.filter(|handle| handle.id() != self.tab_bar_context_menu.id()) self.last_focused_view_by_item.get(&active_item.id())
{ {
cx.focus(last_focused_view); if let Some(last_focused_view) = weak_last_focused_view.upgrade(cx) {
cx.focus(last_focused_view);
return;
} else {
self.last_focused_view_by_item.remove(&active_item.id());
}
}
cx.focus(active_item);
} else { } else {
self.focus_active_item(cx); self.last_focused_view_by_item
.insert(active_item.id(), focused.downgrade());
} }
} else {
self.last_focused_view = Some(focused.downgrade());
} }
cx.emit(Event::Focused);
} }
} }

22
crates/workspace/src/workspace.rs
View file

@ -1,5 +1,4 @@
/// NOTE: Focus only 'takes' after an update has flushed_effects. Pane sends an event in on_focus_in /// NOTE: Focus only 'takes' after an update has flushed_effects.
/// which the workspace uses to change the activated pane.
/// ///
/// This may cause issues when you're trying to write tests that use workspace focus to add items at /// This may cause issues when you're trying to write tests that use workspace focus to add items at
/// specific locations. /// specific locations.
@ -971,7 +970,7 @@ pub struct Workspace {
panes: Vec<ViewHandle<Pane>>, panes: Vec<ViewHandle<Pane>>,
panes_by_item: HashMap<usize, WeakViewHandle<Pane>>, panes_by_item: HashMap<usize, WeakViewHandle<Pane>>,
active_pane: ViewHandle<Pane>, active_pane: ViewHandle<Pane>,
last_active_center_pane: Option<ViewHandle<Pane>>, last_active_center_pane: Option<WeakViewHandle<Pane>>,
status_bar: ViewHandle<StatusBar>, status_bar: ViewHandle<StatusBar>,
titlebar_item: Option<AnyViewHandle>, titlebar_item: Option<AnyViewHandle>,
dock: Dock, dock: Dock,
@ -1111,7 +1110,7 @@ impl Workspace {
panes: vec![dock_pane, center_pane.clone()], panes: vec![dock_pane, center_pane.clone()],
panes_by_item: Default::default(), panes_by_item: Default::default(),
active_pane: center_pane.clone(), active_pane: center_pane.clone(),
last_active_center_pane: Some(center_pane.clone()), last_active_center_pane: Some(center_pane.downgrade()),
status_bar, status_bar,
titlebar_item: None, titlebar_item: None,
notifications: Default::default(), notifications: Default::default(),
@ -1850,7 +1849,7 @@ impl Workspace {
if &pane == self.dock_pane() { if &pane == self.dock_pane() {
Dock::show(self, cx); Dock::show(self, cx);
} else { } else {
self.last_active_center_pane = Some(pane.clone()); self.last_active_center_pane = Some(pane.downgrade());
if self.dock.is_anchored_at(DockAnchor::Expanded) { if self.dock.is_anchored_at(DockAnchor::Expanded) {
Dock::hide(self, cx); Dock::hide(self, cx);
} }
@ -1881,7 +1880,6 @@ impl Workspace {
} }
pane::Event::Remove if !is_dock => self.remove_pane(pane, cx), pane::Event::Remove if !is_dock => self.remove_pane(pane, cx),
pane::Event::Remove if is_dock => Dock::hide(self, cx), pane::Event::Remove if is_dock => Dock::hide(self, cx),
pane::Event::Focused => self.handle_pane_focused(pane, cx),
pane::Event::ActivateItem { local } => { pane::Event::ActivateItem { local } => {
if *local { if *local {
self.unfollow(&pane, cx); self.unfollow(&pane, cx);
@ -1942,7 +1940,7 @@ impl Workspace {
for removed_item in pane.read(cx).items() { for removed_item in pane.read(cx).items() {
self.panes_by_item.remove(&removed_item.id()); self.panes_by_item.remove(&removed_item.id());
} }
if self.last_active_center_pane == Some(pane) { if self.last_active_center_pane == Some(pane.downgrade()) {
self.last_active_center_pane = None; self.last_active_center_pane = None;
} }
@ -2652,9 +2650,17 @@ impl View for Workspace {
.named("workspace") .named("workspace")
} }
fn on_focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext<Self>) { fn on_focus_in(&mut self, view: AnyViewHandle, cx: &mut ViewContext<Self>) {
if cx.is_self_focused() { if cx.is_self_focused() {
cx.focus(&self.active_pane); cx.focus(&self.active_pane);
} else {
for pane in self.panes() {
let view = view.clone();
if pane.update(cx, |_, cx| cx.is_child(view)) {
self.handle_pane_focused(pane.clone(), cx);
break;
}
}
} }
} }