fix(wheel): exclude Mac Chrome linear wheelDelta from legacy mouse detection#306
Merged
Merged
Conversation
…tection Fast trackpad swipes on Mac Chrome/YaBrowser were misclassified as zoom because wheelDelta ≈ 3×deltaY crossed the legacy threshold. Treat that ratio as trackpad. Co-authored-by: Cursor <cursoragent@cursor.com>
Reviewer's GuideRefines wheel intent resolution to avoid treating Mac Chrome/YaBrowser linear wheelDelta trackpad scrolls as legacy mouse zoom, adds explicit wheel input device modes, introduces a rapid-stream sticky intent post-pass, and updates docs, tests, stories, and settings wiring accordingly. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Preview is ready. |
Explicit trackpad/mouse/auto mode for ambiguous Mac Chrome input; I5:sticky-stream prevents pan/zoom flips mid-gesture. Document camera behavior and Storybook controls. Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes e2e CI webServer startup: tsc failed on metaKey in makeMacChromeTrackpadWheelEvent. Co-authored-by: Cursor <cursoragent@cursor.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The rapid-stream sticky logic relies on hardcoded rule ids (e.g. comparing to "I5:last-intent" and tests checking rule prefixes), which could become brittle as rules evolve — consider centralizing rule identifiers in constants or an enum to avoid string coupling.
- In
resolveExplicitMouseIntent,markMouseWheelBurstis called for all branches including the fallbackI4:input-device-mousewithout classic mouse signals; review whether the burst window should be limited to truly mouse-like steps to avoid unintended smoothing behavior in explicit mouse mode.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The rapid-stream sticky logic relies on hardcoded rule ids (e.g. comparing to "I5:last-intent" and tests checking rule prefixes), which could become brittle as rules evolve — consider centralizing rule identifiers in constants or an enum to avoid string coupling.
- In `resolveExplicitMouseIntent`, `markMouseWheelBurst` is called for all branches including the fallback `I4:input-device-mouse` without classic mouse signals; review whether the burst window should be limited to truly mouse-like steps to avoid unintended smoothing behavior in explicit mouse mode.
## Individual Comments
### Comment 1
<location path="src/utils/functions/wheelIntent.ts" line_range="503" />
<code_context>
+ * and Mac Chrome/YaBrowser heuristics are ambiguous.
*/
-export function createWheelIntentResolver(): TResolveWheelIntent {
+export function createWheelIntentResolver(options: TCreateWheelIntentResolverOptions = {}): TResolveWheelIntent {
+ const inputDevice = options.inputDevice ?? "auto";
let lastIntent: EWheelIntent = EWheelIntent.Zoom;
</code_context>
<issue_to_address>
**issue (complexity):** Consider introducing a cohesive TWheelStreamState object and passing it through helpers to centralize stream-related state and simplify resolver branching and helper signatures.
The added input-device and sticky-stream behavior is valuable, but the resolver’s branching and helper signatures have grown enough that a small refactor would reduce cognitive load without changing behavior.
You can keep all functionality while:
1. **Centralizing stream-related state into a small object**
2. **Passing that object into helpers (`resolveExplicitMouseIntent`, `applyRapidStreamStickyIntent`, `emitDebugEntry`) instead of many positional arguments**
This keeps the main decision tree linear and reduces coupling/parameter surface.
### 1. Introduce a `TWheelStreamState` object
```ts
type TWheelStreamState = {
now: number;
timeSinceLastWheel: number;
isRapidStream: boolean;
inMouseWheelBurst: boolean;
mouseWheelBurstRemainingMs: number | null;
lastIntentBefore: EWheelIntent;
lastRuleBefore: string;
};
```
Build it once inside the resolver:
```ts
return (event: WheelEvent, mouseWheelBehavior: TMouseWheelBehavior): EWheelIntent => {
const now = performance.now();
const timeSince = lastTimestamp !== null ? now - lastTimestamp : Number.POSITIVE_INFINITY;
lastTimestamp = now;
const isRapidStream = timeSince < RAPID_STREAM_MS;
const inMouseWheelBurst = isInMouseWheelBurst(now);
const mouseWheelBurstRemainingMs =
mouseWheelBurstUntil !== null ? Math.max(0, mouseWheelBurstUntil - now) : null;
const ctx = createWheelContext(event);
const signals = buildWheelSignals(ctx);
const streamState: TWheelStreamState = {
now,
timeSinceLastWheel: timeSince,
isRapidStream,
inMouseWheelBurst,
mouseWheelBurstRemainingMs,
lastIntentBefore: lastIntent,
lastRuleBefore: lastRule,
};
let intent: EWheelIntent;
let rule: string;
// ... main decision tree ...
};
```
### 2. Use `streamState` in helpers instead of long parameter lists
`resolveExplicitMouseIntent` becomes simpler and less coupled:
```ts
const resolveExplicitMouseIntent = (
ctx: TWheelContext,
signals: TWheelIntentDebugEntry["signals"],
mouseWheelBehavior: TMouseWheelBehavior,
stream: TWheelStreamState
): { intent: EWheelIntent; rule: string } => {
const { isRapidStream, inMouseWheelBurst, now } = stream;
if (signals.isDominantAxisLargeWheel || signals.isClassicMouseWheelStep || signals.hasLegacyMouseWheelDelta) {
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: signals.isClassicMouseWheelStep ? "I4:mouse-wheel-step" : "I4:large-step",
};
}
if (isSlowFractionalMouseWheelStep(ctx, isRapidStream, inMouseWheelBurst)) {
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: "I4:fractional-mouse",
};
}
if (inMouseWheelBurst && signals.isVerticalOnly && isTrackpadLikeRapidSmall(ctx, isRapidStream)) {
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: "I4-burst:smoothing",
};
}
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: "I4:input-device-mouse",
};
};
```
`applyRapidStreamStickyIntent` can use the same `streamState`, avoiding extra parameters and making the contract clearer:
```ts
function applyRapidStreamStickyIntent(
intent: EWheelIntent,
rule: string,
signals: TWheelIntentDebugEntry["signals"],
stream: TWheelStreamState
): { intent: EWheelIntent; rule: string } {
const { isRapidStream, lastIntentBefore, lastRuleBefore } = stream;
if (
!isRapidStream ||
signals.isPinchZoom ||
signals.isDiagonalScroll ||
signals.isPredominantHorizontalScroll ||
intent === lastIntentBefore ||
lastRuleBefore === "I5:last-intent"
) {
return { intent, rule };
}
return { intent: lastIntentBefore, rule: "I5:sticky-stream" };
}
```
Call sites stay linear:
```ts
if (inputDevice === "mouse") {
({ intent, rule } = resolveExplicitMouseIntent(ctx, signals, mouseWheelBehavior, streamState));
} else if (isIntegerPixelTrackpadScroll(ctx, streamState.isRapidStream)) {
// ...
}
// After main rules:
({ intent, rule } = applyRapidStreamStickyIntent(intent, rule, signals, streamState));
lastIntent = intent;
lastRule = rule;
```
### 3. Use `streamState` and `inputDevice` in debug plumbing
`emitDebugEntry` can accept a single `stream` object plus `inputDevice`, keeping the boundary small:
```ts
function emitDebugEntry(
ctx: TWheelContext,
mouseWheelBehavior: TMouseWheelBehavior,
inputDevice: TWheelInputDevice,
stream: TWheelStreamState,
signals: TWheelIntentDebugEntry["signals"],
rule: string,
result: EWheelIntent
): void {
const debugLogger = getWheelIntentDebugLogger();
if (debugLogger === null) return;
const { event, normX, normY } = ctx;
const {
timeSinceLastWheel,
isRapidStream,
inMouseWheelBurst,
mouseWheelBurstRemainingMs,
lastIntentBefore,
} = stream;
debugLogger({
mouseWheelBehavior,
inputDevice,
input: {
deltaX: event.deltaX,
deltaY: event.deltaY,
deltaMode: event.deltaMode,
deltaModeLabel: deltaModeLabel(event.deltaMode),
ctrlKey: event.ctrlKey,
metaKey: event.metaKey,
shiftKey: event.shiftKey,
altKey: event.altKey,
},
ctx: {
normX,
normY,
timeSinceLastWheel,
isRapidStream,
isInMouseWheelBurst: inMouseWheelBurst,
mouseWheelBurstRemainingMs,
lastIntentBefore,
signals,
},
rule,
result,
});
}
```
This keeps all existing functionality (explicit `inputDevice`, rapid-stream sticky behavior, burst tracking, Mac Chrome wheelDelta heuristics) while making the resolver easier to follow: stream-related state is built once, passed as a cohesive unit, and both helpers and debug output depend on it rather than on wide positional argument lists.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| * and Mac Chrome/YaBrowser heuristics are ambiguous. | ||
| */ | ||
| export function createWheelIntentResolver(): TResolveWheelIntent { | ||
| export function createWheelIntentResolver(options: TCreateWheelIntentResolverOptions = {}): TResolveWheelIntent { |
There was a problem hiding this comment.
issue (complexity): Consider introducing a cohesive TWheelStreamState object and passing it through helpers to centralize stream-related state and simplify resolver branching and helper signatures.
The added input-device and sticky-stream behavior is valuable, but the resolver’s branching and helper signatures have grown enough that a small refactor would reduce cognitive load without changing behavior.
You can keep all functionality while:
- Centralizing stream-related state into a small object
- Passing that object into helpers (
resolveExplicitMouseIntent,applyRapidStreamStickyIntent,emitDebugEntry) instead of many positional arguments
This keeps the main decision tree linear and reduces coupling/parameter surface.
1. Introduce a TWheelStreamState object
type TWheelStreamState = {
now: number;
timeSinceLastWheel: number;
isRapidStream: boolean;
inMouseWheelBurst: boolean;
mouseWheelBurstRemainingMs: number | null;
lastIntentBefore: EWheelIntent;
lastRuleBefore: string;
};Build it once inside the resolver:
return (event: WheelEvent, mouseWheelBehavior: TMouseWheelBehavior): EWheelIntent => {
const now = performance.now();
const timeSince = lastTimestamp !== null ? now - lastTimestamp : Number.POSITIVE_INFINITY;
lastTimestamp = now;
const isRapidStream = timeSince < RAPID_STREAM_MS;
const inMouseWheelBurst = isInMouseWheelBurst(now);
const mouseWheelBurstRemainingMs =
mouseWheelBurstUntil !== null ? Math.max(0, mouseWheelBurstUntil - now) : null;
const ctx = createWheelContext(event);
const signals = buildWheelSignals(ctx);
const streamState: TWheelStreamState = {
now,
timeSinceLastWheel: timeSince,
isRapidStream,
inMouseWheelBurst,
mouseWheelBurstRemainingMs,
lastIntentBefore: lastIntent,
lastRuleBefore: lastRule,
};
let intent: EWheelIntent;
let rule: string;
// ... main decision tree ...
};2. Use streamState in helpers instead of long parameter lists
resolveExplicitMouseIntent becomes simpler and less coupled:
const resolveExplicitMouseIntent = (
ctx: TWheelContext,
signals: TWheelIntentDebugEntry["signals"],
mouseWheelBehavior: TMouseWheelBehavior,
stream: TWheelStreamState
): { intent: EWheelIntent; rule: string } => {
const { isRapidStream, inMouseWheelBurst, now } = stream;
if (signals.isDominantAxisLargeWheel || signals.isClassicMouseWheelStep || signals.hasLegacyMouseWheelDelta) {
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: signals.isClassicMouseWheelStep ? "I4:mouse-wheel-step" : "I4:large-step",
};
}
if (isSlowFractionalMouseWheelStep(ctx, isRapidStream, inMouseWheelBurst)) {
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: "I4:fractional-mouse",
};
}
if (inMouseWheelBurst && signals.isVerticalOnly && isTrackpadLikeRapidSmall(ctx, isRapidStream)) {
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: "I4-burst:smoothing",
};
}
markMouseWheelBurst(now);
return {
intent: intentFromMouseWheelBehavior(mouseWheelBehavior),
rule: "I4:input-device-mouse",
};
};applyRapidStreamStickyIntent can use the same streamState, avoiding extra parameters and making the contract clearer:
function applyRapidStreamStickyIntent(
intent: EWheelIntent,
rule: string,
signals: TWheelIntentDebugEntry["signals"],
stream: TWheelStreamState
): { intent: EWheelIntent; rule: string } {
const { isRapidStream, lastIntentBefore, lastRuleBefore } = stream;
if (
!isRapidStream ||
signals.isPinchZoom ||
signals.isDiagonalScroll ||
signals.isPredominantHorizontalScroll ||
intent === lastIntentBefore ||
lastRuleBefore === "I5:last-intent"
) {
return { intent, rule };
}
return { intent: lastIntentBefore, rule: "I5:sticky-stream" };
}Call sites stay linear:
if (inputDevice === "mouse") {
({ intent, rule } = resolveExplicitMouseIntent(ctx, signals, mouseWheelBehavior, streamState));
} else if (isIntegerPixelTrackpadScroll(ctx, streamState.isRapidStream)) {
// ...
}
// After main rules:
({ intent, rule } = applyRapidStreamStickyIntent(intent, rule, signals, streamState));
lastIntent = intent;
lastRule = rule;3. Use streamState and inputDevice in debug plumbing
emitDebugEntry can accept a single stream object plus inputDevice, keeping the boundary small:
function emitDebugEntry(
ctx: TWheelContext,
mouseWheelBehavior: TMouseWheelBehavior,
inputDevice: TWheelInputDevice,
stream: TWheelStreamState,
signals: TWheelIntentDebugEntry["signals"],
rule: string,
result: EWheelIntent
): void {
const debugLogger = getWheelIntentDebugLogger();
if (debugLogger === null) return;
const { event, normX, normY } = ctx;
const {
timeSinceLastWheel,
isRapidStream,
inMouseWheelBurst,
mouseWheelBurstRemainingMs,
lastIntentBefore,
} = stream;
debugLogger({
mouseWheelBehavior,
inputDevice,
input: {
deltaX: event.deltaX,
deltaY: event.deltaY,
deltaMode: event.deltaMode,
deltaModeLabel: deltaModeLabel(event.deltaMode),
ctrlKey: event.ctrlKey,
metaKey: event.metaKey,
shiftKey: event.shiftKey,
altKey: event.altKey,
},
ctx: {
normX,
normY,
timeSinceLastWheel,
isRapidStream,
isInMouseWheelBurst: inMouseWheelBurst,
mouseWheelBurstRemainingMs,
lastIntentBefore,
signals,
},
rule,
result,
});
}This keeps all existing functionality (explicit inputDevice, rapid-stream sticky behavior, burst tracking, Mac Chrome wheelDelta heuristics) while making the resolver easier to follow: stream-related state is built once, passed as a cohesive unit, and both helpers and debug output depend on it rather than on wide positional argument lists.
Replace string literals and startsWith checks with WHEEL_INTENT_RULE and Set-based helpers for faster, type-safe rule comparisons. Co-authored-by: Cursor <cursoragent@cursor.com>
…stants Move WHEEL_INPUT_DEVICE to camera constants and bundle mouseWheelBehavior with wheelInputDevice in TResolveWheelIntentOptions so the default resolver no longer needs recreation. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Summary
wheelDelta ≈ 3×deltaY(Mac Chrome trackpad pattern) fromhasLegacyMouseWheelDeltadetectionTest plan
npm test -- wheelIntent.test.ts(28 tests pass)MOUSE_WHEEL_BEHAVIOR=zoomMade with Cursor
Summary by Sourcery
Adjust wheel intent resolution to better distinguish trackpad vs mouse input, prevent mid-gesture intent flips, and expose explicit wheel input device configuration, with updated docs, stories, and tests.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: