From bc4addb8d49fbe626c1c1d46d17355a405b6bb4c Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 20 Oct 2026 15:21:51 +0000 Subject: [PATCH] docking: opening works when locked, and hiding respects closable OpenAt checked the opened panel's movable, so a locked workspace refused OpenAt while Open worked. It checks movable only for a panel already shown. Workspace::set_visible takes the app's permissions, so hiding cannot close a protected canvas. --- crates/ui/src/docking/command.rs | 4 ++-- crates/ui/src/docking/model.rs | 4 +++- crates/ui/src/docking/model_tests.rs | 26 ++++++++++++++++++++------ crates/ui/src/docking/tests.rs | 2 +- crates/ui/src/docking/workspace.rs | 7 ++++--- docs/ui-docking.md | 2 +- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/crates/ui/src/docking/command.rs b/crates/ui/src/docking/command.rs index 93151e6e..2dd7f67a 100644 --- a/crates/ui/src/docking/command.rs +++ b/crates/ui/src/docking/command.rs @@ -517,12 +517,12 @@ impl DockCommand

{ }; if let Some(panel) = show.filter(|p| !workspace.layout.contains(p)) { if self.edge().is_none() { - workspace.set_visible(panel.clone(), true, None)?; + workspace.set_visible(panel.clone(), true, None, &permissions)?; return Ok(Vec::new()); } // Shown, then moved to the edge, all or nothing. let mut shown = workspace.clone(); - shown.set_visible(panel.clone(), true, None)?; + shown.set_visible(panel.clone(), true, None, &permissions)?; let closed = shown.apply_all(self.to_actions(&shown, right_to_left, space)?, permissions)?; *workspace = shown; return Ok(closed); diff --git a/crates/ui/src/docking/model.rs b/crates/ui/src/docking/model.rs index e1fc5541..641e6373 100644 --- a/crates/ui/src/docking/model.rs +++ b/crates/ui/src/docking/model.rs @@ -762,7 +762,9 @@ impl Layout

{ self.validate()?; match &action { Action::Move { panel, anchor, placement } | Action::OpenAt { panel, anchor, placement } => { - if !permissions(panel).movable { + // Opening an absent panel moves nothing, so it works in a locked workspace too. + let opens = matches!(action, Action::OpenAt { .. }) && !self.contains(panel); + if !opens && !permissions(panel).movable { return Err(DockError::NotAllowed); } if !placement_allowed(placement, permissions(anchor)) { diff --git a/crates/ui/src/docking/model_tests.rs b/crates/ui/src/docking/model_tests.rs index 74eab84c..6e70d5ae 100644 --- a/crates/ui/src/docking/model_tests.rs +++ b/crates/ui/src/docking/model_tests.rs @@ -170,8 +170,8 @@ fn close_group_and_close_window_record_a_restorable_location_per_member() { assert_eq!(workspace.apply(Action::CloseGroup { panel: 3 }, allow).unwrap(), [2, 3]); assert_eq!(workspace.hidden.iter().map(|(p, _)| *p).collect::>(), [2, 3]); assert_eq!(panels_of(&dock(&workspace.layout, Side::End)[0].groups), [vec![4]]); - workspace.set_visible(2, true, None).unwrap(); - workspace.set_visible(3, true, None).unwrap(); + workspace.set_visible(2, true, None, |_| Permissions::default()).unwrap(); + workspace.set_visible(3, true, None, |_| Permissions::default()).unwrap(); assert_eq!(panels_of(&dock(&workspace.layout, Side::End)[0].groups), [vec![2, 3], vec![4]]); assert_eq!(workspace.layout.closed_by(&Action::CloseWindow { panel: 7 }).unwrap(), [6, 7, 8]); @@ -179,7 +179,7 @@ fn close_group_and_close_window_record_a_restorable_location_per_member() { assert!(workspace.layout.floating.is_empty()); assert_eq!(workspace.hidden.len(), 3); for panel in [6, 7, 8] { - workspace.set_visible(panel, true, None).unwrap(); + workspace.set_visible(panel, true, None, |_| Permissions::default()).unwrap(); } assert_eq!(workspace.layout.floating.len(), 1); assert_eq!(panels_of(&workspace.layout.floating[0].groups), [vec![6, 7], vec![8]]); @@ -187,7 +187,7 @@ fn close_group_and_close_window_record_a_restorable_location_per_member() { workspace.apply(Action::CloseOthers { panel: 12 }, allow).unwrap(); assert_eq!(group_state(&workspace.layout, 12).0, [12]); - workspace.set_visible(11, true, None).unwrap(); + workspace.set_visible(11, true, None, |_| Permissions::default()).unwrap(); assert_eq!(group_state(&workspace.layout, 12).0, [11, 12]); assert!(workspace.hidden.is_empty()); assert_eq!(sorted(&workspace.layout), ALL); @@ -299,7 +299,7 @@ fn locked_workspace_rejects_structure_but_allows_activate_open_minimize_and_resi assert_eq!(workspace.apply(action.clone(), allow), Err(DockError::NotAllowed), "{action:?}"); assert_eq!(workspace, before); } - assert_eq!(workspace.set_visible(4, false, None), Err(DockError::NotAllowed)); + assert_eq!(workspace.set_visible(4, false, None, |_| Permissions::default()), Err(DockError::NotAllowed)); let allowed = [ Action::Activate { panel: 2 }, Action::Open { panel: 12, anchor: Some(11) }, @@ -317,6 +317,20 @@ fn locked_workspace_rejects_structure_but_allows_activate_open_minimize_and_resi assert_eq!(workspace.layout.panel_state(&5), PanelState::Open); assert_eq!(workspace.layout.panel_state(&2), PanelState::Front); assert_eq!(workspace.layout.panel_state(&3), PanelState::Open); + // Opening at a chosen place works too; moving a shown panel there does not. + workspace.apply(Action::OpenAt { panel: 13, anchor: 11, placement: Placement::Split(Zone::Bottom) }, allow).unwrap(); + assert!(workspace.layout.contains(&13)); + assert_eq!(workspace.apply(Action::OpenAt { panel: 2, anchor: 11, placement: Placement::Split(Zone::Bottom) }, allow), Err(DockError::NotAllowed)); +} + +#[test] +fn hiding_a_panel_that_may_not_close_is_refused() { + let mut workspace = Workspace::new(docked()); + let canvas = |p: &u32| if *p == 1 { Permissions { accepts_splits: true, ..Permissions::PROTECTED } } else { Permissions::default() }; + assert_eq!(workspace.set_visible(1, false, None, canvas), Err(DockError::NotAllowed)); + assert!(workspace.layout.contains(&1) && workspace.hidden.is_empty()); + workspace.set_visible(2, false, None, canvas).unwrap(); + assert!(!workspace.layout.contains(&2)); } #[test] @@ -334,7 +348,7 @@ fn reconcile_drops_unknown_panels_and_places_missing_registered_ones() { // Unregistered panels wait in `hidden` and come back once registered. assert_eq!(workspace.hidden.iter().map(|(p, _)| *p).collect::>(), [98, 99]); assert_eq!(sorted(&workspace.layout), ALL); - workspace.set_visible(99, true, None).unwrap(); + workspace.set_visible(99, true, None, |_| Permissions::default()).unwrap(); assert_eq!(group_state(&workspace.layout, 2).0, [2, 3, 99]); assert_eq!(dock(&workspace.layout, Side::End).len(), 2); assert_eq!(group_state(&workspace.layout, 11).0, [11, 12]); diff --git a/crates/ui/src/docking/tests.rs b/crates/ui/src/docking/tests.rs index 77d197e7..7ed5d392 100644 --- a/crates/ui/src/docking/tests.rs +++ b/crates/ui/src/docking/tests.rs @@ -314,7 +314,7 @@ fn long_action_sequence_never_loses_or_duplicates_a_panel() { { let fallback = workspace.layout.panels().first().map(|p| **p); let before = workspace.clone(); - if workspace.set_visible(hidden, true, fallback).is_err() { + if workspace.set_visible(hidden, true, fallback, |_| Permissions::default()).is_err() { assert_eq!(workspace, before); } } diff --git a/crates/ui/src/docking/workspace.rs b/crates/ui/src/docking/workspace.rs index 92eb4c06..da414e78 100644 --- a/crates/ui/src/docking/workspace.rs +++ b/crates/ui/src/docking/workspace.rs @@ -114,14 +114,15 @@ impl Workspace

{ /// 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 beside `fallback`. - /// A shown panel becomes the active tab of its group. - pub fn set_visible(&mut self, panel: P, visible: bool, fallback: Option

) -> Result<(), DockError> { + /// 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

, permissions: impl Fn(&P) -> Permissions) -> Result<(), DockError> { self.validate()?; if !visible { if !self.layout.contains(&panel) { return Ok(()); } - return self.apply(Action::Close { panel }, |_| Permissions::default()).map(drop); + return self.apply(Action::Close { panel }, permissions).map(drop); } if self.layout.contains(&panel) { return self.apply(Action::Activate { panel }, |_| Permissions::default()).map(drop); diff --git a/docs/ui-docking.md b/docs/ui-docking.md index 2763ab59..40c8b329 100644 --- a/docs/ui-docking.md +++ b/docs/ui-docking.md @@ -88,7 +88,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. | -| `set_visible(panel, visible, fallback)` | Hide a panel, or show it at its recorded place, else beside `fallback`. | +| `set_visible(panel, visible, fallback, permissions)` | Hide a panel that may close, or show it at its recorded place, else beside `fallback`. | | `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. |