fix: old x axis labels were not destroyed during data update if parsing was set to false or an object with custom axis keys #9601
Conversation
|
Would definitely be good to have a test for this |
|
Looks like this needs a rebase and the test being added. |
|
Closing as stale |
|
@etimberg I was too busy with my work these days but I was planning to update this one. |
|
Yup, not a problem 😄 |
CatchABus
force-pushed
the
custom-parsing-labels-fix
branch
from
86c5916
to
eec303c
Compare
CatchABus
force-pushed
the
custom-parsing-labels-fix
branch
from
c7c086e
to
7994da3
Compare
| @@ -249,6 +249,69 @@ describe('Chart.DatasetController', function() { | |||
| expect(parsedYValues).toEqual([20, 30]); | |||
| }); | |||
|
|
|||
| it('should synchronize labels array with X values when chart existing dataset is updated and parsing is off', function() { | |||
There was a problem hiding this comment.
Does this case work properly when parsing is on, or should this fix actually apply in that case too?
There was a problem hiding this comment.
The bug occurs when parsing is off or has provided keys, that's why I target those 2 cases.
There was a problem hiding this comment.
@kurkle I totally missed this one but it seems issue occurs with parsing set to true as well.
Do you think I should add a 3rd test or maybe change test description to ...when parsing has no provided keys?
UPDATED: My bad. Not setting parsing inside tests defaults to false so I got confused. It seems bug does really NOT occur when parsing is set to true.
There was a problem hiding this comment.
After few more tests, it seems I get different results for parsing: true now and this case needs to be covered.
I'm not sure if there is any cache mechanism, but unit tests suddenly failed after few more attempts.
I marked PR as draft until I resolve this.
|
@etimberg I added 2 new tests for this case. I made use of |
|
Sorry @dimitrisrk for hijacking your fix, but we are getting eager to publish 3.6.1 |
I'm the one who should apologize for my delay. It appears my approach did not cover all cases so I hit a dead end. |
When parsing was set to false or an object with
xAxisKeyoryAxisKey, there were issues if chart data was updated more than once.Apparently, old x labels were not destroyed, resulting in merging old and new x labels.
This PR destroys old x labels every time data is updated, provided that chart has parsing set to
falseor an object with custom keys.According to docs: https://www.chartjs.org/docs/next/general/data-structures.html,
labelsshould only be set manually when importing primitive data so this does not break any kind of labels list set on chart initialization.Fixes #9599