Merge pull request #4193 from dotCooCoo/fix/appearance-add-keys-need-selection

Appearance: Add New Fill and Add New Stroke are off with nothing selected

crates/ui-egui/src/menus.rs

@@ -2446,12 +2446,25 @@ pub(crate) fn waits_for_ime(app: &VectorcraftApp, id: &str) -> bool {
             || id.starts_with("edit.paste"))
 }
 
+/// Engine commands that take `ids` and otherwise act on the Appearance panel's subject (the
+/// targeted layer, group or object, else the selection). Without `ids` and without a subject they
+/// are off, as the panel's buttons are: in menus, in the command palette and on their keys.
+const SUBJECT_COMMANDS: [&str; 2] = ["appearance.addFill", "appearance.addStroke"];
+
+/// Is there an Appearance panel subject: a targeted layer, group or object, or a selected one?
+fn has_subject(app: &VectorcraftApp) -> bool {
+    app.session.active().is_some_and(|st| !st.selection.subjects().is_empty())
+}
+
 /// Can command `id` run now? An engine command asks the engine, a UI command the app's state;
 /// none that would act on text an input method is still composing.
 pub fn enabled(app: &VectorcraftApp, id: &str) -> bool {
     if waits_for_ime(app, id) {
         return false;
     }
+    if SUBJECT_COMMANDS.contains(&id) {
+        return has_subject(app);
+    }
     if let Some(c) = vectorcraft_engine::find_command(id) {
         // The system clipboard's contents can be pasted with an empty internal clipboard.
         return (c.enabled)(&app.session).is_ok() || app.system_paste && id.starts_with("edit.paste");
@@ -3682,6 +3695,9 @@ pub fn invoke(app: &mut VectorcraftApp, id: &str, p: Value) {
     if waits_for_ime(app, id) {
         return;
     }
+    if SUBJECT_COMMANDS.contains(&id) && p.get("ids").is_none() && p.get("id").is_none() && !has_subject(app) {
+        return;
+    }
     // Params first: some dialogs' fields take a look through the session (Move's last move).
     if p.as_object().is_none_or(|o| o.is_empty())
         && let Some((kind, fields)) = menu_dialog(app, id)

crates/ui-egui/src/panels/tests_appearance.rs

@@ -202,6 +202,32 @@ fn cmd_slash_adds_a_fill_and_cmd_alt_slash_a_stroke_as_the_panel_buttons_do() {
     assert_eq!(stacks(&app, &ids), before);
 }
 
+/// With nothing selected, Add New Fill and Add New Stroke are off in menus and the command palette
+/// as the Appearance panel's buttons are, and their keys run nothing (no error in the status bar).
+/// Given ids, they still add to those objects.
+#[test]
+fn add_new_fill_and_stroke_are_off_without_a_selection() {
+    let mut app = app_with_rect();
+    let rect = first_selected(&app).expect("the rectangle").id;
+    let items = |app: &VectorcraftApp| app.session.active().expect("a document").doc.node(rect).expect("the rectangle").appearance.items.len();
+    assert!(crate::menus::enabled(&app, "appearance.addFill") && crate::menus::enabled(&app, "appearance.addStroke"));
+    run(&mut app, "select.none", json!({}));
+    let before = items(&app);
+    for (id, modifiers) in
+        [("appearance.addFill", egui::Modifiers::COMMAND), ("appearance.addStroke", egui::Modifiers::COMMAND | egui::Modifiers::ALT)]
+    {
+        assert!(!crate::menus::enabled(&app, id), "{id} is off");
+        let ctx = egui::Context::default();
+        let ev = egui::Event::Key { key: egui::Key::Slash, physical_key: None, pressed: true, repeat: false, modifiers };
+        let mut out = ctx.run_ui(egui::RawInput { events: vec![ev], ..Default::default() }, |ui| crate::shortcuts::handle(&mut app, ui.ctx()));
+        out.textures_delta.clear();
+        crate::menus::invoke(&mut app, id, json!({}));
+        assert_eq!((items(&app), app.ui.status.as_str()), (before, ""), "{id} ran nothing");
+    }
+    crate::menus::invoke(&mut app, "appearance.addFill", json!({"ids": [rect.0]}));
+    assert_eq!((items(&app), app.ui.status.as_str()), (before + 1, ""), "given ids, it adds a fill");
+}
+
 #[test]
 fn transparency_panel_reads_the_active_item() {
     let mut app = app_with_rect();