Settings: fix mouse desync in scrolled search results and wheel reach of the last result - #2950
Merged
Conversation
Only the visible search-result rows are registered in the settings layout, so `hit_test` returned a viewport-slot index (0 = first visible row) while its consumers — the hover compare in the renderer and the click handler — treat the value as an absolute index into the state's `search_results`. Once the list was scrolled, hover highlighted and clicks activated the result `search_scroll_offset` rows above the pointer (issue #2860). Store the absolute result index on each registered row and return that from `hit_test`, so hover and click stay in the same index space as the state no matter how far the list is scrolled. The web frontend already sends absolute indices for its `searchResult` hits, so its behavior is unchanged. Fixes half of #2860; the wheel-scroll clamp half is a separate commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
Wheel-scrolling the settings search results clamps the scroll offset at `len - max_visible` and pins the selection to the viewport edge, so at the end of the list further wheel-downs were no-ops and the selection could never land on the last result (issue #2860) — while keyboard navigation reaches it fine. Once the viewport is already at the bottom, keep advancing the selection on each wheel-down until it reaches the last result, and symmetrically walk the selection up to the first result when wheeling up at the top, matching the keyboard behavior. Fixes the second half of #2860. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
…icks Two rendered-output reproducers for the #2860 mouse desync (click a result row after a wheel notch and land on exactly that setting; wheel past the end of the list and watch the selection marker walk onto the last visible result) — both fail without the two preceding fixes. Also add regression coverage for the remaining mouse half of #1112: clicking a Toggle's checkbox flips it and clicking a Dropdown's button opens the option list. Investigating that issue showed the behavior already works (the widget-derived hit rects are registered correctly); only the Number value-cell click had e2e coverage, so pin down the other two scalar controls too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
sinelaw
force-pushed
the
claude/fix-settings-mouse
branch
from
August 10, 2026 15:18
192987f to
20d5dc8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2860
Refs #1112
Motivation
In the Settings dialog's search mode, mouse interaction desyncs as soon as the result list is scrolled (#2860):
hit_testreturned a viewport-slot index (0 = first visible row) while its consumers (hover compare in the renderer, click handler) treat the value as an absolute index intosearch_results. After scrolling, hover highlighted — and clicks activated — the resultsearch_scroll_offsetrows above the pointer.len - max_visibleand pins the selection to the viewport edge, so at the end of the list further wheel-downs were no-ops and the last result could never be selected with the wheel (keyboard reaches it fine).The fix
SearchResultLayoutnow stores the absolute result index each visible row was registered with, andhit_testreturns that, so hover/click stay in the same index space as the state regardless of scrolling. The web frontend already sends absolute indices forsearchResulthits, so it is unaffected.search_scroll_downkeeps advancing the selection once the viewport is at the bottom (until the last result), andsearch_scroll_upsymmetrically walks the selection up to the first result at the top — matching keyboard navigation.#1112 (remaining mouse half)
The recent status sweep on #1112 reported that clicks on scalar controls (Number value cell, Toggle checkbox, Dropdown button) are inert. I could not reproduce that at HEAD: driving the debug build in tmux (200x50 and 120x40) and in the e2e harness, clicking the value cell starts inline editing, clicking a checkbox flips it, and clicking a dropdown button opens the inline option list — the widget-derived hit rects are registered at the correct screen cells. (The
-/+number buttons from the original report were removed by design in favor of direct typing.) Since only the Number value-cell click had e2e coverage, this PR adds regression coverage for the Toggle and Dropdown click paths so the working behavior is pinned down.Tests
New reproducers (all fail without the fix commits, pass with them):
view::settings::layout::tests::test_hit_test_search_result_scrolled_uses_absolute_index(unit)view::settings::state::tests::test_search_wheel_down_reaches_last_result,test_search_wheel_up_reaches_first_result(unit)e2e::settings::test_settings_search_result_click_after_wheel_scroll,test_settings_search_wheel_selects_last_result(e2e, rendered-output assertions only)New regression coverage (passes before and after; pins existing behavior for #1112):
e2e::settings::test_settings_toggle_checkbox_click_flips_value,test_settings_dropdown_button_click_opens_optionsRan:
cargo test -p fresh-editor --lib view::settings— 112 passedcargo test -p fresh-editor --test e2e_tests -- e2e::settings— 124 passed, 0 failed (2 pre-existing ignored)cargo check --all-targets -p fresh-editor,cargo fmt,cargo clippy -p fresh-editor --tests— clean on touched files🤖 Generated with Claude Code
https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
Generated by Claude Code