A refused cell draft stops the command; an untouched F2 draft changes nothing

- File and window commands stop when the workbook refuses the cell or
  text-box draft (validation Stop, bad formula, protected text box): the
  draft stays open in its cell with the reason shown, to fix or Esc. The
  per-frame backstop brings a refused draft's workbook back to the front.
- An F2 draft committed with its text unchanged writes nothing: no undo
  step, and the workbook stays clean through Save, Print or New.
- macOS Window menu: the active workbook's check is put back every poll,
  since choosing a check item flips it.
This commit is contained in:
aiongg
2026-10-22 05:12:23 +00:00
parent 1794c389e5
commit a0d54b49d4
4 changed files with 86 additions and 39 deletions
+22 -17
View File
@@ -283,27 +283,32 @@ impl NativeMenu {
/// step with the session.
fn sync_windows(&mut self, app: &SheetApp) {
let windows = app.open_windows();
if windows == self.windows {
return;
if windows != self.windows
&& let Some(menu) = &self.window_menu
{
let same_docs = windows.len() == self.windows.len() && windows.iter().zip(&self.windows).all(|(a, b)| a.doc == b.doc);
if same_docs {
for (item, w) in self.window_items.iter().zip(&windows) {
item.set_text(w.label());
}
} else {
for item in self.window_items.drain(..) {
let _ = menu.remove(&item);
}
for w in &windows {
let item = CheckMenuItem::with_id(format!("{WINDOW_ITEM}{}", w.doc), w.label(), true, w.active, None);
let _ = menu.append(&item);
self.window_items.push(item);
}
}
self.windows = windows;
}
let Some(menu) = &self.window_menu else { return };
let same_docs = windows.len() == self.windows.len() && windows.iter().zip(&self.windows).all(|(a, b)| a.doc == b.doc);
if same_docs {
for (item, w) in self.window_items.iter().zip(&windows) {
item.set_text(w.label());
// AppKit flips a check item when it is chosen, so the active check is put back every poll.
for (item, w) in self.window_items.iter().zip(&self.windows) {
if item.is_checked() != w.active {
item.set_checked(w.active);
}
} else {
for item in self.window_items.drain(..) {
let _ = menu.remove(&item);
}
for w in &windows {
let item = CheckMenuItem::with_id(format!("{WINDOW_ITEM}{}", w.doc), w.label(), true, w.active, None);
let _ = menu.append(&item);
self.window_items.push(item);
}
}
self.windows = windows;
}
pub fn poll(&mut self, app: &mut SheetApp, ctx: &egui::Context) {
+5
View File
@@ -45,11 +45,15 @@ pub struct EditState {
pub completion: Option<String>,
/// Move the caret to the end on the next frame.
pub caret_to_end: bool,
/// The cell's text when an F2-style edit began. Committing it unchanged leaves the cell alone
/// (no undo step, the workbook stays clean). `None` for an edit started by typing.
pub original: Option<String>,
}
impl EditState {
pub fn new(doc: u64, sheet: usize, cell: CellRef, text: String, enter_mode: bool, from_formula_bar: bool) -> EditState {
let caret = text.chars().count();
let original = (!enter_mode).then(|| text.clone());
EditState {
doc,
sheet,
@@ -68,6 +72,7 @@ impl EditState {
sync_caret: true,
caret_to_end: true,
completion: None,
original,
}
}
+25 -18
View File
@@ -412,7 +412,7 @@ impl SheetApp {
|| matches!(id, "view.newWindow" | "view.hideWindow" | "view.unhideWindow")
|| (id == "view.switchWindows" && params.get("doc").is_some_and(|d| !d.is_null()));
if id.starts_with("file.") || switches_window {
self.settle_drafts();
self.settle_drafts()?;
}
if matches!(id, "file.save" | "file.saveAs" | "file.saveBytes") {
self.store_view_state();
@@ -575,8 +575,11 @@ impl SheetApp {
self.open_switch_windows = true;
return;
}
if let Err(e) = self.run(id, params) {
self.alert(e);
match self.run(id, params) {
// A refused cell draft already shows its own message (with the rule's title).
Err(e) if self.message.as_ref().is_some_and(|(_, m)| *m == e) => {}
Err(e) => self.alert(e),
Ok(_) => {}
}
}
@@ -1013,8 +1016,8 @@ impl SheetApp {
/// Commits the edit; `then` moves the selection (dr, dc) afterwards.
///
/// The draft goes into the workbook it was typed in. If another workbook is showing, that
/// workbook stays active afterwards, and a draft its own workbook refuses (validation, a bad
/// formula) is dropped rather than left open over the wrong grid.
/// workbook stays active afterwards; a draft its own workbook refuses (validation, a bad
/// formula) brings that workbook back to the front with the draft still open.
pub fn commit_edit(&mut self, dr: i64, dc: i64, array: bool, fill_selection: bool) -> bool {
let Some(doc) = self.editor.as_ref().map(|ed| ed.doc) else { return true };
let shown = self.session.active().map(|d| d.uid);
@@ -1028,25 +1031,21 @@ impl SheetApp {
let back = self.session.active_index();
self.session.set_active(owner);
if !self.commit_edit_here(dr, dc, array, fill_selection) {
self.cancel_edit();
// Refused: its workbook stays in front with the draft open and the reason shown.
return false;
}
self.session.set_active(back);
true
}
/// Settles the cell and text-box drafts before a command that can show another workbook or
/// write this one to a file: each is committed as Enter would, or dropped when it can't be
/// (the refusal is still reported), so a draft never lands in another workbook.
fn settle_drafts(&mut self) {
/// Commits the cell and text-box drafts, as Enter would, before a command that can show
/// another workbook or write this one to a file, so a draft never lands in another workbook.
/// A draft its workbook refuses stays open with the reason shown, and the command doesn't run.
fn settle_drafts(&mut self) -> Result<(), String> {
if !self.commit_edit(0, 0, false, false) {
self.cancel_edit();
}
if let Err(e) = self.commit_text_box_edit() {
self.cancel_text_box_edit();
if self.message.is_none() {
self.message = Some(("Text Box".into(), clean_error(&e)));
}
return Err(self.message.as_ref().map(|(_, m)| m.clone()).unwrap_or_else(|| "The cell being edited can't be entered.".into()));
}
self.commit_text_box_edit()
}
fn commit_edit_here(&mut self, dr: i64, dc: i64, array: bool, fill_selection: bool) -> bool {
@@ -1056,6 +1055,13 @@ impl SheetApp {
Some(full) if full.to_lowercase().starts_with(&ed.text.to_lowercase()) => full.clone(),
_ => ed.text.clone(),
};
// F2 then Enter (or a file command) on an untouched draft: nothing to write.
if !fill_selection && !array && ed.original.as_ref() == Some(&text) {
if dr != 0 || dc != 0 {
self.move_after_enter(dr, dc);
}
return true;
}
let text = match self.session.active().map(|d| formula_locale::canonical(&text, ed.locale, &d.wb, ed.sheet, ed.cell)) {
Some(Ok(text)) => text,
Some(Err(error)) => {
@@ -1355,7 +1361,8 @@ impl SheetApp {
let t0 = now_ms();
text_box::before_ui(self, &ctx);
// Another workbook came forward without going through a command (which settles drafts
// first): the draft goes into its own workbook instead of floating over this one.
// first): the draft goes into its own workbook instead of floating over this one. If that
// workbook refuses it, it comes back to the front with the draft open.
if self.editor.as_ref().is_some_and(|ed| self.session.active().map(|d| d.uid) != Some(ed.doc)) {
self.commit_edit(0, 0, false, false);
}
+34 -4
View File
@@ -351,19 +351,49 @@ fn new_workbook_keeps_the_typed_cell_in_the_first_and_switch_windows_brings_it_b
}
#[test]
fn a_draft_its_workbook_refuses_is_dropped_when_switching_away() {
fn a_draft_its_workbook_refuses_stops_the_switch_and_stays_open() {
let mut h = harness(Services::default());
let book1 = typing_in_book1_then_new(&mut h);
let book2 = h.state().session.doc().unwrap().uid;
h.state_mut().session.execute("data.validation", serde_json::json!({"range": "A1", "type": "list", "formula1": "\"Red,Green\""})).unwrap();
h.input_mut().events.push(egui::Event::Text("Purple".into()));
h.run_steps(2);
assert!(h.state().editor.is_some());
h.state_mut().run_or_alert("view.switchWindows", serde_json::json!({"doc": book1}));
h.run_steps(2);
assert!(h.state().editor.is_none());
assert_eq!(h.state().session.doc().unwrap().uid, book1, "the switch still happens");
assert_eq!(h.state().session.doc().unwrap().uid, book2, "the switch doesn't happen");
assert_eq!(h.state().editor.as_ref().map(|ed| ed.text.as_str()), Some("Purple"), "the draft stays open to fix");
assert_eq!(a1(&h, 1), Value::Empty, "Book2 is unchanged");
assert!(h.state().message.is_some(), "the refusal is reported");
// Esc drops the draft; then the switch goes through.
h.state_mut().message = None;
h.state_mut().cancel_edit();
h.state_mut().run_or_alert("view.switchWindows", serde_json::json!({"doc": book1}));
h.run_steps(2);
assert_eq!(h.state().session.doc().unwrap().uid, book1);
assert!(h.state().message.is_none());
}
#[test]
fn an_untouched_f2_draft_leaves_the_workbook_clean_on_a_file_command() {
let dir = std::env::temp_dir().join(format!("gridcraft-f2-draft-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join("Book.xlsx").to_string_lossy().into_owned();
let mut h = harness(Services::default());
h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "=1+2"})).unwrap();
h.state_mut().run("file.save", serde_json::json!({"path": &path})).unwrap();
h.run_steps(2);
let undo = h.state().session.doc().unwrap().undo.len();
h.key_press(egui::Key::F2);
h.run_steps(2);
assert_eq!(h.state().editor.as_ref().map(|ed| ed.text.as_str()), Some("=1+2"));
h.state_mut().run_or_alert("file.new", serde_json::json!({}));
h.run_steps(2);
assert!(h.state().editor.is_none());
let book1 = &h.state().session.documents()[0];
assert!(!book1.is_dirty(), "nothing was typed, so nothing changed");
assert_eq!(book1.undo.len(), undo, "and no undo step was added");
let _ = std::fs::remove_dir_all(&dir);
}
fn pdf_picker_harness(dir: &std::path::Path) -> (Arc<Mutex<Vec<String>>>, egui_kittest::Harness<'static, SheetApp>) {