From 2eb4e8441180a114da40cf0f3e498d75f45e6d66 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 6 Nov 2026 21:23:30 -0800 Subject: [PATCH 1/4] Use toolbar order for fresh tool shortcuts --- crates/ui-egui/src/input/shortcuts.rs | 11 ++- .../src/input/tool_key_default_tests.rs | 85 +++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 crates/ui-egui/src/input/tool_key_default_tests.rs diff --git a/crates/ui-egui/src/input/shortcuts.rs b/crates/ui-egui/src/input/shortcuts.rs index 0abd096530..63444f628a 100644 --- a/crates/ui-egui/src/input/shortcuts.rs +++ b/crates/ui-egui/src/input/shortcuts.rs @@ -916,15 +916,16 @@ 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_tool(t.key()).filter(|last| group.contains(last)).unwrap_or(group[0]), + None => app.ui.group_tool(t.key()).filter(|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. @@ -942,3 +943,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 0000000000..6e5347ed17 --- /dev/null +++ b/crates/ui-egui/src/input/tool_key_default_tests.rs @@ -0,0 +1,85 @@ +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(); +} + +#[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::>()); + } +} + +#[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}"); + } +} + +#[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); +} From 26a8215043ece846add6b03b745cb2bd55c7d799 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 6 Nov 2026 21:58:06 -0800 Subject: [PATCH 2/4] Format the fresh tool shortcut fallback --- crates/ui-egui/src/input/shortcuts.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/input/shortcuts.rs b/crates/ui-egui/src/input/shortcuts.rs index 63444f628a..212d16b63d 100644 --- a/crates/ui-egui/src/input/shortcuts.rs +++ b/crates/ui-egui/src/input/shortcuts.rs @@ -923,9 +923,11 @@ fn select_tool_by_key(app: &mut PhotocraftApp, ctx: &egui::Context, pressed: &im 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_tool(t.key()).filter(|last| group.contains(last)).unwrap_or_else(|| { - group.iter().copied().min_by_key(|tool| crate::panels::toolbar::toolbar_position(*tool)).unwrap_or(t) - }), + None => app + .ui + .group_tool(t.key()) + .filter(|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. From 4551b2a2007bd378629654089074391e7e47c2cf Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 6 Nov 2026 23:13:48 -0800 Subject: [PATCH 3/4] Document tool shortcut selection behavior --- crates/ui-egui/src/input/shortcuts.rs | 5 +++-- crates/ui-egui/src/input/tool_key_default_tests.rs | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/input/shortcuts.rs b/crates/ui-egui/src/input/shortcuts.rs index 212d16b63d..cf23556c35 100644 --- a/crates/ui-egui/src/input/shortcuts.rs +++ b/crates/ui-egui/src/input/shortcuts.rs @@ -903,8 +903,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 { diff --git a/crates/ui-egui/src/input/tool_key_default_tests.rs b/crates/ui-egui/src/input/tool_key_default_tests.rs index 6e5347ed17..5b846d5e7f 100644 --- a/crates/ui-egui/src/input/tool_key_default_tests.rs +++ b/crates/ui-egui/src/input/tool_key_default_tests.rs @@ -21,6 +21,7 @@ fn press(app: &mut PhotocraftApp, ctx: &egui::Context, key: Key, modifiers: Modi 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] { @@ -43,6 +44,7 @@ fn fresh_tool_letters_pick_the_first_tool_in_toolbar_order() { } } +/// 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] { @@ -59,6 +61,8 @@ fn plain_w_returns_to_the_remembered_magic_wand() { } } +/// 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(); From f52206293613436e518a7c964ce30c2e415c35fe Mon Sep 17 00:00:00 2001 From: Codex Date: Sat, 7 Nov 2026 02:45:20 -0800 Subject: [PATCH 4/4] Expect Object Selection on the first W press --- crates/ui-egui/src/tool_shift_cycle_order_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 83c3ca33cc..e49077c4ff 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"); }