Fix a bar overlap issue with unordered labels #4674
Conversation
|
@nagix your jsfiddle don't work for me, I think that's because of |
|
@simonbrunel I replaced the links with |
|
@nagix the fiddles work now! I like the results |
| }); | ||
| for (i = 0, ilen = pixels.length; i < ilen; ++i) { | ||
| orderedPixels.push(pixels[i].value); | ||
| order[pixels[i].index] = i; |
There was a problem hiding this comment.
Not sure about this index translation. I feel like we should sort the data at the beginning but that has a number of downsides. It changes the data the user specified and also requires adjusting the data arrays in each dataset accordingly.
There was a problem hiding this comment.
I don't think we need to adjust the data arrays in datasets because the order array translates an original index into an ordered index, and the ordered indexes and pixels are only used for bar width calculation. Do you have any specific problematic cases in mind?
There was a problem hiding this comment.
I think the problematic cases are mostly for debugging purposes. It's mostly code style / personal preference. But i will agree that this is the simplest way to achieve this
There was a problem hiding this comment.
I added comments to avoid confusions in the future.
|
Just want to be sure we want to handle that special case since it requires iterating the data 3 times (instead of only 1) and deal with index translation / intermediate structure, and I feel that most of the time the data are in the correct order? |
There was a problem hiding this comment.
@simonbrunel I agree. I think the user typically passes in data in the correct order
I'd prefer adding a check to ensure a strict ordering. This would not require iterating the data an extra time and would result in simpler code
|
Closing due to inactivity |
When a time scale is used and labels or datasets for bars are unordered, bars will overlap each other. This PR fixes the issue by sorting bar positions when creating the ruler.
Current version: https://jsfiddle.net/msL6xL83/
Proposed version: https://jsfiddle.net/xdajx1mb/