Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize defaultWheelDelta #157

Closed
wants to merge 1 commit into from
Closed

Normalize defaultWheelDelta #157

wants to merge 1 commit into from

Conversation

Dakkaron
Copy link
Contributor

Normalized defaultWheelDelta so that it returns the same scaling speed in Chrome/FF/Edge. IE is still slightly different. Works with all three deltaModes (0 => Pixel, 1 => Lines , 2 => Pages). Also uses event.wheelDeltaY when possible (Chrome and Edge), because it returns the same value on both browsers, while event.deltaY varies widely.

Related: #100

Normalized defaultWheelDelta so that it returns the same scaling speed in Chrome/FF/Edge. IE is still slightly different.
@mbostock
Copy link
Member

mbostock commented Nov 17, 2018

My interpretation is that you are proposing two independent changes to the default wheel normalization behavior, so I’d like to disentangle these changes and discuss them separately.

First, wheel vs. mousewheel. d3-zoom listens only to wheel events, which is the recommended standard:

.on("wheel.zoom", wheeled)

event.wheelDeltaY is part of the deprecated mousewheel event. While Chrome and other browsers treat the deprecated mousewheel event as an alias for the wheel event and report the deprecated mousewheel properties on wheel events, we shouldn’t use deprecated fields in browsers that support recommended standards. (And even if we did want to use the deprecated wheelDeltaY, it probably doesn’t make sense to apply a multiplier derived from non-deprecated deltaMode.)

Second, this changes the delta multiplier for DOM_DELTA_LINE (deltaMode = 1) from 120 to 40, and the multiplier for DOM_DELTA_PAGE (deltaMode = 2) from 120 to 800. Is that your intention, or was your primary intention just to favor the deprecated mousewheel event? If so, the new discrete multipliers seems reasonable, but I don’t have a discrete mouse to test on.

@Herst
Copy link
Contributor

Herst commented Nov 18, 2018

Here is a list of the values of wheelDeltaY vs. deltaY (albeit a bit older but I sure it is still largely valid): https://stackoverflow.com/a/24595588

@mbostock
Copy link
Member

mbostock commented Aug 5, 2019

Merged as f21cdfd, then adjusted as 30d47e0 so that one “page” scroll corresponds to exactly one power-of-two zoom level, and one “line” scroll corresponds to 1/20th of a page.

So, to summarize:

For deltaMode = 0, the multiplier is still 0.002 (1 / 500).
For deltaMode = 1, the multiplier decreased from 0.24 (120 / 500) to 0.05.
For deltaMode = 2, the multiplier increased from 0.24 (120 / 500) to 1.

@mbostock mbostock closed this Aug 5, 2019
@Jones-S
Copy link

Jones-S commented May 11, 2023

I see that this was solved by the new multipliers.
But I still have a problem though: on windows computers (it seems) my event.deltaY is either 120,108 or 102. Now if I use the default wheelDelta it makes huge jumps. e.g. from 100% to 411% after one "roll". (I need to mention that in my case 1 roll does nothing, but if my wheel rolls 3 "clicks", I have an actual wheelDelta of 306 (probably 3 * 102). And so I land directly at 411% zoom. If I cause one more zoom step I end up directly at 1200% (which is my max zoom).

I first thought I could just check for

if (Math.abs(event.deltaY >= 102) {
// return a smaller value
}

But (!) if I scroll quickly on an apple magic mouse or if I use a trackpad and scroll very fast, the delta value sometimes goes above 102 as well and in this case it is wanted behaviour. Now google for example must have a solution for their map zooming as well, but I can't really figure out how I could catch this. Do I have to try to check for the OS?

I would be very glad for any ideas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants