Fix invert point scale logic to get option closest to mouse pointer #150
Conversation
|
|
||
| return domain[nearestLeftPointIndex]; | ||
| if (rangePoints.length <= 1) { |
There was a problem hiding this comment.
is there a use case where there is a 0? is the undefined caught in this use case?
There was a problem hiding this comment.
inspecting d3's code domain looks like it'll always return an array https://github.com/d3/d3-scale/blob/master/src/continuous.js#L91
| return Math.abs(current - number) < Math.abs(list[closestI] - number) | ||
| ? i | ||
| : closestI; | ||
| }, 0); |
There was a problem hiding this comment.
Hi! Thanks for keeping the Reactochart dream alive!
Just FYI - the original reason i wrote it <-- that way was because we were assuming the input data would be sorted (by domain, either ascending or descending). With this assumption, you can use _.sortedIndex to do a binary search in O(log n) time. With ^the new method here, it works for all data regardless of sort order, but always searches every data point, so it's O(n) which can get slow for large datasets.
IIRC, the plan was to eventually allow a sorted flag (on the XYPlot i guess?) that would tell Reactochart whether the data was sorted or not - it would default false, but if true would use tricks like this to get better performance for sorted data. Might be a nice enhancement for the future.
Hope all is well with y'all :)
| if (isDescending) { | ||
| domain.reverse(); | ||
| rangePoints.reverse(); | ||
| } |
There was a problem hiding this comment.
FWIW I don't think you need this descending/reverse logic anymore since you're not assuming anything about sort order.
There was a problem hiding this comment.
that's a good point, will actually opt to update the code above and just use lodash's sortedIndex
Bug fix to get the correct option/datum/category that the user's mouse is hovering over when the chart is using ordinal/point scale