The connector handles were unreachable, because hover was the wrong question - #46
Merged
Merged
Conversation
…uestion Reported from the app, and true: the handles could not be pressed. They are drawn *outside* the border — they have to be, or they stand on the resize handles — so a pointer moving out to one leaves the object before it gets there. The offer was `.obj:hover`, so it went away on the way to itself. Walked out across the edge two pixels at a time, the handles are gone from the first pixel past the border and never come back. So the offer is by **reach**, not by hover: the input layer tracks the pointer and marks the topmost object whose box, grown by thirty screen pixels, contains it. That is the whole affordance — a region bigger than the element, which is not a thing `:hover` can express. **Every test in that block moved the pointer in one hop**, which is a path no hand takes, and that is why eight tests covered a feature nobody could use. The new one walks: out from the middle of the card, across the border, to the handle, two pixels at a time — and it fails against the version this replaces, which I checked before keeping it. One more thing found on the way, by the suite rather than by me: `e.target` is not always a node. An event dispatched at the window has the window as its target and `Node.contains` throws rather than answering false — on a listener that runs on every pointer move, which is every pointer move throwing. Four tests in `interact.test.js` said so immediately; the `nodeType` check is what tells a node from a window. 1060 tests, 97.2% lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PTizw7U35rC2MPquwr4Fv5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughConnector handles now appear when the pointer is within 30 screen pixels of an object edge. The input layer selects the topmost nearby object and applies its ChangesConnector handle proximity
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Pointer
participant InputController
participant PlacedObjects
Pointer->>InputController: Move pointer
InputController->>PlacedObjects: Check scale-adjusted reach
PlacedObjects-->>InputController: Return topmost nearby object
InputController->>PlacedObjects: Set near state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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
2 participants
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.
Reported from the app, and true: the handles a connector is dragged from could not be pressed.
They are drawn outside an object's border — they have to be, or they stand on the resize handles — so a pointer travelling out to one leaves the object before it arrives. The offer was
.obj:hover, so it went away on the way to itself. Walking a pointer out across the edge two pixels at a time, the handles are gone from the first pixel past the border and never come back.Reach, not hover
The input layer tracks the pointer and marks the topmost object whose box, grown by thirty screen pixels, contains it. That is the whole affordance — a region bigger than the element, which is not something
:hovercan express — and it is why the class is applied from JavaScript rather than by a selector.Near rather than selected is still what keeps these clear of the resize handles: those appear for a single selected object at the eight compass points, so the two sets are never under the pointer at the same moment.
Why eight tests covered a feature nobody could use
Every one of them moved the pointer in a single hop, from the middle of the card to the handle — a path no hand takes, and one that never passes through the dead zone. CDP will happily teleport a pointer; a person cannot.
The new test walks it: out from the middle, across the border, to the handle, two pixels at a time. It fails against the version this replaces, which I checked before keeping it.
And one the suite caught immediately
e.targetis not always a node. An event dispatched at the window has the window as its target, andNode.containsthrows rather than answering false — on a listener that runs on every pointer move, which is every pointer move throwing. Four tests ininteract.test.jssaid so; thenodeTypecheck is what tells a node from a window.Checks
1060 tests, all passing, 97.2% line coverage.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PTizw7U35rC2MPquwr4Fv5
Summary by CodeRabbit
New Features
Bug Fixes