Skip to content

fix(desktop): refuse sidebar drags from a press with no button held - #5619

Open
kunsanglee wants to merge 1 commit into
block:mainfrom
kunsanglee:fix/sidebar-drag-swallows-click
Open

fix(desktop): refuse sidebar drags from a press with no button held#5619
kunsanglee wants to merge 1 commit into
block:mainfrom
kunsanglee:fix/sidebar-drag-swallows-click

Conversation

@kunsanglee

Copy link
Copy Markdown

Summary

Clicking a channel in the sidebar sometimes started a drag nobody asked for: a
preview followed the cursor and the channel never opened.

dnd-kit's PointerSensor decides to start a drag from pointer travel alone and
never re-reads the button state, so a pointerup the page does not receive
leaves the sensor armed. The next cursor move begins a drag, and because dnd-kit
installs a capture-phase click blocker as soon as a drag activates
(@dnd-kit/core 6.3.1, in handleStart), the click that move belonged to is
swallowed. The channel selection never runs.

The releases really do go missing. I instrumented the macOS webview and
logged every pointer event over 38 gestures of ordinary sidebar use:

pointerdown pointerup count
deliberate press-and-hold buttons: 1 prompt 4, of which all 3 attempted drags worked
tap-to-click buttons: 0 deferred 356ms – 1.2s+; 8 times not within 700ms 34, of which 23 would have started a drag

A tap is recognised after the finger has already lifted, so the webview reports
pointerdown with no button pressed and defers the matching pointerup. During
that window dnd-kit is still armed, and moving the cursor a few pixels toward
the next channel starts a drag.

The two gesture shapes never crossed in the measurements: every buttons: 0
press was a click, every buttons: 1 press was a real drag. So the fix is to
not start. dnd-kit only instantiates a sensor when the activator returns exactly
true, and reads activators off the sensor class, so a subclass that checks
buttons before delegating to the base handler means no session, no click
blocker, and nothing to unwind.

export class PressAwarePointerSensor extends PointerSensor {
  static activators = [
    {
      eventName: "onPointerDown" as const,
      handler: (event: ReactPointerEvent, options: PointerSensorOptions) =>
        event.nativeEvent.buttons !== 0 &&
        PointerSensor.activators[0].handler(event, options),
    },
  ];
}

Key decisions

  • Declining before instantiation, rather than cancelling a live session. An
    earlier revision watched for button-free movement and dispatched a synthetic
    pointercancel. That was 45 lines and it could tear down a drag that was
    going fine, on the strength of a single move event from the same webview whose
    button reporting the change exists to distrust. Declining at pointerdown
    cannot do that, and it also sidesteps dnd-kit deferring its document-listener
    teardown by 50ms, which leaves the click blocker up for a moment after a
    cancel.
  • buttons !== 0, not a bitmask test. The base handler already rejects
    non-primary and non-left presses; the only new claim is that a press with no
    button pressed is not a press. Touch contact and pen contact both report a
    button, so neither is affected.
  • The base handler is delegated to rather than replaced, so onActivation
    and the primary/left checks keep coming from dnd-kit.
  • The activation distance is unchanged (6px). Measured travel during the bug
    was 21–170px, so no threshold value separates these gestures; raising it would
    have been an unrelated change that fixed nothing.
  • The community rail gets the same sensor. It is the same bug there, and a
    swallowed click means a failed community switch, which is heavier than a
    missed channel.

One case is deliberately not covered: a drag that starts with a button held and
loses its release later. It never appeared in the measurements, and Esc
already cancels a live drag, so nothing is built for it until it is reported.

Related issue

None found. I searched open issues and PRs for drag, sidebar, pointerup,
dnd-kit and click drag sidebar — no existing report of this behaviour.

Adjacent, not duplicates:

Testing

just ci passes locally.

Regression coverage is e2e rather than a unit test because the behaviour needs a
real DOM and dnd-kit's full sensor pipeline; a *.test.mjs beside the module
could only re-assert the code it is testing.

Each test reproduces the production sequence — a pointerdown reporting no
button held, which page.mouse.down() cannot produce — and asserts what the
user loses, that the following click selects its target:

  • sidebar-drag-activation.spec.ts — a refused press does not arm a drag or eat
    the next channel click; and a real press-and-drag still commits its section
    reorder right after one was refused.
  • community-rail.spec.ts — the same for the rail, asserting the community
    switch lands.

Each test was validated by mutation, both directions, checking the build
exit code separately so a type error could not silently serve a stale bundle:

mutation refused-press tests deliberate-drag tests
drop the button check fail (click swallowed, target never activates) pass
refuse every press pass fail (sidebar-drag-activation, and the existing rail reorder test at community-rail.spec.ts:1005)

Manual verification on macOS, in two passes. The instrumented 38-gesture session
above was taken with an earlier revision of this change; it is where the
buttons separation and the count of 23 come from. This revision was then
checked by using a dev build normally — the recording below is that check.

Recording

Nothing renders differently, so before/after screenshots would be identical
pixels; these are the interaction instead, from a dev build on macOS with
tap-to-click.

Clicking channels — each click opens its channel, and no drag preview appears:

clicking channels opens them

Deliberately pressing and dragging — the preview appears, follows the cursor,
and the drop lands:

a deliberate drag still works

Deferred to follow-up PRs

  • desktop/src/features/home/useResizableInboxListWidth.ts and
    desktop/src/shared/hooks/useThreadPanelWidth.ts have the same defect class —
    they listen for pointerup with { once: true } and handle no
    pointercancel, so a lost release leaves document.body.style.cursor and
    userSelect: none stuck app-wide.
  • Neither DndContext wires onDragCancel, so cancelling a live drag with
    Esc leaves the dimmed source row dimmed. Pre-existing, unrelated to
    the press that starts it.
  • A drop that changes nothing still writes localStorage and publishes a
    kind:KIND_CHANNEL_SECTIONS event (useChannelSections.ts). Not caused by
    this bug, but it made each spurious drag reach the relay.
  • SidebarDndContext advertises aria-roledescription="draggable" without
    registering a KeyboardSensor (the rail has one), so keyboard users are told
    about an interaction they cannot perform.

Clicking a channel in the sidebar sometimes started a drag nobody asked
for: a preview followed the cursor and the channel never opened.

dnd-kit's PointerSensor decides to start a drag from pointer travel alone
and never re-reads the button state, so a `pointerup` the page does not
receive leaves the sensor armed. The next cursor move begins a drag, and
because dnd-kit installs a capture-phase `click` blocker as soon as a drag
activates, the click that move belonged to is swallowed.

The macOS webview reports tap-to-click exactly that way. Instrumenting it
over 38 gestures: a tap is recognised after the finger has already lifted,
so `pointerdown` arrives with `buttons: 0` (34 of 38) and the matching
`pointerup` is deferred — 356ms to over 1.2s, and 8 times not within 700ms
at all. A deliberate press-and-hold reports `buttons: 1` throughout. The
two never crossed: every measured `buttons: 0` press was a click, and all
three deliberate drags started from `buttons: 1`.

So the sensor declines at that moment rather than arming. dnd-kit only
instantiates a sensor when the activator returns exactly `true`, and reads
`activators` off the sensor class, so a subclass that checks `buttons`
before delegating to the base handler means no session, no click blocker,
and nothing to unwind. The base handler still does the rest, including the
primary/left-button checks and `onActivation`.

The community rail gets the same sensor: a swallowed click there fails a
community switch, which is heavier than a missed channel.

Covered by e2e rather than a unit test because the behaviour needs a real
DOM and dnd-kit's sensor pipeline, and each test reproduces the production
sequence — a `pointerdown` reporting no button held, which
`page.mouse.down()` cannot produce — then asserts the following click
still selects its target. Validated by mutation in both directions:
dropping the button check fails both refused-press tests, and refusing
every press instead fails the deliberate-drag tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: kunsanglee <85242378+kunsanglee@users.noreply.github.com>
@kunsanglee
kunsanglee requested a review from a team as a code owner August 12, 2026 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant