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

Refactored scrolling functionality to fail gracefully #1186

Merged
merged 1 commit into from
May 2, 2013

Conversation

kenearley
Copy link

@pfiller Here's a much better, cleaner version of the scroll fix. This checks for the correct properties and fails gracefully if there is not delta. This is for Issue #1184

More info on wheelDeta here: http://stackoverflow.com/questions/8886281/event-wheeldelta-returns-undefined

@kenearley kenearley mentioned this pull request May 1, 2013
if target.scrollHeight > target.getHeight() and ((delta < 0 and bottom_overflow) or (delta > 0 and top_overflow))
evt.preventDefault()

delta = -evt.wheelDelta or evt.detail
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this work in the Prototype version, but we have to use originalEvent in the jQuery version?

Copy link
Author

Choose a reason for hiding this comment

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

Newer versions of jQuery do not support evt.wheelData or evt.detail. Apparently jQuery overrides the event but shoves all the original event properties in originalEvent. There's not a lot of info I can find.

http://stackoverflow.com/questions/8886281/event-wheeldelta-returns-undefined

Copy link
Collaborator

Choose a reason for hiding this comment

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

jQuery listeners receive a jQuery.Event object normalizing differences between browsers. evt.originalEvent is the native event object being wrapped by jQuery. See http://api.jquery.com/category/events/event-object/

Copy link
Author

Choose a reason for hiding this comment

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

Yeah. What @stof said 😃

@pfiller
Copy link
Contributor

pfiller commented May 1, 2013

I have one question (possibly non-blocking depending on your answer), but otherwise this looks good to me. I like the removal of the Firefox-specific event handler.

@pfiller
Copy link
Contributor

pfiller commented May 2, 2013

:shipit:

kenearley pushed a commit that referenced this pull request May 2, 2013
Refactored scrolling functionality to fail gracefully
@kenearley kenearley merged commit 8faa45f into master May 2, 2013
@kenearley kenearley deleted the fix-scrolling-issue-take2 branch May 2, 2013 15:49
pfiller added a commit that referenced this pull request May 2, 2013
Fixes bug with Isolated Scrolling #1186
Remove unused get_side_border_padding #1169
Fix issue when using both jQuery & Prototype #1168
Fix choices_click method #1163
Isolate Chosen Scrolling #1155
Fix Right-to-Left scrollbar issue #1159
Add support for labels #1152
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.

3 participants