Open the Insert Chart dialog from Recommended Charts instead of inserting a chart (#1399)

crates/ui-egui/src/lib.rs

@@ -505,6 +505,11 @@ impl SheetApp {
         if id == "file.exportPdf" && self.services.pick_save.is_some() && params.get("path").and_then(Json::as_str).is_none_or(str::is_empty) {
             return self.export_pdf_with_picker(params);
         }
+        // Recommended Charts with no chart type named (ribbon button, keytip, command search) asks
+        // which chart to insert; programmatic callers of `run` still get the first recommendation.
+        if id == "insert.recommendedCharts" && params.get("type").is_none() {
+            return self.open_dialog("recommendedCharts", json!({}));
+        }
         if let Err(e) = self.run(id, params) {
             self.alert(e);
         }

crates/ui-egui/src/ribbon.rs

@@ -1434,13 +1434,8 @@ fn insert(app: &mut SheetApp, ui: &mut Ui) {
     let __k = big_button(ui, Icon::Chart, "Recommended\nCharts", "Recommended Charts", false);
     control_keytip(app, ui, &__k, "insert.recommendedCharts");
     if __k.clicked() {
-        match app.run("insert.recommendedCharts", json!({})) {
-            // The new chart is selected, as after inserting any other object (see insert.chart
-            // in dialogs.rs) — otherwise the prior cell/range selection is left in place, and
-            // Del right after inserting deletes the source data instead of the chart.
-            Ok(r) => app.selected_chart = r.get("chart").and_then(serde_json::Value::as_u64).and_then(|id| u32::try_from(id).ok()),
-            Err(e) => app.message = Some(("GridCraft".into(), crate::clean_error(&e))),
-        }
+        // Menu-style: with no chart type, run_or_alert opens the Insert Chart dialog.
+        act(app, "insert.recommendedCharts", json!({}));
     }
     ui.vertical(|ui| {
         ui.horizontal(|ui| {

crates/ui-egui/tests/interaction.rs

@@ -485,6 +485,19 @@ fn recommended_charts_dialog_inserts_the_chosen_chart() {
     assert!(h.state().message.is_some());
 }
 
+#[test]
+fn recommended_charts_command_without_a_type_opens_the_dialog() {
+    // #1399: the ribbon button and the NR keytip ran the command directly and inserted the first
+    // recommendation (a scatter plot) instead of showing the chart picker.
+    let mut s = blank();
+    s.execute("range.setValues", json!({"range": "A1", "values": [["Fruit", "Sales"], ["Apples", 5], ["Pears", 3], ["Plums", 2]]})).unwrap();
+    s.execute("selection.set", json!({"range": "A1:B4"})).unwrap();
+    let mut h = harness(s);
+    h.state_mut().run_or_alert("insert.recommendedCharts", json!({}));
+    assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("recommendedCharts"));
+    assert!(h.state().session.active().unwrap().wb.active().unwrap().charts.is_empty(), "no chart is inserted before one is picked");
+}
+
 #[test]
 fn select_data_dialog_sets_the_category_axis_labels() {
     // #287: Chart Design › Select Data edits the Horizontal (Category) Axis Labels and removes the