Settings: fix mouse desync in scrolled search results and wheel reach of the last result

crates/fresh-editor/src/view/settings/layout.rs

@@ -50,6 +50,11 @@ pub struct SettingsLayout {
 /// Layout info for a search result
 #[derive(Debug, Clone)]
 pub struct SearchResultLayout {
+    /// Absolute index into the state's `search_results` list. Only the
+    /// visible rows are registered in the layout, so this is NOT the same
+    /// as the position within `SettingsLayout::search_results` once the
+    /// list is scrolled (#2860).
+    pub result_index: usize,
     /// Page index (category)
     pub page_index: usize,
     /// Item index within the page
@@ -131,9 +136,18 @@ impl SettingsLayout {
         });
     }
 
-    /// Add a search result to the layout
-    pub fn add_search_result(&mut self, page_index: usize, item_index: usize, area: Rect) {
+    /// Add a search result to the layout. `result_index` is the absolute
+    /// index into the state's `search_results` list (not the on-screen
+    /// slot), so hit-testing keeps working when the list is scrolled.
+    pub fn add_search_result(
+        &mut self,
+        result_index: usize,
+        page_index: usize,
+        item_index: usize,
+        area: Rect,
+    ) {
         self.search_results.push(SearchResultLayout {
+            result_index,
             page_index,
             item_index,
             area,
@@ -215,10 +229,14 @@ impl SettingsLayout {
             }
         }
 
-        // Check search results (before regular items, since they replace the item list during search)
-        for (idx, result) in self.search_results.iter().enumerate() {
+        // Check search results (before regular items, since they replace the
+        // item list during search). The hit carries the ABSOLUTE result
+        // index: only visible rows are registered here, so the position in
+        // this vec is a viewport slot and would be off by the scroll offset
+        // once the list is scrolled (#2860).
+        for result in &self.search_results {
             if point_in_rect(result.area, x, y) {
-                return Some(SettingsHit::SearchResult(idx));
+                return Some(SettingsHit::SearchResult(result.result_index));
             }
         }
 
@@ -392,7 +410,8 @@ pub enum SettingsHit {
     CategoriesScrollbar,
     /// Click on a setting item (index)
     Item(usize),
-    /// Click on a search result (index in search_results)
+    /// Click on a search result (absolute index into the state's
+    /// `search_results`, not the on-screen slot)
     SearchResult(usize),
     /// Click on toggle control
     ControlToggle(usize),
@@ -520,6 +539,27 @@ mod tests {
         assert_eq!(layout.hit_test(40, 11), Some(SettingsHit::Item(0)));
     }
 
+    /// Reproducer for issue #2860: only VISIBLE search results are registered
+    /// in the layout, so when the list is scrolled the first registered row
+    /// is not result 0. `hit_test` must report the absolute result index the
+    /// row was registered with, not the row's position in the layout vec —
+    /// otherwise hover and click resolve to a result `scroll_offset` rows
+    /// above the pointer.
+    #[test]
+    fn test_hit_test_search_result_scrolled_uses_absolute_index() {
+        let modal = Rect::new(0, 0, 100, 40);
+        let mut layout = SettingsLayout::new(modal);
+
+        // Scrolled viewport: visible rows are results 3, 4, 5 (3 rows each).
+        layout.add_search_result(3, 0, 3, Rect::new(25, 3, 70, 3));
+        layout.add_search_result(4, 0, 4, Rect::new(25, 6, 70, 3));
+        layout.add_search_result(5, 0, 5, Rect::new(25, 9, 70, 3));
+
+        assert_eq!(layout.hit_test(30, 4), Some(SettingsHit::SearchResult(3)));
+        assert_eq!(layout.hit_test(30, 7), Some(SettingsHit::SearchResult(4)));
+        assert_eq!(layout.hit_test(30, 10), Some(SettingsHit::SearchResult(5)));
+    }
+
     /// Reproducer for issue #1825: clicking on the value area between the
     /// brackets of a Number control used to return `Item`, which only
     /// changes selection. It must now return a dedicated hit that the mouse

crates/fresh-editor/src/view/settings/render.rs

@@ -2579,6 +2579,7 @@ fn render_search_results(
             frame,
             item_area,
             result,
+            idx,
             is_selected,
             is_hovered,
             theme,
@@ -2615,11 +2616,15 @@ fn render_search_results(
     }
 }
 
-/// Render a single search result with breadcrumb
+/// Render a single search result with breadcrumb. `result_index` is the
+/// absolute index into the state's `search_results` (needed for hit-testing
+/// because only the visible rows get registered in the layout).
+#[allow(clippy::too_many_arguments)]
 fn render_search_result_item(
     frame: &mut Frame,
     area: Rect,
     result: &SearchResult,
+    result_index: usize,
     is_selected: bool,
     is_hovered: bool,
     theme: &Theme,
@@ -2717,7 +2722,7 @@ fn render_search_result_item(
     }
 
     // Track this item in layout
-    layout.add_search_result(result.page_index, result.item_index, area);
+    layout.add_search_result(result_index, result.page_index, result.item_index, area);
 }
 
 /// Build a line with highlighted match positions

crates/fresh-editor/src/view/settings/state.rs

@@ -1562,9 +1562,19 @@ impl SettingsState {
 
     /// Scroll search results up by delta items
     pub fn search_scroll_up(&mut self, delta: usize) -> bool {
-        if self.search_results.is_empty() || self.search_scroll_offset == 0 {
+        if self.search_results.is_empty() {
             return false;
         }
+        if self.search_scroll_offset == 0 {
+            // Viewport already at the top: keep walking the selection up so
+            // the wheel can reach (and select) the first result, matching
+            // keyboard navigation (#2860).
+            if self.selected_search_result == 0 {
+                return false;
+            }
+            self.selected_search_result = self.selected_search_result.saturating_sub(delta);
+            return true;
+        }
         self.search_scroll_offset = self.search_scroll_offset.saturating_sub(delta);
         // Keep selection visible
         if self.selected_search_result >= self.search_scroll_offset + self.search_max_visible {
@@ -1583,7 +1593,15 @@ impl SettingsState {
             .len()
             .saturating_sub(self.search_max_visible);
         if self.search_scroll_offset >= max_offset {
-            return false;
+            // Viewport already at the bottom: keep walking the selection down
+            // so the wheel can reach (and select) the last result, matching
+            // keyboard navigation (#2860).
+            let last = self.search_results.len() - 1;
+            if self.selected_search_result >= last {
+                return false;
+            }
+            self.selected_search_result = (self.selected_search_result + delta).min(last);
+            return true;
         }
         self.search_scroll_offset = (self.search_scroll_offset + delta).min(max_offset);
         // Keep selection visible
@@ -4236,4 +4254,88 @@ mod tests {
         state.search_delete();
         assert_eq!(state.search_query(), "ที่");
     }
+
+    /// Schema with enough same-prefix settings that a search for "opt" has
+    /// more results than a small viewport can show — used to exercise the
+    /// mouse-wheel scroll clamping of the search-result list.
+    const TEST_SCHEMA_MANY_OPTS: &str = r#"
+{
+  "type": "object",
+  "properties": {
+    "opt_a": { "type": "boolean", "default": true },
+    "opt_b": { "type": "boolean", "default": true },
+    "opt_c": { "type": "boolean", "default": true },
+    "opt_d": { "type": "boolean", "default": true },
+    "opt_e": { "type": "boolean", "default": true },
+    "opt_f": { "type": "boolean", "default": true }
+  },
+  "$defs": {}
+}
+"#;
+
+    fn search_scroll_state() -> SettingsState {
+        let config = test_config();
+        let mut state = SettingsState::new(TEST_SCHEMA_MANY_OPTS, &config).unwrap();
+        state.show();
+        state.search_active = true;
+        for c in "opt".chars() {
+            state.search_push_char(c);
+        }
+        assert!(
+            state.search_results.len() >= 6,
+            "expected all opt_* settings to match, got {}",
+            state.search_results.len()
+        );
+        // Small viewport: 2 visible rows, so the list scrolls.
+        state.search_max_visible = 2;
+        state
+    }
+
+    /// Reproducer for issue #2860 (wheel can't reach the last result):
+    /// wheel-down used to clamp the scroll offset at `len - max_visible` and
+    /// pin the selection to the viewport top, so the selection could never
+    /// land on the last result. Once the viewport is at the bottom, further
+    /// wheel-downs must keep advancing the selection — matching keyboard
+    /// navigation, which reaches the last item fine.
+    #[test]
+    fn test_search_wheel_down_reaches_last_result() {
+        let mut state = search_scroll_state();
+        let last = state.search_results.len() - 1;
+
+        // Wheel down more than enough notches to pass the end of the list.
+        for _ in 0..(2 * state.search_results.len()) {
+            state.search_scroll_down(1);
+        }
+
+        assert_eq!(
+            state.selected_search_result, last,
+            "wheel-down must be able to select the last search result"
+        );
+        // One more wheel-down at the very end is a no-op.
+        assert!(!state.search_scroll_down(1));
+        assert_eq!(state.selected_search_result, last);
+    }
+
+    /// Symmetric to `test_search_wheel_down_reaches_last_result`: once the
+    /// viewport is back at the top, further wheel-ups keep moving the
+    /// selection until it reaches the first result.
+    #[test]
+    fn test_search_wheel_up_reaches_first_result() {
+        let mut state = search_scroll_state();
+
+        // Go all the way down first, then all the way back up.
+        for _ in 0..(2 * state.search_results.len()) {
+            state.search_scroll_down(1);
+        }
+        for _ in 0..(2 * state.search_results.len()) {
+            state.search_scroll_up(1);
+        }
+
+        assert_eq!(state.search_scroll_offset, 0);
+        assert_eq!(
+            state.selected_search_result, 0,
+            "wheel-up must be able to select the first search result"
+        );
+        assert!(!state.search_scroll_up(1));
+    }
 }