Primitives · Focus: overlays dropped together hand focus back past every one of them

src/overlays/tests/mod.rs

@@ -24,6 +24,7 @@ enum Open {
     Confirm,
     Prompt,
     Tall,
+    Nested,
 }
 
 /// A popover, a hover card and one dialog at a time, recording what they do.
@@ -46,7 +47,10 @@ impl Stage {
         move |_, cx| view.update(cx, |stage, _| stage.log.push(what.into()))
     }
 
-    fn shut(&self, cx: &mut Context<Self>) -> impl Fn(&mut Window, &mut gpui::App) + 'static {
+    fn shut(
+        &self,
+        cx: &mut Context<Self>,
+    ) -> impl Fn(&mut Window, &mut gpui::App) + Clone + 'static {
         let view = cx.entity();
         move |_, cx| {
             view.update(cx, |stage, cx| {
@@ -100,6 +104,9 @@ impl Render for Stage {
         );
         let sent = cx.entity();
         let overlay = self.open.map(|open| match open {
+            Open::Nested => Dialog::new("outer", "Outer", shut.clone())
+                .child(Dialog::new("inner", "Inner", shut.clone()))
+                .into_any_element(),
             Open::Plain => Dialog::new("plain", "Plain", shut)
                 .detail("A line of detail")
                 .into_any_element(),
@@ -274,6 +281,19 @@ fn a_dialog_its_owner_drops_hands_focus_back(cx: &mut TestAppContext) {
     assert!(before_focused(&view, cx), "focus went back");
 }
 
+#[gpui::test]
+fn nested_dialogs_dropped_together_hand_focus_back(cx: &mut TestAppContext) {
+    let (view, cx) = stage(cx);
+    show(&view, Open::Nested, cx);
+    assert!(!before_focused(&view, cx), "the dialogs hold focus");
+    view.update(cx, |stage, cx| {
+        stage.open = None;
+        cx.notify();
+    });
+    settle(cx);
+    assert!(before_focused(&view, cx), "focus went back past both");
+}
+
 #[gpui::test]
 fn the_scrim_closes_a_dialog_but_not_an_alert(cx: &mut TestAppContext) {
     let (view, cx) = stage(cx);

src/primitives/focus.rs

@@ -126,23 +126,39 @@ fn step(scope: &FocusHandle, trap: bool, forward: bool, window: &mut Window, cx:
     }
 }
 
-/// Open overlays holding focus, oldest first, with what they took it from.
+/// Overlays holding focus: their focus and the prior one.
 #[derive(Default)]
-struct Holders(Vec<(WeakEntity<Takeover>, AnyWindowHandle, Option<FocusHandle>)>);
+struct Holders(Vec<Holder>);
+
+struct Holder {
+    takeover: WeakEntity<Takeover>,
+    window: AnyWindowHandle,
+    focus: FocusHandle,
+    previous: Option<FocusHandle>,
+}
 
 impl Global for Holders {}
 
-/// Forgets overlays gone without closing; the newest one's prior focus.
+/// Forgets overlays gone unclosed; the focus before them all.
 fn dropped_overlay(window: &Window, cx: &mut App) -> Option<FocusHandle> {
     let here = window.window_handle();
-    let mut back = None;
-    cx.default_global::<Holders>().0.retain(|(holder, at, previous)| {
-        let gone = holder.upgrade().is_none();
-        if gone && *at == here {
-            back = previous.clone();
-        }
-        !gone
-    });
+    let holders = &mut cx.default_global::<Holders>().0;
+    let (gone, open): (Vec<Holder>, Vec<Holder>) = holders
+        .drain(..)
+        .partition(|holder| holder.takeover.upgrade().is_none());
+    *holders = open;
+    let gone: Vec<Holder> = gone
+        .into_iter()
+        .filter(|holder| holder.window == here)
+        .collect();
+    // Walk back past every gone overlay.
+    let mut back = gone.last()?.previous.clone();
+    while let Some(holder) = gone
+        .iter()
+        .find(|holder| Some(&holder.focus) == back.as_ref())
+    {
+        back = holder.previous.clone();
+    }
     back
 }
 
@@ -176,7 +192,12 @@ pub(crate) fn take_focus(
     if !state.read(cx).taken {
         let previous = window.focused(cx);
         window.focus(&state.read(cx).focus.clone(), cx);
-        let holder = (state.downgrade(), window.window_handle(), previous.clone());
+        let holder = Holder {
+            takeover: state.downgrade(),
+            window: window.window_handle(),
+            focus: state.read(cx).focus.clone(),
+            previous: previous.clone(),
+        };
         cx.default_global::<Holders>().0.push(holder);
         state.update(cx, |takeover, _| {
             takeover.previous = previous;
@@ -189,7 +210,9 @@ pub(crate) fn take_focus(
 /// Returns focus to whatever held it before the overlay opened.
 pub(crate) fn give_back(state: &Entity<Takeover>, window: &mut Window, cx: &mut App) {
     let closed = state.downgrade();
-    cx.default_global::<Holders>().0.retain(|(holder, ..)| *holder != closed);
+    cx.default_global::<Holders>()
+        .0
+        .retain(|holder| holder.takeover != closed);
     let previous = state.update(cx, |takeover, _| {
         takeover.returned = true;
         takeover.previous.clone()