Centralize vertical navigation in TUI list views (#54081)

## What changed

Add `ListKeymap::vertical_action_for` and `ScrollState::apply_vertical_action` to share key resolution, selection movement, and visibility handling across the experimental features, hooks, memories, skills, and multi-select views. Preserve navigation priority, search input handling, and item reordering precedence.

## Testing

Update scroll-state tests to exercise the shared action handler and picker tests to navigate through key events. Cover overlapping page-down and move-left bindings when item reordering is disabled.

GitOrigin-RevId: 29927b0a4de0fbeb4c0dd19c418b8f60fffb8a7d

codex-rs/tui/src/bottom_pane/experimental_features_view.rs

@@ -163,48 +163,6 @@ impl ExperimentalFeaturesView {
         rows
     }
 
-    fn move_up(&mut self) {
-        let len = self.visible_len();
-        if len == 0 {
-            return;
-        }
-        self.state.move_up_wrap(len);
-        self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
-    }
-
-    fn move_down(&mut self) {
-        let len = self.visible_len();
-        if len == 0 {
-            return;
-        }
-        self.state.move_down_wrap(len);
-        self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
-    }
-
-    fn page_up(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.state.page_up_clamped(len, visible);
-    }
-
-    fn page_down(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.state.page_down_clamped(len, visible);
-    }
-
-    fn jump_top(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.state.jump_top(len, visible);
-    }
-
-    fn jump_bottom(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.state.jump_bottom(len, visible);
-    }
-
     fn toggle_selected(&mut self) {
         let Some(selected_idx) = self.state.selected_idx else {
             return;
@@ -375,13 +333,22 @@ impl BottomPaneView for ExperimentalFeaturesView {
     }
 
     fn handle_key_event(&mut self, key_event: KeyEvent) {
+        if let Some(action) = self.keymap.vertical_action_for(key_event) {
+            let len = self.visible_len();
+            if len == 0
+                && matches!(
+                    action,
+                    crate::keymap::ListAction::MoveUp | crate::keymap::ListAction::MoveDown
+                )
+            {
+                return;
+            }
+            self.state
+                .apply_vertical_action(action, len, MAX_POPUP_ROWS.min(len));
+            return;
+        }
+
         match key_event {
-            _ if self.keymap.move_up.is_pressed(key_event) => self.move_up(),
-            _ if self.keymap.move_down.is_pressed(key_event) => self.move_down(),
-            _ if self.keymap.page_up.is_pressed(key_event) => self.page_up(),
-            _ if self.keymap.page_down.is_pressed(key_event) => self.page_down(),
-            _ if self.keymap.jump_top.is_pressed(key_event) => self.jump_top(),
-            _ if self.keymap.jump_bottom.is_pressed(key_event) => self.jump_bottom(),
             KeyEvent {
                 code: KeyCode::Char(' '),
                 modifiers: KeyModifiers::NONE,

codex-rs/tui/src/bottom_pane/hooks_browser_view.rs

@@ -165,38 +165,6 @@ impl HooksBrowserView {
             .and_then(|idx| self.entry.hooks.get(idx))
     }
 
-    fn move_up(&mut self) {
-        let len = self.page_len();
-        self.state.move_up_wrap(len);
-        self.state.ensure_visible(len, self.max_visible_rows());
-    }
-
-    fn move_down(&mut self) {
-        let len = self.page_len();
-        self.state.move_down_wrap(len);
-        self.state.ensure_visible(len, self.max_visible_rows());
-    }
-
-    fn page_up(&mut self) {
-        let len = self.page_len();
-        self.state.page_up_clamped(len, self.max_visible_rows());
-    }
-
-    fn page_down(&mut self) {
-        let len = self.page_len();
-        self.state.page_down_clamped(len, self.max_visible_rows());
-    }
-
-    fn jump_top(&mut self) {
-        let len = self.page_len();
-        self.state.jump_top(len, self.max_visible_rows());
-    }
-
-    fn jump_bottom(&mut self) {
-        let len = self.page_len();
-        self.state.jump_bottom(len, self.max_visible_rows());
-    }
-
     fn page_len(&self) -> usize {
         match self.page {
             HooksBrowserPage::Events => codex_protocol::protocol::HookEventName::iter().count(),
@@ -204,10 +172,6 @@ impl HooksBrowserView {
         }
     }
 
-    fn max_visible_rows(&self) -> usize {
-        MAX_POPUP_ROWS.min(self.page_len().max(1))
-    }
-
     fn open_selected_event(&mut self) {
         let Some(event_name) = self.selected_event() else {
             return;
@@ -613,13 +577,14 @@ impl BottomPaneView for HooksBrowserView {
     }
 
     fn handle_key_event(&mut self, key_event: KeyEvent) {
+        if let Some(action) = self.keymap.vertical_action_for(key_event) {
+            let len = self.page_len();
+            self.state
+                .apply_vertical_action(action, len, MAX_POPUP_ROWS.min(len.max(1)));
+            return;
+        }
+
         match key_event {
-            _ if self.keymap.move_up.is_pressed(key_event) => self.move_up(),
-            _ if self.keymap.move_down.is_pressed(key_event) => self.move_down(),
-            _ if self.keymap.page_up.is_pressed(key_event) => self.page_up(),
-            _ if self.keymap.page_down.is_pressed(key_event) => self.page_down(),
-            _ if self.keymap.jump_top.is_pressed(key_event) => self.jump_top(),
-            _ if self.keymap.jump_bottom.is_pressed(key_event) => self.jump_bottom(),
             _ if self.keymap.accept.is_pressed(key_event)
                 && self.page == HooksBrowserPage::Events =>
             {

codex-rs/tui/src/bottom_pane/memories_settings_view.rs

@@ -223,50 +223,6 @@ impl MemoriesSettingsView {
             .collect()
     }
 
-    fn move_up(&mut self) {
-        let len = self.visible_len();
-        if len == 0 {
-            return;
-        }
-        let state = self.active_state_mut();
-        state.move_up_wrap(len);
-        state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
-    }
-
-    fn move_down(&mut self) {
-        let len = self.visible_len();
-        if len == 0 {
-            return;
-        }
-        let state = self.active_state_mut();
-        state.move_down_wrap(len);
-        state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
-    }
-
-    fn page_up(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.active_state_mut().page_up_clamped(len, visible);
-    }
-
-    fn page_down(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.active_state_mut().page_down_clamped(len, visible);
-    }
-
-    fn jump_top(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.active_state_mut().jump_top(len, visible);
-    }
-
-    fn jump_bottom(&mut self) {
-        let len = self.visible_len();
-        let visible = MAX_POPUP_ROWS.min(len);
-        self.active_state_mut().jump_bottom(len, visible);
-    }
-
     fn toggle_selected(&mut self) {
         if self.reset_confirmation.is_some() {
             return;
@@ -321,13 +277,22 @@ impl BottomPaneView for MemoriesSettingsView {
     }
 
     fn handle_key_event(&mut self, key_event: KeyEvent) {
+        if let Some(action) = self.keymap.vertical_action_for(key_event) {
+            let len = self.visible_len();
+            if len == 0
+                && matches!(
+                    action,
+                    crate::keymap::ListAction::MoveUp | crate::keymap::ListAction::MoveDown
+                )
+            {
+                return;
+            }
+            self.active_state_mut()
+                .apply_vertical_action(action, len, MAX_POPUP_ROWS.min(len));
+            return;
+        }
+
         match key_event {
-            _ if self.keymap.move_up.is_pressed(key_event) => self.move_up(),
-            _ if self.keymap.move_down.is_pressed(key_event) => self.move_down(),
-            _ if self.keymap.page_up.is_pressed(key_event) => self.page_up(),
-            _ if self.keymap.page_down.is_pressed(key_event) => self.page_down(),
-            _ if self.keymap.jump_top.is_pressed(key_event) => self.jump_top(),
-            _ if self.keymap.jump_bottom.is_pressed(key_event) => self.jump_bottom(),
             KeyEvent {
                 code: KeyCode::Char(' '),
                 modifiers: KeyModifiers::NONE,

codex-rs/tui/src/bottom_pane/multi_select_picker.rs

@@ -353,46 +353,6 @@ impl MultiSelectPicker {
         }
     }
 
-    /// Moves the selection cursor up, wrapping to the bottom if at the top.
-    fn move_up(&mut self) {
-        let len = self.visible_len();
-        self.state.move_up_wrap(len);
-        let visible = Self::max_visible_rows(len);
-        self.state.ensure_visible(len, visible);
-    }
-
-    /// Moves the selection cursor down, wrapping to the top if at the bottom.
-    fn move_down(&mut self) {
-        let len = self.visible_len();
-        self.state.move_down_wrap(len);
-        let visible = Self::max_visible_rows(len);
-        self.state.ensure_visible(len, visible);
-    }
-
-    fn page_up(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.page_up_clamped(len, visible);
-    }
-
-    fn page_down(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.page_down_clamped(len, visible);
-    }
-
-    fn jump_top(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.jump_top(len, visible);
-    }
-
-    fn jump_bottom(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.jump_bottom(len, visible);
-    }
-
     /// Toggles the enabled state of the currently selected item.
     ///
     /// Updates the preview line and invokes the `on_change` callback if set.
@@ -543,37 +503,30 @@ impl BottomPaneView for MultiSelectPicker {
         // plain j/k only apply through non-text events or modified bindings.
         let allow_plain_char_navigation = !is_plain_text_key_event(key_event);
 
+        if allow_plain_char_navigation
+            && self.ordering_enabled
+            && self.keymap.move_left.is_pressed(key_event)
+        {
+            self.move_selected_item(Direction::Up);
+            return;
+        }
+        if allow_plain_char_navigation
+            && self.ordering_enabled
+            && self.keymap.move_right.is_pressed(key_event)
+        {
+            self.move_selected_item(Direction::Down);
+            return;
+        }
+        if allow_plain_char_navigation
+            && let Some(action) = self.keymap.vertical_action_for(key_event)
+        {
+            let len = self.visible_len();
+            self.state
+                .apply_vertical_action(action, len, Self::max_visible_rows(len));
+            return;
+        }
+
         match key_event {
-            _ if allow_plain_char_navigation
-                && self.ordering_enabled
-                && self.keymap.move_left.is_pressed(key_event) =>
-            {
-                self.move_selected_item(Direction::Up);
-            }
-            _ if allow_plain_char_navigation
-                && self.ordering_enabled
-                && self.keymap.move_right.is_pressed(key_event) =>
-            {
-                self.move_selected_item(Direction::Down);
-            }
-            _ if allow_plain_char_navigation && self.keymap.move_up.is_pressed(key_event) => {
-                self.move_up()
-            }
-            _ if allow_plain_char_navigation && self.keymap.move_down.is_pressed(key_event) => {
-                self.move_down()
-            }
-            _ if allow_plain_char_navigation && self.keymap.page_up.is_pressed(key_event) => {
-                self.page_up()
-            }
-            _ if allow_plain_char_navigation && self.keymap.page_down.is_pressed(key_event) => {
-                self.page_down()
-            }
-            _ if allow_plain_char_navigation && self.keymap.jump_top.is_pressed(key_event) => {
-                self.jump_top()
-            }
-            _ if allow_plain_char_navigation && self.keymap.jump_bottom.is_pressed(key_event) => {
-                self.jump_bottom()
-            }
             KeyEvent {
                 code: KeyCode::Backspace,
                 ..
@@ -958,7 +911,7 @@ mod tests {
             vec!["theme-colors", "model", "branch"]
         );
 
-        picker.move_down();
+        picker.handle_key_event(KeyEvent::from(KeyCode::Down));
         picker.move_selected_item(Direction::Up);
         assert_eq!(
             picker
@@ -1050,6 +1003,7 @@ mod tests {
         let (tx, _rx) = unbounded_channel::<AppEvent>();
         let mut keymap = RuntimeKeymap::defaults().list;
         keymap.page_down = vec![key_hint::ctrl(KeyCode::Char('d'))];
+        keymap.move_left = keymap.page_down.clone();
         keymap.page_up = vec![key_hint::ctrl(KeyCode::Char('u'))];
         keymap.jump_bottom = vec![key_hint::ctrl(KeyCode::Char('e'))];
         keymap.jump_top = vec![key_hint::ctrl(KeyCode::Char('a'))];
@@ -1063,7 +1017,7 @@ mod tests {
                 .map(|idx| {
                     item(
                         &format!("item-{idx}"),
-                        /*orderable*/ true,
+                        /*orderable*/ false,
                         /*section_break_after*/ false,
                     )
                 })
@@ -1102,7 +1056,7 @@ mod tests {
                 .collect(),
         );
         picker.header = Box::new(());
-        picker.jump_bottom();
+        picker.handle_key_event(KeyEvent::from(KeyCode::End));
         let mut snapshots = Vec::new();
         for height in 1..=7 {
             let area = Rect::new(/*x*/ 0, /*y*/ 0, /*width*/ 80, height);
@@ -1183,7 +1137,7 @@ mod tests {
             buf
         };
         let first = render(&picker);
-        picker.jump_bottom();
+        picker.handle_key_event(KeyEvent::from(KeyCode::End));
         picker.toggle_selected();
         picker.move_selected_item(Direction::Up);
         let last = render(&picker);

codex-rs/tui/src/bottom_pane/scroll_state.rs

@@ -1,3 +1,5 @@
+use crate::keymap::ListAction;
+
 /// Generic scroll/selection state for a vertical list menu.
 ///
 /// Encapsulates the common behavior of a selectable list that supports:
@@ -25,6 +27,28 @@ impl ScrollState {
         }
     }
 
+    /// Apply a vertical list action and keep the resulting selection visible.
+    pub fn apply_vertical_action(&mut self, action: ListAction, len: usize, visible_rows: usize) {
+        match action {
+            ListAction::MoveUp => {
+                self.move_up_wrap(len);
+                self.ensure_visible(len, visible_rows);
+            }
+            ListAction::MoveDown => {
+                self.move_down_wrap(len);
+                self.ensure_visible(len, visible_rows);
+            }
+            ListAction::PageUp => self.page_up_clamped(len, visible_rows),
+            ListAction::PageDown => self.page_down_clamped(len, visible_rows),
+            ListAction::JumpTop => self.jump_top(len, visible_rows),
+            ListAction::JumpBottom => self.jump_bottom(len, visible_rows),
+            ListAction::MoveLeft
+            | ListAction::MoveRight
+            | ListAction::Accept
+            | ListAction::Cancel => {}
+        }
+    }
+
     /// Reset selection and scroll.
     pub fn reset(&mut self) {
         self.selected_idx = None;
@@ -144,6 +168,7 @@ impl ScrollState {
 #[cfg(test)]
 mod tests {
     use super::ScrollState;
+    use crate::keymap::ListAction;
 
     #[test]
     fn wrap_navigation_and_visibility() {
@@ -156,16 +181,14 @@ mod tests {
         s.ensure_visible(len, vis);
         assert_eq!(s.scroll_top, 0);
 
-        s.move_up_wrap(len);
-        s.ensure_visible(len, vis);
+        s.apply_vertical_action(ListAction::MoveUp, len, vis);
         assert_eq!(s.selected_idx, Some(len - 1));
         match s.selected_idx {
             Some(sel) => assert!(s.scroll_top <= sel),
             None => panic!("expected Some(selected_idx) after wrap"),
         }
 
-        s.move_down_wrap(len);
-        s.ensure_visible(len, vis);
+        s.apply_vertical_action(ListAction::MoveDown, len, vis);
         assert_eq!(s.selected_idx, Some(0));
         assert_eq!(s.scroll_top, 0);
     }
@@ -177,27 +200,27 @@ mod tests {
         let vis = 4;
 
         s.clamp_selection(len);
-        s.page_down_clamped(len, vis);
+        s.apply_vertical_action(ListAction::PageDown, len, vis);
         assert_eq!(s.selected_idx, Some(4));
         assert_eq!(s.scroll_top, 1);
 
-        s.page_down_clamped(len, vis);
+        s.apply_vertical_action(ListAction::PageDown, len, vis);
         assert_eq!(s.selected_idx, Some(8));
         assert_eq!(s.scroll_top, 5);
 
-        s.page_down_clamped(len, vis);
+        s.apply_vertical_action(ListAction::PageDown, len, vis);
         assert_eq!(s.selected_idx, Some(9));
         assert_eq!(s.scroll_top, 6);
 
-        s.page_up_clamped(len, vis);
+        s.apply_vertical_action(ListAction::PageUp, len, vis);
         assert_eq!(s.selected_idx, Some(5));
         assert_eq!(s.scroll_top, 5);
 
-        s.jump_top(len, vis);
+        s.apply_vertical_action(ListAction::JumpTop, len, vis);
         assert_eq!(s.selected_idx, Some(0));
         assert_eq!(s.scroll_top, 0);
 
-        s.jump_bottom(len, vis);
+        s.apply_vertical_action(ListAction::JumpBottom, len, vis);
         assert_eq!(s.selected_idx, Some(9));
         assert_eq!(s.scroll_top, 6);
     }

codex-rs/tui/src/bottom_pane/skills_toggle_view.rs

@@ -162,44 +162,6 @@ impl SkillsToggleView {
             .collect()
     }
 
-    fn move_up(&mut self) {
-        let len = self.visible_len();
-        self.state.move_up_wrap(len);
-        let visible = Self::max_visible_rows(len);
-        self.state.ensure_visible(len, visible);
-    }
-
-    fn move_down(&mut self) {
-        let len = self.visible_len();
-        self.state.move_down_wrap(len);
-        let visible = Self::max_visible_rows(len);
-        self.state.ensure_visible(len, visible);
-    }
-
-    fn page_up(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.page_up_clamped(len, visible);
-    }
-
-    fn page_down(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.page_down_clamped(len, visible);
-    }
-
-    fn jump_top(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.jump_top(len, visible);
-    }
-
-    fn jump_bottom(&mut self) {
-        let len = self.visible_len();
-        let visible = Self::max_visible_rows(len);
-        self.state.jump_bottom(len, visible);
-    }
-
     fn toggle_selected(&mut self) {
         let Some(idx) = self.state.selected_idx else {
             return;
@@ -241,25 +203,16 @@ impl BottomPaneView for SkillsToggleView {
         // plain j/k only apply through non-text events or modified bindings.
         let allow_plain_char_navigation = !is_plain_text_key_event(key_event);
 
+        if allow_plain_char_navigation
+            && let Some(action) = self.keymap.vertical_action_for(key_event)
+        {
+            let len = self.visible_len();
+            self.state
+                .apply_vertical_action(action, len, Self::max_visible_rows(len));
+            return;
+        }
+
         match key_event {
-            _ if allow_plain_char_navigation && self.keymap.move_up.is_pressed(key_event) => {
-                self.move_up()
-            }
-            _ if allow_plain_char_navigation && self.keymap.move_down.is_pressed(key_event) => {
-                self.move_down()
-            }
-            _ if allow_plain_char_navigation && self.keymap.page_up.is_pressed(key_event) => {
-                self.page_up()
-            }
-            _ if allow_plain_char_navigation && self.keymap.page_down.is_pressed(key_event) => {
-                self.page_down()
-            }
-            _ if allow_plain_char_navigation && self.keymap.jump_top.is_pressed(key_event) => {
-                self.jump_top()
-            }
-            _ if allow_plain_char_navigation && self.keymap.jump_bottom.is_pressed(key_event) => {
-                self.jump_bottom()
-            }
             KeyEvent {
                 code: KeyCode::Backspace,
                 ..

codex-rs/tui/src/keymap.rs

@@ -413,6 +413,20 @@ impl ListKeymap {
         .find(|action| self.bindings_for(*action).is_pressed(event))
     }
 
+    /// Resolve only vertical navigation, preserving its historical priority.
+    pub(crate) fn vertical_action_for(&self, event: KeyEvent) -> Option<ListAction> {
+        [
+            ListAction::MoveUp,
+            ListAction::MoveDown,
+            ListAction::PageUp,
+            ListAction::PageDown,
+            ListAction::JumpTop,
+            ListAction::JumpBottom,
+        ]
+        .into_iter()
+        .find(|action| self.bindings_for(*action).is_pressed(event))
+    }
+
     pub(crate) fn has_chord_prefix(&self, event: KeyEvent) -> bool {
         self.chord_hints.bindings.iter().any(|binding| {
             binding.action.context.overlaps(KeymapContext::List)