Series.update breaks adaptToUpdatedData=false for multiple navigator series

[josteinkjellsen]

Expected behaviour

Series[index].update(options) combined with navigator: { adaptToUpdatedData: false, series: navdata } works the same way for multiple series in navigator as a single series.

Actual behaviour

When doing a dynamic update using series[index].update(options) combined with navigator: { adaptToUpdatedData: false, series: navdata }, all navigation series are updated with data from the main series. So it's not possible to update line color without getting problems with the time range in navigator. As a workaround I'm now recreating the whole chart with the new set option, but this kind of defeats the purpose of the series[index].update(options) function. Am I missing an options here somwhere, or is this a bug? Note that this only happens with multiple series in navigator, array instead of single series object.

Live demo with steps to reproduce

Click the 'Update' button to see the problem reproduced example:
http://jsfiddle.net/3mf5dv56/

Working without multiple series (array) in navigator: http://jsfiddle.net/3mf5dv56/1/

Affected browser(s)

Same behaviour in Chrome and IE

[stpoa]

In version 4.2.7 works as expected:
http://jsfiddle.net/pvbnw62h/

In current version problem exists:
http://jsfiddle.net/pvbnw62h/1

[josteinkjellsen]

Hi stpoa, and thank you for the quick response. It "works" in that version because it does not support multiple series in naviation. See here: http://jsfiddle.net/kf2wvos9/ and old version http://jsfiddle.net/kf2wvos9/1/

[josteinkjellsen]

I've stepped though it now and everything seems ok after the update(options, redraw) function has executed. I can see the color change etc, but problem is that the update function has marked navigation data to be deleted.

update: function(options, redraw)
So when render() triggers after the update, it breaks the navigator with the remove inactive ticks loop.

