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

elasticY does not work if all values are negative (ordinal x axis) #879

Closed
jtsherba opened this issue Mar 11, 2015 · 7 comments
Closed

elasticY does not work if all values are negative (ordinal x axis) #879

jtsherba opened this issue Mar 11, 2015 · 7 comments
Milestone

Comments

@jtsherba
Copy link

I believe I have found a little bug.

For an ordinal bar chart with only negative values, the bar for the highest value does not draw correctly.

The issue is not present when there are one or more positive values included.

See example here: https://jsfiddle.net/tjsd4yqs/6/

@jtsherba
Copy link
Author

Never mind, just needed to set the y axis domain to start from 0.

@gordonwoodhull
Copy link
Contributor

Thanks @jtsherba. I think there is a bug here, because .elasticY(true) does not work if all values are negative.

https://jsfiddle.net/tjsd4yqs/12/

Retitling.

@gordonwoodhull gordonwoodhull changed the title Ordinal bar chart with all negatives displaying incorrectly elasticY does not work if all values are negative (ordinal x axis) Mar 11, 2015
@gordonwoodhull gordonwoodhull added this to the v2.0 milestone Mar 20, 2015
@arjain
Copy link

arjain commented Mar 8, 2016

Are your aware of any workarounds to make elasticY work with negative values?

@sebgrohn
Copy link
Contributor

sebgrohn commented Jun 3, 2016

We recently ran into this issue in our product as well, both with bar charts and row charts. After some investigation, we think that the following changes would fix the issue:

  • For bar charts and others using Stack mixin, change dc.stackMixin.yAxisMax to behave like yAxisMin, capping each stack's value to at least y0 / 0:

    _chart.yAxisMax = function () {
        var max = d3.max(flattenStack(), function (p) {
            return (p.y + p.y0 > p.y0) ? (p.y + p.y0) : p.y0;
        });
    
        return dc.utils.add(max, _chart.yAxisPadding());
    };
  • For row chart, add a condition in calculateAxisScale in dc.rowChart for the domain max, like the one for the minimum:

    function calculateAxisScale () {
        if (!_x || _elasticX) {
            var extent = d3.extent(_rowData, _chart.cappedValueAccessor);
            if (extent[0] > 0) {
                extent[0] = 0;
            }
            if (extent[1] < 0) {
                extent[1] = 0;
            }
            _x = d3.scale.linear().domain(extent)
                .range([0, _chart.effectiveWidth()]);
        }
        _xAxis.scale(_x);
    }
  • Other chart types that don't use Stack mixin and should have a corresponding fix?

I would be happy to prepare a proper PR in the beginning of next week, if you think this is the way to go. :)

Ping @stillesjo

@sebgrohn
Copy link
Contributor

sebgrohn commented Jun 7, 2016

I have opened a PR (#1156) with the proposed solution above, but there are some issues that need to be discussed. All input is welcome!

@sebgrohn
Copy link
Contributor

sebgrohn commented Jun 9, 2016

@gordonwoodhull: what do you think about this solution?

@gordonwoodhull
Copy link
Contributor

Fixed by #1156 in 2.0 beta 32.

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

4 participants