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

Expose the wheel zoom speed in Python #3125

Closed
liuyxpp opened this Issue Nov 14, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@liuyxpp

liuyxpp commented Nov 14, 2015

The sensitivity of the wheel zoom cannot be configured on the Python side. However, there is a speed property which can be set on the JS side. It would be better to expose this property to the Python side too.

@birdsarah

This comment has been minimized.

Member

birdsarah commented Nov 15, 2015

@liuyxpp we are actually in the middle of a big clean-up to ensure that the properties on the python and JS side match each other.

@havocp WheelZoomTool has a default for speed on the JS side (https://github.com/bokeh/bokeh/blob/master/bokehjs/src/coffee/tool/gestures/wheel_zoom_tool.coffee#L119) but not a python property.

However running the test in #3099 doesn't pick up this discrepancy. Thoughts?

@birdsarah

This comment has been minimized.

Member

birdsarah commented Nov 15, 2015

Maybe the test only picks up differences in defaults not when there's missing properties?

@birdsarah

This comment has been minimized.

Member

birdsarah commented Nov 15, 2015

great spot @bryevdv, do you know why that would be?

@bryevdv

This comment has been minimized.

Member

bryevdv commented Nov 15, 2015

Why that line exists? @Havoc added that to get things working at all. Currently the client models add all kind of new attributes with abandon and most of them should not get serialized back to the server. But a few like this one should be, but were just overlooked. We should have been more careful in the first place and made client only "viewmodel" objects to store any new, extra, "not intended to be serialized" data, and and kept the main models pure and 1-1 in sync with Python (no extra attrs added). We still should do this some day.

On Nov 15, 2015, at 00:16, Sarah Bird notifications@github.com wrote:

great spot @bryevdv, do you know why that would be?


Reply to this email directly or view it on GitHub.

@havocp

This comment has been minimized.

Contributor

havocp commented Nov 15, 2015

Python would raise an exception at one point if it saw an unknown property... now it just warns. So I was having to mark tons of props as nonserializable. It sounds like the better fix here was to simply add the prop to Python.

@birdsarah

This comment has been minimized.

Member

birdsarah commented Nov 17, 2015

thanks for the background @havocp and @bryevdv still learning how the all the pieces of the properties mechanism works.

@bryevdv

This comment has been minimized.

Member

bryevdv commented Apr 2, 2016

We can certainly expose the speed, but unless we can also find a way to make a given speed somewhat uniform across browsers I am dubious of the utility. But changing to feature in any case.

@bryevdv bryevdv added this to the short-term milestone Apr 2, 2016

@damianavila

This comment has been minimized.

Contributor

damianavila commented Jul 19, 2016

Reopening because the PR was reverted by #4831

@damianavila damianavila reopened this Jul 19, 2016

@bryevdv bryevdv modified the milestones: short-term, 0.12.x, 0.12.16 Mar 16, 2018

bryevdv added a commit that referenced this issue Apr 4, 2018

@bryevdv bryevdv referenced this issue Apr 4, 2018

Merged

add option to disable wheel zoom on axes #7743

2 of 2 tasks complete

@bryevdv bryevdv closed this in #7743 Apr 4, 2018

bryevdv added a commit that referenced this issue Apr 4, 2018

add option to disable wheel zoom on axes (#7743)
* add option to disable wheel zoom on axes

* fix #3125
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment