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

Incorrect series order in legend after removing old series and adding new ones. #5960

Closed
Rashaine opened this Issue Nov 10, 2016 · 1 comment

Comments

Projects
None yet
2 participants
@Rashaine

Rashaine commented Nov 10, 2016

I tracked this down to series.init (series._i = chartSeries.length - 1;) which sets the series sorting index to the length of the series, however this doesn't account for other series which already had a sorting index set above the chartSeries.length.

I also noted while trying to come up with a workaround that series.update() overrides series.options.index with series.index if no new index is provided in the update which seems weird and wrong.

Expected behaviour

Adding a new series should always add it as the last series unless an index is specified.

Actual behaviour

Adding a new series works as expected until existing series are deleted to create gaps/holes in the saved sort indices.

Live demo with steps to reproduce

http://jsfiddle.net/a9pnwwjp/2/

  1. Remove 3 series with the remove button.
  2. Add new series with the add button and note that the first 2 series added go between series 1 and series 5, then the rest are added after series 5.

Affected browser(s)

Chrome

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Nov 14, 2016

Collaborator

Thanks for reporting!

Regarding your comment on series.update(), perhaps we should add a check for existing index option:

        // Do the merge, with some forced options
        newOptions = merge(oldOptions, {
            animation: false,
            index: pick(oldOptions.index, this.index), // <= changed here
            pointStart: this.xData[0] // when updating after addPoint
        }, { data: this.options.data }, newOptions);
Collaborator

TorsteinHonsi commented Nov 14, 2016

Thanks for reporting!

Regarding your comment on series.update(), perhaps we should add a check for existing index option:

        // Do the merge, with some forced options
        newOptions = merge(oldOptions, {
            animation: false,
            index: pick(oldOptions.index, this.index), // <= changed here
            pointStart: this.xData[0] // when updating after addPoint
        }, { data: this.options.data }, newOptions);

TorsteinHonsi added a commit that referenced this issue Nov 17, 2016

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