-
Notifications
You must be signed in to change notification settings - Fork 604
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
Crosshair plugin - initial canvas size fix #1038
base: master
Are you sure you want to change the base?
Conversation
src/extras/crosshair.js
Outdated
@@ -28,11 +28,19 @@ Dygraph.Plugins.Crosshair = (function _extras_crosshair_closure() { | |||
|
|||
var crosshair = function crosshair(opt_options) { | |||
this.canvas_ = document.createElement("canvas"); | |||
this.updateCanvasSize(0, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would 1, 1
work as well? bit afraid of hitting corner cases here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think size is not important until canvas is added to DOM. Moved to activate
callback, where it can be set to graph actual size
I also have added a size change check before updating it to remove possible unnecessary layout updates and to remove a bit of a garbage on mouse movements |
n-gist dixit:
I think size is not important until canvas is added to DOM. Moved to
`activate` callback, where it can be set to graph actual size
Ah, good plan.
|
Crosshair plugin creates canvas to draw its lines, but if the graph is small enough, this canvas can block interaction with other elements until you hover over a graph giving it a chance to resize. Canvas initializes with 300x150 which is default Canvas element - Wikipedia
In the example below it overlaps the '+' button preventing clicking it
To fix it I have set the size to [0,0] in plugin constructor.