bugfix/18658-sunburst-labels-blinking #18697
Conversation
File size comparisonNo differences found |
Visual test results - No difference found |
There was a problem hiding this comment.
@vazonik When removing code, it is important to consider closely what is lost. One thing I can see is that the data labels now suddenly appear initially, instead of easing/fading in. This is what should be controlled by the dataLabels.defer option, and in the current state of this PR, it behaves like defer is always false.
I recommend taking a closer look at what is going on in that code block, hopefully we can do it in a more consistent way. Maybe we can implement the defer option in another place, more consistent with other series types. If things were done correctly, it shouldn't be necessary with series-specific overrides like this.
|
@TorsteinHonsi Thanks! Ignoring the My change causes removing the fadein-fadeout animation of data labels during drill down, but with @pawellysy we found that it could also be unnecessary. |
@vazonik In that case it is important that we find it and fix it before the next release. Do you have a demo of this? It seems to work well here (on the master): https://jsfiddle.net/highcharts/rz82ywtp/ |
@TorsteinHonsi Yes: |
|
Okay, it's caused by this regression: #18478 . If we set a container height in the CSS, we fix the redraw issue and the data labels behave like they should. |
|
Ok, so now it works like this: https://jsfiddle.net/BlackLabel/Lg5c0nwu/ |
There was a problem hiding this comment.
I now remembered what the hackDataLabelAnimation is about. It is for traversing the sunburst. See http://localhost:3030/samples/#view/highcharts/demo/sunburst and click Europe. Without the hack, the data labels animate in a chaotic way. That's why we decided to just temporary hide them and fade back in instead of trying to animate between different rotations and text paths. I think what you need to do is to keep the logic is it was, but rather inspect when the hackDataLabelAnimation is activated. So that it does not get activated when just doing a simple point update. In fact, looking at the previous line 851, I would expect it to only happen when the root changed, which is does not do in this case.
|
Thank you! While we're in this part of the code, I want to ask you to to check out something I have been noticing for a while. See https://jsfiddle.net/highcharts/6eka4tmn/. If you click the middle element, "Oceanic", you also get the blinking of the data labels. I would expect that in this case the root id was identical to the previous root id, since we're not allowed to traverse further up, and subsequently the labels shouldn't blink. Can you have a look at that? |
Fixed #18658, all sunburst's data labels were disappearing and showing back after one point update.