diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index a6c25771f..d5dffb23d 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -710,7 +710,11 @@ impl Dialog { }], json!({"kind": "blanks"}), ), - "sort" => Dialog::custom("sort", "Sort", json!({"header": true, "levels": [{"column": "A", "order": "asc"}]})), + "sort" => { + // Seed "Sort by" from the column the user is actually in (#1906). + let col = app.session.active().map(|d| gridcraft_engine::core::col_to_letters(d.selection.active.col)).unwrap_or_else(|| "A".into()); + Dialog::custom("sort", "Sort", json!({"header": true, "levels": [{"column": col, "order": "asc"}]})) + } "nameManager" => Dialog::custom("nameManager", "Name Manager", json!({})), "manageRules" => Dialog::custom("manageRules", "Conditional Formatting Rules Manager", json!({})), "commandSearch" => Dialog::custom("commandSearch", "Search Commands", json!({})), @@ -2038,12 +2042,43 @@ fn sort(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut boo ui.checkbox(&mut header, "My list has headers"); d.values.insert("header".into(), json!(header)); let mut levels: Vec = d.values.get("levels").and_then(Json::as_array).cloned().unwrap_or_default(); + // Columns of the region around the active cell, labelled with their header (or first value). + let columns: Vec<(String, String)> = app + .session + .active() + .and_then(|d| { + let sh = d.wb.active()?; + let region = gridcraft_engine::selection::current_region(sh, d.selection.active); + Some( + (region.start.col..=region.end.col) + .map(|c| { + let letter = gridcraft_engine::core::col_to_letters(c); + // The region starts at the header row when one is present, so the + // label always comes from start.row: header text with headers on, + // first data value with them off. + let label_row = region.start.row; + let text = gridcraft_engine::display::cell_text(&d.wb, sh, gridcraft_engine::core::CellRef::new(label_row, c)); + let text: String = text.chars().take(24).collect(); + (letter.clone(), if text.is_empty() { letter } else { format!("{letter}: {text}") }) + }) + .collect(), + ) + }) + .unwrap_or_default(); let mut remove = None; for (i, lv) in levels.iter_mut().enumerate() { ui.horizontal(|ui| { ui.label(if i == 0 { "Sort by" } else { "Then by" }); let mut col = lv["column"].as_str().unwrap_or("A").to_string(); - ui.add(egui::TextEdit::singleline(&mut col).desired_width(50.0)); + if columns.is_empty() { + ui.add(egui::TextEdit::singleline(&mut col).desired_width(50.0)); + } else { + egui::ComboBox::from_id_salt(("sort_col", i)).selected_text(col.clone()).show_ui(ui, |ui| { + for (letter, label) in &columns { + ui.selectable_value(&mut col, letter.clone(), label.as_str()); + } + }); + } lv["column"] = json!(col.to_ascii_uppercase()); let mut by = lv["by"].as_str().unwrap_or("values").to_string(); egui::ComboBox::from_id_salt(("by", i)) @@ -2073,7 +2108,15 @@ fn sort(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut boo levels.remove(i); } if ui.button("+ Add Level").clicked() { - levels.push(json!({"column": "B", "order": "asc"})); + let used: Vec = levels.iter().filter_map(|l| l.get("column").and_then(Json::as_str).map(str::to_string)).collect(); + let next = columns + .iter() + .map(|(l, _)| l) + .find(|l| !used.contains(l)) + .cloned() + .or_else(|| columns.last().map(|(l, _)| l.clone())) + .unwrap_or_else(|| "B".into()); + levels.push(json!({"column": next, "order": "asc"})); } d.values.insert("levels".into(), Json::Array(levels.clone())); let mut open = true; diff --git a/crates/ui-egui/tests/sort_dialog_columns.rs b/crates/ui-egui/tests/sort_dialog_columns.rs new file mode 100644 index 000000000..fb685c3e0 --- /dev/null +++ b/crates/ui-egui/tests/sort_dialog_columns.rs @@ -0,0 +1,81 @@ +//! #1906: the Sort dialog seeds "Sort by" from the active column instead of a +//! free-text box stuck on "A", and the column list offers the region's columns. + +use egui::{Key, Modifiers}; +use egui_kittest::kittest::Queryable; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness(session: Session) -> Harness { + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +/// Header row + three rows; the user's cursor sits in column B (Revenue). +fn sales() -> Session { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Month", "Revenue"], ["Mar", 30], ["Jan", 10], ["Feb", 20]]})).unwrap(); + s.execute("selection.set", json!({"range": "B2"})).unwrap(); + s +} + +fn col(h: &Harness) -> String { + h.state().dialog.as_ref().unwrap().values["levels"][0]["column"].as_str().unwrap_or("").to_string() +} + +#[test] +fn sort_dialog_seeds_the_active_column() { + let mut h = harness(sales()); + h.state_mut().open_dialog("sort", json!({})); + h.run_steps(2); + assert_eq!(col(&h), "B", "Sort by starts on the column the cursor is in, not A"); +} + +#[test] +fn add_level_picks_a_column_no_level_uses_yet() { + let mut h = harness(sales()); + h.state_mut().open_dialog("sort", json!({})); + h.run_steps(2); + // Level 1 seeded B; clicking "+ Add Level" must not default level 2 to B too (#1906 review). + h.run_steps(6); + h.get_by_label("+ Add Level").click(); + h.run_steps(6); + let cols: Vec = h.state().dialog.as_ref().unwrap().values["levels"] + .as_array() + .map(|a| a.iter().filter_map(|l| l["column"].as_str().map(str::to_string)).collect()) + .unwrap_or_default(); + assert_eq!(cols.len(), 2, "two levels after Add Level"); + assert_ne!(cols[0], cols[1], "the second level defaults to a different column: {cols:?}"); +} + +#[test] +fn sorting_by_the_seeded_column_orders_the_rows() { + let mut h = harness(sales()); + h.state_mut().open_dialog("sort", json!({})); + h.run_steps(2); + // Accept the dialog: rows must come back ordered by Revenue (10, 20, 30). + h.input_mut().events.push(egui::Event::Key { + key: Key::Enter, + physical_key: Some(Key::Enter), + pressed: true, + repeat: false, + modifiers: Modifiers::NONE, + }); + h.run_steps(2); + let d = h.state().session.active().unwrap(); + let sh = d.wb.active().unwrap(); + let nums: Vec = (1..4).map(|r| sh.value(gridcraft_engine::core::CellRef::new(r, 1))).filter_map(|v| v.as_f64().map(|n| n as i64)).collect(); + assert_eq!(nums, vec![10, 20, 30], "sort ran on the seeded column B"); +}