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

Fix 2598 - mousewheel dom3 and wheel property #2633

Merged
merged 6 commits into from
Jul 23, 2014

Conversation

SergioCrisostomo
Copy link
Member

This pull requests fixes

  • the mousewheel event in Browsers using DOM3 wheel event
  • the .wheel property of MooTools's DOMEvent which would otherwise be broken in Firefox.

It also updates the normalize of the e.wheel value according to each browsers return on event.wheelDelta, event.detail and event.deltaY.

The reason why deltaY is mixed in here is because Firefox returns undefined for event.wheelDelta and 0 for event.detail, as can be tested in this jsFiddle: http://jsfiddle.net/mNdwW/

Tested in SauceLabs and manually on IE7-11, Opera (latest) Win & Mac, Safari 7, Chrome (latest) Win & Mac, Firefox (latest) Win & Mac

fixes #2598, closes #2599, closes #2620

@SergioCrisostomo SergioCrisostomo changed the title Fix 2598 - mousewheel dom3 and normalize wheel speed Fix 2598 - mousewheel dom3 and wheel property Jul 11, 2014
var rawAmount = evt.deltaY ? evt.deltaY : evt.detail;
normalized = -(rawAmount % 3 ? rawAmount * 10 : rawAmount / 3);
}
return normalized;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just:

return event.wheelDelta ? event.wheelData / 120 : -(event.deltaY || event.detail || 0) / 3

I don't really understand the (x % 120 - 0) == -0 or the rawAmount % 3.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use event instead of evt

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arian I wanted to check event.wheelDelta % 120 but since that can return -0 I thought doing (-0) - (-0) so I get a consistent number (always -0) which I can compare with == -0, would there be a better way? Math.abs? Have || for both 0 and -0?

The rawAmount % 3 is because FIrefox returns 3 and 0.1 multiples in respective Mac and Windows. This would detect that and normalize it.

We could just do the return as you suggest. That would not normalize the value but stll give a direction indicator and make it easier to maintain.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so there are four cases:

  1. multiples of 120
  2. multiples of 12
  3. multiples of 3
  4. multiples of 0.1

Don't you have to do the - 0 trick for the % 3 as well?
In any case, -0 == 0 returns true, so it shouldn't matter actually.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arian you are right, -0 == 0 returns true anyway.
Will wait for travis to get back and re-test it.

@SergioCrisostomo
Copy link
Member Author

@arian great feedback, thank you.

Updated, re-tested, re-based also.

if (event.wheelDelta){
normalized = event.wheelDelta % 120 == 0 ? event.wheelDelta / 120 : event.wheelDelta / 12;
} else {
var rawAmount = event.deltaY ? event.deltaY : event.detail;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var rawAmount = event.deltaY || event.detail || 0;

@SergioCrisostomo SergioCrisostomo added this to the 1.5.1 milestone Jul 20, 2014
@kentaromiura
Copy link
Member

Ship it

@SergioCrisostomo
Copy link
Member Author

@kentaromiura: nice!

Can someone check this so we moo on to 1.5.1 ?

if (type == 'DOMMouseScroll' || type == 'mousewheel')
this.wheel = (event.wheelDelta) ? event.wheelDelta / 120 : -(event.detail || 0) / 3;

if (type == 'DOMMouseScroll' || type == 'wheel' || type == 'mousewheel')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, that would be more clear, will fix.

ibolmo added a commit that referenced this pull request Jul 23, 2014
Fix 2598 - mousewheel dom3 and wheel property
@ibolmo ibolmo merged commit 20ed691 into mootools:master Jul 23, 2014
@SergioCrisostomo SergioCrisostomo deleted the fix-2598 branch July 26, 2014 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

delta and wheel property in mouse event mousewheel event on chrome and firefox
4 participants