Skip to content
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

No correct support for multiples yAxis #201

Closed
ValentinH opened this issue Oct 7, 2014 · 22 comments
Closed

No correct support for multiples yAxis #201

ValentinH opened this issue Oct 7, 2014 · 22 comments

Comments

@ValentinH
Copy link
Contributor

Highcharts allows to specify several yAxis for a given chart (a demo can be see here: http://www.highcharts.com/demo/combo-multi-axes).

However, this is not properly handle by highcharts-ng:

  • multiple yAxis configuration can be passed as an array of axis to the configuration.
  • BUT all the extremes (min/max) management is done expecting a single axis instead of a possible array.

For instance, this (link to source):

var processExtremes = function(chart, axis, axisName) {
      if(axis.currentMin || axis.currentMax) {
        chart[axisName][0].setExtremes(axis.currentMin, axis.currentMax, true);
      }
    };

sets only the first axe extremes provided axis is an Axis on not an Array of Axis.

Again, for the watch on config.yAxis, the code is:

angular.forEach(axisNames, function(axisName) {
          scope.$watch('config.' + axisName, function (newAxes, oldAxes) {
            if (newAxes === oldAxes) return;
            if(newAxes) {
              chart[axisName][0].update(newAxes, false);
              updateZoom(chart[axisName][0], angular.copy(newAxes));
              chart.redraw();
            }
          }, true);
        });

and for my opinion it should be something like this:

angular.forEach(axisNames, function(axisName) {
          scope.$watch('config.' + axisName, function (newAxes, oldAxes) {
            if (newAxes === oldAxes) return;
            if(newAxes) {
                if(newAxes instanceof Array) {
                    for(var axe_index in newAxes) {
                        var axe = newAxes[axe_index];
                        if(axe_index < chart[axisName].length) {
                            chart[axisName][axe_index].update(axe, false);
                            updateZoom(chart[axisName][axe_index], angular.copy(axe));
                        }
                    }
                }
                else {
                    chart[axisName][0].update(newAxes, false);
                    updateZoom(chart[axisName][0], angular.copy(newAxes));
                }
                chart.redraw();
            }
          }, true);
        });

What do you think? If this last suggestion is OK, I can make a pull request.

Thanks by advance,
Valentin

@pablojim
Copy link
Owner

Thanks for this. It looks looks generally good. Could you create a pull request and a few jsfiddles showing its use.

@xuye
Copy link

xuye commented Oct 22, 2014

It has another situation like this:
I have three tab to display different data. Each tab has different number serie and yAxis. when i to switch tab to show chart. yAxis doesn't change.

use @ValentinH's method it just can solve multiples yAxis, but can't solve multiples tab with multiples yAxis. finally i use
angular.forEach(axisNames, function(axisName) {
scope.$watch('config.' + axisName, function (newAxes, oldAxes) {
if (newAxes === oldAxes) return;
if(oldAxes && axisName == 'yAxis'){
initChart();
}
if(newAxes) {
chart[axisName][0].update(newAxes, false);
updateZoom(chart[axisName][0], angular.copy(newAxes));
chart.redraw();
}
}, true);
to reinit chart. Thanks by advance

@ValentinH
Copy link
Contributor Author

I think this is a different issue. You are giving a solution that works for your problem but that doesn't work anymore for multiple axis. Indeed, your code still uses chart[axisName][0] which means only the first axis is updated.

@xuye
Copy link

xuye commented Oct 22, 2014

OK , thank you for your advise.

@moses-seeq
Copy link

I have run into this as well, trying to manipulate a single y axis on a chart with multiple y axes. Have you made any progress on this?

@moses-seeq
Copy link

Borrowing this JSFiddle (http://jsfiddle.net/ashwinp/J7LmP/4/) to illustrate that the application behaves the same regardless of the use of linkedTo - delete it and re-run and you will see the behavior remains.

@ValentinH
Copy link
Contributor Author

The code I provided only handle the watch on multiple Y-Axis...

@moses-seeq
Copy link

Understood, I wasn't sure if more had been done. Thanks for the quick reply.

@ValentinH
Copy link
Contributor Author

What kind of manipulation do you wanna do? Cause I added a getter on the Highcharts object from which you can get each axis.

@moses-seeq
Copy link

I am trying to add functionality to scroll and zoom axes of only some of the set of y axes in the chart.

@moses-seeq
Copy link

I fixed my issue. The code that I thought was setting the y axis for each series was using an incorrect variable (copy/paste issue embarrassingly).

@seiyria
Copy link

seiyria commented Jan 12, 2015

So, is there an easy way to get this functionality? Should I fork and patch it in and use my fork?

@ValentinH
Copy link
Contributor Author

The code I posted in this issue works well for handling multiple axis but I haven't submitted a pull request because some other parts deal with axis and I don't really understand these parts...

@linkedrank
Copy link

Was this ever addressed? I would also like to the use the multiple y-axis functionality

@cosme-benito
Copy link

The suggestion for the configuration of axes is not enough because it does not support creating/removing axis dynamically. I have done it like this:

  angular.forEach(axisNames, function(axisName) {
      scope.$watch('config.' + axisName, function(newAxes, oldAxes) {
        if (newAxes === oldAxes || !newAxes) {
          return;
        }

        if (angular.isArray(newAxes)) {

          for (var axisIndex = 0; axisIndex < newAxes.length; axisIndex++) {
            var axis = newAxes[axisIndex];

            if (axisIndex < chart[axisName].length) {
              chart[axisName][axisIndex].update(axis, false);
              updateZoom(chart[axisName][axisIndex], angular.copy(axis));
            }
            else {
              //Create axis if it does not exist: Cosme
              chart.addAxis(axis);
            }

            //Remove extra axis
            for (var axisIndex = newAxes.length; axisIndex < chart[axisName].length; axisIndex++) {
              chart[axisName][axisIndex].remove();
            }

          }

        } else {
          // update single axis
          chart[axisName][0].update(newAxes, false);
          updateZoom(chart[axisName][0], angular.copy(newAxes));

          //Remove extra axis
            for (var axisIndex = 1; axisIndex < chart[axisName].length; axisIndex++) {
              chart[axisName][axisIndex].remove();
            }

        }
        chart.redraw();
      }, true);
    });

This way at least supports creating/deleting axis. I am unsure of how to correct the processExtremes function, though.

@philippone
Copy link

Is the support for multiple axis removed completely in version 1.0.0?
It's not working with an array for xAxis or yAxis but dictionary for an axis is working.

@pablojim
Copy link
Owner

New issue with a jsfiddle please

@philippone
Copy link

Added a title for xAxis and yAxis to config in 'basic-example'
axis: dictionary -> titles are applied: http://jsfiddle.net/o5dg0kjh/2/

Tried to add multiple yAxis (like http://www.highcharts.com/demo/combo-multi-axes):
axis: array -> multiple axis not applied, default yAxis is used: http://jsfiddle.net/wd0h79z9/2/

@philippone
Copy link

It's working if I'm changing the x/yAxis to array instead of dictionary in defaultoptions (https://github.com/pablojim/highcharts-ng/blob/master/src/highcharts-ng.js#L78):

var defaultOptions = { chart: { events: {} }, title: {}, subtitle: {}, series: [], credits: {}, plotOptions: {}, navigator: {}, xAxis: [{ events: {} }], yAxis: [{ events: {} }] };

@pablojim pablojim reopened this Jan 30, 2017
@pablojim
Copy link
Owner

Thanks, it looks like angular merge has some slightly strange behaviour merging dicts and arrays. angular.merge({foo: {a: 1}}, {foo: [1,2]})
I'll try add a fix.

@pablojim
Copy link
Owner

@philippone this is now partially fixed. It now supports multiple axes but does not support the dynamic addition & deletion of axes.

This would require tracking of the axes like we track series here: https://github.com/pablojim/highcharts-ng/blob/master/src/highcharts-ng.js#L42

PRs accepted!

@fintrader
Copy link

@pablojim I made a PR - let me know if this works: #584

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants