Merge pull request #303 from vangel76/fix/chart-category-axis

Charts: Select Data edits the category axis labels; blank corner makes column 1 the categories (#287)

# Conflicts:
#	crates/engine/src/tests.rs
#	crates/ui-egui/src/dialogs.rs
#	crates/ui-egui/tests/interaction.rs
#	docs/chart-parity.md

crates/engine/src/cmd/extra.rs

@@ -34,7 +34,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Select Data…",
             ["Chart Design", "Data"],
             None,
-            "{chart?, range: \"Sheet1!A1:D7\", byRows?: bool}",
+            "{chart?, range?: \"Sheet1!A1:D7\", byRows?: bool, categories?: \"Sheet1!A2:A7\" (Horizontal (Category) Axis Labels for every series; \"\" clears), removeSeries?: [index…]}",
             has_doc,
             select_data
         ),
@@ -350,22 +350,70 @@ fn change_colors(s: &mut Session, p: &Json) -> Result<Json> {
 }
 
 fn select_data(s: &mut Session, p: &Json) -> Result<Json> {
-    let range = str_param(p, "range").ok_or_else(|| bad("chart.selectData", "missing `range`"))?.to_string();
-    let (sheet_name, body) = split_sheet(&range);
     let d = s.doc()?;
-    let src_si = match sheet_name {
-        Some(n) => d.wb.sheet_index(&n).ok_or_else(|| bad("chart.selectData", "no such sheet"))?,
-        None => d.wb.active_sheet,
+    // A new data range rebuilds the series, as in Excel's Select Data › Chart data range.
+    let rebuilt = match str_param(p, "range") {
+        Some(range) => {
+            let range = range.trim_start_matches('=');
+            let (sheet_name, body) = split_sheet(range);
+            let src_si = match sheet_name {
+                Some(n) => d.wb.sheet_index(&n).ok_or_else(|| bad("chart.selectData", "no such sheet"))?,
+                None => d.wb.active_sheet,
+            };
+            let r = RangeRef::parse(body).ok_or_else(|| bad("chart.selectData", "not a range"))?;
+            if r.count() > 1_000_000 {
+                return Err(bad("chart.selectData", "range too large for a chart"));
+            }
+            let src = d.wb.sheet(src_si).ok_or(EngineError::NoDocument)?;
+            let by_rows = bool_param(p, "byRows").unwrap_or(r.width() > r.height() + 1);
+            let series = super::insert::series_from_range(src, r, by_rows);
+            Some((series, format!("{}!{}", gridcraft_formula::quote_sheet(&src.name), r.a1()), by_rows))
+        }
+        None => None,
     };
-    let r = RangeRef::parse(body).ok_or_else(|| bad("chart.selectData", "not a range"))?;
-    let src = d.wb.sheet(src_si).ok_or(EngineError::NoDocument)?;
-    let by_rows = bool_param(p, "byRows").unwrap_or(r.width() > r.height() + 1);
-    let series = super::insert::series_from_range(src, r, by_rows);
-    let source = format!("{}!{}", gridcraft_formula::quote_sheet(&src.name), r.a1());
+    // Horizontal (Category) Axis Labels › Edit: one labels range for every series ("" clears).
+    let categories = match str_param(p, "categories") {
+        None => None,
+        Some(t) if t.trim().trim_start_matches('=').is_empty() => Some(None),
+        Some(t) => {
+            let t = t.trim().trim_start_matches('=');
+            let (sheet_name, body) = split_sheet(t);
+            let si = match sheet_name {
+                Some(n) => d.wb.sheet_index(&n).ok_or_else(|| bad("chart.selectData", "no such sheet"))?,
+                None => d.wb.active_sheet,
+            };
+            let r = RangeRef::parse(body).ok_or_else(|| bad("chart.selectData", "`categories` is not a range"))?;
+            let name = d.wb.sheet(si).map(|sh| sh.name.clone()).ok_or(EngineError::NoDocument)?;
+            let abs = |c: CellRef| format!("${}${}", gridcraft_core::col_to_letters(c.col), c.row + 1);
+            Some(Some(format!("{}!{}:{}", gridcraft_formula::quote_sheet(&name), abs(r.start), abs(r.end))))
+        }
+    };
+    let mut remove: Vec<usize> = p
+        .get("removeSeries")
+        .and_then(Json::as_array)
+        .map(|a| a.iter().filter_map(Json::as_u64).filter_map(|i| usize::try_from(i).ok()).collect())
+        .unwrap_or_default();
+    remove.sort_unstable();
+    remove.dedup();
+    if rebuilt.is_none() && categories.is_none() && remove.is_empty() {
+        return Err(bad("chart.selectData", "missing `range`, `categories` or `removeSeries`"));
+    }
     with_chart(s, p, move |c| {
-        c.series = series;
-        c.source = Some(source);
-        c.by_rows = by_rows;
+        if let Some((series, source, by_rows)) = rebuilt {
+            c.series = series;
+            c.source = Some(source);
+            c.by_rows = by_rows;
+        }
+        for i in remove.iter().rev() {
+            if *i < c.series.len() && c.series.len() > 1 {
+                c.series.remove(*i);
+            }
+        }
+        if let Some(cats) = categories {
+            for se in &mut c.series {
+                se.categories = cats.clone();
+            }
+        }
         Ok(())
     })
 }

crates/engine/src/cmd/insert.rs

@@ -532,7 +532,12 @@ pub fn series_from_range(sh: &Sheet, r: RangeRef, by_rows: bool) -> Vec<Series>
     let header_row = r.height() > 1
         && (r.start.col..=r.end.col).any(|c| sh.value(CellRef::new(r.start.row, c)).is_text())
         && (r.start.col..=r.end.col).filter(|c| *c != r.start.col).all(|c| is_text(CellRef::new(r.start.row, c)));
-    let label_col = r.width() > 1 && (r.start.row + header_row as u32..=r.end.row).all(|row| is_text(CellRef::new(row, r.start.col)));
+    // A blank top-left cell above a header row marks the first column as the category labels even
+    // when it holds numbers (years, X values), the usual spreadsheet convention, so it isn't
+    // plotted as a series of its own (#287).
+    let blank_corner = header_row && sh.value(r.start).is_empty();
+    let label_col =
+        r.width() > 1 && (blank_corner || (r.start.row + header_row as u32..=r.end.row).all(|row| is_text(CellRef::new(row, r.start.col))));
     let data = RangeRef::new(CellRef::new(r.start.row + header_row as u32, r.start.col + label_col as u32), r.end);
     let mut out = Vec::new();
     if !by_rows {

crates/engine/src/tests.rs

@@ -125,6 +125,37 @@ fn chart_recommendations_follow_the_data_shape() {
     assert!(s.ui_requests.is_empty(), "programmatic calls never open the dialog");
 }
 
+#[test]
+fn chart_category_axis_from_blank_corner_and_select_data() {
+    // #287: numeric X values must be the category axis, not a series of their own.
+    let mut s = s();
+    let series = |s: &Session| -> Vec<(Option<String>, String)> {
+        let d = s.doc().unwrap();
+        d.wb.active().unwrap().charts.last().unwrap().series.iter().map(|x| (x.categories.clone(), x.values.clone())).collect()
+    };
+    // A blank top-left cell marks the first column as category labels, numbers or not.
+    s.execute("range.setValues", json!({"range": "A1", "values": [[null, "Sales"], [2020, 5], [2021, 7], [2022, 9]]})).unwrap();
+    s.execute("insert.chart", json!({"range": "A1:B4", "type": "line"})).unwrap();
+    assert_eq!(series(&s), [(Some("Sheet1!$A$2:$A$4".to_string()), "Sheet1!$B$2:$B$4".to_string())]);
+
+    // With a "Year" header the year column is a series, as before; Select Data fixes it:
+    // Horizontal (Category) Axis Labels and Remove on the year series.
+    s.execute("range.setValues", json!({"range": "D1", "values": [["Year", "Sales"], [2020, 5], [2021, 7], [2022, 9]]})).unwrap();
+    s.execute("insert.chart", json!({"range": "D1:E4", "type": "line"})).unwrap();
+    assert_eq!(series(&s).len(), 2);
+    assert!(s.execute("chart.selectData", json!({})).is_err(), "nothing to change");
+    assert!(s.execute("chart.selectData", json!({"categories": "not a range"})).is_err());
+    s.execute("chart.selectData", json!({"categories": "=D2:D4", "removeSeries": [0, 7]})).unwrap();
+    assert_eq!(series(&s), [(Some("Sheet1!$D$2:$D$4".to_string()), "Sheet1!$E$2:$E$4".to_string())]);
+    let d = s.doc().unwrap();
+    let sh = d.wb.active().unwrap();
+    let data = gridcraft_chart::resolve(&d.wb, d.wb.active_sheet, sh.charts.last().unwrap());
+    assert_eq!(data.categories, ["2020", "2021", "2022"]);
+    // The last series can't be removed; "" clears the labels.
+    s.execute("chart.selectData", json!({"categories": "", "removeSeries": [0]})).unwrap();
+    assert_eq!(series(&s), [(None, "Sheet1!$E$2:$E$4".to_string())]);
+}
+
 #[test]
 fn entry_and_text_paste_use_inherited_formats_and_cell_overrides() {
     let mut s = s();