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

Series.update breaks adaptToUpdatedData=false for multiple navigator series #5846

Closed
josteinkjellsen opened this Issue Oct 19, 2016 · 56 comments

Comments

Projects
None yet
7 participants
@josteinkjellsen

josteinkjellsen commented Oct 19, 2016

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

This comment has been minimized.

Show comment
Hide comment
@stpoa

stpoa Oct 19, 2016

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

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

stpoa commented Oct 19, 2016

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

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

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 19, 2016

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 commented Oct 19, 2016

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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 19, 2016

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 commented Oct 19, 2016

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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 19, 2016

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

josteinkjellsen commented Oct 19, 2016

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

@oysteinmoseng oysteinmoseng added the Bug label Oct 24, 2016

@oysteinmoseng oysteinmoseng changed the title from Series[index].update() breaks adaptToUpdatedData = false to Series.update breaks adaptToUpdatedData=false for multiple navigator series Oct 24, 2016

@oysteinmoseng oysteinmoseng self-assigned this Oct 24, 2016

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 27, 2016

@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.

josteinkjellsen commented Oct 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Oct 27, 2016

Collaborator

@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?

Collaborator

oysteinmoseng commented Oct 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 27, 2016

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

josteinkjellsen commented Oct 27, 2016

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

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 27, 2016

@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/.

josteinkjellsen commented Oct 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@Skuriles

Skuriles Nov 29, 2016

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 ??

Skuriles commented Nov 29, 2016

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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Nov 29, 2016

Collaborator

@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.

Collaborator

oysteinmoseng commented Nov 29, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@Skuriles

Skuriles Nov 29, 2016

@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?

Skuriles commented Nov 29, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Nov 29, 2016

Collaborator

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

Collaborator

oysteinmoseng commented Nov 29, 2016

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

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Jan 18, 2017

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

josteinkjellsen commented Jan 18, 2017

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

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Jan 19, 2017

Collaborator

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

Collaborator

oysteinmoseng commented Jan 19, 2017

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

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Jan 19, 2017

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

josteinkjellsen commented Jan 19, 2017

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

@emileindik

This comment has been minimized.

Show comment
Hide comment
@emileindik

emileindik Feb 27, 2017

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

Thanks!

emileindik commented Feb 27, 2017

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

Thanks!

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Mar 1, 2017

Collaborator

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

Collaborator

TorsteinHonsi commented Mar 1, 2017

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

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Mar 10, 2017

Collaborator

@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.

Collaborator

oysteinmoseng commented Mar 10, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen May 21, 2017

Hi, any status updates on this bug?

josteinkjellsen commented May 21, 2017

Hi, any status updates on this bug?

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng May 22, 2017

Collaborator

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

Collaborator

oysteinmoseng commented May 22, 2017

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

oysteinmoseng added a commit that referenced this issue Jun 15, 2017

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Jun 15, 2017

Collaborator

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.

Collaborator

oysteinmoseng commented Jun 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Jun 15, 2017

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

josteinkjellsen commented Jun 15, 2017

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

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Jun 16, 2017

Collaborator

@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/.

Collaborator

oysteinmoseng commented Jun 16, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Jun 16, 2017

@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 commented Jun 16, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Jun 19, 2017

@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.

josteinkjellsen commented Jun 19, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Jun 19, 2017

Collaborator

@josteinkjellsen Looking into it

Collaborator

oysteinmoseng commented Jun 19, 2017

@josteinkjellsen Looking into it

@oysteinmoseng oysteinmoseng reopened this Jun 19, 2017

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Jun 21, 2017

@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.

josteinkjellsen commented Jun 21, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Jun 21, 2017

Collaborator

@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.

Collaborator

oysteinmoseng commented Jun 21, 2017

@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.

@TorsteinHonsi TorsteinHonsi removed this from the 5.0.13 milestone Jul 27, 2017

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Jul 31, 2017

@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/)?

josteinkjellsen commented Jul 31, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Aug 2, 2017

Collaborator

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

Collaborator

TorsteinHonsi commented Aug 2, 2017

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

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Aug 9, 2017

Collaborator

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

Collaborator

oysteinmoseng commented Aug 9, 2017

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

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Aug 31, 2017

@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/

josteinkjellsen commented Aug 31, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Aug 31, 2017

Collaborator

@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.

Collaborator

oysteinmoseng commented Aug 31, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Aug 31, 2017

Collaborator

@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.

Collaborator

TorsteinHonsi commented Aug 31, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Aug 31, 2017

@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/

josteinkjellsen commented Aug 31, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Sep 1, 2017

Collaborator

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.

Collaborator

TorsteinHonsi commented Sep 1, 2017

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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Sep 1, 2017

Collaborator

@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.

Collaborator

oysteinmoseng commented Sep 1, 2017

@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 added a commit that referenced this issue Sep 29, 2017

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Oct 5, 2017

Collaborator

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.

Collaborator

oysteinmoseng commented Oct 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 18, 2017

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.

josteinkjellsen commented Oct 18, 2017

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

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Oct 18, 2017

Collaborator

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

Collaborator

oysteinmoseng commented Oct 18, 2017

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

@oysteinmoseng

This comment has been minimized.

Show comment
Hide comment
@oysteinmoseng

oysteinmoseng Oct 23, 2017

Collaborator

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

Collaborator

oysteinmoseng commented Oct 23, 2017

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

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 25, 2017

@oysteinmoseng Sounds good! Anyway to test it?

josteinkjellsen commented Oct 25, 2017

@oysteinmoseng Sounds good! Anyway to test it?

@pawelfus

This comment has been minimized.

Show comment
Hide comment
Contributor

pawelfus commented Oct 25, 2017

@josteinkjellsen

This comment has been minimized.

Show comment
Hide comment
@josteinkjellsen

josteinkjellsen Oct 25, 2017

@pawelfus Thanks! Works great now 👍

josteinkjellsen commented Oct 25, 2017

@pawelfus Thanks! Works great now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment