fix(web): suppress session menu during touch drag - #4065
Open
btli wants to merge 22 commits into
Open
Conversation
Contributor
|
@btli This PR is a Bug fix, Feature, or UI / frontend change but the Demo section is missing or only contains a placeholder. These change types require a screenshot or screen recording so reviewers can see the new behaviour without checking out the branch. Please update the Demo section with:
Use |
This was referenced Aug 5, 2026
Open
Contributor
Author
|
Heads-up: no push here yet, deliberately. The fixes for this PR's findings were implemented on top of the unified row-gesture recognizer, which replaced this branch's approach and builds on #3985 — it can't be expressed against main alone. The recognizer work (drag-suppressed context menu, cancellation on any second touch, dnd-kit lifecycle reset so a viewport resize can't leave the gesture armed, and the Recommended merge order — this PR is one of seven fixed and verified together; the fully integrated reference (all cross-PR conflicts resolved, 5,003 web tests + 273/273 Android tests green) is Android track (suggest landing first — security fixes, and the later two resolve against it):
Web track (file-disjoint from the Android track; order within it matters): |
Add a configurable horizontal swipe gesture on left-panel session rows for touch devices. Each direction (left/right) maps to an action — archive, delete, or none — persisted per-device and set from Settings → Appearance. Swipe→archive reuses the kebab's stop-then-archive handler; swipe→delete opens the same confirm dialog the kebab uses, so delete is never immediate. The gesture locks by axis and yields to dnd-kit's long-press drag, leaving drag-to-file-into-project unaffected. The whole row — link, session-state badge, and pin/kebab controls — lives on one translating surface, so trailing icons travel with the text instead of the text sliding out from under them. Swipe-only markup and the opaque row background are gated on an in-progress swipe, so a row at rest renders exactly as it did before the gesture existed. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
Follow-ups from review of the swipe gesture: - Commit off a synchronously-tracked offset instead of the rendered `dx`. A fast flick can lift the finger before React commits the render for the final pointermove, so the action was decided from a stale offset — a swipe past the threshold could fail to fire, and one that snapped back could fire anyway. - Claim the horizontal axis with `touch-pan-y` where a swipe can fire, so the browser can't take the pan (or a back-navigation gesture) and cancel the drag mid-swipe. Vertical scrolling stays native. - Ignore pointerdowns that bubbled out of a portal. The row's dialogs are React children rendered in portals, so a drag inside the open delete dialog would otherwise start a swipe on the row behind it. - Reset the swipe preference with "Reset appearance", which promises to clear every appearance choice. - Transition the row transform only at rest, so it tracks the finger 1:1 while swiping instead of easing behind it. - Scale the hint glyph past the commit point so the "will fire on release" state isn't carried by color alone. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
The archive direction looked unfinished next to delete: the hint used `bg-primary/15`, and --primary is near-black in light mode, so it rendered as a flat grey block that read as a disabled button. It now uses the accent pair, which is blue in both modes, and deepens its tint past the commit point. The row also inset-slides instead of translating. A translate pushed the title past the panel boundary, where it was cut mid-word: revealing the hint needs ~48px of gap but the title only has ~18px of slack, so clipping was inherent to translating. Insetting from the swiped edge lets the title re-truncate with its existing ellipsis and keeps every row control inside the panel. Travel past the commit point is also damped to a third and hard-capped, so a long drag resists rather than dragging the row further than the action needs. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
Archiving stopped issuing a client-side stop some time ago — the server stops the runner once the flag commits, and a client stop would race it. The swipe test still asserted the old stop-then-archive sequence, so it failed against code that is behaving correctly. Assert the real contract instead, including that no client stop fires. Signed-off-by: Bryan Li <bryan.li@gmail.com>
Cover the Settings direction mapping end to end, reveal/action pairing, the exact 72px commit boundary, single dispatch, and a throwing localStorage getter. Unrecognized stored actions now normalize to none instead of silently arming archive. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
Fold the settings-select drive into one helper, let a held swipe release itself instead of re-passing dx, and share the reveal icon assertions. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
An unfolded foldable or tablet is wider than the md breakpoint, so the width gate turned swipe-to-archive/delete off on a touch device. The gate now asks whether a coarse pointer exists (any-pointer: coarse); useRowSwipe still accepts only touch pointers, so mouse paths are untouched at any width. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
- Suppress the trailing click after any beyond-slop swipe so a committed swipe can never also navigate into the session (pointer-capture + preventDefault behavior varies across mobile browsers). Scoped to the row itself so portalled dialog/menu clicks pass through. - Mirror the row state in the swipe reveal: an archived row shows the unarchive glyph, since the gesture restores rather than re-archives. - Clamp the drag offset to rest when a locked swipe reverses into a direction mapped to none, instead of sliding the row over bare canvas. - Disarm the gesture (and drop the touch-pan-y override) when both directions are mapped to none, so the browser keeps horizontal gestures. - Extract a shared useMediaQuery store (one native change listener per query) and reimplement useCoarsePointer/useIsMobileViewport on it. - Cache the swipe-preference snapshot; refresh it on the change events instead of re-reading and re-parsing localStorage on every row render. - Drop the swipeActionRef indirection: useRowSwipe already tracks onAction in its handler deps, so the callback is passed directly. Tests cover the new behaviors: trailing-click suppression (asserted via the router location, not test-side preventDefault), the archived-row hint, the none-direction clamp, and the touch-action gating. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
One module-level listener pair feeds all swipe-preference subscribers (the useMediaQuery shape) instead of two window listeners per row, and actionFor() centralizes the offset-direction -> configured-action map that was spelled out in four places. Collapse a duplicated damping-test step to a single hard-cap assertion. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
Signed-off-by: Bryan Li <bryan.li@gmail.com>
Signed-off-by: Bryan Li <bryan.li@gmail.com>
- Reset the shared context-menu open state when the row's render branch changes, so a remounted ContextMenu can't reopen anchored at (0,0) - Drop the isDraggingRef mirror; a [isDragging]-dep callback is enough - DRY the e2e touch tests with a touch_page fixture plus _touch/_center helpers, and delete the project created by the drop test on teardown Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
A session row had three touch consumers arming on independent thresholds with no arbiter: dnd-kit's drag sensor (250ms hold), Radix's context menu (700ms long-press), and the horizontal row swipe (12px). dnd-kit won every stationary hold, so Radix's later open request landed during a live drag and was rejected — long-press could not reach the menu at all — and a slow swipe that stayed inside the sensor's tolerance became a drag instead. Replace the racing timers with `useRowGesture`, which owns the gesture and dispatches one intent: horizontal travel swipes, other travel scrolls, and a still hold arms a pick-up that either drags on movement or opens the menu on release. `RowGestureTouchSensor` binds its activator to touchmove and consults the recognizer, so dnd-kit is registered without an activation constraint and arms no timer of its own — there is nothing left to race. Radix's trigger keeps its own 700ms timer, which would open a second menu mid-hold, so touch pointerdown is defaultPrevented on the row link; pen still reaches Radix, which the recognizer does not claim. The hold also honours a drift tolerance, restoring what the removed sensor constraint provided: a finger still creeping is scrolling slowly, not holding still. Declare VIBRATE so the pick-up haptic is audible to the WebView. Signed-off-by: Bryan Li <bryan.li@gmail.com> Co-authored-by: Isaac
btli
force-pushed
the
polly/session-row-gesture-fix
branch
from
August 6, 2026 18:45
f81afe4 to
bebff4a
Compare
Contributor
Author
|
This PR is part of a stacked train rebased onto current main so the whole set merges conflict-free in order. Full order: #3985 → #4065 → #4060 → #4057 (web/Sidebar train) and #3587 → #3589 → #3800 (android train). The two trains are independent of each other. This PR is 2nd in the web train, stacked on #3985. Until its predecessors merge, this PR's diff temporarily includes their commits; it collapses to a clean, self-contained diff automatically as each predecessor lands. |
Cancel the recognizer (and dnd-kit's drag) on any second touch anywhere, block the native context menu while a row drag is live, clear suppressClick on every fresh press, and follow dnd-kit's own drag end/cancel so a viewport resize can't leave the recognizer armed. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
Name the armed-or-drag touch-action check, drop the `state.current` guards that reset() now makes redundant, and reuse a second-touch pointer constant in the gesture tests. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
…uch capability A real fingertip wobbles well past 12px during a 400ms hold, so the pending phase demoted to scroll before the drag could arm. The explicit scroll fallback widens to a 25px circle (native pan-y still wins earlier) and the hold tolerance to 20px; swipe keeps its 12px horizontal-dominant lock, and 25/sqrt(2) > 12 keeps the two regions disjoint. Gestures now enable wherever a coarse pointer exists instead of below the md breakpoint, so an unfolded foldable keeps swipe and long-press drag. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
The context menu now appears the moment the 400ms hold arms (with the haptic and lift) instead of on release. Pulling 10px from the arm point dismisses the menu once (Escape through Radix's layer) and hands the row to dnd-kit; anything less keeps the menu, and release leaves it open through the trailing click. The threshold measures from the arm point so legal pre-arm drift can't pre-spend the drag budget. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
…ess gesture Three leaks on real touch devices: Chrome samples touch-action at touchstart, so nothing stopped it claiming the armed gesture's first vertical moves as a pan (pointercancel, no drag) — a native non-passive touchmove guard now runs from arm until reset. The row menus rendered modally, whose body pointer-events lock let touches fall through the sidebar overlay and start text selection on the chat beneath — they are non-modal now. And the menu surface itself carried no user-select or touch-action, so drags starting on it seeded selection behind it. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
…owns the touch At ~500ms Android fires its own contextmenu on the still-held touch — 100ms after the recognizer opened the menu. Pointer capture retargets it to the row root, bypassing Radix's trigger, and unprevented it starts text selection and cancels the pointer stream, which killed the pending drag. The recognizer's own dispatch is tagged and passes through; everything else is prevented while the gesture is armed or dragging. Root-caused with real Chrome touch dispatch over CDP: with touch-action pan-y, pointercancel fires through pointer capture unless the pan and the OS menu are both forestalled. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
…re owns the touch The OS long-press hit-tests the element under the finger — once the row menu opens at the press point, that is the menu portal, past the row-level guard, so Android still engaged selection and cancelled the pointer stream (drag worked only when the pull beat the ~500ms long-press). The contextmenu/selectstart suppression now rides the armed-gesture guard at the document, capture phase; the recognizer's tagged dispatch passes through. Root-caused live on the device: a CDP-synthesized gesture (which bypasses the OS long-press) completed the full menu->dismiss->drag->drop pipeline, isolating the failure to the View-layer path only real fingers take. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
…ess survives Chrome Android hands a stationary long-press on a draggable link to its native drag machinery, firing an unconditional pointercancel before the contextmenu event — no preventDefault can save the pointer stream. With draggable=false the long-press has no native claimant left and the row gesture's menu-then-drag works from a stationary hold. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
…synthetic Escape A document-visible Escape keydown is exactly what dnd-kit's pointer sensors listen to for drag cancellation — dismissing the menu that way races the drag it is meant to make room for. Controlled open state closes the menu directly with no key event. Co-authored-by: Isaac Signed-off-by: Bryan Li <bryan.li@gmail.com>
btli
force-pushed
the
polly/session-row-gesture-fix
branch
from
August 6, 2026 19:05
bebff4a to
8e08d21
Compare
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.
Related issue
N/A
Summary
ELI5: the row now checks whether it is already being dragged before letting its second long-press timer open a menu.
Test Plan
test_touch_drag_does_not_open_session_context_menufailed after proving the row was dragging:rename-conversationhad count 1.export PATH="/opt/homebrew/opt/node@22/bin:$PATH" && pnpm --filter web run test -- src/shell/Sidebar(4,892 passed, 3 expected failures, 1 skipped)uv run pytest tests/e2e_ui/sessions/test_sidebar_context_menu.py tests/e2e_ui/sessions/test_sidebar_projects.py tests/e2e_ui/sessions/test_sidebar_session_gestures.py -v --ui-skip-build(8 passed)pre-commit run --all-files(all hooks passed)Demo
N/A — this is a transient behavioral gesture collision with no persistent visual delta. The CDP regression holds a genuine touch past both timers and asserts that the drag stays active while the context menu remains absent.
Type of change
Test coverage
Coverage notes
The new E2E tests use
Input.dispatchTouchEventin ahas_touch=TrueChromium context. They positively verify drag activation, native sidebar scrolling, and the resulting session-to-project membership change; the existing desktop right-click E2E remains green.Changelog
Long-pressing a session now starts a drag without opening its context menu on top.