feat: Tweeq-style drag scrubbing for ScrubableNumberInput - #12418
Open
LittleSound wants to merge 3 commits into
Open
feat: Tweeq-style drag scrubbing for ScrubableNumberInput#12418LittleSound wants to merge 3 commits into
LittleSound wants to merge 3 commits into
Conversation
X-axis drags value, Y-axis adjusts sensitivity with an SVG dot-ruler overlay showing the active precision. Pointer-lock hides the cursor during scrub and returns it to the press position on release.
dosubot
Bot
added
the
size:XL
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors ScrubableNumberInput to a gesture-driven scrubbing system: pure gesture interpreter, scrub state composable, pointer-event drag composable, BallRuler SVG visualization, plus integration and tests for gesture math and pointer handling. ChangesDrag-based Number Input Scrubbing System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🎭 Playwright: ❌ 1637 passed, 1 failed · 1 flaky❌ Failed Tests📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/common/scrubableNumberInput/useDragGesture.ts`:
- Around line 35-141: Add behavioral unit tests for the useDragGesture
composable covering its lifecycle: write tests that mount a dummy element with
useDragGesture and assert that onClick is called for short taps,
onDrag/onDragEnd are called when movement exceeds the threshold in
onPointerMove, the dragDelay path triggers fireStart after the timeout, pointer
lock is requested when lockPointer is true and unlock is called on pointer up,
and teardown behavior runs on pointercancel/pointerleave (pointer capture
released and timers cleared). Target the functions useDragGesture,
onPointerDown, onPointerMove, onPointerUp, fireStart and teardown by dispatching
synthetic PointerEvents (varying pointerType, button, isPrimary, movement
distances) and use fake timers to simulate dragDelay; assert calls to
onDrag/onClick/onDragStart/onDragEnd and that lock/unlock are invoked and
pointer capture/release are performed.
- Around line 87-113: In onPointerMove (inside useDragGesture) stop relying
solely on event.movementX/movementY for dx/dy; detect when movementX/movementY
are 0/undefined (common for touch) and compute deltas from successive
event.clientX/clientY instead. Add a small state (e.g., lastClientX/lastClientY
or pointerLastPos) that you initialize from pointerDownAt when drag starts and
update on every pointer move; compute dx/dy = (movementX/movementY) /
browserZoom when available else (event.clientX - lastClientX) / browserZoom and
similarly for Y, then call options.onDrag(dx, dy, event) and update the
lastClient values. Ensure this logic runs after fireStart() creates the drag
state so the fallback has an initial lastClient value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8f134d2-94a5-44e5-a861-f56fe612d939
📒 Files selected for processing (6)
src/components/common/ScrubableNumberInput.vuesrc/components/common/scrubableNumberInput/BallRuler.vuesrc/components/common/scrubableNumberInput/interpretGesture.test.tssrc/components/common/scrubableNumberInput/interpretGesture.tssrc/components/common/scrubableNumberInput/useDragGesture.tssrc/components/common/scrubableNumberInput/useScrubValue.ts
Comment on lines
+35
to
+141
| export function useDragGesture( | ||
| target: MaybeRef<HTMLElement | null | undefined>, | ||
| options: DragGestureOptions = {} | ||
| ): { dragging: Readonly<ReturnType<typeof ref<boolean>>> } { | ||
| const dragging = ref(false) | ||
| const allowedTypes = options.pointerType ?? ['mouse', 'pen', 'touch'] | ||
| const dragDelay = options.dragDelaySeconds ?? 0 | ||
|
|
||
| const { lock, unlock } = usePointerLock(target) | ||
|
|
||
| let pointerId: number | null = null | ||
| let pointerDownAt: [number, number] | null = null | ||
| let dragDelayTimer: ReturnType<typeof setTimeout> | undefined | ||
| let pointerLocked = false | ||
|
|
||
| function teardown() { | ||
| if (dragDelayTimer !== undefined) { | ||
| clearTimeout(dragDelayTimer) | ||
| dragDelayTimer = undefined | ||
| } | ||
| pointerDownAt = null | ||
| pointerId = null | ||
| } | ||
|
|
||
| function fireStart(event: PointerEvent) { | ||
| dragging.value = true | ||
| if (unref(options.lockPointer) && !pointerLocked) { | ||
| pointerLocked = true | ||
| void lock(event).catch(() => { | ||
| pointerLocked = false | ||
| }) | ||
| } | ||
| options.onDragStart?.(event) | ||
| } | ||
|
|
||
| function onPointerDown(event: PointerEvent) { | ||
| if (unref(options.disabled)) return | ||
| if (event.button !== 0 || !event.isPrimary) return | ||
| if (!allowedTypes.includes(event.pointerType as DragPointerType)) return | ||
|
|
||
| pointerId = event.pointerId | ||
| pointerDownAt = [event.clientX, event.clientY] | ||
| const el = unref(target) | ||
| el?.setPointerCapture(pointerId) | ||
|
|
||
| // Drag commitment is decided later — either by the movement-distance | ||
| // threshold in onPointerMove, or by this long-press timer expiring while | ||
| // the pointer is still down. Until then it's just a potential click. | ||
| if (dragDelay === 0) return | ||
| dragDelayTimer = setTimeout(() => fireStart(event), dragDelay * 1000) | ||
| } | ||
|
|
||
| function onPointerMove(event: PointerEvent) { | ||
| if (pointerId !== event.pointerId || pointerDownAt === null) return | ||
|
|
||
| if (!dragging.value) { | ||
| // Lock engages inside fireStart, not yet — so clientX/Y is still valid | ||
| // for the drag-vs-click distance threshold. | ||
| const minDist = event.pointerType === 'mouse' ? 1 : 5 | ||
| const moved = Math.hypot( | ||
| event.clientX - pointerDownAt[0], | ||
| event.clientY - pointerDownAt[1] | ||
| ) | ||
| if (moved < minDist) return | ||
| if (dragDelayTimer !== undefined) { | ||
| clearTimeout(dragDelayTimer) | ||
| dragDelayTimer = undefined | ||
| } | ||
| fireStart(event) | ||
| } | ||
|
|
||
| // Compensate for browser zoom (Cmd +/-). event.movementX/Y report in | ||
| // device-pixel-like units that don't honor the browser zoom level; the | ||
| // ratio outerWidth/innerWidth backs that out. | ||
| const browserZoom = window.outerWidth / window.innerWidth || 1 | ||
| const dx = (event.movementX || 0) / browserZoom | ||
| const dy = (event.movementY || 0) / browserZoom | ||
| options.onDrag?.(dx, dy, event) | ||
| } | ||
|
|
||
| function onPointerUp(event: PointerEvent) { | ||
| if (pointerId !== event.pointerId) return | ||
| const el = unref(target) | ||
| el?.releasePointerCapture(event.pointerId) | ||
|
|
||
| const wasDragging = dragging.value | ||
| if (pointerLocked) { | ||
| void unlock() | ||
| pointerLocked = false | ||
| } | ||
| if (wasDragging) { | ||
| options.onDragEnd?.(event) | ||
| } else { | ||
| options.onClick?.(event) | ||
| } | ||
| dragging.value = false | ||
| teardown() | ||
| } | ||
|
|
||
| useEventListener(target, 'pointerdown', onPointerDown) | ||
| useEventListener(target, 'pointermove', onPointerMove) | ||
| useEventListener(target, 'pointerup', onPointerUp) | ||
| useEventListener(target, 'pointercancel', onPointerUp) | ||
| useEventListener(target, 'pointerleave', onPointerUp) | ||
|
|
||
| return { dragging: readonly(dragging) } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Add behavioral tests for drag lifecycle paths in this composable.
This file introduces core interaction logic (delay threshold, click-vs-drag split, pointer lock path, pointer cancel/leave cleanup), but there are no direct tests for it in this PR. Please add src/**/*.test.ts coverage for these paths.
As per coding guidelines "src/**/*.test.ts: ... Aim for behavioral coverage of critical and new features in unit tests" and "**/*.{test,spec}.ts: Write tests for all changes, especially bug fixes to catch future regressions".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/common/scrubableNumberInput/useDragGesture.ts` around lines 35
- 141, Add behavioral unit tests for the useDragGesture composable covering its
lifecycle: write tests that mount a dummy element with useDragGesture and assert
that onClick is called for short taps, onDrag/onDragEnd are called when movement
exceeds the threshold in onPointerMove, the dragDelay path triggers fireStart
after the timeout, pointer lock is requested when lockPointer is true and unlock
is called on pointer up, and teardown behavior runs on
pointercancel/pointerleave (pointer capture released and timers cleared). Target
the functions useDragGesture, onPointerDown, onPointerMove, onPointerUp,
fireStart and teardown by dispatching synthetic PointerEvents (varying
pointerType, button, isPrimary, movement distances) and use fake timers to
simulate dragDelay; assert calls to onDrag/onClick/onDragStart/onDragEnd and
that lock/unlock are invoked and pointer capture/release are performed.
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #12418 +/- ##
===========================================
- Coverage 75.07% 60.31% -14.76%
===========================================
Files 1526 1420 -106
Lines 96710 72782 -23928
Branches 28279 20273 -8006
===========================================
- Hits 72602 43901 -28701
- Misses 23214 28405 +5191
+ Partials 894 476 -418
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1049 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Add behavioral tests for the full pointer lifecycle (click vs drag, long-press, pointer lock, pointercancel/leave cleanup). Derive scrub deltas from clientX/Y when not pointer-locked so touch — where movementX/Y is often 0/undefined — works; keep movementX/Y under lock where clientX/Y is pinned.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/common/scrubableNumberInput/useDragGesture.test.ts (1)
34-54: ⚡ Quick winTighten
mountoptions typing to prevent silent test misconfiguration.Line 34 and Line 53 currently allow any option key via
Record<string, unknown>and then force-cast. Typing this asPartial<Parameters<typeof useDragGesture>[1]>keeps test helpers type-safe and catches option typos at compile time.♻️ Proposed change
-function mount(options: Record<string, unknown> = {}) { +function mount(options: Partial<Parameters<typeof useDragGesture>[1]> = {}) { @@ - const merged = { ...cb, ...options } as Parameters<typeof useDragGesture>[1] + const merged: Parameters<typeof useDragGesture>[1] = { ...cb, ...options } scope.run(() => useDragGesture(ref(el), merged)) }As per coding guidelines, "Use TypeScript for type safety throughout the codebase."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/common/scrubableNumberInput/useDragGesture.test.ts` around lines 34 - 54, The mount helper currently types options as Record<string, unknown> then force-casts into the useDragGesture options which hides misconfigurations; change the mount signature to accept options: Partial<Parameters<typeof useDragGesture>[1]> (and remove the forced cast when creating merged) so TypeScript will catch invalid option keys — update the mount function, the options parameter, and the merged variable creation (which currently spreads cb and options) to use that Partial type to ensure type-safe test setup for useDragGesture.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/common/scrubableNumberInput/useDragGesture.test.ts`:
- Around line 199-207: Add a new sibling test case after the existing
pointercancel test that validates cleanup behavior for the pointerleave event.
The test should follow the same pattern as the current test: mount the
component, dispatch pointerdown and pointermove events to initiate a drag, then
dispatch a pointerleave event instead of pointercancel. Assert that both
cb.onDragEnd is called once and el.releasePointerCapture is invoked with the
pointer ID (1) to ensure pointerleave properly ends the drag and releases
pointer capture just as pointercancel does.
---
Nitpick comments:
In `@src/components/common/scrubableNumberInput/useDragGesture.test.ts`:
- Around line 34-54: The mount helper currently types options as Record<string,
unknown> then force-casts into the useDragGesture options which hides
misconfigurations; change the mount signature to accept options:
Partial<Parameters<typeof useDragGesture>[1]> (and remove the forced cast when
creating merged) so TypeScript will catch invalid option keys — update the mount
function, the options parameter, and the merged variable creation (which
currently spreads cb and options) to use that Partial type to ensure type-safe
test setup for useDragGesture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35b39815-d3b6-4aed-82e0-50976f2d9302
📒 Files selected for processing (2)
src/components/common/scrubableNumberInput/useDragGesture.test.tssrc/components/common/scrubableNumberInput/useDragGesture.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/scrubableNumberInput/useDragGesture.ts
Add a pointerleave sibling to the pointercancel test, and type the mount helper's options as Partial<DragOptions> with explicitly-typed callback mocks so option-key typos are caught at compile time instead of cast away.
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
Reworks
ScrubableNumberInputinto a Tweeq-style two-axis drag scrub: X moves value, Y rescales sensitivity through orders of magnitude. Pointer-lock hides the cursor during scrub; an SVG dot-ruler overlay visualises the active precision.Changes
scrubableNumberInput/folder with a layered split —interpretGesture.ts(pure algorithm),useScrubValue.ts(state),useDragGesture.ts(DOM pointer wrangling + pointer-lock),BallRuler.vue(pure SVG view).ScrubableNumberInput.vuebecomes a thin orchestrator. Drag now uses long-press (0.5s) or 1-px threshold to commit; click still focuses the input. While scrubbing, the ± buttons fade out and four chevron hints appear on the edges.WidgetInputNumberInput,WidgetBoundingBox,RangeEditor,LinearControls) work unmodified.Review Focus
A few design tastes worth knowing before reviewing:
DRAG_PX_FOR_FULL_RANGEandDRAG_PX_PER_STEP_AT_FLOOR— describe the Y-axis bounds in plain English ("drag N px to cover the range at max sensitivity"). Translate tominSpeed/maxSpeedinline; no hidden coefficients.useScrubValueaccumulates a raw value and runs the validator only on the way out — a tiny per-frame delta still accrues until it crosses a step boundary.reset()clears the EMA / weight but notspeedMult, so a slip-up release doesn't force re-calibration. Each input has its own closure → independent per slider for free.useElementSize(ResizeObservercontentRect→ logical px, transform-independent) and the bar fill iswidth: %(also logical). Sensitivity usesstep, no width involved.BallRulerdashoffset anchoring is mathematically exact in bar mode.dashOffset = (value − min)/(max − min) × widthguarantees one ball lies on the handle for any dash gap (j = ⌊D/N⌋trick). In free mode the+width/2term phase-aligns a ball to centre at value = 0, paired with pointer-lock so the locked cursor is the visual anchor.Screenshots
CleanShot.2026-05-21.at.22.10.22-converted.mp4
CleanShot.2026-05-22.at.19.10.04.mp4