diff --git a/crates/ui-egui/src/canvas/overlays.rs b/crates/ui-egui/src/canvas/overlays.rs index da5b68f2f..deb284959 100644 --- a/crates/ui-egui/src/canvas/overlays.rs +++ b/crates/ui-egui/src/canvas/overlays.rs @@ -763,7 +763,7 @@ pub(super) fn accent(p: &egui::Painter) -> Color32 { /// Pick select: the pink outline on the layer the keys walked to (#1836). fn pick_outline(app: &FigApp, painter: &egui::Painter, doc: &Document) { - if let Some(p) = app.ui.pick.filter(|p| doc.node(*p).is_some()) { + if let Some(p) = app.picked().filter(|p| doc.node(*p).is_some()) { draw_outline(app, painter, doc, p, stroke(crate::handles::PINK, 2.0)); } } diff --git a/crates/ui-egui/src/layers_keyboard_tests.rs b/crates/ui-egui/src/layers_keyboard_tests.rs index adbb9a510..5a9f78597 100644 --- a/crates/ui-egui/src/layers_keyboard_tests.rs +++ b/crates/ui-egui/src/layers_keyboard_tests.rs @@ -239,44 +239,61 @@ fn tab_switches_the_selection_box_to_pick_select() { press(&mut u, Key::F8, Modifiers::NONE); press(&mut u, Key::Tab, Modifiers::NONE); assert!(u.app.ui.key_box.is_none(), "the box becomes the pick cursor"); - assert_eq!(u.app.ui.pick, Some(a), "on the selected layer"); + assert_eq!(u.app.picked(), Some(a), "on the selected layer"); assert_eq!(u.app.session.selection(), vec![a], "nothing selected yet"); // Tab walks the siblings as the Layers panel lists them (the frame on top, then B, then A), // wrapping; Shift+Tab goes the other way. press(&mut u, Key::Tab, Modifiers::NONE); - assert_eq!(u.app.ui.pick, Some(f), "below A comes the top of the list"); + assert_eq!(u.app.picked(), Some(f), "below A comes the top of the list"); press(&mut u, Key::Tab, Modifiers::SHIFT); - assert_eq!(u.app.ui.pick, Some(a)); + assert_eq!(u.app.picked(), Some(a)); press(&mut u, Key::Tab, Modifiers::SHIFT); - assert_eq!(u.app.ui.pick, Some(b)); + assert_eq!(u.app.picked(), Some(b)); // Enter goes into a frame's top child, Shift+Enter back out; the page is never picked. press(&mut u, Key::Tab, Modifiers::SHIFT); - assert_eq!(u.app.ui.pick, Some(f)); + assert_eq!(u.app.picked(), Some(f)); press(&mut u, Key::Enter, Modifiers::NONE); - assert_eq!(u.app.ui.pick, Some(c)); + assert_eq!(u.app.picked(), Some(c)); press(&mut u, Key::Enter, Modifiers::SHIFT); press(&mut u, Key::Enter, Modifiers::SHIFT); - assert_eq!(u.app.ui.pick, Some(f)); + assert_eq!(u.app.picked(), Some(f)); assert_eq!(u.app.session.selection(), vec![a], "walking selects nothing"); // Space selects the picked layer and ends pick select. press(&mut u, Key::Space, Modifiers::NONE); assert_eq!(u.app.session.selection(), vec![f]); - assert!(u.app.ui.pick.is_none()); + assert!(u.app.picked().is_none()); // The chord again while the box shows switches too; Escape ends it, the selection kept. press(&mut u, Key::F8, Modifiers::NONE); press(&mut u, Key::F8, Modifiers::NONE); assert!(u.app.ui.key_box.is_none()); - assert_eq!(u.app.ui.pick, Some(f)); + assert_eq!(u.app.picked(), Some(f)); press(&mut u, Key::Escape, Modifiers::NONE); - assert!(u.app.ui.pick.is_none()); + assert!(u.app.picked().is_none()); assert_eq!(u.app.session.selection(), vec![f]); // Nothing selected: the pick starts on the page's top layer; a press on the canvas ends it. u.app.run("select.none", json!({})); press(&mut u, Key::F8, Modifiers::NONE); press(&mut u, Key::Tab, Modifiers::NONE); - assert_eq!(u.app.ui.pick, Some(f)); + assert_eq!(u.app.picked(), Some(f)); let p = u.app.canvas.rect.center(); u.button(p, true); u.button(p, false); + assert!(u.app.picked().is_none()); + // Picked in one document, the cursor stays behind when another comes to the front, even where + // that one has a layer with the same id (each document numbers its layers from 1): Space + // there selects nothing, and the stale pick is dropped. + u.app.run("select.set", json!({"ids": [a]})); + press(&mut u, Key::F8, Modifiers::NONE); + press(&mut u, Key::Tab, Modifiers::NONE); + assert_eq!(u.app.picked(), Some(a)); + u.app.run("file.new", json!({})); + for _ in 0..3 { + u.app.run("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10})); + } + u.app.run("select.none", json!({})); + assert!(u.app.session.doc().unwrap().node(a).is_some(), "the new document has a layer with the picked id"); + assert_eq!(u.app.picked(), None); + press(&mut u, Key::Space, Modifiers::NONE); + assert!(u.app.session.selection().is_empty(), "nothing selected in the new document"); assert!(u.app.ui.pick.is_none()); } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 9857a3320..6207b59dd 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -482,8 +482,9 @@ pub struct UiState { /// marquee's start at `x0, y0`. pub key_box: Option<[f64; 4]>, /// Pick select (#1836): the layer the pink outline is on while the keys walk the layer tree - /// (Tab or the selection-box chord again while the box shows; Space selects it). - pub pick: Option, + /// (Tab or the selection-box chord again while the box shows; Space selects it), with the uid + /// of its document: layer ids are only unique within a document ([`FigApp::picked`]). + pub pick: Option<(u64, NodeId)>, /// Layers panel search: open, the filter text, the shift-click range anchor and the selection /// the tree last revealed (ancestors expand and the row scrolls into view when it changes). pub layer_search: bool, diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index dce0861d2..2f0826e74 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -576,12 +576,17 @@ fn pick_sibling(doc: &figcraft_doc::Document, id: figcraft_doc::NodeId, dir: i64 /// move between parent and child layers. Press Space to make you selection. Press Esc or click /// anywhere on the canvas to exit selection mode.", Accessibility at Figma, #1836). pub(crate) fn key_pick(app: &mut FigApp, key: Key, m: Modifiers) -> bool { - let Some(at) = app.ui.pick else { return false }; - let Ok(doc) = app.session.doc() else { return false }; - let Some(node) = doc.node(at) else { + if app.ui.pick.is_none() { + return false; + } + // Picked in another document (one switched away from or closed since) or deleted: gone. + let Some(at) = app.picked() else { app.ui.pick = None; return false; }; + let Ok(uid) = app.session.ds().map(|d| d.uid) else { return false }; + let Ok(doc) = app.session.doc() else { return false }; + let Some(node) = doc.node(at) else { return false }; let plain = !m.command && !m.alt && !m.ctrl; let next = match key { Key::Tab if plain => pick_sibling(doc, at, if m.shift { -1 } else { 1 }), @@ -599,12 +604,20 @@ pub(crate) fn key_pick(app: &mut FigApp, key: Key, m: Modifiers) -> bool { _ => return false, }; if let Some(n) = next { - app.ui.pick = Some(n); + app.ui.pick = Some((uid, n)); } true } impl FigApp { + /// The layer the pick cursor is on, while its document is the one in front and the layer is + /// still there (#1836). + pub(crate) fn picked(&self) -> Option { + let (uid, id) = self.ui.pick?; + let ds = self.session.ds().ok()?; + (ds.uid == uid && ds.doc.node(id).is_some()).then_some(id) + } + /// Pick select: the pink outline on the first selected layer, or on the page's top layer when /// nothing is selected ("Once activated, a pink outline is placed on a selected objects on the /// canvas, or any node if nothing is selected", Accessibility at Figma); the keys walk the @@ -616,7 +629,7 @@ impl FigApp { self.toast(crate::i18n::tr("Nothing to pick: the page is empty")); return; }; - self.ui.pick = Some(at); + self.ui.pick = Some((ds.uid, at)); self.toast(crate::i18n::tr( "Pick select: Tab and Shift+Tab move between layers, Enter goes into one, Shift+Enter out, Space selects, Escape cancels", ));