diff --git a/crates/ui-egui/src/input/shortcuts.rs b/crates/ui-egui/src/input/shortcuts.rs index c3ef3778741..d525dc2a0b3 100644 --- a/crates/ui-egui/src/input/shortcuts.rs +++ b/crates/ui-egui/src/input/shortcuts.rs @@ -907,8 +907,9 @@ fn handle_escape_and_view_keys(app: &mut PhotocraftApp, ctx: &egui::Context) { crate::opacity_keys::handle(app, ctx); } -/// Selects a tool from its letter key. Pressing the key of the current group cycles within it; with -/// the Shift-switch preference only ⇧+key cycles. Returns true when a tool key was handled. +/// Selects a tool from its letter key. Entering a group uses its remembered eligible tool or the +/// first eligible tool in toolbar order. Pressing the key of the current group cycles within it. +/// With the Shift-switch preference, only ⇧+key cycles. Returns true when a tool key was handled. fn select_tool_by_key(app: &mut PhotocraftApp, ctx: &egui::Context, pressed: &impl Fn(Key) -> bool) -> bool { let shift_switch = app.session.prefs().tools.use_shift_key_for_tool_switch; for t in Tool::ALL { @@ -920,15 +921,20 @@ fn select_tool_by_key(app: &mut PhotocraftApp, ctx: &egui::Context, pressed: &im if cycle_shift || pressed(k) { let before = app.ui.tool; let mut group: Vec = Tool::ALL.iter().copied().filter(|x| x.key() == t.key() && crate::tool_shortcuts::uses_default(app, *x)).collect(); - // Shift+key (the cycle and the fallback to the group's first tool) uses the toolbar - // flyout's order; plain keys keep the `Tool::ALL` order. + // Shift+key cycles in toolbar flyout order. if cycle_shift { group.sort_by_key(|x| crate::panels::toolbar::toolbar_position(*x)); } app.ui.tool = match group.iter().position(|x| *x == app.ui.tool) { Some(i) if shift_switch && !cycle_shift => group[i], Some(i) => group[(i + 1) % group.len()], - None => app.ui.group_tools.iter().copied().find(|last| group.contains(last)).unwrap_or(group[0]), + None => app + .ui + .group_tools + .iter() + .copied() + .find(|last| group.contains(last)) + .unwrap_or_else(|| group.iter().copied().min_by_key(|tool| crate::panels::toolbar::toolbar_position(*tool)).unwrap_or(t)), }; crate::tool_memory::ended_by_tool(app); // `consume_key` ignores extra ⇧ and ⌥: a ⇧/⌥ switch is never spring-loaded. @@ -946,3 +952,7 @@ fn select_tool_by_key(app: &mut PhotocraftApp, ctx: &egui::Context, pressed: &im #[cfg(test)] mod tests; + +#[cfg(test)] +#[path = "tool_key_default_tests.rs"] +mod tool_key_default_tests; diff --git a/crates/ui-egui/src/input/tool_key_default_tests.rs b/crates/ui-egui/src/input/tool_key_default_tests.rs new file mode 100644 index 00000000000..5b846d5e7f2 --- /dev/null +++ b/crates/ui-egui/src/input/tool_key_default_tests.rs @@ -0,0 +1,89 @@ +use super::{Key, Modifiers, PhotocraftApp, Tool, handle}; +use crate::panels::toolbar::TOOL_SECTIONS; +use crate::spring_tools::key_of; +use std::collections::HashSet; + +fn app() -> PhotocraftApp { + PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()) +} + +fn press(app: &mut PhotocraftApp, ctx: &egui::Context, key: Key, modifiers: Modifiers) { + let raw = egui::RawInput { + events: vec![ + egui::Event::ModifiersChanged(modifiers), + egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }, + egui::Event::Key { key, physical_key: None, pressed: false, repeat: false, modifiers }, + ], + ..Default::default() + }; + ctx.begin_pass(raw); + handle(app, ctx); + ctx.end_pass().textures_delta.clear(); +} + +/// A fresh app enters each default letter group at its first toolbar tool under either preference. +#[test] +fn fresh_tool_letters_pick_the_first_tool_in_toolbar_order() { + for shift_switch in [true, false] { + let mut checked = HashSet::new(); + for tool in TOOL_SECTIONS.iter().flat_map(|section| section.iter()).flat_map(|slot| slot.iter()).copied() { + let Some(key) = key_of(tool.key()) else { continue }; + if !checked.insert(key) { + continue; + } + + let mut app = app(); + app.session.edit_prefs(|prefs| prefs.tools.use_shift_key_for_tool_switch = shift_switch); + if app.ui.tool.key() == tool.key() { + app.ui.tool = Tool::Brush; + } + press(&mut app, &egui::Context::default(), key, Modifiers::NONE); + assert_eq!(app.ui.tool, tool, "{key:?}, Shift switch: {shift_switch}"); + } + assert_eq!(checked, Tool::ALL.into_iter().filter_map(|tool| key_of(tool.key())).collect::>()); + } +} + +/// A remembered Magic Wand takes precedence over the fresh W default, even when hidden. +#[test] +fn plain_w_returns_to_the_remembered_magic_wand() { + for hidden in [false, true] { + let mut app = app(); + let ctx = egui::Context::default(); + if hidden { + app.session.edit_prefs(|prefs| prefs.toolbar.hidden = vec!["MagicWand".to_owned()]); + } + app.ui.tool = Tool::MagicWand; + press(&mut app, &ctx, Key::B, Modifiers::NONE); + assert_eq!(app.ui.tool, Tool::Brush); + press(&mut app, &ctx, Key::W, Modifiers::NONE); + assert_eq!(app.ui.tool, Tool::MagicWand, "hidden: {hidden}"); + } +} + +/// Shift+W follows the flyout order and wraps. With the default preference, plain W keeps +/// the current tool when it already belongs to the group. +#[test] +fn shift_w_cycles_in_toolbar_order() { + let mut app = app(); + let ctx = egui::Context::default(); + app.ui.tool = Tool::ObjectSelection; + + for expected in [Tool::QuickSelection, Tool::MagicWand, Tool::ObjectSelection] { + press(&mut app, &ctx, Key::W, Modifiers::SHIFT); + assert_eq!(app.ui.tool, expected); + } + press(&mut app, &ctx, Key::W, Modifiers::NONE); + assert_eq!(app.ui.tool, Tool::ObjectSelection); +} + +#[test] +fn remapped_first_tool_is_skipped_by_its_default_letter() { + let mut app = app(); + let ctx = egui::Context::default(); + app.run("prefs.set", serde_json::json!({"path": "shortcuts.tools.select.objectSelection", "value": "F6"})).unwrap(); + press(&mut app, &ctx, Key::W, Modifiers::NONE); + assert_eq!(app.ui.tool, Tool::QuickSelection); + press(&mut app, &ctx, Key::F6, Modifiers::NONE); + assert_eq!(app.ui.tool, Tool::ObjectSelection); +} diff --git a/crates/ui-egui/src/tool_shift_cycle_order_tests.rs b/crates/ui-egui/src/tool_shift_cycle_order_tests.rs index 83c3ca33cc6..e49077c4ff2 100644 --- a/crates/ui-egui/src/tool_shift_cycle_order_tests.rs +++ b/crates/ui-egui/src/tool_shift_cycle_order_tests.rs @@ -160,5 +160,5 @@ fn a_plain_w_from_outside_the_group_keeps_selecting_what_it_selected_before() { ctx.begin_pass(raw); crate::shortcuts::handle(&mut app, &ctx); ctx.end_pass().textures_delta.clear(); - assert_eq!(app.ui.tool, Tool::MagicWand, "the first W tool in Tool::ALL, not the first in the flyout"); + assert_eq!(app.ui.tool, Tool::ObjectSelection, "fresh W selects the first tool in the flyout"); }