docking: panels closed one by one reopen into one group (#290)

* docking: panels closed one by one reopen into one group

Showing a hidden panel whose anchor is hidden too follows the chain of
hidden anchors to its far end, so a floating location no longer opens a
second window. The hidden panels the restore passed then anchor back on
the panel before them in the chain, on the recorded side, so reopening
them joins the restored panel instead of rebuilding their group or
window. A restored tab whose next tab is not in the anchor's group goes
last.

* docking: a panel beside an edge frame keeps its own location on reopen

* docking: a panel reopened through a hidden neighbour keeps its own group state; the chain stops reversing at an edge frame

crates/ui/src/docking/model.rs

@@ -784,7 +784,8 @@ impl<P: Clone + Eq + Hash> Layout<P> {
 
     /// Restore an absent panel at a captured location. Missing anchors are reported without
     /// changes, allowing the app to choose an explicit default; a floating panel whose
-    /// window is gone floats at its last rect instead. Unrelated tabs retain selection.
+    /// window is gone floats at its last rect instead. A tab whose next tab is not in the
+    /// anchor's group goes last. Unrelated tabs retain selection.
     pub fn restore(&mut self, panel: P, location: &Location<P>) -> Result<(), DockError> {
         self.validate()?;
         if self.contains(&panel) {
@@ -856,7 +857,12 @@ impl<P: Clone + Eq + Hash> Layout<P> {
             }
             // A subtree that changed since (a migrated or reconciled layout) splits the anchor.
             if !around {
-                proposed.apply_inner(Action::OpenAt { panel: panel.clone(), anchor: anchor.clone(), placement: location.placement.clone() })?;
+                // The tab that followed may have closed or moved since; the panel then goes last.
+                let placement = match &location.placement {
+                    Placement::Tab { before: Some(before) } if !proposed.same_group(before, anchor) => Placement::Tab { before: None },
+                    placement => placement.clone(),
+                };
+                proposed.apply_inner(Action::OpenAt { panel: panel.clone(), anchor: anchor.clone(), placement })?;
                 if let Some(size) = location.size {
                     proposed.trees_mut().any(|root| restore_parent_size(root, &panel, size));
                 }

crates/ui/src/docking/model_tests.rs

@@ -205,6 +205,154 @@ fn close_group_and_close_window_record_a_restorable_location_per_member() {
     assert_eq!(workspace, before);
 }
 
+#[test]
+fn panels_closed_one_by_one_reopen_into_one_group() {
+    let allow = |_: &u32| Permissions::default();
+    let rect = [100.0, 100.0, 300.0, 400.0];
+    let cycle = |workspace: &mut Workspace<u32>, close: &[u32], reopen: &[u32]| {
+        for &panel in close {
+            workspace.set_visible(panel, false, None, allow).unwrap();
+        }
+        for &panel in reopen {
+            workspace.set_visible(panel, true, None, allow).unwrap_or_else(|e| panic!("{close:?} {reopen:?} {panel}: {e}"));
+        }
+        assert!(workspace.hidden.is_empty());
+    };
+    let orders2 = [[2, 3], [3, 2]];
+    for close in orders2 {
+        for reopen in orders2 {
+            let mut workspace = Workspace::new(docked());
+            cycle(&mut workspace, &close, &reopen);
+            let columns = dock(&workspace.layout, Side::End);
+            assert_eq!(
+                columns.iter().map(|c| panels_of(&c.groups)).collect::<Vec<_>>(),
+                [vec![vec![2, 3], vec![4]], vec![vec![5]]],
+                "{close:?} {reopen:?}"
+            );
+        }
+    }
+    let orders2 = [[10, 20], [20, 10]];
+    for close in orders2 {
+        for reopen in orders2 {
+            let mut workspace = Workspace::new(Layout { root: Some(tabs(&[1], 0)), floating: vec![Floating::single(vec![10, 20], 0, rect)] });
+            cycle(&mut workspace, &close, &reopen);
+            let [window] = &workspace.layout.floating[..] else { panic!("{close:?} {reopen:?}: {:?}", workspace.layout.floating) };
+            assert_eq!((window.panels().copied().collect::<Vec<_>>(), window.rect), (vec![10, 20], rect), "{close:?} {reopen:?}");
+        }
+    }
+    // Three tabs: the locations do not always record the order, only the window.
+    let orders3 = [[10, 20, 30], [10, 30, 20], [20, 10, 30], [20, 30, 10], [30, 10, 20], [30, 20, 10]];
+    for close in orders3 {
+        for reopen in orders3 {
+            let mut workspace = Workspace::new(Layout { root: Some(tabs(&[1], 0)), floating: vec![Floating::single(vec![10, 20, 30], 0, rect)] });
+            cycle(&mut workspace, &close, &reopen);
+            let [window] = &workspace.layout.floating[..] else { panic!("{close:?} {reopen:?}: {:?}", workspace.layout.floating) };
+            let mut panels: Vec<u32> = window.panels().copied().collect();
+            panels.sort_unstable();
+            assert_eq!(panels, [10, 20, 30], "{close:?} {reopen:?}");
+        }
+    }
+}
+
+#[test]
+fn a_panel_reopened_through_a_hidden_neighbour_keeps_its_own_group_state() {
+    let allow = |_: &u32| Permissions::default();
+    let dock_of = |columns: Vec<Column<u32>>| Layout {
+        root: Some(hsplit(SplitSize::FixedSecond(480.0), tabs(&[1], 0), Node::Dock { side: Side::End, columns })),
+        floating: Vec::new(),
+    };
+    // Groups [2] and [3] stacked with their own heights: 2 comes back at its height, not 3's.
+    let stacked = dock_of(vec![
+        Column::new(vec![Group { height: Some(120.0), ..g(&[2], 0) }, Group { height: Some(300.0), ..g(&[3], 0) }], 240.0),
+        Column::new(vec![g(&[5], 0)], 240.0),
+    ]);
+    let mut workspace = Workspace::new(stacked);
+    workspace.set_visible(2, false, None, allow).unwrap();
+    workspace.set_visible(3, false, None, allow).unwrap();
+    workspace.set_visible(2, true, None, allow).unwrap();
+    let group = |w: &Workspace<u32>, panel: u32| {
+        dock(&w.layout, Side::End).iter().flat_map(|c| c.groups.iter()).find(|g| g.panels.contains(&panel)).cloned().unwrap()
+    };
+    assert_eq!(group(&workspace, 2).height, Some(120.0), "{:?}", workspace.layout);
+    workspace.set_visible(3, true, None, allow).unwrap();
+    assert_eq!(group(&workspace, 3).height, Some(300.0), "{:?}", workspace.layout);
+
+    // [2] beside an icon-only column holding [3]: 2 comes back in a column of its own width,
+    // not as an icon.
+    let iconic = Column { iconic: true, ..Column::new(vec![g(&[3], 0)], 200.0) };
+    let mut workspace = Workspace::new(dock_of(vec![Column::new(vec![g(&[2], 0)], 260.0), iconic]));
+    workspace.set_visible(2, false, None, allow).unwrap();
+    workspace.set_visible(3, false, None, allow).unwrap();
+    workspace.set_visible(2, true, None, allow).unwrap();
+    let column = dock(&workspace.layout, Side::End).iter().find(|c| c.groups.iter().any(|g| g.panels.contains(&2))).cloned().unwrap();
+    assert!(!column.iconic, "{:?}", workspace.layout);
+    assert_eq!(column.width, 260.0);
+}
+
+#[test]
+fn panels_closed_one_by_one_reopen_on_their_own_sides() {
+    let allow = |_: &u32| Permissions::default();
+    let orders = [[2, 3], [3, 2]];
+    let frames = Layout {
+        root: Some(hsplit(SplitSize::Ratio(0.3), tabs(&[1], 0), hsplit(SplitSize::Ratio(0.5), tabs(&[2], 0), tabs(&[3], 0)))),
+        floating: Vec::new(),
+    };
+    let columns = Layout {
+        root: Some(hsplit(
+            SplitSize::FixedSecond(480.0),
+            tabs(&[1], 0),
+            Node::Dock { side: Side::End, columns: vec![Column::new(vec![g(&[2], 0)], 240.0), Column::new(vec![g(&[3], 0)], 240.0)] },
+        )),
+        floating: Vec::new(),
+    };
+    for close in orders {
+        for reopen in orders {
+            let mut workspace = Workspace::new(frames.clone());
+            for (panel, visible) in close.iter().map(|p| (*p, false)).chain(reopen.iter().map(|p| (*p, true))) {
+                workspace.set_visible(panel, visible, None, allow).unwrap();
+            }
+            let order: Vec<u32> = workspace.layout.panels().into_iter().copied().collect();
+            assert_eq!(order, [1, 2, 3], "{close:?} {reopen:?}: {:?}", workspace.layout);
+            assert!(order.iter().all(|p| matches!(workspace.layout.place(p), Some(Place::Node(_)))));
+            assert_ne!(workspace.layout.place(&2), workspace.layout.place(&3), "{close:?} {reopen:?}");
+
+            let mut workspace = Workspace::new(columns.clone());
+            for (panel, visible) in close.iter().map(|p| (*p, false)).chain(reopen.iter().map(|p| (*p, true))) {
+                workspace.set_visible(panel, visible, None, allow).unwrap();
+            }
+            let groups: Vec<_> = dock(&workspace.layout, Side::End).iter().map(|c| panels_of(&c.groups)).collect();
+            assert_eq!(groups, [vec![vec![2]], vec![vec![3]]], "{close:?} {reopen:?}");
+        }
+    }
+    // A panel that sat at the edge beside a hidden one keeps that one's own location: 3 opens
+    // right of 1, not as a tab beside 2.
+    let mut workspace = Workspace::new(Layout { root: Some(tabs(&[1], 0)), floating: Vec::new() });
+    let at = |anchor: u32, placement: Placement<u32>| Location {
+        floating: None,
+        anchor: Some(anchor),
+        placement,
+        size: None,
+        stack: None,
+        sibling: Vec::new(),
+        group: None,
+    };
+    workspace.hidden = vec![(2, at(3, Placement::Edge(Zone::Right))), (3, at(1, Placement::Split(Zone::Right)))];
+    workspace.set_visible(2, true, None, allow).unwrap();
+    workspace.set_visible(3, true, None, allow).unwrap();
+    assert!(matches!(workspace.layout.place(&3), Some(Place::Node(_))));
+    assert_ne!(workspace.layout.place(&2), workspace.layout.place(&3), "{:?}", workspace.layout);
+    // Past the edge frame the chain keeps its forward anchors: 4 does not anchor on 3 while 3
+    // anchors on 4, and both still reopen.
+    let mut workspace = Workspace::new(Layout { root: Some(tabs(&[1], 0)), floating: Vec::new() });
+    workspace.hidden =
+        vec![(2, at(3, Placement::Edge(Zone::Right))), (3, at(4, Placement::Split(Zone::Right))), (4, at(1, Placement::Split(Zone::Right)))];
+    workspace.set_visible(2, true, None, allow).unwrap();
+    assert_eq!(workspace.hidden.iter().find(|(p, _)| *p == 4).and_then(|(_, l)| l.anchor), Some(1), "{:?}", workspace.hidden);
+    workspace.set_visible(3, true, None, allow).unwrap();
+    workspace.set_visible(4, true, None, allow).unwrap();
+    assert!(workspace.hidden.is_empty() && [1, 2, 3, 4].iter().all(|p| workspace.layout.contains(p)), "{:?}", workspace.layout);
+}
+
 #[test]
 fn restore_into_a_column_group_seam_and_new_column_round_trips() {
     let column_a = Column::new(vec![g(&[2, 3], 1), Group { minimized: true, height: Some(150.0), ..g(&[4], 0) }, g(&[9], 0)], 180.0);

crates/ui/src/docking/workspace.rs

@@ -6,7 +6,7 @@
 
 use std::{collections::HashSet, hash::Hash};
 
-use super::model::{Action, DockError, Layout, Location, MAX_PANELS, Permissions};
+use super::model::{Action, DockError, Layout, Location, MAX_PANELS, Permissions, Placement, Zone};
 
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -121,7 +121,9 @@ impl<P: Clone + Eq + Hash> Workspace<P> {
 
     /// Show or hide one panel. Showing restores its recorded location, following the chain of
     /// hidden anchors when its own anchor is hidden too; otherwise it opens as
-    /// [`Action::Open`] with `fallback` as the anchor, following `permissions`.
+    /// [`Action::Open`] with `fallback` as the anchor, following `permissions`. The hidden
+    /// panels on that chain reopen beside it later, so a group or window closed one panel at a
+    /// time comes back as one.
     /// A shown panel becomes the active tab of its group. Hiding follows `permissions` and
     /// [`Self::locked`], as [`Self::apply`] does, so a protected canvas stays shown.
     pub fn set_visible(&mut self, panel: P, visible: bool, fallback: Option<P>, permissions: impl Fn(&P) -> Permissions) -> Result<(), DockError> {
@@ -136,7 +138,7 @@ impl<P: Clone + Eq + Hash> Workspace<P> {
             return self.apply(Action::Activate { panel }, |_| Permissions::default()).map(drop);
         }
         let mut layout = self.layout.clone();
-        if self.restore_into(&mut layout, &panel).is_err() {
+        if self.restore_from_hidden(&mut layout, &panel).is_err() {
             layout = self.layout.clone();
             layout.apply_with_permissions(Action::Open { panel: panel.clone(), anchor: fallback }, &permissions)?;
         }
@@ -146,19 +148,42 @@ impl<P: Clone + Eq + Hash> Workspace<P> {
         Ok(())
     }
 
-    fn restore_into(&self, layout: &mut Layout<P>, panel: &P) -> Result<(), DockError> {
+    /// Restore `panel` where the chain of hidden anchors from its location ends, else at the
+    /// nearest location before that end that restores. The hidden panels it passed then anchor
+    /// on the panel before them in the chain, on the recorded side, so reopening them joins it
+    /// instead of rebuilding their group or window a second time.
+    fn restore_from_hidden(&mut self, layout: &mut Layout<P>, panel: &P) -> Result<(), DockError> {
         let find = |p: &P| self.hidden.iter().find(|(q, _)| q == p).map(|(_, location)| location);
-        let mut location = find(panel).ok_or(DockError::MissingTarget)?;
-        for _ in 0..=self.hidden.len() {
-            match layout.restore(panel.clone(), location) {
-                Err(DockError::MissingTarget) => {
-                    let next = location.anchor.as_ref().filter(|anchor| *anchor != panel).and_then(find);
-                    location = next.ok_or(DockError::MissingTarget)?;
-                }
-                result => return result,
-            }
+        let mut chain = vec![(panel.clone(), find(panel).ok_or(DockError::MissingTarget)?.clone())];
+        while let Some(anchor) = chain.last().and_then(|(_, location)| location.anchor.as_ref())
+            && !chain.iter().any(|(p, _)| p == anchor)
+            && let Some(location) = find(anchor)
+        {
+            chain.push((anchor.clone(), location.clone()));
         }
-        Err(DockError::MissingTarget)
+        // A floating location whose anchor is hidden would open a window of its own, even when
+        // the chain leads to an open one. A neighbour's location brings the panel's own group
+        // and column state (height, width, icon-only), not the neighbour's.
+        let own = chain.first().and_then(|(_, location)| location.group.clone());
+        let used = (0..chain.len())
+            .rev()
+            .find_map(|i| {
+                let location = Location { group: own.clone().or_else(|| chain[i].1.group.clone()), ..chain[i].1.clone() };
+                match layout.restore(panel.clone(), &location) {
+                    Err(DockError::MissingTarget) => None,
+                    result => Some(result.map(|()| i)),
+                }
+            })
+            .unwrap_or(Err(DockError::MissingTarget))?;
+        // Past an outer-edge frame the relation can't be reversed: the rest of the chain keeps
+        // its forward anchors, so no two hidden panels anchor on each other.
+        for i in 1..=used {
+            let (before, location) = &chain[i - 1];
+            let Some((_, own)) = self.hidden.iter_mut().find(|(p, _)| p == &chain[i].0) else { continue };
+            let Some(location) = reversed(own, location, before) else { break };
+            *own = location;
+        }
+        Ok(())
     }
 
     /// Fit a loaded workspace to the panels the application registers now. An invalid
@@ -216,7 +241,7 @@ impl<P: Clone + Eq + Hash> Workspace<P> {
         let stuck: Vec<P> = self.hidden.iter().map(|(p, _)| p).filter(|p| registered(p) && !permissions(p).closable).cloned().collect();
         for panel in stuck {
             let mut layout = self.layout.clone();
-            let shown = self.restore_into(&mut layout, &panel).is_ok()
+            let shown = self.restore_from_hidden(&mut layout, &panel).is_ok()
                 || default.layout.location(&panel).is_ok_and(|location| layout.restore(panel.clone(), &location).is_ok())
                 || layout.open_at_edge(panel.clone()).is_ok();
             if shown {
@@ -268,3 +293,26 @@ fn close_locations<P: Clone + Eq + Hash>(layout: &Layout<P>, closed: &[P]) -> Ve
     locations.reverse();
     locations
 }
+
+/// The location of a hidden panel beside `panel`, whose `location` anchors on it: the same
+/// relation from the other side. `None` for an outer-edge frame, which only sat beside it.
+fn reversed<P: Clone + Eq>(hidden: &Location<P>, location: &Location<P>, panel: &P) -> Option<Location<P>> {
+    let next = match &hidden.placement {
+        Placement::Tab { before } => before.clone(),
+        _ => None,
+    };
+    let (placement, size) = match &location.placement {
+        // `before` names the anchor only when `panel` came first and the anchor second; the
+        // anchor's own `before` then still names the tab after it.
+        Placement::Tab { before } if before == &location.anchor => (Placement::Tab { before: next }, None),
+        Placement::Tab { .. } | Placement::Split(Zone::Center) => (Placement::Tab { before: Some(panel.clone()) }, None),
+        Placement::Split(Zone::Left) => (Placement::Split(Zone::Right), location.size),
+        Placement::Split(Zone::Right) => (Placement::Split(Zone::Left), location.size),
+        Placement::Split(Zone::Top) => (Placement::Split(Zone::Bottom), location.size),
+        Placement::Split(Zone::Bottom) => (Placement::Split(Zone::Top), location.size),
+        Placement::Group { below, .. } => (Placement::Group { below: !below, height: None }, None),
+        Placement::Column { after } => (Placement::Column { after: !after }, None),
+        Placement::Edge(_) => return None,
+    };
+    Some(Location { floating: None, anchor: Some(panel.clone()), placement, size, sibling: Vec::new(), ..hidden.clone() })
+}

docs/ui-docking.md

@@ -113,7 +113,7 @@ an optional saved arrangement for Reset, and `locked`.
 | Method | Does |
 |---|---|
 | `apply`, `apply_all` | Apply actions all or none, with the app's permissions. Closed panels are recorded in `hidden`. While `locked`, moving, floating and closing are refused; `SetNative`, which records the OS windows, still applies. |
-| `set_visible(panel, visible, fallback, permissions)` | Hide a panel that may close, or show it at its recorded place, else as `Open` beside `fallback`, or where `Open` without an anchor puts it. |
+| `set_visible(panel, visible, fallback, permissions)` | Hide a panel that may close, or show it at its recorded place, else as `Open` beside `fallback`, or where `Open` without an anchor puts it. A group or window closed one panel at a time comes back as one group as its panels are shown again. |
 | `reconcile(registered, permissions, default)` | Fit a loaded workspace to the panels the app has now. Unknown panels move to `hidden`, so they come back when registered again. A registered panel that cannot close, such as the canvas, is always shown. |
 | `reset(default)` | Return to the saved arrangement, or to `default` for a built-in workspace. |
 | `validate` | Check a loaded workspace before use. |