Drag an arrow out of an object - #44
Merged
Merged
Conversation
Hover an object and four handles appear on its edges; press one and a line follows the pointer; release over another object and the arrow exists. One gesture, where joining two things used to be a click, a shift-click and a press on a bar — which is still there, and still the way to take an arrow away again. **On hover rather than on selection.** That is what keeps these out of the resize handles' way: 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. Four rather than eight, because the corners belong to resizing and because four is what an arrow means — it leaves an object through a side. They are hidden while any gesture is under way, since the card being dragged around is hovered by definition, and while text is being edited, where the pointer is a caret. That last one needed the input layer to say when a gesture is running at all, so every gesture now goes through one `begin` and one `finish` and the stage carries a class for it. Escape then has something to cancel: while an arrow is being dragged, the thing Escape means is not this *arrow* rather than not this selection. **The preview is drawn in the connectors' own SVG**, not in the overlay with the marquee and the guides. It is the thing being made rather than a mark about the gesture: world coordinates, the same geometry the real one will have — head and all, once it is over something it can land on — and inside the box that element sizes to itself, since drawn anywhere else it would be clipped out of its own preview. Over open board it is dashed and headless, because a question should not look like an answer, and releasing there makes nothing: a connector to nowhere is not a thing this document can hold. What is under the pointer is asked of `elementFromPoint` rather than of the board's own geometry. That answers with what a person can *see* there — the card on top rather than the envelope behind it — which is the one they are pointing at. 1051 tests, 97.2% lines. Eight of them are this gesture: the handles appearing and not appearing, the arrow made in the right direction, the object it started from left unselected and unmoved, a release on nothing and on itself, Escape, and a second drag between a pair that is already joined. 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds drag-to-connect interactions. Objects expose edge handles. Input manages connector gestures and cancellation. The connector layer renders previews and landed connections. Browser tests and README documentation cover the new behavior. ChangesDrag-to-connect workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Pointer
participant Input
participant Object
participant ConnectorLayer
Pointer->>Input: Press an edge connection handle
Input->>Object: Identify the source object
Pointer->>Input: Move across the canvas
Input->>Object: Detect a valid target
Input->>ConnectorLayer: Render connector preview
Pointer->>Input: Release
Input->>ConnectorLayer: Hide preview
Input->>Object: Create connector when target is valid
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/styles/canvas.css`:
- Around line 1342-1345: Update the handle visibility rules around
.connect-handles and .obj.handles-on so the two handle sets cannot be
simultaneously visible for a selected hovered object; assign one set clear
priority in the overlapping state. Add a browser test covering a selected,
hovered object and verify pointer presses target the prioritized handles rather
than the later connector elements.
In `@test/browser/connectors.test.js`:
- Around line 248-263: The hover and dragFrom helpers rely on fixed sleeps
instead of verifying readiness. Replace the 40 ms, 30 ms, and 60 ms waits in
hover and dragFrom, including the corresponding waits around lines 350-357, with
condition-based waits that observe the relevant computed style, preview
visibility, selection state, or connector count before continuing.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d03f0f31-cf11-40f3-93ea-a2f043099023
📒 Files selected for processing (7)
README.mdsrc/app.jssrc/platform/connectors.jssrc/platform/input.jssrc/platform/renderer.jssrc/styles/canvas.csstest/browser/connectors.test.js
…proved nothing - **The two handle sets met at a seam.** A selected object that is also hovered shows both: the resize handles reach from five pixels outside the border to four inside it, and the connector handles started fourteen out — so a single pixel of edge belonged to whichever came later in the document, which is not a rule anybody could have known. They start twenty out now, five clear pixels from the others at any zoom, since both are counter-scaled. Hiding one set instead was the other option and the worse one: the object you want to drag an arrow *from* is very often the one you have just made, which is selected. A test presses each of the two on the same edge of the same selected object and asserts each still does its own job. - **Three fixed sleeps proved nothing.** The gesture has real conditions and they are cheap to wait for: the handles' computed display for the hover, the preview appearing for the drag, and the preview going for the end of it — which is true whether an arrow was made or not, so even the tests that assert nothing happened now wait for something that did. The moves in between are not waited on at all: CDP dispatches them in order and the release is processed after them. 1052 tests, 97.2% lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PTizw7U35rC2MPquwr4Fv5
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/browser/connectors.test.js (1)
387-395: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the handle-separation assertion prove non-overlap.
connect.x > resize.leftdoes not prove that the rectangles are separate. The handles can overlap while this condition remains true. Return both bounding rectangles and assert that the connector rectangle is outside the resize rectangle. Apply the check to all four directions.As per the PR objective, selected objects must keep connector and resize handles separate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/browser/connectors.test.js` around lines 387 - 395, Update the handle-separation assertions in the connector tests around handleOf and the resize rectangle measurement to return both complete bounding rectangles, then verify the connector and resize rectangles do not overlap. Apply equivalent non-overlap checks for all four handle directions, preserving the requirement that selected objects keep connector and resize handles separate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/browser/connectors.test.js`:
- Around line 271-278: Update the dragFrom helper to wait for PREVIEW !== 'none'
after mousePressed and before processing further movement or releasing, ensuring
the connector gesture has visibly started. Preserve the existing completion wait
for PREVIEW === 'none' after mouseReleased.
---
Nitpick comments:
In `@test/browser/connectors.test.js`:
- Around line 387-395: Update the handle-separation assertions in the connector
tests around handleOf and the resize rectangle measurement to return both
complete bounding rectangles, then verify the connector and resize rectangles do
not overlap. Apply equivalent non-overlap checks for all four handle directions,
preserving the requirement that selected objects keep connector and resize
handles separate.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 39c7f829-26a9-401e-8efb-43c5e1a0c19f
📒 Files selected for processing (2)
src/styles/canvas.csstest/browser/connectors.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/styles/canvas.css
The preview is hidden before a press as well as after one, so waiting only for it to go let every one of these pass whether or not the gesture ever started — and the assertions about what a drag did would have been about a drag that did not happen. The first move now waits for the arrow to be *being drawn*. The drop-on-itself case goes out over open board and comes back, because a pointer that never leaves the object it started on draws nothing to wait for. Checked by making the handle press do nothing: seven of the eight fail, the first of them on that wait. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PTizw7U35rC2MPquwr4Fv5
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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.
Hover an object and four handles appear on its edges; press one and a line follows the pointer; release over another object and the arrow exists. One gesture, where joining two things used to be a click, a shift-click and a press on a bar — which is still there, and still the way to take an arrow away again.
On hover rather than on selection
That is what keeps these out of the resize handles' way: 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. Four rather than eight, because the corners belong to resizing and because four is what an arrow means — it leaves an object through a side.
They are hidden while any gesture is under way, since the card being dragged around is hovered by definition, and while text is being edited, where the pointer belongs to the caret.
Which meant the input layer had to know when it is busy
Every gesture now goes through one
beginand onefinish, and the stage carries a class for the duration. That is what the stylesheet hides the handles on — and it givesEscapesomething to cancel: while an arrow is being dragged, the thingEscapemeans is not this arrow rather than not this selection.The preview is drawn in the connectors' own SVG
Not in the overlay with the marquee and the guides. It is the thing being made rather than a mark about the gesture: world coordinates, the same geometry the real one will have — head and all, once it is over something it can land on — and inside the box that element sizes to itself, since drawn anywhere else it would be clipped out of its own preview.
Over open board it is dashed and headless, because a question should not look like an answer. Releasing there makes nothing: a connector to nowhere is not a thing this document can hold, and inventing an object to end it on would be a second feature answering for this one.
What is under the pointer
Asked of
elementFromPointrather than of the board's own geometry, because that answers with what a person can see there — the card on top rather than the envelope behind it, which is the one they are pointing at.Not in this change
An arrow dropped on open board makes nothing, where most tools would make a card to land on. That is the obvious next step and a decision of its own; it is written into the README's limits with the rest.
Checks
1051 tests, all passing, 97.2% line coverage. Eight of them are this gesture: the handles appearing on hover and not before it, staying away while text is edited, the arrow made in the right direction, the object it started from left unselected and unmoved, a release on nothing, a release back on itself,
Escapemid-drag, and a second drag between a pair that is already joined.🤖 Generated with Claude Code
https://claude.ai/code/session_01PTizw7U35rC2MPquwr4Fv5
Summary by CodeRabbit
New Features
Documentation