diff --git a/Cargo.lock b/Cargo.lock index ceba9ddc69..5a0721606f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3834,6 +3834,7 @@ dependencies = [ "neomacs-video-model", "neomacs-webview", "neovm-core", + "num_enum", "objc2", "objc2-app-kit", "objc2-foundation", diff --git a/crates/neomacs-display-runtime/Cargo.toml b/crates/neomacs-display-runtime/Cargo.toml index cfa9867880..c36f406ccd 100644 --- a/crates/neomacs-display-runtime/Cargo.toml +++ b/crates/neomacs-display-runtime/Cargo.toml @@ -37,6 +37,7 @@ tracing.workspace = true tracing-subscriber.workspace = true serde_json.workspace = true strum.workspace = true +num_enum.workspace = true thiserror.workspace = true bitflags.workspace = true cfg-if.workspace = true diff --git a/crates/neomacs-display-runtime/src/menus/interaction.rs b/crates/neomacs-display-runtime/src/menus/interaction.rs index 93c392b81a..fd62ac2101 100644 --- a/crates/neomacs-display-runtime/src/menus/interaction.rs +++ b/crates/neomacs-display-runtime/src/menus/interaction.rs @@ -1,6 +1,6 @@ //! Translate desktop keys into menu navigation, never editor input. -use super::session::MenuSession; +use super::session::{MenuDirection, MenuSession}; use winit::keyboard::{Key, NamedKey}; pub(super) fn cancels(key: &Key, modifiers: winit::keyboard::ModifiersState) -> bool { @@ -18,14 +18,14 @@ pub(super) fn key( } match key.as_ref() { Key::Named(NamedKey::ArrowDown) => { - session.move_hover(1); + session.move_hover(MenuDirection::Next); } Key::Named(NamedKey::ArrowUp) => { - session.move_hover(-1); + session.move_hover(MenuDirection::Previous); } Key::Named(NamedKey::ArrowRight) => { if session.open_submenu() { - session.move_hover(1); + session.move_hover(MenuDirection::Next); } } Key::Named(NamedKey::ArrowLeft) => { @@ -33,11 +33,11 @@ pub(super) fn key( } Key::Named(NamedKey::Home) => { session.active_panel_mut().hover_index = -1; - session.move_hover(1); + session.move_hover(MenuDirection::Next); } Key::Named(NamedKey::End) => { session.active_panel_mut().hover_index = -1; - session.move_hover(-1); + session.move_hover(MenuDirection::Previous); } Key::Named(NamedKey::Enter) => { return session.activate_panel(session.submenu_panels.len()); diff --git a/crates/neomacs-display-runtime/src/menus/session.rs b/crates/neomacs-display-runtime/src/menus/session.rs index 83500f8a24..71292d78f9 100644 --- a/crates/neomacs-display-runtime/src/menus/session.rs +++ b/crates/neomacs-display-runtime/src/menus/session.rs @@ -77,6 +77,14 @@ impl MenuLifetime { } } +/// Keyboard traversal through the selectable items in a menu panel. +#[derive(Clone, Copy, Debug, PartialEq, Eq, num_enum::IntoPrimitive)] +#[repr(i32)] +pub(super) enum MenuDirection { + Next = 1, + Previous = -1, +} + pub struct MenuSession { menu: MeasuredMenu, /// The main (root) menu panel @@ -178,7 +186,8 @@ impl MenuSession { } /// Move hover in the active panel. Returns true if changed. - pub fn move_hover(&mut self, direction: i32) -> bool { + pub(super) fn move_hover(&mut self, direction: MenuDirection) -> bool { + let step: i32 = direction.into(); // Read panel state without mutable borrow let panel = self.active_panel(); let len = panel.item_indices.len() as i32; @@ -188,7 +197,7 @@ impl MenuSession { let current_hover = panel.hover_index; let indices: Vec = panel.item_indices.clone(); - let mut idx = current_hover + direction; + let mut idx = current_hover + step; for _ in 0..len { if idx < 0 { idx = len - 1; @@ -205,7 +214,7 @@ impl MenuSession { } return false; } - idx += direction; + idx += step; } false } diff --git a/crates/neomacs-display-runtime/src/menus/session/tests/session_test.rs b/crates/neomacs-display-runtime/src/menus/session/tests/session_test.rs index da4560a428..be3b5caf9b 100644 --- a/crates/neomacs-display-runtime/src/menus/session/tests/session_test.rs +++ b/crates/neomacs-display-runtime/src/menus/session/tests/session_test.rs @@ -521,7 +521,7 @@ fn move_hover_down_from_none() { item("C", true, 0), ]); // hover starts at -1, moving down (+1) should go to index 0 - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); assert_eq!(state.root_panel.hover_index, 0); } @@ -532,10 +532,10 @@ fn move_hover_down_sequential() { item("B", true, 0), item("C", true, 0), ]); - state.move_hover(1); // -> 0 - assert!(state.move_hover(1)); // -> 1 + state.move_hover(MenuDirection::Next); // -> 0 + assert!(state.move_hover(MenuDirection::Next)); // -> 1 assert_eq!(state.root_panel.hover_index, 1); - assert!(state.move_hover(1)); // -> 2 + assert!(state.move_hover(MenuDirection::Next)); // -> 2 assert_eq!(state.root_panel.hover_index, 2); } @@ -544,7 +544,7 @@ fn move_hover_wraps_around_bottom() { let mut state = simple_menu(vec![item("A", true, 0), item("B", true, 0)]); state.root_panel.hover_index = 1; // Moving down from last should wrap to 0. - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); assert_eq!(state.root_panel.hover_index, 0); } @@ -553,7 +553,7 @@ fn move_hover_wraps_around_top() { let mut state = simple_menu(vec![item("A", true, 0), item("B", true, 0)]); state.root_panel.hover_index = 0; // Moving up from first should wrap to last. - assert!(state.move_hover(-1)); + assert!(state.move_hover(MenuDirection::Previous)); assert_eq!(state.root_panel.hover_index, 1); } @@ -561,7 +561,7 @@ fn move_hover_wraps_around_top() { fn move_hover_skips_separators() { let mut state = simple_menu(vec![item("A", true, 0), separator(0), item("B", true, 0)]); state.root_panel.hover_index = 0; - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); // Should skip separator at index 1 and land on index 2. assert_eq!(state.root_panel.hover_index, 2); } @@ -574,7 +574,7 @@ fn move_hover_skips_disabled_items() { item("C", true, 0), ]); state.root_panel.hover_index = 0; - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); // Should skip disabled item at index 1. assert_eq!(state.root_panel.hover_index, 2); } @@ -583,27 +583,27 @@ fn move_hover_skips_disabled_items() { fn move_hover_empty_panel() { // No depth-0 items means root panel is empty. let mut state = simple_menu(vec![item("Child", true, 1)]); - assert!(!state.move_hover(1)); - assert!(!state.move_hover(-1)); + assert!(!state.move_hover(MenuDirection::Next)); + assert!(!state.move_hover(MenuDirection::Previous)); } #[test] fn move_hover_all_disabled_returns_false() { let mut state = simple_menu(vec![item("A", false, 0), item("B", false, 0)]); - assert!(!state.move_hover(1)); + assert!(!state.move_hover(MenuDirection::Next)); } #[test] fn move_hover_all_separators_returns_false() { let mut state = simple_menu(vec![separator(0), separator(0)]); - assert!(!state.move_hover(1)); + assert!(!state.move_hover(MenuDirection::Next)); } #[test] fn move_hover_single_enabled_item_from_none() { let mut state = simple_menu(vec![item("Only", true, 0)]); // From -1 moving down lands on 0. - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); assert_eq!(state.root_panel.hover_index, 0); } @@ -612,7 +612,7 @@ fn move_hover_single_enabled_item_already_there() { let mut state = simple_menu(vec![item("Only", true, 0)]); state.root_panel.hover_index = 0; // Moving from the only item should not change (wraps back to same). - assert!(!state.move_hover(1)); + assert!(!state.move_hover(MenuDirection::Next)); assert_eq!(state.root_panel.hover_index, 0); } @@ -623,8 +623,8 @@ fn move_hover_up_from_none() { item("B", true, 0), item("C", true, 0), ]); - // hover = -1, direction = -1 => idx = -2 => wraps to len-1 = 2 - assert!(state.move_hover(-1)); + // Moving backward from no hover selects the last item. + assert!(state.move_hover(MenuDirection::Previous)); assert_eq!(state.root_panel.hover_index, 2); } @@ -790,14 +790,14 @@ fn move_hover_in_submenu() { assert_eq!(state.active_panel().hover_index, -1); // Move down in submenu - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); assert_eq!(state.active_panel().hover_index, 0); // "File1.txt" - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); assert_eq!(state.active_panel().hover_index, 1); // "File2.txt" - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); assert_eq!(state.active_panel().hover_index, 2); // "File3.txt" // Wraps back - assert!(state.move_hover(1)); + assert!(state.move_hover(MenuDirection::Next)); assert_eq!(state.active_panel().hover_index, 0); } diff --git a/crates/neomacs-display-runtime/src/menus/tests/native_test.rs b/crates/neomacs-display-runtime/src/menus/tests/native_test.rs index 11cbc08e50..fe248f2d8e 100644 --- a/crates/neomacs-display-runtime/src/menus/tests/native_test.rs +++ b/crates/neomacs-display-runtime/src/menus/tests/native_test.rs @@ -1,5 +1,6 @@ //! Opt-in compositor test. Run with WAYLAND_DEBUG=1 to inspect xdg_popup roles. +use super::session::MenuDirection; use super::{MenuPresentation, MenuRequest, MenuSession}; use crate::presentation::PopupCommit; use std::collections::HashSet; @@ -342,7 +343,7 @@ impl ApplicationHandler for Smoke { &gpu.queue, ); let mut session = MenuSession::new(0.0, 0.0, menu, 14.0, 18.0); - session.move_hover(1); + session.move_hover(MenuDirection::Next); assert!(session.open_submenu()); self.menus.open(MenuRequest { tooltips: self.with_tooltips.then(|| {