HumbleSoftware / Flotr2

Graphs and Charts for Canvas in JavaScript.
http://www.humblesoftware.com/flotr2/
MIT License
2.45k stars 528 forks source link

Fix hit plugin with stacked lines #256

Closed cesutherland closed 10 years ago

cesutherland commented 10 years ago

Thanks for the changes! Relates to #136.

One quick note then it should be good to merge.

cesutherland commented 10 years ago

@ioguix could you take a look at the comments here? I'll pull in your fixes.

-Carl

ioguix commented 10 years ago

@cesutherland: I just added a new commit to remove forgotten comments and use a specific member for the stack offset in the array.

I was not aware the third value of this array could be used by users for some other things. I am not really comfortable with this solution because it mixes arrays and object. Actually, I wonder if it forces javascript to transform the array (x, y, o) in an array of object or something...Why not using an official "properties" object in the 3rd value of the object like that:

data[i][2] = { 
    // some user properties...
    tooltip: "blah",
    stackOffset: stack1
}

I don't know if my comment worth the pain it can causes to other users though. Feel free to pull the code as-is or keep discussing my comment.

ioguix commented 10 years ago

I've found my answer about what's going on with using "data[i].y0", see:

So the array is not transformed to an object, but it is not that clean neither. But I can leave with that if the solution would breaks tons of code out there.

Cheers,

cesutherland commented 10 years ago

Thanks for looking more into it! Arrays are indeed objects.

I believe this the solution most likely to cause the least harm. The issue with the 3rd value is that some chart types have more than 2 dimensions (OHLC, bubble, etc.) which do use data[2]. A user may wish to use that same data in multiple chart types.

ioguix commented 10 years ago

Oh, ok, that make sense then.

Anything else to consider on this issue before pulling it ?

cesutherland commented 10 years ago

Just that we were in sync on it!