Merge pull request #865 from AganzeFelicite/fix/delete-page-key-859

fix(pages): Delete deletes the page picked in the Pages list (#859)

contributors/people.toml

@@ -59,3 +59,6 @@ sync_github_name = true
 
 [people.MortenHusted]
 sync_github_name = true
+
+[people.AganzeFelicite]
+sync_github_name = true

crates/ui-egui/locales/cs.toml

@@ -930,6 +930,7 @@ status = "draft — needs native review"
 "PDF print settings" = "Nastavení tisku PDF"
 "Padding" = "Vnitřní odsazení"
 "Page" = "Stránka"
+"Page deleted" = "Stránka smazána"
 "Pages" = "Stránky"
 "Pages added to the document" = "Stránky přidány do dokumentu"
 "Placed on the current page" = "Umístěno na aktuální stránku"

crates/ui-egui/locales/de.toml

@@ -899,6 +899,7 @@ status = "draft — needs native review"
 "PDF print settings" = "PDF-Druckeinstellungen"
 "Padding" = "Innenabstand"
 "Page" = "Seite"
+"Page deleted" = "Seite gelöscht"
 "Pages" = "Seiten"
 "Pages added to the document" = "Seiten zum Dokument hinzugefügt"
 "Placed on the current page" = "Auf der aktuellen Seite platziert"

crates/ui-egui/locales/en.toml

@@ -951,6 +951,7 @@ name = "English"
 "PDF print settings" = "PDF print settings"
 "Padding" = "Padding"
 "Page" = "Page"
+"Page deleted" = "Page deleted"
 "Pages" = "Pages"
 "Pages added to the document" = "Pages added to the document"
 "Placed on the current page" = "Placed on the current page"

crates/ui-egui/locales/es.toml

@@ -952,6 +952,7 @@ status = "draft — needs native review"
 "PDF print settings" = "Ajustes de impresión PDF"
 "Padding" = "Margen interno"
 "Page" = "Página"
+"Page deleted" = "Página eliminada"
 "Pages" = "Páginas"
 "Pages added to the document" = "Páginas añadidas al documento"
 "Placed on the current page" = "Colocado en la página actual"

crates/ui-egui/locales/fi.toml

@@ -953,6 +953,7 @@ status = "draft — needs native review"
 "PDF print settings" = "PDF-tulostusasetukset"
 "Padding" = "Sisämarginaali"
 "Page" = "Sivu"
+"Page deleted" = "Sivu poistettu"
 "Pages" = "Sivut"
 "Pages added to the document" = "Sivut lisätty asiakirjaan"
 "Pages: {pages}" = "Sivut: {pages}"

crates/ui-egui/locales/fr.toml

@@ -948,6 +948,7 @@ status = "draft — needs native review"
 "PDF print settings" = "Réglages d’impression PDF"
 "Padding" = "Marge intérieure"
 "Page" = "Page"
+"Page deleted" = "Page supprimée"
 "Pages" = "Pages"
 "Pages added to the document" = "Pages ajoutées au document"
 "Pick the Frame tool, then drag on the canvas or choose a device size" = "Choisissez l’outil Cadre, puis faites glisser sur le canevas ou choisissez une taille d’appareil"

crates/ui-egui/locales/id.toml

@@ -934,6 +934,7 @@ status = "draft — needs native review"
 "PDF print settings" = "Pengaturan cetak PDF"
 "Padding" = "Padding"
 "Page" = "Halaman"
+"Page deleted" = "Halaman dihapus"
 "Pages" = "Halaman"
 "Pages added to the document" = "Halaman ditambahkan ke dokumen"
 "Placed on the current page" = "Ditempatkan di halaman saat ini"

crates/ui-egui/locales/ja.toml

@@ -939,6 +939,7 @@ status = "draft — needs native review"
 "PDF print settings" = "PDF 印刷設定"
 "Padding" = "パディング"
 "Page" = "ページ"
+"Page deleted" = "ページは削除済み"
 "Pages" = "ページ"
 "Pages added to the document" = "ページをドキュメントに追加した"
 "Placed on the current page" = "現在のページに配置した"

crates/ui-egui/locales/ko.toml

@@ -893,6 +893,7 @@ status = "draft — needs native review"
 "PDF print settings" = "PDF 인쇄 설정"
 "Padding" = "패딩"
 "Page" = "페이지"
+"Page deleted" = "페이지가 삭제됨"
 "Pages" = "페이지"
 "Pages added to the document" = "문서에 페이지 추가됨"
 "Placed on the current page" = "현재 페이지에 배치됨"

crates/ui-egui/locales/nl.toml

@@ -953,6 +953,7 @@ status = "draft — needs native review"
 "PDF print settings" = "PDF-afdrukinstellingen"
 "Padding" = "Padding"
 "Page" = "Pagina"
+"Page deleted" = "Pagina verwijderd"
 "Pages" = "Pagina's"
 "Pages added to the document" = "Pagina's toegevoegd aan het document"
 "Pages: {pages}" = "Pagina's: {pages}"

crates/ui-egui/locales/pt-BR.toml

@@ -892,6 +892,7 @@ status = "draft — needs native review"
 "PDF print settings" = "Configurações de impressão em PDF"
 "Padding" = "Espaçamento interno"
 "Page" = "Página"
+"Page deleted" = "Página excluída"
 "Pages" = "Páginas"
 "Pages added to the document" = "Páginas adicionadas ao documento"
 "Placed on the current page" = "Colocado na página atual"

crates/ui-egui/locales/ru.toml

@@ -933,6 +933,7 @@ name = "Русский"
 "PDF print settings" = "Настройки печати PDF"
 "Padding" = "Внутренние отступы"
 "Page" = "Страница"
+"Page deleted" = "Страница удалена"
 "Pages" = "Страницы"
 "Pages added to the document" = "Страницы добавлены в документ"
 "Placed on the current page" = "Размещено на текущей странице"

crates/ui-egui/locales/sv.toml

@@ -943,6 +943,7 @@ name = "Svenska"
 "PDF print settings" = "PDF-utskriftsinställningar"
 "Padding" = "Utfyllnad"
 "Page" = "Sida"
+"Page deleted" = "Sidan har raderats"
 "Pages" = "Sidor"
 "Pages added to the document" = "Sidor tillagda i dokumentet"
 "Placed on the current page" = "Placerat på den aktuella sidan"

crates/ui-egui/locales/tr.toml

@@ -934,6 +934,7 @@ name = "Türkçe"
 "PDF print settings" = "PDF baskı ayarları"
 "Padding" = "İç boşluk"
 "Page" = "Sayfa"
+"Page deleted" = "Sayfa silindi"
 "Pages" = "Sayfalar"
 "Pages added to the document" = "Sayfalar belgeye eklendi"
 "Placed on the current page" = "Geçerli sayfaya yerleştirildi"

crates/ui-egui/locales/zh-Hans.toml

@@ -894,6 +894,7 @@ status = "draft — needs native review"
 "PDF print settings" = "PDF 打印设置"
 "Padding" = "内边距"
 "Page" = "页面"
+"Page deleted" = "页面已删除"
 "Pages" = "页面"
 "Pages added to the document" = "页面已添加到文档"
 "Placed on the current page" = "已放置在当前页面"

crates/ui-egui/src/canvas.rs

@@ -491,6 +491,8 @@ fn zoom_tool(app: &mut FigApp, ui: &Ui, resp: &egui::Response, pos: Pos2, mods:
 /// then doesn't show.
 fn input(app: &mut FigApp, ui: &mut Ui, resp: &egui::Response) -> bool {
     if crate::proto_canvas::input(app, ui, resp) {
+        // A prototype gesture is a press on the canvas too: the Pages list gives up the keyboard.
+        app.ui.pages_focus = None;
         return true;
     }
     let ctx = ui.ctx().clone();
@@ -541,6 +543,7 @@ fn input(app: &mut FigApp, ui: &mut Ui, resp: &egui::Response) -> bool {
     // A press on the canvas gives the keyboard back to it (arrows nudge again).
     if resp.hovered() && ctx.input(|i| i.pointer.any_pressed()) {
         app.ui.layers_focus = false;
+        app.ui.pages_focus = None;
         app.ui.key_box = None;
     }
     // Zoom (pinch, Cmd/Ctrl+scroll) about the pointer; plain scroll pans.

crates/ui-egui/src/layers.rs

@@ -199,6 +199,7 @@ fn pages(app: &mut FigApp, ui: &mut Ui) {
     }
     icons::paint(ui.painter(), ar.shrink(5.0), Icon::Plus, t.icon);
     if aresp.clicked() {
+        app.ui.pages_focus = app.session.ds().ok().map(|d| d.uid);
         app.run("page.add", json!({}));
     }
     if !app.ui.pages_open {
@@ -208,6 +209,7 @@ fn pages(app: &mut FigApp, ui: &mut Ui) {
     let pages: Vec<(NodeId, String, bool)> =
         ds.doc.pages.iter().filter_map(|p| ds.doc.node(*p).map(|n| (*p, n.name.clone(), ds.doc.is_divider(*p)))).collect();
     let cur = ds.page;
+    let uid = ds.uid;
     let sel = ds.selection.clone();
     let mut select = None;
     let mut rename_done: Option<(NodeId, String)> = None;
@@ -257,6 +259,8 @@ fn pages(app: &mut FigApp, ui: &mut Ui) {
             }
             if resp.clicked() && !divider {
                 select = Some(id);
+                app.ui.pages_focus = Some(uid);
+                app.ui.layers_focus = false;
             }
             if resp.double_clicked() {
                 app.ui.rename_page = Some((id, name.clone()));
@@ -274,7 +278,7 @@ fn pages(app: &mut FigApp, ui: &mut Ui) {
                     ui.close();
                 }
                 if ui.button(tr("Delete page")).clicked() {
-                    app.run("page.delete", json!({"id": id}));
+                    delete_page(app, id);
                     ui.close();
                 }
             });
@@ -311,6 +315,20 @@ fn pages(app: &mut FigApp, ui: &mut Ui) {
     pages_edge(app, ui, moved.inner_rect.height().min(moved.content_size.y));
 }
 
+/// The Pages list has the keyboard: a page row (or the add-page button) of the open file was
+/// clicked last, and the list is still on screen.
+pub(crate) fn pages_have_keyboard(app: &FigApp) -> bool {
+    let Ok(ds) = app.session.ds() else { return false };
+    app.ui.pages_focus == Some(ds.uid) && app.ui.pages_open && !app.ui.left_collapsed && !ds.view.ui_hidden && app.ui.left_tab == LeftTab::File
+}
+
+/// Delete `page` (no confirmation: Undo brings it back) and say so.
+pub(crate) fn delete_page(app: &mut FigApp, page: NodeId) {
+    if app.run("page.delete", json!({"id": page})).is_some() {
+        app.toast(tr("Page deleted"));
+    }
+}
+
 /// The pages list's height in a panel with `avail` below the Pages header: the height asked for
 /// (by default 40% of it), at least three rows, and leaving the layers their header and three rows.
 pub(crate) fn pages_list_height(asked: Option<f32>, avail: f32) -> f32 {
@@ -1064,6 +1082,7 @@ fn layers(app: &mut FigApp, ui: &mut Ui) {
     }
     if let Some((id, m)) = clicked {
         app.ui.layers_focus = true;
+        app.ui.pages_focus = None;
         let order: Vec<NodeId> = rows.iter().map(|r| r.id).collect();
         let pos = |x: NodeId| order.iter().position(|o| *o == x);
         let anchor = app.ui.layer_anchor.filter(|a| sel.contains(a)).and_then(pos);

crates/ui-egui/src/layers_keys_tests.rs

@@ -1,8 +1,8 @@
 //! Arrow keys in the layers panel once a row has been clicked: up/down through the rows (Shift
 //! extends), left/right to collapse, expand, and go to the parent or first child; a press on the
-//! canvas gives arrows back to nudging.
+//! canvas gives arrows back to nudging. Delete after a click in the Pages list deletes the page.
 
-use egui::{Event, Key, Modifiers, pos2};
+use egui::{Event, Key, Modifiers, Pos2, pos2, vec2};
 use figcraft_doc::NodeId;
 use figcraft_engine::Session;
 use serde_json::json;
@@ -107,3 +107,125 @@ fn a_canvas_press_gives_arrows_back_to_nudging() {
     let doc = u.app.session.doc().unwrap();
     assert_eq!(doc.node(c).unwrap().transform.translation().x, 201.0);
 }
+
+/// The middle of `page`'s row in the Pages list, found by clicking down the list until it opens;
+/// then back to `back`.
+fn page_row(u: &mut Ui, page: NodeId, back: NodeId) -> Pos2 {
+    for y in (90..400).step_by(4) {
+        let p = pos2(150.0, y as f32);
+        u.button(p, true);
+        u.button(p, false);
+        if u.app.session.page().unwrap() == page {
+            u.app.session.execute("page.select", &json!({"id": back})).unwrap();
+            u.frame(vec![]);
+            // Rows are 32 high: aim at the middle, past the hit tolerance at its edge.
+            return p + vec2(0.0, 12.0);
+        }
+    }
+    panic!("no row for page {page:?}");
+}
+
+fn pages(u: &Ui) -> usize {
+    u.app.session.doc().unwrap().pages.len()
+}
+
+#[test]
+fn delete_after_clicking_a_page_deletes_it_without_asking() {
+    // #859: Delete on a page picked in the Pages list said "nothing is selected".
+    let (mut u, ..) = setup();
+    let first = u.app.session.page().unwrap();
+    let second = id(u.app.session.execute("page.add", &json!({})).unwrap());
+    u.app.session.execute("page.select", &json!({"id": first})).unwrap();
+    u.frame(vec![]);
+    let row = page_row(&mut u, second, first);
+    u.button(row, true);
+    u.button(row, false);
+    assert_eq!(u.app.session.page().unwrap(), second);
+    key(&mut u, Key::Delete, false);
+    // Gone at once, no confirmation: a toast says so and Undo brings it back.
+    assert_eq!(pages(&u), 1);
+    assert!(!u.app.session.doc().unwrap().pages.contains(&second));
+    assert_eq!(u.app.ui.toast.as_ref().map(|t| t.0.as_str()), Some("Page deleted"));
+    u.app.run("edit.undo", json!({}));
+    assert_eq!(pages(&u), 2);
+    // Backspace (the Mac's delete key) does the same.
+    let row = page_row(&mut u, second, first);
+    u.button(row, true);
+    u.button(row, false);
+    key(&mut u, Key::Backspace, false);
+    assert_eq!(pages(&u), 1);
+    // The last page stays.
+    key(&mut u, Key::Delete, false);
+    assert_eq!(pages(&u), 1);
+}
+
+#[test]
+fn selected_layers_or_a_canvas_press_take_delete_back_from_the_pages_list() {
+    let (mut u, a, ..) = setup();
+    let first = u.app.session.page().unwrap();
+    let second = id(u.app.session.execute("page.add", &json!({})).unwrap());
+    u.frame(vec![]);
+    let row = page_row(&mut u, first, second);
+    u.button(row, true);
+    u.button(row, false);
+    assert_eq!(u.app.session.page().unwrap(), first);
+    assert!(u.app.ui.pages_focus.is_some());
+    // Layers selected after the click (Select all, say): Delete deletes them, not the page.
+    u.app.run("select.set", json!({"ids": [a]}));
+    key(&mut u, Key::Delete, false);
+    assert_eq!(pages(&u), 2);
+    assert!(u.app.session.doc().unwrap().node(a).is_none());
+    // Nothing is selected now, but the list gave up the keyboard: a second Delete keeps the page.
+    assert!(u.app.ui.pages_focus.is_none());
+    key(&mut u, Key::Delete, false);
+    assert_eq!(pages(&u), 2, "a second Delete keeps the page");
+    // A press on the canvas gives Delete back to it.
+    u.button(row, true);
+    u.button(row, false);
+    assert!(u.app.ui.pages_focus.is_some());
+    let p = pos2(900.0, 700.0);
+    u.button(p, true);
+    u.button(p, false);
+    assert!(u.app.ui.pages_focus.is_none());
+    u.app.run("select.none", json!({}));
+    key(&mut u, Key::Delete, false);
+    assert_eq!(pages(&u), 2, "Delete with nothing selected on the canvas deletes no page");
+}
+
+#[test]
+fn delete_deletes_no_page_once_the_pages_list_is_off_screen_or_another_file_is_open() {
+    let (mut u, ..) = setup();
+    let first = u.app.session.page().unwrap();
+    let second = id(u.app.session.execute("page.add", &json!({})).unwrap());
+    u.app.session.execute("page.select", &json!({"id": first})).unwrap();
+    u.frame(vec![]);
+    let row = page_row(&mut u, second, first);
+    let click = |u: &mut Ui| {
+        u.button(row, true);
+        u.button(row, false);
+        assert!(u.app.ui.pages_focus.is_some());
+    };
+    // The Assets tab, a collapsed Pages list, a collapsed left panel.
+    let hide: [fn(&mut Ui, bool); 3] = [
+        |u, on| u.app.ui.left_tab = if on { crate::LeftTab::Assets } else { crate::LeftTab::File },
+        |u, on| u.app.ui.pages_open = !on,
+        |u, on| u.app.ui.left_collapsed = on,
+    ];
+    for (k, hide) in hide.iter().enumerate() {
+        click(&mut u);
+        hide(&mut u, true);
+        u.frame(vec![]);
+        key(&mut u, Key::Delete, false);
+        assert_eq!(pages(&u), 2, "case {k}: the list is off screen");
+        hide(&mut u, false);
+        u.frame(vec![]);
+    }
+    // Another file open: its pages are safe.
+    click(&mut u);
+    u.app.run("file.new", json!({}));
+    u.app.run("page.add", json!({}));
+    u.frame(vec![]);
+    assert_eq!(pages(&u), 2);
+    key(&mut u, Key::Delete, false);
+    assert_eq!(pages(&u), 2, "the other file keeps its pages");
+}

crates/ui-egui/src/lib.rs

@@ -552,6 +552,10 @@ pub struct UiState {
     /// The layers panel has the keyboard: arrow keys move through its rows instead of nudging
     /// (set by a click on a row, cleared by a press on the canvas).
     pub layers_focus: bool,
+    /// The file (its `uid`) whose pages list has the keyboard: Delete with no layers selected deletes
+    /// its current page (set by a click on a page row or the add-page button, cleared by a click on a
+    /// layer row or a press on the canvas).
+    pub pages_focus: Option<u64>,
     pub layers_seen_sel: Vec<NodeId>,
     pub dark: bool,
     pub follow_system: bool,
@@ -686,6 +690,7 @@ impl Default for UiState {
             layer_filter: String::new(),
             layer_anchor: None,
             layers_focus: false,
+            pages_focus: None,
             layers_seen_sel: Vec::new(),
             dark: false,
             follow_system: false,

crates/ui-egui/src/menus.rs

@@ -119,6 +119,19 @@ pub fn invoke(app: &mut FigApp, id: &str) {
         // Edit ▸ Delete (and the Delete binding) with an interaction open in the Prototype tab
         // removes that interaction, not the layer (#767).
         "edit.delete" if app.ui_context.clone().is_some_and(|ctx| crate::proto_panel::delete_selected_interaction(app, &ctx)) => {}
+        // After a click in the Pages list, while the list is on screen and its file open, with no
+        // layers selected, it deletes the current page (#859).
+        "edit.delete" if crate::layers::pages_have_keyboard(app) && app.session.selection().is_empty() => {
+            if let Ok(page) = app.session.page() {
+                crate::layers::delete_page(app, page);
+            }
+        }
+        // Otherwise the list gives up the keyboard: once Delete has taken layers selected since the
+        // click (Select all, say), a second Delete must not take the page.
+        "edit.delete" if app.ui.pages_focus.is_some() => {
+            app.ui.pages_focus = None;
+            app.run(id, json!({}));
+        }
         "file.open" => app.open_dialog(),
         "plugin.import" => crate::plugins_ui::import_dialog(app),
         "file.import" => app.import_dialog(false),

crates/ui-egui/src/proto_canvas.rs

@@ -295,6 +295,26 @@ pub fn input(app: &mut FigApp, ui: &egui::Ui, resp: &egui::Response) -> bool {
 mod tests {
     use super::*;
 
+    /// A press on a connection handle takes the keyboard from the Pages list, as a press anywhere
+    /// else on the canvas does, so a later Delete can't take the page (#859).
+    #[test]
+    fn a_press_on_a_connection_handle_ends_the_pages_list_focus() {
+        let mut s = figcraft_engine::Session::new();
+        let a = s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 200, "height": 150})).unwrap();
+        let a = crate::present_tests::id(&a);
+        s.execute("select.set", &json!({"ids": [a]})).unwrap();
+        let mut u = crate::present_tests::Ui::new(s);
+        u.app.ui.right_tab = RightTab::Prototype;
+        u.run("view.zoomSelection", json!({}));
+        u.frames(2, Vec::new);
+        u.app.ui.pages_focus = u.app.session.ds().ok().map(|d| d.uid);
+        let doc = u.app.session.doc().unwrap().clone();
+        let (_, at) = *handles(&u.app, &doc, &[a]).first().unwrap();
+        u.button(at, true);
+        assert!(u.app.ui.pages_focus.is_none());
+        u.button(at, false);
+    }
+
     #[test]
     fn curves_leave_the_facing_side() {
         let a = Rect::from_min_size(pos2(0.0, 0.0), vec2(50.0, 20.0));

crates/ui-egui/src/proto_editor_tests.rs

@@ -304,3 +304,26 @@ fn escape_closes_prototype_editor_without_deselecting_layer() {
     u.key(pos2(100.0, 100.0), egui::Key::Escape);
     assert!(u.app.session.selection().is_empty(), "selection cleared on third escape");
 }
+
+/// After a click in the Pages list, Delete that removes an interaction ends the list's hold on the
+/// keyboard: with the layer deselected afterwards, Delete doesn't take the page (#859).
+#[test]
+fn deleting_an_interaction_ends_the_pages_list_focus() {
+    let (s, _a, _b, btn) = setup();
+    let mut u = Ui::new(s);
+    let first = u.app.session.page().unwrap();
+    u.run("page.add", json!({}));
+    u.run("page.select", json!({"id": first}));
+    // As after a click on the page's row (layers_keys_tests clicks the real row).
+    u.app.ui.pages_focus = u.app.session.ds().ok().map(|d| d.uid);
+    u.app.ui.right_tab = crate::RightTab::Prototype;
+    u.run("select.set", json!({"ids": [btn]}));
+    crate::proto_panel::open_editor(&u.ctx, Some((btn, 0)));
+    u.frames(1, Vec::new);
+    u.key(pos2(100.0, 100.0), egui::Key::Delete);
+    assert_eq!(u.app.session.doc().unwrap().node(btn).unwrap().interactions.len(), 0);
+    assert!(u.app.ui.pages_focus.is_none());
+    u.run("select.none", json!({}));
+    u.key(pos2(100.0, 100.0), egui::Key::Delete);
+    assert_eq!(u.app.session.doc().unwrap().pages.len(), 2, "the page stays");
+}

crates/ui-egui/src/proto_panel.rs

@@ -53,6 +53,8 @@ pub fn delete_selected_interaction(app: &mut FigApp, ctx: &egui::Context) -> boo
         return false;
     }
     open_editor(ctx, None);
+    // Delete went to the interaction, so a later one doesn't go to the Pages list.
+    app.ui.pages_focus = None;
     let ids = targets_for(app, node);
     app.run("proto.interaction.remove", json!({"ids": ids, "index": index}));
     true

crates/ui-egui/src/selection_spacing.rs

@@ -217,6 +217,7 @@ pub fn input(app: &mut FigApp, ctx: &egui::Context, pos: Option<Pos2>, hovered:
                 let drag =
                     Drag { ids, axis: axis.clone(), start: p, value: h.value, uid: ds.uid, page: ds.page, moved: false, last_value: axis.value };
                 app.ui.layers_focus = false;
+                app.ui.pages_focus = None;
                 app.ui.key_box = None;
                 ctx.memory_mut(|m| {
                     if let Some(f) = m.focused() {