-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Improved browser compatibility for table scroll #5051
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
Conversation
Could you please investigate why these two tests are failing? plotly.js/test/jasmine/tests/table_test.js Line 446 in a54c502
plotly.js/test/jasmine/tests/table_test.js Line 472 in a54c502
|
Yeah, apparently the tests check that the 'mousewheel' event is raised so they fail when 'wheel' is used instead. |
Those tests were added in #3327 so I'll request a review from @alexcjohnson. |
Sorry it's my first PR and I'm not sure what's the expected thing to do in such cases, do I add a change to the test to handle the 'wheel' event? |
Nice fix @ManelBH ! In fact looking around our code, we have a number of places that only plotly.js/src/plots/cartesian/dragbox.js Lines 1228 to 1232 in a54c502
But we don't need to clean all of that up now. It should be fine to change the test to use |
@ManelBH after 7f983c5 there is still one failing test: plotly.js/test/jasmine/tests/table_test.js Line 446 in a54c502
|
The |
Fixes #5052.
Currently the table scroll uses the non-standard 'mousewheel' event which is not supported by firefox (and maybe other browsers). Changed it to the standard 'wheel' event, supported by most browsers, with a fallback to 'mousewheel' just in case.