refactor: lift pointer hit-testing out of Editor into a pure layer - #2960
Open
sinelaw wants to merge 2 commits into
Open
refactor: lift pointer hit-testing out of Editor into a pure layer#2960sinelaw wants to merge 2 commits into
sinelaw wants to merge 2 commits into
Conversation
`mouse_input.rs` carried the "what is under the pointer?" ladder as `impl Editor` methods reading `self.active_layout()` / `self.active_chrome()` directly, so the precedence rules — which overlay wins where two overlap — could only be exercised by building a whole editor. They never were. Move the ladder into `app/hit_test.rs` as free functions over a view that names each rect and slice it reads: - `hover_target` = `floating_overlay_target` (context menus, suggestions, popups, file browser) then `chrome_target` (menu bar, dropdown, explorer, separators, split controls, tabs, scrollbars, status bar, search options). - `menu_dropdown_target` and `file_browser_target` join it; their former `impl Editor` wrappers in menu_actions.rs / file_open_input.rs now delegate, so there is one implementation of each probe rather than two. The view borrows exactly what the decision needs — not the layout caches themselves. That keeps a new layout field from silently drifting into the hit-test, and lets the tests construct precise values rather than blanking a cache and trusting the zeros. One probe cannot be pure: the file-explorer status indicator resolves its columns through the theme, the plugin decoration caches and the explorer renderer's slot layout. It stays on `Editor` as `explorer_status_indicator_at` and is passed in pre-resolved, so the *ordering* still lives in the ladder even though that probe's geometry does not. Behaviour-preserving: the ladder is the same sequence of the same comparisons. 10 new unit tests pin the precedence rules that had no coverage before — overlay-over-chrome, topmost-popup-wins, scroll-offset suggestion rows, explorer close button vs indicator vs border, split controls on the tab row, scrollbar thumb vs track, separator orientation, status-bar span. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014cyMxXzFLeS94CrnXuXbJf
Lifting the hover ladder out of `Editor` had the shell resolve the file-explorer status indicator up front, before the ladder ran. That probe is not cheap — it scans the window's buffers for unsaved changes, takes the theme lock, and runs the explorer's slot layout — and `compute_hover_target` runs on every mouse-move event. Any hover a popup, context menu or the menu bar claimed first was paying for a result that was then discarded; previously that work sat inline, after those checks, so it never ran. Pass a closure instead of a value, called at the one rung that needs it. The probe keeps its place in the ladder (after the explorer close button, before the resize border) and regains its laziness. The new test pins the property rather than just the ordering: it counts calls and asserts an overlay hit and a close-button hit resolve without consulting the probe at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014cyMxXzFLeS94CrnXuXbJf
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.
Follow-on to #2943, applying the same decision-core/imperative-shell split to the mouse side.
The problem
mouse_input.rscarried the "what is under the pointer?" ladder asimpl Editormethods readingself.active_layout()/self.active_chrome()directly. The precedence rules — which overlay wins where two of them cover the same cell — could only be exercised by constructing a whole editor, so they never were. These are the rules that surface to users as "the button doesn't work".There were also two implementations of the same probes:
compute_menu_dropdown_hover(menu_actions.rs) andcompute_file_browser_hover(file_open_input.rs) each duplicated hit-tests the hover ladder also performed.The change
app/hit_test.rs(new, noimpl Editor) holds the ladder as free functions:hover_target=floating_overlay_target(context menus → suggestions → popups → file browser) thenchrome_target(menu bar → open dropdown → explorer → separators → split controls → tabs → scrollbars → status bar → search options).menu_dropdown_targetandfile_browser_targetlive here too; their formerimpl Editorwrappers now delegate, so each probe has one implementation instead of two.The view borrows exactly the rects and slices the decision reads — deliberately not the layout caches themselves. Naming each field means the shell has to state what it hands over, a newly-added layout field cannot silently drift into the hit-test, and tests construct precise values rather than blanking a cache and trusting that the zeros are meaningful.
One probe cannot be pure: the file-explorer status indicator resolves its columns through the theme, the plugin decoration/slot caches and the explorer renderer's slot layout. It stays on
Editorasexplorer_status_indicator_atand is passed in pre-resolved — so the ordering still lives in the ladder even though that probe's geometry does not.Notes for review
mouse_input.rsdrops ~400 lines net;PopupAreaLayoutis re-exportedpub(crate)for the view's popup slice.Still ahead (not in this PR)
handle_mouse_clickis a second ~1,100-line ladder over the same geometry — menu bar, tabs, scrollbars, status bar, explorer, split controls — with hit-test and effect fused together. Two hand-maintained ladders over identical rects is the drift risk the overlayLayerwork was created to eliminate. With the pure ladder in place, the click path can become resolve target → match → apply effect, the same shapehandle_keynow has.Testing
cargo check/cargo clippy --all-targets/cargo fmt --check(--all-features) clean — no warnings in touched filescargo test --all-features --lib— 3,313 passed, 0 failed🤖 Generated with Claude Code
https://claude.ai/code/session_014cyMxXzFLeS94CrnXuXbJf
Generated by Claude Code