refactor(runtime): type menu navigation directions

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

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,26 +18,26 @@ 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) => {
             session.close_submenu();
         }
         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());

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<usize> = 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
     }

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,15 +553,15 @@ 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);
 }
 
 #[test]
 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);
 }
 

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(|| {