Fix connector endpoint drag UX (no-crash translate + ghost line preview)#265
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Verification: verify:selfResult: ✅ PASS in 221.2s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The selection-translate paths (body drag, keyboard nudge, align, distribute, rotate) all funnelled every selected id through `store.updateElementFrame`, which rejects connectors — their frame is derived from `start`/`end` endpoints. So a connector in the selection always threw on commit. Route translations through new `commitTranslate(store, slideId, el, dx, dy)` in `interactions/drag.ts`: connectors update their free endpoints (attached endpoints stay anchored to their host shape), non-connectors get the same `updateElementFrame` call as before. The pure `translateElement` twin powers `paintLive` so the drag preview and the commit produce identical state — fixes a latent bug where the dragged connector's cached frame moved during the preview but the rendered line (which reads endpoint positions) stayed put until commit. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Previously the live drag re-rendered the canvas with the connector's endpoint moved to the cursor, so the real line disappeared from its original position and the user lost their reference point. Keep the real line and its handles anchored at the pre-drag state; layer a translucent ghost copy of the connector through the renderer's existing `forceRender(slide, doc, ghost)` slot (same path the shape-insert hover preview uses) so the user sees where the endpoint will land without losing the original as a comparison anchor. Also gate `onInsertHoverLeave` with a new `endpointDragging` flag. The canvas `pointerleave` listener fires every time the cursor crosses overlay DOM (handle, snap-site dots, ghost) — without the gate it nulls `connectorCursor` and re-paints the overlay, which flickers the snap-site dots and (when the previous handle-anchor implementation was in place) wiped the ghost on every move. And swap the endpoint handle cursor from `pointer` to `crosshair` to match the connector-insert tool: in both cases the user is targeting a point that will become an endpoint. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Pin the "attachment wins" semantic with a body-drag test on a connector whose both endpoints attach to shapes outside the selection — without this, a refactor switching attached endpoints to "also translate by delta" would silently regress. - Add a renderer-level test that `forceRender(slide, doc, ghost)` with a connector ghost strokes the line with `GHOST_ALPHA` and brackets the paint with `save`/`restore`. Closes the gap the editor-side test comment hand-waved as "covered by renderer tests." - Update the `forceRender` JSDoc to name both uses (hover-preview insert AND endpoint-drag preview) and the fact that a half- attached ghost connector resolves through the slide lookup. - Document the `paintLive` `el.type`-preservation invariant so a future caller can't quietly substitute a connector for a shape. - Note the `commitTranslate(0,0)` no-op suppression in the lessons file — silent behavior shift from the prior `updateElementFrame` path that emitted a redundant op. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
All todo items checked off; lessons captured; review feedback addressed in the same branch. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
hackerwins
force-pushed
the
fix/slides-connector-drag-ux
branch
from
May 18, 2026 17:39
3ea202a to
2382c39
Compare
hackerwins
added a commit
that referenced
this pull request
May 18, 2026
Rebase folded in main's depth-aware `hitTestSlide` (added with the group / ungroup work in #263 and reused by the connector endpoint drag fix in #265). The integration commit reroutes that function through the new precise `hitTestElement`, which two existing tests now need: - `hit-test-elements.test.ts` — pass a `ctx` option to every `hitTestSlide` call, and give the shape factory a `fill` so the rect has a visible body for `isPointInPath` to land on. (The previous fixture relied on bbox-only hit-test which the precise test rejects for empty shapes.) - `editor.test.ts` body-drag-of-half-attached-connector — the click used to land in empty bbox space; move it onto the line's midpoint with the same drag delta so the connector still gets selected. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
1 participant
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.
Summary
store.updateElementFrame, which rejects connectors because theirframeis derived fromstart/endendpoints. Routes connectors through a newcommitTranslate(store, slideId, el, dx, dy)helper that updates free endpoints viaupdateConnectorEndpointand leaves attached endpoints anchored to their host.forceRender(slide, doc, ghost)slot (same path the shape-insert hover preview uses). On release, the store commit + repaint teleports the line to where the ghost was.crosshair. Matches the connector-insert tool: in both cases the user is targeting a point that will become an endpoint.Secondary cleanup that fell out of the work:
applyDragdeleted (unused in production; its tests now exercise the newtranslateElementdirectly, with explicit connector + attached-endpoint coverage).applyFrameUpdatesno longer takes a redundantslideIdparameter.endpointDraggingflag prevents the canvaspointerleavehandler from clobbering snap-site dots when the cursor crosses overlay DOM mid-drag.Test plan
pnpm verify:fastgreen (792 tests across packages; slides package at 1031).packages/slides/test/view/editor/:translateElementtranslates a shape's frame, preserves rotation/size, doesn't mutate input, translates a connector's free endpoints + cached frame, leaves attached endpoints anchored to their host.pnpm dev): select a connector, drag start/end endpoint — confirm the real line stays in place and a faded ghost line follows the cursor; release commits the move. Drag the connector body — confirm both endpoints translate (free) / stay anchored (attached). Keyboard nudge + align on a selection that includes a connector — no crash.🤖 Generated with Claude Code