render: function()
....
 // Remove inactive ticks
                each([ticks, minorTicks, alternateBands], function(coll) {
                    var pos,
                        i,
                        forDestruction = [],
                        delay = animation.duration,
                        destroyInactiveItems = function() {
                            i = forDestruction.length;
                            [b]while (i--) {
                                // When resizing rapidly, the same items may be destroyed in different timeouts,
                                // or the may be reactivated
                                if (coll[forDestruction[i]] && !coll[forDestruction[i]].isActive) {
                                    coll[forDestruction[i]].destroy();
                                    delete coll[forDestruction[i]];
                                }
                            }[/b]
                        };

[josteinkjellsen]

The (partly working) dirty workaround: http://jsfiddle.net/z4b37typ/

[josteinkjellsen]

@oysteinmoseng Hi Øystein, just for curious about the priority of this bug. Do you think it may be fixed in next release? I see there is also an issue with navigator when adding and removing series when we have multiple navigator series and the adaptToUpdateData = false. It's most likely handled similar as the update function.

[oysteinmoseng]

@josteinkjellsen We'll certainly do our best to solve it by the next release. Do you have a demo of the issue with adding/removing series?

[josteinkjellsen]

@oysteinmoseng Sounds good. I will make an example as soon as I find time for it and post it here.

[josteinkjellsen]

@oysteinmoseng Here is and example of the broken navigator after chart.addSeries: http://jsfiddle.net/js6krg1u/. I was mistaken, series.remove does seem to work: http://jsfiddle.net/o7a7uLea/.

[Skuriles]

If you are rewriting the whole navigator stuff, will there be an update included which hides navigator serie if the according "main series" visibility is hidden like that:

serie.visible ? serie.hide() : serie.show();

Or has this to be done manually by finding the correct navigator series with
serie.navigatorSeries._id ??

[oysteinmoseng]

@Skuriles If you want this behavior I suggest using series.navigatorSeries.hide() in the hide event of a series for now. Example: http://jsfiddle.net/qq0dc9ah. It can of course be discussed if this should be the default behavior.

[Skuriles]

@oysteinmoseng thanks, much better approach than dealing with internal properties!!

As you said, think it should be default behaviour, or are there any arguments to keep a navigator series if the according line is hidden. This has only negative effects due to scaling etc. of series, or?

[oysteinmoseng]

@Skuriles We briefly discussed it, and decided to make this default behavior. I'll look at it as part of the rewrite.

[josteinkjellsen]

@oysteinmoseng Hi, any info on when this bug will be fixed?

[oysteinmoseng]

@josteinkjellsen Sorry for the wait, this has been pushed down for too long now. We'll take a look at it next week.

[josteinkjellsen]

@oysteinmoseng No worries, I was just curious :) Will just be great to remove the workaround we put on for both update and add.

[emileindik]

Hi there @oysteinmoseng,
Any update on the progress of this bug?

Thanks!

[TorsteinHonsi]

@emileindik Øystein is out of office this week, he'll answer you next week.

[oysteinmoseng]

@emileindik We started a rewrite which would fix this bug, but it has not been finished yet. I'll try to have it done in the next two weeks or so.

[josteinkjellsen]

Hi, any status updates on this bug?

[oysteinmoseng]

@josteinkjellsen Sorry, I thought this was done. Apparently not. Looking into it asap.

[oysteinmoseng]

Navigator update logic has now been rewritten, and should be much more predictable and stable. Let me know if any edge cases have slipped through the cracks.

[josteinkjellsen]

Great! When will it be realeased to the public? I will test it asap.

[oysteinmoseng]

@josteinkjellsen It should be part of the next release (5.0.13). This will probably be out in the next few weeks.

You can test it now by loading Highstock from github.highcharts.com/hc5-fixes. Example: http://jsfiddle.net/3mf5dv56/7/.

[josteinkjellsen]

@oysteinmoseng Found the issue now. It's not null gaps, but the marker we use. After removing it from my simulator output, it no longer gives me #15 after update. Would however be nice if the series.update would not break when using markers :)

Parts of the simualtor output:
[1497619378555,12.1907252299365],[1497619378585,12.2383128339965],[1497619378615,12.9576282865266],[1497619378645,12.8867367761209],[1497619378675,13.0252956269573],[1497619378705,17.0568942519561],**{"x":1497619378735,"y":17.0568942519561,"marker":{"enabled":true,"symbol":"cross","lineColor": "red"}},**[1497619379035,null],[1497619379065,15.5578962405973],[1497619379095,16.7752762177734],[1497619379125,17.1284956314234],[1497619379155,15.7860076296559],[1497619379185,16.4640452009244],[1497619379215,17.9869569935561],[1497619379245,18.5642694184348],[1497619379275,17.641977035732]...

Cross:
Highcharts.SVGRenderer.prototype.symbols.cross = function (x, y, w, h) { return ['M', x, y, 'L', x + w, y + h, 'M', x + w, y, 'L', x, y + h, 'z']; }; if (Highcharts.VMLRenderer) { Highcharts.VMLRenderer.prototype.symbols.cross = Highcharts.SVGRenderer.prototype.symbols.cross; }
Thanks for all your assistance, have a nice Weekend!

[josteinkjellsen]

@oysteinmoseng Seems the type option is not working: http://jsfiddle.net/3mf5dv56/12/. It is working when doing it this way: http://jsfiddle.net/3mf5dv56/13/, but now I've change my solution to use the first approach.

[oysteinmoseng]

@josteinkjellsen Looking into it

[josteinkjellsen]

@oysteinmoseng You are probably working on this still, would just mention that the version that here: http://github.highcharts.com/hc5-fixes/stock/highstock.js now breaks the fix that was made (http://jsfiddle.net/3mf5dv56/7/). For us changing the type option is not that important, as it almost never is changed in run-time, but I thought I would mention it.

[oysteinmoseng]

@josteinkjellsen Yes, we were having some issues with the github.highcharts.com service, so that link didn't serve the latest code. Should be resolved now.

Still looking into the type-issue, and will also take a look at markers.

[josteinkjellsen]

@oysteinmoseng Hi, I see there has been released a new version that fixes a lot of bugs. Any news on the type option bug (http://jsfiddle.net/3mf5dv56/12/)?

[TorsteinHonsi]

Hi Jostein, Øystein is currently out of office, he will reply to you when he's back.

[oysteinmoseng]

@josteinkjellsen Hi Jostein, I will continue to look into the type issue and other navigator issues as soon as possible.

[josteinkjellsen]

@oysteinmoseng Almost perfect now 👍 One small thing, could the navigator series color follow the main series when first created? Now I set main series green, but it always shows the default blue in navigator.

Example: http://jsfiddle.net/3mf5dv56/16/

[oysteinmoseng]

@josteinkjellsen That seems like a good idea. What do you think @TorsteinHonsi?

Internal note: Currently this does not happen because the default navigator options include a color property. If we remove this we somehow need to pull the color directly from the base series, in case the base series does not have a color defined in options.

[TorsteinHonsi]

@josteinkjellsen That seems like a good idea. What do you think @TorsteinHonsi?

Yes, makes sense now that we have multiple navigator series. It will break a lot of auto-visual tests though, but we could do it in the switch to HC6. Have to make sure that the navigator.series.color option is still respected though.

[josteinkjellsen]

@TorsteinHonsi / @oysteinmoseng It used to work like that in previous version (like 5.0.12), but I guess now series.options and series.navigatorSeries.options is not pointing to the same object. See 5.0.12 version here: http://jsfiddle.net/3mf5dv56/17/

[TorsteinHonsi]

Okay, here's something fishy. Apparently, when the navigator.series option is defined as an array, the navigator series' color is inherited from the base series. This also happens in the current v5.0.14: http://jsfiddle.net/highcharts/3mf5dv56/18/. Comment out the line series: [] and the navigator gets blue.

So what we need to find out is why there is a change in behaviour depending on the navigator.series option. It is confusing and seems like a bug. In my opinion we should change it so it always inherits the color from the base series.

[oysteinmoseng]

@TorsteinHonsi Yes, this should not happen anymore with the latest code (http://jsfiddle.net/3mf5dv56/19/), the way we generate navigator series has been normalized to be more consistent across the possible options.

@josteinkjellsen We decided to look at making the nav series inherit color as we approach HC6, to avoid breaking our tests too soon. For now, you could work around the issue like this: http://jsfiddle.net/3mf5dv56/20/ although you will have to explicitly set a color on each base series for this to work.

[oysteinmoseng]

This is now part of HC6 so closing this issue. This issue is getting a bit long, so let's start a new ticket for any further tweaks on the subject.

[josteinkjellsen]

Hi, have you started a new ticket? Still broken in 6.0.1. http://jsfiddle.net/3mf5dv56/23/

Edit: Strange, See here: http://jsfiddle.net/3mf5dv56/25/ . It works if you click update 1, then breaks when clicking update2. Also the oposite, it works when clicking update2 and breaks when clicking update1 after.

[oysteinmoseng]

@josteinkjellsen No new ticket open, feel free to start one. I'll take a look this week

[oysteinmoseng]

@josteinkjellsen Pushed fix for this, everything should work now as far as I know.

[josteinkjellsen]

@oysteinmoseng Sounds good! Anyway to test it?

[pawelfus]

You can use http://github.highcharts.com/bugfix/highstock.src.js - updated demo: http://jsfiddle.net/3mf5dv56/26/

[josteinkjellsen]

@pawelfus Thanks! Works great now 👍