-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Try to fix wrong position bug for BarChartView + HorizontalBarChartView #221
Conversation
…et.entryCount to calculate the offset and position
@danielgindi as you have changed lots since 2.1.1, this PR cannot be merged with green light. I won't be able to merge newer code to this branch until I got some time to resolve the conflicts... Also need you to review the whole logic. I might miss something somewhere. |
alright, lucky night I got time to deal with the conflicts.. hope you don't mind a few more commits introduced. |
Do you know how to squash commits? |
If you are using SourceTree, you could right click the latest commit, Rebase, and let it rebase the code on the latest commit. |
Are you saying this?https://www.sourcetreeapp.com I can take a look at it tomorrow. Need to handle some other stuff now. Should I follow the same steps you provide?
|
Alright, thanks for the lesson! I managed to squash the commits. |
I am using v3.0.1 but still have the same problem. the bars are misplaced. Please if u can help me with that check #3154 |
try to fix wrong position bug for issue #214
The idea is, because not every xIndex will have a dataEntry, if the data is invalid (nil/nan/null), the entryCount will be less than xValsCount.
For example, xVals: [0,1,2,3,4,5,6,7,8,9,10], if there are two dataSets, like below:
xIndex:0 value:100, xIndex:10 value:200
xIndex:0 value:1000, xIndex:10 value:2000
Current logic will use entryCount instead of xValsCount to calculate the matrix and position, which will lead to shorter xPosition value on xAxis. issue #214 has the screenshot.
What I changed is I always use xValsCount and actual xIndex value to calculate.
@danielgindi There might be something I missed if I am not correct. On the other hand, if you think I am right, I would also miss places to apply the new logic, because while I am testing, I found I missed to change
getMarkerPosition
. There could be more.I also feel it might has such issues for all bar-like charts, but I don't have time to dig out more charts. If I ran into this, I will continue trying to fix.