refactor(ui): one gesture core — recognizers/constants/press-hold/click-suppression (#12188 Phase 2) - #12684
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
f674760 to
bc12fc5
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
bc12fc5 to
4a85a7e
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…d-hold in the shared core (#12188) Finish the constants centralization the gesture core (#12445) started: gestures/constants.ts is now the single definition site for every tuned gesture value, with named per-surface overrides (PAGER_*, COPY_HOLD_MS, GRAPH_PAN_ENGAGE_SLOP, SHEET_DETENT_OVERSHOOT_SCALE) documenting each deliberate divergence. No value changes. - constants: add TOUCH_TAP_MOVE_SLOP (the three identical 10px move slops), GRAPH_PAN_ENGAGE_SLOP, the full hold table (DEFAULT_HOLD_MS moved in, COPY_HOLD_MS, PUSH_TO_TALK_HOLD_MS moved in), PAGER_AXIS_COMMIT_SLOP / PAGER_AXIS_DOMINANCE_RATIO / PAGER_FLICK_VELOCITY, OVERSHOOT_RESISTANCE, SHEET_DETENT_OVERSHOOT_SCALE. - recognizers: add sqrtRubberBand (the chat sheet's sqrt detent damping). - new usePointerPressAndHold: pointer-contract long-press with a move-cancel slop (the touch usePressAndHold cancels on any move). - useHorizontalPager / use-notification-pull import their tuned values from the shared table (aliased, behavior byte-identical). - chat-message tap-reveal slop = shared TOUCH_TAP_MOVE_SLOP. - RelationshipsGraphPanel: hand-rolled suppress-click ref + timeout replaced by shared useClickSuppression; pan engage slop named. - gestures.test.ts: sqrtRubberBand math, usePointerPressAndHold contract, and a tuned-constants drift-gate table (now 97 assertions across 5 suites). Part of #12188 Phase 2 residuals after #12445/#12454. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) ContinuousChatOverlay drops its last three hand-rolled gesture copies: - ThreadLine's 420ms hold-to-copy timer (local COPY_HOLD_MS / COPY_MOVE_CANCEL_PX + manual timer/slop refs) delegates to the shared usePointerPressAndHold with the centralized COPY_HOLD_MS. - The local sqrt rubber-band function is the shared sqrtRubberBand with the named SHEET_DETENT_OVERSHOOT_SCALE. - The keyboard-storm viewport sync's hand-rolled one-rAF-per-frame pattern is the shared useRafCoalescer (same schedule/cancel semantics). - OUTSIDE_SHEET_TAP_SLOP aliases the shared TOUCH_TAP_MOVE_SLOP (same 10px). The 750ms-windowed outside-click swallower is deliberately NOT migrated to useClickSuppression: it is a document-level gate on the next outside click after a swallowed pointerup, not a synthesized-click suppressor, and its window is 750ms, not a macrotask. Behavior-preserving; all tuned values unchanged. Part of #12188 Phase 2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-name marker The chat-gesture-coverage scanner treats a literal gesture-hook name in file content as a handler-site marker; constants.ts is a pure value table, so its per-surface comments name the surface, not the hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ject (#12188) useRafCoalescer returns a fresh object literal each render (the schedule/flush/ cancel callbacks inside are stable, the wrapper is not). The overlay's viewport-sync effect depended on the whole object, so it re-ran every render and re-fired settleDragRef mid-drag — stranding an in-progress sheet gesture at the wrong detent (the chat-sheet e2e runner's COLLAPSED->HALF flick was overshooting to a full-overscroll ~638px instead of 377px). Depend on the destructured stable methods instead; the effect now runs once. The three merged adapters already destructure the stable methods, so only this call site was affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4a85a7e to
7c9dd82
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Part of #12188 Phase 2. Builds on the merged gesture-core groundwork (#12445 recognizers/constants scaffold, #12454/#12345 push-to-talk) by finishing the centralization the earlier passes left partial: every tuned gesture value now has one definition site, and the last hand-rolled gesture copies at the call sites become adapters over the shared core.
Duplication eliminated (§C rollup — before → after)
gestures/constants.ts:DEFAULT_HOLD_MS450 ·COPY_HOLD_MS420 ·PUSH_TO_TALK_HOLD_MS200. The 180ms composer timing was already unified to 200 in #12345; overlay copy-hold now uses sharedusePointerPressAndHold.TAP_SLOP8 ·TOUCH_TAP_MOVE_SLOP10 (the three identical 10px move-slops: copy-hold, tap-reveal, outside-sheet) ·GRAPH_PAN_ENGAGE_SLOP4AXIS_COMMIT_SLOP8 (default) + namedPAGER_AXIS_COMMIT_SLOP6 overrideHORIZONTAL_DOMINANCE_RATIO0.8 + namedPAGER_AXIS_DOMINANCE_RATIO1.15 overrideDEFAULT_PULL_VELOCITY0.5 ·DEFAULT_SWIPE_VELOCITY0.4 ·PAGER_FLICK_VELOCITY0.45; notification-pull velocity aliasesDEFAULT_PULL_VELOCITYrubberBand+OVERSHOOT_RESISTANCE0.35 (notification + pager edge); newsqrtRubberBand+SHEET_DETENT_OVERSHOOT_SCALE6 (overlay detent)useClickSuppression— RelationshipsGraphPanel migrated here (dropped a hand-rolled ref +setTimeout(…,0)); the pager/TopicGroup/conv-item already delegate (#12445). The overlay's 750ms-windowed outside-click gate is deliberately kept (it's a next-outside-click gate, not a synthesized-click swallow — see note below).useRafCoalescer— overlay viewport-sync migrated (dropped its hand-rolled one-rAF-per-frame loop)usePointerPressAndHold(new pointer-contract recognizer)Centralized-constants table — deliberate unifications vs kept per-surface overrides
PUSH_TO_TALK_HOLD_MS = 200. No new value change is introduced in this PR.PAGER_AXIS_COMMIT_SLOP(6 vs 8),PAGER_AXIS_DOMINANCE_RATIO(1.15 vs 0.8 — the pager wants horizontal to beat vertical, the pull surfaces want the widened diagonal cone),PAGER_FLICK_VELOCITY(0.45),COPY_HOLD_MS(420 vs 450),GRAPH_PAN_ENGAGE_SLOP(4),SHEET_DETENT_OVERSHOOT_SCALE(6, sqrt damping). Each carries a comment explaining the divergence.gestures.test.ts→ "tuned constants table") pins every default and every per-surface override plus its direction of divergence, so a future silent behavior change fails CI.No tuned value was changed in this PR. All hooks keep their public APIs; the three gesture engines stay separate (DD2).
useNotificationPullkeeps its raw-TouchEvent non-passivepreventDefaultpath.Regression net (merged unified runners #12465) — results
test:launcher-e2ePAGER_*constants are byte-identical)test:chatux-gesture-e2etest:home-screen-e2e"hyperliquid" absent from launcher— unrelated launcher-content/dedup assertion (develop's #12521 wallet-subpage hiding), not gesture mathtest:chat-sheet-e2echat-gesture-coveragegate (packages/app)iwermissing-dep in untouchedspatial/__e2e__(CI install resolves)Regression found & fixed mid-review
The chat-sheet runner initially failed 7 detent assertions (COLLAPSED→HALF flick overshooting to ~638px vs 377px). Root cause:
useRafCoalescerreturns a fresh object literal each render (its inner methods are stable, the wrapper is not); the overlay's viewport-sync effect depended on the whole object, so it re-ran every render and re-firedsettleDragRefmid-drag, stranding the in-progress sheet gesture. Fixed by depending on the destructured stableschedule/cancelmethods. The three merged adapters already destructure the stable methods, so only this new call site was affected. Runner green after the fix.Line delta
usePointerPressAndHold+sqrtRubberBand+ the documented constants table): +184The raw call-site dedup lands negative; the module grows because this pass adds the documented tuned-value table and a reusable pointer press-and-hold recognizer that will absorb further duplication in Phase 3.
audit:app
N/A this run — shared-worktree node_modules + long dev-server boot make a full
audit:appunreliable here. These are gesture-logic changes with zero visual-token / render-output changes (same tuned values, same DOM), and the e2e runners above capture rest+gesture screenshots proving the surfaces render and respond correctly.What Phase 3 builds on
The composer/message-row merge (Phase 3) now consumes:
usePointerPressAndHoldandusePushToTalk(hold recognizers),useClickSuppression,TOUCH_TAP_MOVE_SLOP/ the full hold + slop table, andsqrtRubberBand— so the retiring inline composers/bubbles adopt the shared gesture core instead of re-deriving it.🤖 Generated with Claude Code