fix: make tab dragging deliberate with directional split previews

[kshivang]

Small pointer slips while clicking a pane-strip or horizontal tab could start a drag and create an unintended split on release. The shared tab gesture now waits for the standard touch movement threshold instead of Compose's 0.125dp mouse threshold, leaving sub-threshold motion to the tab's click handler. Deliberate drags remain immediate once the threshold is crossed; no hold delay is required.

The handler preserves drag ownership and cancellation cleanup, accepts only primary mouse drags, and starts at the actual threshold-crossing position without adding that movement twice. This applies to both favicon chips and full tab buttons.

Also show the split preview on the source pane: the drop logic already accepts its edges, but the rendering condition previously excluded that pane. The same-pane center stays a no-op.

Validation: mouse-input regression covers repeated click jitter, click delivery, deliberate drag activation/position, and single drop. Existing release, replacement, removal, and cancellation tests remain in the focused suite. Desktop compilation, focused gesture/session tests, ktlintCheck and detekt pass.

Split drops now retain their chosen side: left/right/top/bottom placement is forwarded to the existing split layout operation, with only that edge highlighted. Sidebar-to-tab drops use the same placement. A regression exercises all four edge targets through preview, drop result and resulting split tree.

[supabase]

This pull request has been ignored for the connected project pcnwqamqdnsadranufjv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[github-actions]

Claude diff review of fe13bf0 (no tests executed).

Review — PR #1755 @ fe13bf0

Scope: replacement of detectDragGestures with a hand-rolled awaitEachGesture pipeline in TabDragGesture.kt, plus one new desktop UI test. Line numbers below are derived from the diff hunks (@@ -13,21 +19,29 @@, @@ -127,6 +128,64 @@); I only read the diff, so nothing about TabDraggableComponent, DragSession, or the pre-existing tests is verified.

What the change looks correct about

• session.update(change.positionChange()) is called before change.consume() (TabDragGesture.kt ~L35–36). This ordering is load-bearing: positionChange() returns Offset.Zero once the change is consumed, so swapping these two lines would silently freeze the drag. Worth a short inline comment so a future refactor doesn't reorder it.
• Dropping the overSlop delta (comment at ~L31) is consistent with passing start.position to onStart: the old onDragStart(drag.position) + onDrag(change, overSlop) pair did add the slop remainder on top of an already-post-slop position, so the tracked position should now match the real pointer position. (Behaviour of session.start/update not verifiable from the diff.)
• drag(start.id) (not down.id) is the right id, since awaitTouchSlopOrCancellation may hand back a different pointer if the original one lifts.

Behaviour changes that look unintended / undocumented

1. Non-primary mouse buttons can no longer start a drag, and a primary press is swallowed while another button is held — TabDragGesture.kt ~L24:

   if (down.type == PointerType.Mouse && !currentEvent.buttons.isPrimaryPressed) return@awaitEachGesture

   detectDragGestures had no button filter, so middle/secondary-button tab drags (if any UI relied on them) are a regression. Additionally, awaitEachGesture waits for all pointers to be released after the block returns, so if the user presses secondary first and then primary, the early return blocks the whole gesture until both buttons are up — the drag never starts. The diff comment only explains the slop change, not the button gate, and no test covers it.

2. The button gate is start-only (~L24 vs. the drag loop at ~L33). On desktop a mouse pointer stays pressed while any button is down, so press-primary → start drag → press secondary → release primary keeps the drag alive and still commits a drop on final release. Inconsistent with the entry condition; uncertain whether it matters for tabs, but it is a new asymmetry introduced here.

3. Slop threshold jump for mouse. Using awaitTouchSlopOrCancellation applies the touch slop (18.dp by default) to mouse input. That is the stated intent, but it is a large threshold for pointer dragging: the tab will not begin following the cursor until ~18dp of travel. If any consumer derives a grab anchor from the onStart offset, the drag preview anchor is now taken up to 18dp away from where the user actually pressed. Flagging as a UX/product decision to confirm, not a defect.

4. session.start returning false now silently aborts the gesture — ~L30. Previously the return value was ignored and detectDragGestures still drove onDrag/onDragEnd with isOwner guards. Now, when start fails, the slop-crossing change has already been consumed (~L28), so the press produces neither a drag nor a click, and onEnd is never invoked for that gesture. Whether that is reachable depends on DragSession.start semantics, which are not in the diff — please confirm.

5. completed == true with isOwner == false does no cleanup — ~L39–43: neither onEnd nor session.cancel() runs; the only recovery is the finally at pointer-input teardown. This mirrors the old onDragEnd guard, so it is pre-existing rather than new, but the restructure makes it easy to fix here if externally-cancelled sessions need a notification.

6. Pointer types other than Mouse/Touch. Stylus/eraser/Unknown bypass the button check and get touch slop. If any target platform reports mouse input as PointerType.Unknown, or reports empty buttons on the down event, mouse tab dragging breaks entirely there. Only desktop is exercised by the new test — uncertain, but the guard is platform-behaviour-dependent code in commonMain.

Test gaps

• No test for the new button gate (TabDragGestureTest.kt): a secondary/middle-button press-and-drag should assert starts == 0 and that a subsequent primary drag still works.
• No coverage of the new cancellation path (drag() returning false → session.cancel() → onEnd(null), ~L41). The implementation moved from onDragCancel to drag()'s return value; if an existing test covered the callback path it may no longer exercise the same code, and I can't tell from the diff whether one exists.
• No touch-pointer test asserting touch drags still cross at touch slop after the rewrite.
• The new test asserts the position only at the slop-crossing point (assertEquals(Offset(110f, 30f), component.getCurrentPosition()), ~L176). The "delta not applied twice" claim in the implementation comment is only half-checked; a second moveTo plus a position assertion would catch an off-by-one-delta regression mid-drag.
• No assertion on component.dropTarget/sourceIndex in the drag phase of the new test (only drops == 1), so the session.end(sourceIndex()) path is weakly covered.

Test fragility (likely actionable)

• TabDragGestureTest.kt ~L171–174: the second performMouseInput { press(); moveTo(Offset(110f, 30f)) } block calls press() with no preceding moveTo, relying on the cursor position persisting from the previous injection scope (last moveTo(34f, 33f)). Add an explicit moveTo before press() so the press location — and therefore the 76px travel that must exceed slop — is stated rather than inherited.
• The jitter loop (~L160–166) depends on awaitTouchSlopOrCancellation accumulating position change as a vector (so +3,+2 then -3,-2 cancels). That is my reading of the Compose implementation, not something the diff proves; if accumulation were distance-based the 20 moves would total ~72px and the test would be asserting the opposite of the intent. A single moveTo just under slop (e.g. Offset(30f + slopMinusEpsilon, 30f)) in addition to the jitter loop would pin the actual contract independently of that assumption.
• Both phases depend on the default ViewConfiguration.touchSlop at test density; the magic numbers (3–4px vs. 80px) are safely on either side, but deriving them from viewConfiguration.touchSlop would make the intent explicit.

Minor

• ~L24 is a long single line (if (...) return@awaitEachGesture, ~110 chars); check it against the project's ktlint/detekt max-line-length.
• The comment "Compose's mouse drag slop is only 0.125dp" hardcodes an upstream implementation detail; consider phrasing it as "much smaller than touch slop" so it doesn't rot on a Compose bump.

No confirmed functional defect in the happy path that I can demonstrate from the diff alone; items 1 and 4 are the ones I'd want resolved (or explicitly justified) before merge, along with a test for the button gate and the cancellation path.

[github-actions]

Claude diff review of 993c7b0 (no tests executed).

I'll examine the repository context for the files in this diff.

bash:

cd /home/runner/work/_temp && ls && find . -maxdepth 3 -name "*.kt" -path "*TabDrag*" 2>/dev/null | head

toolbash: command not found