From 6c0c5f4c6e1dc19a782e7ad07f6fac8db59f3d55 Mon Sep 17 00:00:00 2001 From: Luke Cotter <4013877+lukecotter@users.noreply.github.com> Date: Wed, 22 Jul 2026 23:39:40 +0100 Subject: [PATCH] fix(timeline): unify and clamp cross-platform wheel zoom The three timeline wheel handlers each used an unbounded linear factor (1 + -deltaY*0.001), which zoomed in large jumps on Windows mice, could go <=0 on fast scrolls, and drifted (in-then-out did not return to the same zoom). Extract one wheelZoomFactor() helper (clamped, exponential) in ViewportUtils and share it across the main, minimap and metric-strip handlers. --- CHANGELOG.md | 4 + .../timeline/optimised/ViewportUtils.ts | 33 +++ .../optimised/__tests__/ViewportUtils.test.ts | 65 ++++++ .../interaction/TimelineInteractionHandler.ts | 33 +-- .../TimelineInteractionHandler.test.ts | 216 ++++++++++++++++++ .../metric-strip/MetricStripOrchestrator.ts | 11 +- .../minimap/MinimapInteractionHandler.ts | 5 +- 7 files changed, 333 insertions(+), 34 deletions(-) create mode 100644 log-viewer/src/features/timeline/optimised/__tests__/ViewportUtils.test.ts create mode 100644 log-viewer/src/features/timeline/optimised/interaction/__tests__/TimelineInteractionHandler.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7041efda..6588936d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 🚧 **Timeline truncation markers** now end where the log recovers, so trusted sections are no longer greyed out. ([#828]) - πŸ—‚οΈ **Call Tree + Database styling**: VS Code style tree icons, and rows indent under their group headings. ([#832]). +### Fixed + +- πŸ–±οΈ **Timeline wheel zoom**: consistent, smooth zoom across platforms and input devices β€” a Windows mouse wheel no longer over-zooms in large jumps, fast scrolls stay bounded, and zooming in then back out returns to the same level. + ## [1.20.0] 2026-06-18 ### Added diff --git a/log-viewer/src/features/timeline/optimised/ViewportUtils.ts b/log-viewer/src/features/timeline/optimised/ViewportUtils.ts index 5ea5ced1..3ac8cdfb 100644 --- a/log-viewer/src/features/timeline/optimised/ViewportUtils.ts +++ b/log-viewer/src/features/timeline/optimised/ViewportUtils.ts @@ -45,3 +45,36 @@ export function calculateViewportBounds(viewport: ViewportState): ViewportBounds depthEnd, }; } + +/** + * Upper bound on the per-event normalized wheel delta (~one WHEEL_DELTA mouse + * notch). Windows mouse notches report a far larger deltaY than a macOS + * trackpad, so clamping keeps a single notch/gesture to a consistent, bounded + * zoom step across platforms and prevents fast scrolls from jumping. + */ +const MAX_WHEEL_DELTA = 120; + +/** + * Map a wheel event to a zoom multiplier, shared by every timeline wheel + * handler (main flame chart, minimap, metric strip) so zoom feel is identical. + * + * Exponential so zoom-in and zoom-out of equal magnitude are reciprocal (no + * drift when scrolling in then out) and the factor is always > 0; the delta is + * clamped so one large event can't produce a jarring jump or a sign flip. + * + * @param deltaY - Raw WheelEvent.deltaY (positive = scroll down = zoom out). + * @param deltaMode - WheelEvent.deltaMode (0 pixel, 1 line, 2 page). + * @param sensitivity - Multiplier on the zoom step. Default 1. + * @returns Zoom multiplier to apply to the current zoom (>0; 1 = no change). + */ +export function wheelZoomFactor(deltaY: number, deltaMode: number, sensitivity = 1): number { + // Scroll up (negative deltaY) zooms in (factor > 1). + let delta = -deltaY; + if (deltaMode === 1) { + delta *= 15; // lines β†’ approx pixels + } else if (deltaMode === 2) { + delta *= 800; // pages β†’ approx pixels + } + delta = Math.max(-MAX_WHEEL_DELTA, Math.min(MAX_WHEEL_DELTA, delta)); + return Math.exp(delta * 0.001 * sensitivity); +} diff --git a/log-viewer/src/features/timeline/optimised/__tests__/ViewportUtils.test.ts b/log-viewer/src/features/timeline/optimised/__tests__/ViewportUtils.test.ts new file mode 100644 index 00000000..91d43856 --- /dev/null +++ b/log-viewer/src/features/timeline/optimised/__tests__/ViewportUtils.test.ts @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2025 Certinia Inc. All rights reserved. + */ + +/** + * Unit tests for wheelZoomFactor β€” the shared wheelβ†’zoom mapping used by the + * main flame chart, minimap, and metric-strip wheel handlers. Covers the + * cross-platform properties that motivated the shared helper (#853 follow-up): + * direction, always-positive factor, in/out symmetry, per-event clamping, and + * deltaMode normalization. + */ +import { describe, expect, it } from '@jest/globals'; + +import { wheelZoomFactor } from '../ViewportUtils.js'; + +describe('wheelZoomFactor', () => { + it('zooms in (factor > 1) on scroll up and out (factor < 1) on scroll down', () => { + expect(wheelZoomFactor(-10, 0)).toBeGreaterThan(1); + expect(wheelZoomFactor(10, 0)).toBeLessThan(1); + }); + + it('returns 1 for a zero delta', () => { + expect(wheelZoomFactor(0, 0)).toBe(1); + }); + + it('is always positive, even for an extreme delta', () => { + // Old linear formula (1 + delta*0.001) went <= 0 at deltaY >= 1000. + expect(wheelZoomFactor(100000, 0)).toBeGreaterThan(0); + expect(wheelZoomFactor(-100000, 0)).toBeGreaterThan(0); + }); + + it('is symmetric: zooming in then out by the same delta returns to the start', () => { + const zoomIn = wheelZoomFactor(-40, 0); + const zoomOut = wheelZoomFactor(40, 0); + expect(zoomIn * zoomOut).toBeCloseTo(1, 10); + }); + + it('clamps the per-event delta so large deltas saturate to the same factor', () => { + // Beyond the clamp (Β±120), factor no longer grows with delta. + const atCap = wheelZoomFactor(-120, 0); + const wayOverCap = wheelZoomFactor(-5000, 0); + expect(wayOverCap).toBeCloseTo(atCap, 10); + // And a Windows-sized notch (~100) is close to, but under, the cap. + expect(wheelZoomFactor(-100, 0)).toBeLessThan(atCap); + }); + + it('normalizes line mode (deltaMode 1) larger than pixel mode', () => { + // Small line deltas map to a bigger step than the same pixel value. + expect(wheelZoomFactor(-3, 1)).toBeGreaterThan(wheelZoomFactor(-3, 0)); + // A line delta of 8 (Γ—15 = 120) reaches the clamp. + expect(wheelZoomFactor(-8, 1)).toBeCloseTo(wheelZoomFactor(-120, 0), 10); + }); + + it('normalizes page mode (deltaMode 2) to the clamp for any real page delta', () => { + // A single page delta (Γ—800) always saturates the clamp. + expect(wheelZoomFactor(-1, 2)).toBeCloseTo(wheelZoomFactor(-120, 0), 10); + }); + + it('scales the step by sensitivity', () => { + const base = wheelZoomFactor(-50, 0, 1); + const sensitive = wheelZoomFactor(-50, 0, 2); + // Doubling sensitivity squares the multiplier (exponential mapping). + expect(sensitive).toBeCloseTo(base * base, 10); + }); +}); diff --git a/log-viewer/src/features/timeline/optimised/interaction/TimelineInteractionHandler.ts b/log-viewer/src/features/timeline/optimised/interaction/TimelineInteractionHandler.ts index 647c54c1..2dd6da3c 100644 --- a/log-viewer/src/features/timeline/optimised/interaction/TimelineInteractionHandler.ts +++ b/log-viewer/src/features/timeline/optimised/interaction/TimelineInteractionHandler.ts @@ -15,6 +15,7 @@ import { type ModifierKeys, } from '../../types/flamechart.types.js'; import type { TimelineViewport } from '../TimelineViewport.js'; +import { wheelZoomFactor } from '../ViewportUtils.js'; /** * Configuration for interaction behavior. @@ -561,30 +562,14 @@ export class TimelineInteractionHandler { const rect = this.canvas.getBoundingClientRect(); const mouseX = event.clientX - rect.left; - // Calculate zoom delta - // deltaY is positive when scrolling down, negative when scrolling up - // Standard behavior: scroll up = zoom in, scroll down = zoom out - let zoomDelta = -event.deltaY; - - // Apply zoom inversion if enabled - if (this.options.invertZoom) { - zoomDelta = -zoomDelta; - } - - // Normalize wheel delta (different browsers report different scales) - // Use deltaMode to handle different units (pixels, lines, pages) - let normalizedDelta = zoomDelta; - if (event.deltaMode === 1) { - // Lines - multiply by typical line height - normalizedDelta *= 15; - } else if (event.deltaMode === 2) { - // Pages - multiply by typical page height - normalizedDelta *= 800; - } - - // Calculate zoom factor - // Use exponential scaling for smooth zoom feel - const zoomFactor = 1 + normalizedDelta * 0.001 * this.options.zoomSensitivity; + // Clamped, exponential zoom factor shared with the minimap and metric-strip + // wheel handlers for consistent feel across platforms and devices. + // invertZoom flips scroll direction by negating deltaY before mapping. + const zoomFactor = wheelZoomFactor( + this.options.invertZoom ? -event.deltaY : event.deltaY, + event.deltaMode, + this.options.zoomSensitivity, + ); // Get current zoom and calculate new zoom const currentState = this.viewport.getState(); diff --git a/log-viewer/src/features/timeline/optimised/interaction/__tests__/TimelineInteractionHandler.test.ts b/log-viewer/src/features/timeline/optimised/interaction/__tests__/TimelineInteractionHandler.test.ts new file mode 100644 index 00000000..62e0b3b4 --- /dev/null +++ b/log-viewer/src/features/timeline/optimised/interaction/__tests__/TimelineInteractionHandler.test.ts @@ -0,0 +1,216 @@ +/** + * @jest-environment jsdom + */ + +/* + * Copyright (c) 2025 Certinia Inc. All rights reserved. + */ + +/** + * Unit tests for TimelineInteractionHandler + * + * Covers the wheel/mouse event wiring implicated in the Windows "zoom not + * working" report (#853) β€” the layer that translates native WheelEvent / + * MouseEvent (deltaMode, shiftKey, altKey) into viewport calls: + * - bare wheel β†’ mouse-anchored zoom (in/out) + * - deltaMode line/page normalization + * - Shift+wheel / Alt+wheel β†’ pan (never zoom) + * - enableZoom:false β†’ wheel no-op + * - Alt/Shift+mousedown β†’ area-zoom / measurement start, with Alt priority + * - non-left button ignored; preventDefault on wheel + */ +import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'; + +import { TimelineViewport } from '../../TimelineViewport.js'; +import { wheelZoomFactor } from '../../ViewportUtils.js'; +import { + type InteractionCallbacks, + TimelineInteractionHandler, +} from '../TimelineInteractionHandler.js'; + +describe('TimelineInteractionHandler', () => { + const DISPLAY_WIDTH = 1000; + const DISPLAY_HEIGHT = 600; + const TOTAL_DURATION = 1_000_000; + const MAX_DEPTH = 10; + + let canvas: HTMLCanvasElement; + let viewport: TimelineViewport; + let handler: TimelineInteractionHandler; + + function createHandler( + options: ConstructorParameters[2] = {}, + callbacks: InteractionCallbacks = {}, + ): TimelineInteractionHandler { + return new TimelineInteractionHandler(canvas, viewport, options, callbacks); + } + + function dispatchWheel(init: WheelEventInit): WheelEvent { + const event = new WheelEvent('wheel', { bubbles: true, cancelable: true, ...init }); + canvas.dispatchEvent(event); + return event; + } + + function dispatchMouseDown(init: MouseEventInit): MouseEvent { + const event = new MouseEvent('mousedown', { bubbles: true, cancelable: true, ...init }); + canvas.dispatchEvent(event); + return event; + } + + beforeEach(() => { + canvas = document.createElement('canvas'); + document.body.appendChild(canvas); + viewport = new TimelineViewport(DISPLAY_WIDTH, DISPLAY_HEIGHT, TOTAL_DURATION, MAX_DEPTH); + }); + + afterEach(() => { + handler.destroy(); + document.body.removeChild(canvas); + jest.restoreAllMocks(); + }); + + describe('wheel zoom', () => { + it('zooms in on scroll up (negative deltaY), anchored at the cursor', () => { + handler = createHandler(); + const setZoom = jest.spyOn(viewport, 'setZoom'); + const startZoom = viewport.getState().zoom; + + dispatchWheel({ deltaY: -10, clientX: 250 }); + + expect(setZoom).toHaveBeenCalledTimes(1); + const [requestedZoom, anchorX] = setZoom.mock.calls[0] as [number, number]; + expect(requestedZoom).toBeGreaterThan(startZoom); + // canvas rect.left is 0 in jsdom, so anchor == clientX + expect(anchorX).toBe(250); + }); + + it('zooms out on scroll down (positive deltaY)', () => { + handler = createHandler(); + const setZoom = jest.spyOn(viewport, 'setZoom'); + const startZoom = viewport.getState().zoom; + + dispatchWheel({ deltaY: 10, clientX: 250 }); + + expect(setZoom).toHaveBeenCalledTimes(1); + const [requestedZoom] = setZoom.mock.calls[0] as [number, number]; + expect(requestedZoom).toBeLessThan(startZoom); + }); + + it('calls preventDefault so the webview does not scroll the page', () => { + handler = createHandler(); + const event = dispatchWheel({ deltaY: -10, clientX: 250 }); + expect(event.defaultPrevented).toBe(true); + }); + + it('applies the shared wheelZoomFactor to the current zoom', () => { + handler = createHandler(); + const setZoom = jest.spyOn(viewport, 'setZoom'); + const startZoom = viewport.getState().zoom; + + dispatchWheel({ deltaY: -10, deltaMode: 0, clientX: 100 }); + + const [requestedZoom] = setZoom.mock.calls[0] as [number, number]; + expect(requestedZoom).toBeCloseTo(startZoom * wheelZoomFactor(-10, 0, 1), 10); + }); + + it('clamps a large delta so one event cannot produce a huge jump', () => { + handler = createHandler(); + const setZoom = jest.spyOn(viewport, 'setZoom'); + const startZoom = viewport.getState().zoom; + + // Windows fast-scroll / momentum: an unclamped linear factor would go + // negative here; clamped exponential stays bounded and positive. + dispatchWheel({ deltaY: -10000, deltaMode: 0, clientX: 100 }); + + const [requestedZoom] = setZoom.mock.calls[0] as [number, number]; + expect(requestedZoom).toBeCloseTo(startZoom * wheelZoomFactor(-10000, 0, 1), 10); + expect(requestedZoom).toBeLessThan(startZoom * 1.2); // bounded, no runaway jump + }); + + it('does not zoom when enableZoom is false', () => { + handler = createHandler({ enableZoom: false }); + const setZoom = jest.spyOn(viewport, 'setZoom'); + + dispatchWheel({ deltaY: -10, clientX: 250 }); + + expect(setZoom).not.toHaveBeenCalled(); + }); + }); + + describe('wheel pan (never zoom)', () => { + it('Shift+wheel pans vertically when deltaY dominates', () => { + handler = createHandler(); + const panBy = jest.spyOn(viewport, 'panBy'); + const setZoom = jest.spyOn(viewport, 'setZoom'); + + dispatchWheel({ deltaY: 40, deltaX: 0, shiftKey: true }); + + expect(setZoom).not.toHaveBeenCalled(); + expect(panBy).toHaveBeenCalledWith(0, 40); + }); + + it('Shift+wheel pans horizontally when deltaX dominates', () => { + handler = createHandler(); + const panBy = jest.spyOn(viewport, 'panBy'); + + dispatchWheel({ deltaX: 40, deltaY: 5, shiftKey: true }); + + expect(panBy).toHaveBeenCalledWith(40, 0); + }); + + it('Alt+wheel pans horizontally using -deltaY', () => { + handler = createHandler(); + const panBy = jest.spyOn(viewport, 'panBy'); + const setZoom = jest.spyOn(viewport, 'setZoom'); + + dispatchWheel({ deltaY: 40, altKey: true }); + + expect(setZoom).not.toHaveBeenCalled(); + expect(panBy).toHaveBeenCalledWith(-40, 0); + }); + }); + + describe('mousedown modes', () => { + it('Alt+mousedown starts area zoom at the cursor', () => { + const onAreaZoomStart = jest.fn<(screenX: number) => void>(); + const onMeasureStart = jest.fn<(screenX: number) => void>(); + handler = createHandler({}, { onAreaZoomStart, onMeasureStart }); + + dispatchMouseDown({ button: 0, altKey: true, clientX: 300 }); + + expect(onAreaZoomStart).toHaveBeenCalledWith(300); + expect(onMeasureStart).not.toHaveBeenCalled(); + }); + + it('Shift+mousedown starts measurement at the cursor', () => { + const onMeasureStart = jest.fn<(screenX: number) => void>(); + handler = createHandler({}, { onMeasureStart }); + + dispatchMouseDown({ button: 0, shiftKey: true, clientX: 300 }); + + expect(onMeasureStart).toHaveBeenCalledWith(300); + }); + + it('gives Alt priority over Shift when both are held', () => { + const onAreaZoomStart = jest.fn<(screenX: number) => void>(); + const onMeasureStart = jest.fn<(screenX: number) => void>(); + handler = createHandler({}, { onAreaZoomStart, onMeasureStart }); + + dispatchMouseDown({ button: 0, altKey: true, shiftKey: true, clientX: 300 }); + + expect(onAreaZoomStart).toHaveBeenCalledTimes(1); + expect(onMeasureStart).not.toHaveBeenCalled(); + }); + + it('ignores non-left mouse buttons', () => { + const onAreaZoomStart = jest.fn<(screenX: number) => void>(); + const onMeasureStart = jest.fn<(screenX: number) => void>(); + handler = createHandler({}, { onAreaZoomStart, onMeasureStart }); + + dispatchMouseDown({ button: 2, altKey: true, clientX: 300 }); + + expect(onAreaZoomStart).not.toHaveBeenCalled(); + expect(onMeasureStart).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/log-viewer/src/features/timeline/optimised/metric-strip/MetricStripOrchestrator.ts b/log-viewer/src/features/timeline/optimised/metric-strip/MetricStripOrchestrator.ts index 74513c6a..efe10914 100644 --- a/log-viewer/src/features/timeline/optimised/metric-strip/MetricStripOrchestrator.ts +++ b/log-viewer/src/features/timeline/optimised/metric-strip/MetricStripOrchestrator.ts @@ -28,6 +28,7 @@ import type { ViewportState, } from '../../types/flamechart.types.js'; import { MeshAxisRenderer } from '../time-axis/MeshAxisRenderer.js'; +import { wheelZoomFactor } from '../ViewportUtils.js'; import { MetricStripRenderer } from './MetricStripRenderer.js'; import { MetricStripTooltipRenderer } from './MetricStripTooltipRenderer.js'; import { MetricTierClassifier } from './MetricTierClassifier.js'; @@ -604,13 +605,9 @@ export class MetricStripOrchestrator { return; } - // Vertical scroll β†’ zoom at cursor position - // Normalize wheel delta (match TimelineInteractionHandler behavior) - let normalizedDelta = -event.deltaY; - if (event.deltaMode === 1) { - normalizedDelta *= 15; // Lines mode - } - const zoomFactor = 1 + normalizedDelta * 0.001; + // Vertical scroll β†’ zoom at cursor position (shared with the main flame + // chart and minimap wheel handlers for consistent feel). + const zoomFactor = wheelZoomFactor(event.deltaY, event.deltaMode); const timeNs = this.screenXToTime(event.offsetX); if (timeNs !== null) { this.callbacks.onZoom?.(zoomFactor, timeNs); diff --git a/log-viewer/src/features/timeline/optimised/minimap/MinimapInteractionHandler.ts b/log-viewer/src/features/timeline/optimised/minimap/MinimapInteractionHandler.ts index 065fda23..f475f9ea 100644 --- a/log-viewer/src/features/timeline/optimised/minimap/MinimapInteractionHandler.ts +++ b/log-viewer/src/features/timeline/optimised/minimap/MinimapInteractionHandler.ts @@ -19,6 +19,7 @@ * - Hover: Show vertical guide on other view (cursor mirror) */ +import { wheelZoomFactor } from '../ViewportUtils.js'; import type { MinimapDragMode, MinimapViewport } from './MinimapViewport.js'; /** @@ -273,9 +274,7 @@ export class MinimapInteractionHandler { } // Vertical scroll = zoom - const zoomDelta = -event.deltaY; - const normalizedDelta = event.deltaMode === 1 ? zoomDelta * 15 : zoomDelta; - const zoomFactor = 1 + normalizedDelta * 0.001; + const zoomFactor = wheelZoomFactor(event.deltaY, event.deltaMode); this.callbacks.onZoom(zoomFactor, anchorTimeNs); }