From a89b44ee2eec63a403f3e004805c42eda6ea595e Mon Sep 17 00:00:00 2001 From: ruchirguitar <113586074+ruchirguitar@users.noreply.github.com> Date: Thu, 5 Nov 2026 14:31:01 +0530 Subject: [PATCH] Document tabs: views stay with their documents when one batch closes and moves documents (#5663) `sync_views` matched views to documents by uid only while the number of documents was unchanged. A `command.batch` (or a recording) that closed a document fell back to matching by position, so the documents after the closed one took their neighbours' views, and a move later in the same batch was not applied to the views. The views are now matched by uid whenever the documents change: a closed document's view is dropped and a new document gets its saved view. Closing from the UI no longer removes the view by hand. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/app/view.rs | 14 ++++------ crates/ui-egui/src/tests/tab_reorder.rs | 34 +++++++++++++++++++++++++ crates/ui-egui/src/unsaved.rs | 5 +--- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/app/view.rs b/crates/ui-egui/src/app/view.rs index a1c898b71..b016445bd 100644 --- a/crates/ui-egui/src/app/view.rs +++ b/crates/ui-egui/src/app/view.rs @@ -5,17 +5,13 @@ impl VectorcraftApp { /// opens in a floating window while Open Documents As Tabs is cleared). The views Window › New /// Window opened go with their documents. /// - /// A view stays with its document when the documents change order (`document.move`, also in a - /// recording or a batch, #5663): while every view still has the document it was synced with, - /// they are put in the documents' order by uid. Code that removed a view by hand (closing a - /// document) leaves them aligned by index, as before. + /// A view stays with its document when documents change order or close (`document.move`, + /// `file.close`, also several of them in one recording or batch, #5663): the views are matched + /// to the documents by the uid each was synced with, a closed document's view is dropped and a + /// new document gets its saved view. pub fn sync_views(&mut self) { let docs = self.session.documents(); - if self.view_uids.len() == self.views.len() - && docs.len() == self.views.len() - && docs.iter().map(|d| d.uid).ne(self.view_uids.iter().copied()) - && docs.iter().all(|d| self.view_uids.contains(&d.uid)) - { + if self.view_uids.len() == self.views.len() && docs.iter().map(|d| d.uid).ne(self.view_uids.iter().copied()) { let mut old: Vec> = std::mem::take(&mut self.views).into_iter().map(Some).collect(); self.views = docs .iter() diff --git a/crates/ui-egui/src/tests/tab_reorder.rs b/crates/ui-egui/src/tests/tab_reorder.rs index 167addce0..c1070fbc7 100644 --- a/crates/ui-egui/src/tests/tab_reorder.rs +++ b/crates/ui-egui/src/tests/tab_reorder.rs @@ -136,6 +136,40 @@ fn views_follow_a_move_made_by_the_engine_command() { assert_eq!(app.views.iter().map(|v| v.zoom).collect::>(), [4.0, 2.0, 3.0]); } +/// One batch that closes documents and moves others (a script, a recording) leaves every view with +/// its own document; the views are put right once, after the whole batch. +#[test] +fn views_follow_closes_and_moves_made_in_one_batch() { + let (mut app, _ctx) = app(); + let zooms = |app: &VectorcraftApp| app.views.iter().map(|v| v.zoom).collect::>(); + for (k, v) in app.views.iter_mut().enumerate() { + v.zoom = 1.0 + k as f64; + } + // A close alone, in the middle: the documents after it keep their own views. + app.run("command.batch", json!({"commands": [{"command": "file.close", "params": {"index": 1}}]})).unwrap(); + assert_eq!(order(&app), ["Alpha", "Charlie", "Delta"]); + assert_eq!(zooms(&app), [1.0, 3.0, 4.0], "Charlie and Delta keep theirs, not Bravo's and Charlie's"); + // A close and a move together. + app.run( + "command.batch", + json!({"commands": [ + {"command": "file.close", "params": {"index": 0}}, + {"command": "document.move", "params": {"from": 1, "to": 0}}, + ]}), + ) + .unwrap(); + assert_eq!(order(&app), ["Delta", "Charlie"]); + assert_eq!(zooms(&app), [4.0, 3.0], "Delta does not take the closed Alpha's view"); + // Closing by index made the document in the closed one's place current (Charlie); it shows its own view. + assert_eq!((current(&app), app.view().map(|v| v.zoom)), ("Charlie".into(), Some(3.0))); + // A document opened afterwards starts with a view of its own. + app.run("file.new", json!({"width": 100, "height": 100, "name": "Echo"})).unwrap(); + app.sync_views(); + assert_eq!(order(&app), ["Delta", "Charlie", "Echo"]); + assert_eq!(app.views.len(), 3); + assert_eq!(&zooms(&app)[..2], [4.0, 3.0]); +} + #[test] fn a_tab_dragged_off_the_strip_still_floats_and_moves_nothing() { let (mut app, ctx) = app(); diff --git a/crates/ui-egui/src/unsaved.rs b/crates/ui-egui/src/unsaved.rs index 01eafaed9..aa5c107e4 100644 --- a/crates/ui-egui/src/unsaved.rs +++ b/crates/ui-egui/src/unsaved.rs @@ -69,12 +69,9 @@ fn ask(app: &mut VectorcraftApp, i: usize, then: &str) -> Result Ok(json!({ "pending": KIND })) } -/// Close document `i` through the engine command, keeping `views` aligned with the documents. +/// Close document `i` through the engine command; [`VectorcraftApp::sync_views`] drops its view. fn close_now(app: &mut VectorcraftApp, i: usize) -> Result { let r = app.session.execute("file.close", &json!({ "index": i })).map_err(|e| e.to_string())?; - if i < app.views.len() { - app.views.remove(i); - } app.sync_views(); if app.session.tool_id() == "artboard" && let Some(n) = app.session.active().map(|d| d.doc.artboards.len())