diff --git a/crates/keymap/src/ui/sets.rs b/crates/keymap/src/ui/sets.rs index a18343a5..7b4cd479 100644 --- a/crates/keymap/src/ui/sets.rs +++ b/crates/keymap/src/ui/sets.rs @@ -43,7 +43,7 @@ impl Draw<'_, '_> { let modified = DialogState::is_modified(self.env); let items = self.set_items(builtin, modified); let mut act = None; - ui.add_enabled_ui(self.state.review.is_none(), |ui| { + ui.add_enabled_ui(!self.state.modal(), |ui| { ui.horizontal(|ui| { let set_label = self.ui("Set:"); let l = ui.label(self.vis(&set_label)); diff --git a/crates/keymap/src/ui/state.rs b/crates/keymap/src/ui/state.rs index d9461647..c53a12d2 100644 --- a/crates/keymap/src/ui/state.rs +++ b/crates/keymap/src/ui/state.rs @@ -787,13 +787,26 @@ impl DialogState { fn load_set(&mut self, env: &mut Env, name: &str) -> Result<(), ProfileError> { let Some((p, b)) = env.sets.as_mut() else { return Err(ProfileError::NotFound) }; p.select(name, b)?; - env.keymap.set_overrides(p.active_overrides(b).clone()); + self.load_active(env); + Ok(()) + } + + /// Loads the active set (no overrides when none is left), with a fresh undo history. + fn load_active(&mut self, env: &mut Env) { + if let Some((p, b)) = env.sets.as_ref() { + env.keymap.set_overrides(p.active_overrides(b).clone()); + } self.session = Some(EditSession::begin(env.keymap)); self.recording = None; self.conflict = None; self.field = None; + self.prompt = None; self.changed = true; - Ok(()) + } + + /// A prompt or the import review is open: the set row, the table and the edit actions wait. + pub(crate) fn modal(&self) -> bool { + self.prompt.is_some() || self.review.is_some() } fn report(&mut self, env: &Env, result: Result<(), ProfileError>) { @@ -805,6 +818,7 @@ impl DialogState { /// The set picker. With unsaved edits it asks to save them first. pub(crate) fn switch_set(&mut self, env: &mut Env, name: &str) { + self.prompt = None; self.after_save_prompt(env, Then::Switch(name.to_string())); } @@ -891,13 +905,22 @@ impl DialogState { } } + /// Deletes a user set. Deleting the active set loads the one that becomes active, or no + /// overrides when no set is left. fn delete_set(&mut self, env: &mut Env, name: &str) { let result = match env.sets.as_mut() { - Some((p, b)) => p.delete(name, b).map(|()| p.active_name(b).to_string()), + Some((p, b)) => { + let was_active = p.active_name(b) == name; + p.delete(name, b).map(|()| was_active) + } None => Err(ProfileError::NotFound), }; - let result = result.and_then(|active| self.load_set(env, &active)); - self.report(env, result); + match result { + Ok(true) => self.load_active(env), + Ok(false) => self.changed = true, + Err(_) => {} + } + self.report(env, result.map(|_| ())); } /// Save: stores the edits; a built-in's become a copy. The undo history stays. @@ -1348,6 +1371,42 @@ mod tests { assert_eq!((fx.km.overrides(), &fx.p), (&km0, &p0)); } + #[test] + fn a_stale_delete_prompt_neither_reloads_nor_drops_edits() { + let mut fx = Fx::new(Options::default()); + fx.p.save_as("Mine", Overrides::new(), &fx.b).ok(); + fx.p.save_as("Other", Overrides::new(), &fx.b).ok(); + let mut st = open(&mut fx); + st.ask_delete_set(&fx.env()); + assert!(st.modal()); + st.switch_set(&mut fx.env(), "Mine"); + assert_eq!((st.prompt.clone(), fx.p.active.as_str()), (None, "Mine"), "switching closes the prompt"); + + record(&mut st, &mut fx, add("object.front"), &["F2"]); + st.prompt = Some(Prompt::DeleteSet("Other".into())); + st.answer(&mut fx.env(), Answer::Yes); + assert!(!fx.p.sets.contains_key("Other")); + assert_eq!((fx.p.active.as_str(), fx.keys("object.front")), ("Mine", vec!["F2".to_string()]), "the active set keeps its edits"); + assert!(st.can_undo()); + } + + #[test] + fn deleting_the_last_set_without_builtins_loads_no_overrides() { + let mut fx = Fx::new(Options::default()); + fx.b.clear(); + let mut mine = Overrides::new(); + mine.insert("object.front", vec!["F2".into()]); + fx.p.save_as("Mine", mine.clone(), &fx.b).ok(); + fx.km.set_overrides(mine); + let mut st = open(&mut fx); + st.ask_delete_set(&fx.env()); + st.answer(&mut fx.env(), Answer::Yes); + assert!(st.message.is_none(), "{:?}", st.message); + assert!(fx.p.sets.is_empty() && fx.km.overrides().is_empty() && !st.can_undo()); + st.ok(&mut fx.env()); + assert!(fx.p.sets.is_empty(), "OK stores no copy of the deleted set"); + } + #[test] fn ok_stores_edits_of_a_builtin_as_a_copy() { let mut fx = Fx::new(Options::default()); diff --git a/crates/keymap/src/ui/view.rs b/crates/keymap/src/ui/view.rs index 7bebed9f..373bbb82 100644 --- a/crates/keymap/src/ui/view.rs +++ b/crates/keymap/src/ui/view.rs @@ -310,7 +310,7 @@ impl Draw<'_, '_> { } return; } - if text_focus || !self.env.options.editable { + if text_focus || !self.env.options.editable || self.state.modal() { return; } let mac = self.env.keymap.platform().is_mac(); @@ -351,7 +351,13 @@ impl Draw<'_, '_> { let mid_bottom = (rest.max.y - bottom_h - gap).max(rest.min.y); let mid = Rect::from_min_max(rest.min, pos2(rest.max.x, mid_bottom)); if !self.review(ui, mid) { - self.middle(ui, mid); + let prompt = self.state.prompt.is_some(); + ui.scope(|ui| { + if prompt { + ui.disable(); + } + self.middle(ui, mid); + }); } let top = (mid.max.y + gap).min(rest.max.y); let bottom = Rect::from_min_max(pos2(rest.min.x, top), rest.max); @@ -1303,7 +1309,7 @@ impl Draw<'_, '_> { if m.goto.is_some() && self.button(ui, true, &self.ui("Go to Conflict"), "shortcuts.message.goto") { act = 1; } - if m.undo && self.button(ui, self.state.can_undo(), &self.ui("Undo"), "shortcuts.message.undo") { + if m.undo && self.button(ui, self.state.can_undo() && !self.state.modal(), &self.ui("Undo"), "shortcuts.message.undo") { act = 2; } }); @@ -1374,15 +1380,16 @@ impl Draw<'_, '_> { } else { vec![(self.ui("Close"), "shortcuts.close")] }; + let idle = !self.state.modal(); ui.horizontal(|ui| { if editable { - if self.button(ui, self.state.can_undo(), &self.ui("Undo"), "shortcuts.undo") { + if self.button(ui, idle && self.state.can_undo(), &self.ui("Undo"), "shortcuts.undo") { self.state.undo(self.env); } - if self.button(ui, self.state.can_redo(), &self.ui("Redo"), "shortcuts.redo") { + if self.button(ui, idle && self.state.can_redo(), &self.ui("Redo"), "shortcuts.redo") { self.state.redo(self.env); } - if self.button(ui, !self.env.keymap.overrides().is_empty(), &self.ui("Reset All…"), "shortcuts.resetAll") { + if self.button(ui, idle && !self.env.keymap.overrides().is_empty(), &self.ui("Reset All…"), "shortcuts.resetAll") { self.state.ask_reset_all(); } } @@ -1397,7 +1404,8 @@ impl Draw<'_, '_> { let width: f32 = right.iter().map(|(t, _)| self.button_width(ui, t) + gap).sum(); ui.add_space((ui.available_width() - width).max(0.0)); for (text, id) in &right { - if self.button(ui, true, text, id) { + // OK waits for the open prompt or review; Cancel and Close never do. + if self.button(ui, idle || *id != "shortcuts.ok", text, id) { if editable && *id == "shortcuts.ok" { self.state.ok(self.env); } else { diff --git a/crates/keymap/tests/ui_dialog.rs b/crates/keymap/tests/ui_dialog.rs index 38a77e28..aed56d40 100644 --- a/crates/keymap/tests/ui_dialog.rs +++ b/crates/keymap/tests/ui_dialog.rs @@ -630,6 +630,24 @@ fn cancel_restores_keymap_and_sets() { assert!(h.state().km.overrides().is_empty()); } +#[test] +fn an_open_prompt_holds_the_set_row_table_and_edit_actions() { + let mut h = with_sets(Profiles::default()); + click(&mut h, "Save: ⌘S"); + press(&mut h, Modifiers::NONE, EguiKey::F2); + new_set(&mut h, "Other"); + click(&mut h, "Delete"); + assert!(has(&h, "Delete the set “Other”?")); + for label in ["Set:", "New Set…", "Save: F2", "Undo", "Reset All…", "OK"] { + assert!(h.get_by_label(label).accesskit_node().is_disabled(), "{label}"); + } + assert!(h.get_all_by_label("Cancel").all(|n| !n.accesskit_node().is_disabled()), "the prompt's and the dialog's"); + h.get_all_by_label("Cancel").next().expect("the prompt's Cancel").click(); + h.run(); + assert!(h.state().state.is_open()); + assert!(!h.get_by_label("OK").accesskit_node().is_disabled()); +} + #[test] fn ok_stores_edits_of_a_builtin_as_a_copy() { let mut h = with_sets(Profiles::default());