From 9f5af15a194343f34098114fd33dd1d5584c8c4b Mon Sep 17 00:00:00 2001 From: rmsff <17902130+rmsff@users.noreply.github.com> Date: Sun, 8 Nov 2026 14:57:55 +0300 Subject: [PATCH] fix(ui): Escape commits a Type Mask session as Command+Return does (#12070) Escape while typing with the Horizontal or Vertical Type Mask tool threw the session away. As in the reference application it now commits: the text outline becomes the selection in one history state and no layer is added, whatever Preferences > Type "Use ESC to commit" says. The options bar's Cancel still throws the session away. Escape on an empty mask text, or while an input method is composing, was not measured and still cancels. A Command transform drag keeps its own Escape, and the Type tool's Escape still follows the preference. The #1055 canvas test that pinned the old Escape now goes through Cancel, which keeps what it guarded: the selection and layers unchanged. --- crates/ui-egui/src/tools/type_tool.rs | 2 + .../src/tools/type_tool/canvas_tests.rs | 8 +- crates/ui-egui/src/tools/type_tool/keys.rs | 6 +- .../src/tools/type_tool/mask_escape_tests.rs | 152 ++++++++++++++++++ 4 files changed, 163 insertions(+), 5 deletions(-) create mode 100644 crates/ui-egui/src/tools/type_tool/mask_escape_tests.rs diff --git a/crates/ui-egui/src/tools/type_tool.rs b/crates/ui-egui/src/tools/type_tool.rs index 42ad346f61..1d4ac7e85f 100644 --- a/crates/ui-egui/src/tools/type_tool.rs +++ b/crates/ui-egui/src/tools/type_tool.rs @@ -146,3 +146,5 @@ mod commit_tests; #[cfg(test)] mod ime_tests; +#[cfg(test)] +mod mask_escape_tests; diff --git a/crates/ui-egui/src/tools/type_tool/canvas_tests.rs b/crates/ui-egui/src/tools/type_tool/canvas_tests.rs index 091a0a22cb..42effdb2eb 100644 --- a/crates/ui-egui/src/tools/type_tool/canvas_tests.rs +++ b/crates/ui-egui/src/tools/type_tool/canvas_tests.rs @@ -2728,10 +2728,10 @@ fn type_mask_commits_the_glyphs_as_the_selection_in_one_step() { assert!(v.height() > 2 * v.width(), "{v:?}"); } -/// #1055: Escape cancels a Type Mask session whatever "Use Escape to Commit" says: the selection -/// and the layers stay as they were. +/// #1055: Cancel throws a Type Mask session away: the selection and the layers stay as they were. +/// (Escape commits it, as in the reference application: #12070, `mask_escape_tests`.) #[test] -fn type_mask_escape_leaves_the_selection_unchanged() { +fn type_mask_cancel_leaves_the_selection_unchanged() { let mut app = new_app(); app.run("select.all", json!({})).unwrap(); let (before, layers) = (selection_bounds(&app), app.session.active().unwrap().doc.layers.len()); @@ -2741,7 +2741,7 @@ fn type_mask_escape_leaves_the_selection_unchanged() { click(&mut h, p); assert!(h.state().ui.text_edit.as_ref().is_some_and(|ed| ed.mask), "typing a mask"); super::insert(h.state_mut(), "I"); - h.key_press(egui::Key::Escape); + super::cancel(h.state_mut()); h.run_steps(2); assert!(h.state().ui.text_edit.is_none()); assert_eq!(selection_bounds(h.state()), before); diff --git a/crates/ui-egui/src/tools/type_tool/keys.rs b/crates/ui-egui/src/tools/type_tool/keys.rs index 74c81c2f7d..0da9b40319 100644 --- a/crates/ui-egui/src/tools/type_tool/keys.rs +++ b/crates/ui-egui/src/tools/type_tool/keys.rs @@ -255,7 +255,11 @@ fn handle_pressed_key( insert(app, "\t"); } Key::Escape if crate::type_transform::active(app) => crate::type_transform::cancel_drag(app), - Key::Escape if edit.edit.mask => cancel(app), + // A Type Mask commits on Escape as on ⌘↩, whatever "Use ESC to commit" says; the options bar's + // Cancel still throws it away (#12070). Escape on an empty mask text, or while an input + // method is composing, was not measured and still cancels. + Key::Escape if edit.edit.mask && (edit.text.is_empty() || edit.edit.preedit.is_some()) => cancel(app), + Key::Escape if edit.edit.mask => commit(app), Key::Escape if app.session.prefs().type_.use_esc_to_commit => commit(app), Key::Escape => cancel(app), Key::A if modifiers.command => { diff --git a/crates/ui-egui/src/tools/type_tool/mask_escape_tests.rs b/crates/ui-egui/src/tools/type_tool/mask_escape_tests.rs new file mode 100644 index 0000000000..30566900b0 --- /dev/null +++ b/crates/ui-egui/src/tools/type_tool/mask_escape_tests.rs @@ -0,0 +1,152 @@ +//! #12070: Escape commits a Type Mask session the way ⌘↩ does, whatever "Use Escape to Commit" +//! says: the text outline becomes the selection in one history state and no layer is added. An +//! empty mask text and a composing input method still cancel, and the Type tool's Escape still +//! follows the preference. (A ⌘ transform drag's Escape arm stays first; masks have no transform +//! box yet, #12082.) Keys go through the app's shortcut handling, as on the canvas. + +use egui_kittest::Harness; +use photocraft_doc::LayerId; +use photocraft_raster::Surface; +use serde_json::json; + +use crate::PhotocraftApp; +use crate::state::Tool; + +fn new_app(tool: Tool, use_esc_to_commit: bool) -> PhotocraftApp { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 400, "height": 200})).unwrap(); + app.run("prefs.set", json!({"path": "type.useEscToCommit", "value": use_esc_to_commit})).unwrap(); + // Away from the text, so New, Add, Subtract and Intersect each leave a different selection. + app.run("select.rect", json!({"x": 300, "y": 150, "width": 60, "height": 30})).unwrap(); + app.sync_views(); + app.ui.tool = tool; + app +} + +/// Start new text with a click and type `s`, as the canvas does. +fn type_new(app: &mut PhotocraftApp, s: &str) -> LayerId { + super::pointer_up(app, [50.0, 100.0], [50.0, 100.0]); + super::insert(app, s); + LayerId(app.ui.text_edit.as_ref().unwrap().layer) +} + +/// One frame of the app's shortcut handling for each of `events`. +fn send(app: PhotocraftApp, events: Vec) -> PhotocraftApp { + let mut h = Harness::builder().build_ui_state( + |ui, app: &mut PhotocraftApp| { + let ctx = ui.ctx().clone(); + // As `canvas_tests`' harness: the shortcuts wait for the app's fonts. + if ctx.fonts(|f| f.families().contains(&egui::FontFamily::Name("medium".into()))) { + crate::shortcuts::handle(app, &ctx); + } + }, + app, + ); + PhotocraftApp::setup_context(&h.ctx, crate::theme::ThemeKind::ALL[0]); + h.run_steps(4); + for event in events { + h.event(event); + h.run_steps(1); + } + h.into_state() +} + +fn key(key: egui::Key, modifiers: egui::Modifiers) -> egui::Event { + egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers } +} + +fn escape(app: PhotocraftApp) -> PhotocraftApp { + send(app, vec![key(egui::Key::Escape, egui::Modifiers::NONE)]) +} + +/// The selection (compared pixel for pixel, shown by its bounds), the history (oldest first) and +/// the layer count. +#[derive(PartialEq)] +struct Snapshot { + selection: Option, + history: Vec, + layers: usize, +} + +impl std::fmt::Debug for Snapshot { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let bounds = self.selection.as_ref().map(Surface::content_bounds); + write!(f, "selection {bounds:?}, history {:?}, {} layers", self.history, self.layers) + } +} + +fn snapshot(app: &PhotocraftApp) -> Snapshot { + let st = app.session.active().unwrap(); + Snapshot { selection: st.doc.selection.clone(), history: st.history.entries(), layers: st.doc.layers.len() } +} + +#[test] +fn escape_commits_a_type_mask_as_command_return_does() { + for tool in [Tool::HorizontalTypeMask, Tool::VerticalTypeMask] { + for use_esc_to_commit in [true, false] { + let what = format!("{tool:?}, useEscToCommit={use_esc_to_commit}"); + let session = || { + let mut app = new_app(tool, use_esc_to_commit); + let before = snapshot(&app); + type_new(&mut app, "AB"); + (app, before) + }; + let (app, before) = session(); + let escaped = escape(app); + let (app, _) = session(); + let returned = send(app, vec![key(egui::Key::Enter, egui::Modifiers::COMMAND)]); + assert!(escaped.ui.text_edit.is_none(), "{what}: the session ends"); + let after = snapshot(&escaped); + let bounds = after.selection.as_ref().map(Surface::content_bounds); + assert!(bounds.is_some_and(|r| r.x1 < 300), "{what}: the text outline alone is the selection: {bounds:?}"); + assert_eq!(after.history.len(), before.history.len() + 1, "{what}: one history state"); + assert_eq!(after.layers, before.layers, "{what}: no layer is added"); + assert_eq!(after, snapshot(&returned), "{what}: the same as ⌘↩"); + } + } +} + +#[test] +fn escape_with_an_empty_mask_text_changes_nothing() { + for tool in [Tool::HorizontalTypeMask, Tool::VerticalTypeMask] { + for use_esc_to_commit in [true, false] { + let what = format!("{tool:?}, useEscToCommit={use_esc_to_commit}"); + let mut app = new_app(tool, use_esc_to_commit); + let before = snapshot(&app); + type_new(&mut app, ""); + let app = escape(app); + assert!(app.ui.text_edit.is_none(), "{what}"); + assert_eq!(snapshot(&app), before, "{what}"); + } + } +} + +#[test] +fn escape_while_an_input_method_composes_still_cancels_a_type_mask() { + let preedit = || egui::Event::Ime(egui::ImeEvent::Preedit { text: "にほ".into(), active_range_chars: None }); + let session = || { + let mut app = new_app(Tool::HorizontalTypeMask, true); + let before = snapshot(&app); + type_new(&mut app, "AB"); + (app, before) + }; + let (app, _) = session(); + let app = send(app, vec![preedit()]); + assert!(app.ui.text_edit.as_ref().is_some_and(|ed| ed.preedit.is_some()), "composing"); + let (app, before) = session(); + let app = send(app, vec![preedit(), key(egui::Key::Escape, egui::Modifiers::NONE)]); + assert!(app.ui.text_edit.is_none()); + assert_eq!(snapshot(&app), before); +} + +#[test] +fn the_type_tools_escape_still_follows_the_preference() { + for (use_esc_to_commit, kept) in [(true, true), (false, false)] { + let mut app = new_app(Tool::Type, use_esc_to_commit); + let id = type_new(&mut app, "AB"); + let app = escape(app); + assert!(app.ui.text_edit.is_none()); + let layer = app.session.active().unwrap().doc.layer(id).is_some(); + assert_eq!(layer, kept, "useEscToCommit={use_esc_to_commit}: committed, or cancelled and the new layer gone"); + } +}