Fix #2995: address PR review on tab reorder and dock drag begin

Reuse catalogued window.panel.move errors, keep legacy dock drags from clearing docking on Escape, reorder floating tabs locally without reconcile re-entry, and fix dock_tab_order test setup.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
nrq6993-a11y
2026-10-21 23:38:02 +07:00
committed by bflatastic
co-authored by Cursor
parent ba2a5bfaed
commit 4cba355f71
3 changed files with 44 additions and 24 deletions
+7 -4
View File
@@ -858,12 +858,15 @@ mod tests {
#[test]
fn partial_dock_tab_order_appends_tabs_missing_from_preferences() {
let mut ui = crate::state::UiState::default();
ui.dock_tab_order = vec![DockTab::Layers];
let ui = crate::state::UiState {
dock_tab: DockTab::Layers,
dock_tab_order: vec![DockTab::Layers, DockTab::Properties],
..Default::default()
};
let order = floating::resolved_dock_tab_order(&ui);
assert_eq!(order.len(), DockTab::ALL.len());
assert_eq!(order[0], DockTab::Layers);
assert!(order.contains(&DockTab::Properties) && order.contains(&DockTab::Libraries));
assert_eq!(order[..2], [DockTab::Layers, DockTab::Properties]);
assert_eq!(order[2], DockTab::Libraries);
}
#[test]
+10 -10
View File
@@ -647,6 +647,7 @@ fn apply(app: &mut VectorcraftApp, ctx: &egui::Context, action: Option<Action>)
return;
}
let before = app.ui.clone();
let skip_reconcile = matches!(action, Some(Action::Reorder(_, _, _)));
match action {
Some(Action::Activate(gi, k)) => {
if let Some(g) = app.ui.floating_panels.get_mut(gi)
@@ -678,16 +679,15 @@ fn apply(app: &mut VectorcraftApp, ctx: &egui::Context, action: Option<Action>)
Some(Action::Detach(gi, left)) => detach(app, ctx, gi, left),
Some(Action::Tear(id, left)) => tear(app, ctx, &[id], id, strip_grab(ctx, left)),
Some(Action::Reorder(gi, from, to)) => {
let ids = app.ui.floating_panels.get(gi).and_then(|g| {
if from == to { None } else { Some((g.panels.get(from).cloned(), g.panels.get(to).cloned())) }
});
if let Some((Some(from_id), Some(to_id))) = ids
&& let Err(error) = app.run(
"window.panel.move",
json!({ "panel": from_id, "anchor": to_id, "before": to_id }),
)
if let Some(g) = app.ui.floating_panels.get_mut(gi)
&& from != to
&& from < g.panels.len()
&& to < g.panels.len()
{
app.ui.status = error;
let active = g.panels.get(g.active).cloned();
let moved = g.panels.remove(from);
g.panels.insert(to, moved);
g.active = active.and_then(|id| g.panels.iter().position(|p| *p == id)).unwrap_or(0);
}
}
Some(Action::Undock(gi, grab)) => {
@@ -705,7 +705,7 @@ fn apply(app: &mut VectorcraftApp, ctx: &egui::Context, action: Option<Action>)
}
None => {}
}
if let Err(error) = crate::panel_docking::reconcile_native(app, before.clone()) {
if !skip_reconcile && let Err(error) = crate::panel_docking::reconcile_native(app, before.clone()) {
app.ui.status = error;
}
ctx.data_mut(|data| {
+27 -10
View File
@@ -146,14 +146,15 @@ fn number(params: &Value, key: &str, default: f32) -> Result<f32, String> {
/// group without promoting to a shared layout.
fn legacy_same_strip_reorder(app: &mut VectorcraftApp, panel: &str, before: &str) -> Result<Value, String> {
use crate::state::DockTab;
const BAD_DEST: &str = "move requires an existing destination panel";
if crate::state::DockTab::from_id(panel).is_some()
&& DockTab::from_id(before).is_some()
&& crate::floating::group_of(&app.ui, panel).is_none()
&& crate::floating::group_of(&app.ui, before).is_none()
{
let mut order: Vec<DockTab> = crate::floating::docked_tabs(&app.ui).collect();
let from = order.iter().position(|t| t.info().0 == panel).ok_or("panel is not in the dock strip")?;
let to = order.iter().position(|t| t.info().0 == before).ok_or("before is not in the dock strip")?;
let from = order.iter().position(|t| t.info().0 == panel).ok_or(BAD_DEST)?;
let to = order.iter().position(|t| t.info().0 == before).ok_or(BAD_DEST)?;
if from != to {
let item = order.remove(from);
order.insert(to, item);
@@ -161,13 +162,13 @@ fn legacy_same_strip_reorder(app: &mut VectorcraftApp, panel: &str, before: &str
}
return Ok(json!({ "panel": panel, "before": before }));
}
let gi = crate::floating::group_of(&app.ui, panel).ok_or("panel is not in a tab group")?;
let gi = crate::floating::group_of(&app.ui, panel).ok_or(BAD_DEST)?;
if crate::floating::group_of(&app.ui, before) != Some(gi) {
return Err("before must name a tab in the same group".into());
return Err(BAD_DEST.into());
}
let g = app.ui.floating_panels.get_mut(gi).ok_or("panel group missing")?;
let from = g.panels.iter().position(|id| id == panel).ok_or("panel is not in the group")?;
let to = g.panels.iter().position(|id| id == before).ok_or("before is not in the group")?;
let g = app.ui.floating_panels.get_mut(gi).ok_or("invalid restored panel group")?;
let from = g.panels.iter().position(|id| id == panel).ok_or(BAD_DEST)?;
let to = g.panels.iter().position(|id| id == before).ok_or(BAD_DEST)?;
if from != to {
let active = g.panels.get(g.active).cloned();
let moved = g.panels.remove(from);
@@ -329,9 +330,25 @@ fn apply_action(app: &mut VectorcraftApp, action: Action<String>) -> Result<Valu
fn change(app: &mut VectorcraftApp, operation: &str, params: &Value) -> Result<Value, String> {
let panel = params.get("panel").and_then(Value::as_str).and_then(normalize).ok_or("panel must name a registered panel")?;
if app.ui.docking.is_none() && operation == "move" && params.get("before").is_some() {
if operation == "move" && params.get("before").is_some() {
let before = params.get("before").and_then(Value::as_str).and_then(normalize).ok_or("before must name a panel")?;
return legacy_same_strip_reorder(app, panel, before);
let in_group = crate::floating::group_of(&app.ui, panel).is_some();
use crate::state::DockTab;
let dock_strip_reorder = DockTab::from_id(panel).is_some()
&& DockTab::from_id(before).is_some()
&& crate::floating::group_of(&app.ui, panel).is_none()
&& crate::floating::group_of(&app.ui, before).is_none();
if app.ui.docking.is_none() || in_group || dock_strip_reorder {
let mirror_native_tabs = crate::floating::group_of(&app.ui, panel)
.and_then(|i| app.ui.floating_panels.get(i))
.is_some_and(decorated);
let snapshot = app.ui.clone();
let result = legacy_same_strip_reorder(app, panel, before);
if mirror_native_tabs && app.ui.docking.is_some() && result.is_ok() {
reconcile_native(app, snapshot)?;
}
return result;
}
}
if app.ui.docking.as_ref().is_some_and(|layout| !valid(layout)) {
return Err("saved panel layout is invalid; reset the workspace first".into());
@@ -935,7 +952,7 @@ pub(crate) fn legacy_tab_in_strip(
let area = egui::Id::new("vectorcraft-panel-docking");
let begun = ui.ctx().data(|d| d.get_temp::<LegacyDrag>(area.with("legacy-origin")).is_some());
let begin = if strip.is_some() {
!inside_strip && !begun && (response.dragged() || response.drag_started())
!begun && (response.drag_started() || (!inside_strip && response.dragged()))
} else {
response.drag_started()
};