Multiplicom / axiom

3 stars 0 forks source link

Revise scroll event handling #49

Closed dlorson closed 5 years ago

dlorson commented 5 years ago

Harmonize scroll event handling across browsers with the wheel event instead of the deprecated mousewheel event.

Tested in FF, Chrome, Safari; could not test yet in IE due to DOMElement issue on master (presumably due to the direct DOM manipulation changes).

Tested using the DataFrame, GenomeBrowser (TrackViewer). Could not locate an instance of PanelHtml where non-browser scrolling is used.

dlorson commented 5 years ago

I see! Mistakenly inverted the scroll direction. Fixed. I'll test further in IE. Even if it was like that before, may as well fix it as part of this issue.

MichaelVyverman commented 5 years ago

@dlorson Let us know if your tests on IE are successful, you can re-request review from Goran for approval.

dlorson commented 5 years ago

I managed to get it running yesterday (IE11/Win8.1) using #7d93d29 and replaying both commits in this PR, as Goran mentioned.

Indeed scrolling vertically nor horizontally seems to work using the trackpad, with either the pre-change and post-change versions. Didn't have a mouse to test, so retrying today. Debugging within the VM is extremely slow, so this might take some time.

dlorson commented 5 years ago

OK, after some debugging in IE, both with the current PR (replayed on #7d93d29) and with an older commit prior to all scroll-related changes (#cc0913f), I observed the following:

In my opinion this is a hopeless case, and IE users should rely on secondary methods for horizontal scrolling (horizontal scrollbar, or panning via dragging in the genome browser)

goranvinterhalter commented 5 years ago

Given the issues on IE & Edge I'm considering the solution good enough (for now).