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

Composite Chart Not Rescaling Bar Width #611

Closed
erikreedstrom opened this issue Jun 6, 2014 · 11 comments
Closed

Composite Chart Not Rescaling Bar Width #611

erikreedstrom opened this issue Jun 6, 2014 · 11 comments
Labels
Milestone

Comments

@erikreedstrom
Copy link

I have been using the following to rescale bar widths when resizing a full-width chart, where width is assigned by the anchor set at 100% in CSS:

chart.on('preRender', function(){
    chart.rescale();
});

$("#some-chart").on("resize", function(){
    chart.render();
});

Which works perfectly if chart is a bar chart. However, if the bar chart is part of a composition, where chart is the composite chart, bar widths no longer rescale.

It seems that _chart.xAxisLength() in calculateBarWidth is not reflecting the new width.

@gordonwoodhull gordonwoodhull added this to the v2.0 milestone Jun 8, 2014
@gordonwoodhull
Copy link
Contributor

Thanks for filing this. I agree it's a bug and I'll take a look for 2.0, as I'm looking at a lot of related stuff.

In the meanwhile, you might have better luck asking for workarounds on the mailing list, as I know a lot of people are doing similar stuff.

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Jul 21, 2014

Indeed it looks like each of width, height, and margins needs to be propagated to children when it is set, and not just in compose.

@r4j4h
Copy link
Contributor

r4j4h commented Dec 16, 2014

+1 Ran into this today

@paloteb
Copy link

paloteb commented Jun 21, 2016

It's almost two years late but it can be fixed if you assign the xAxisLength of the composite chart to the xAxisLength of the child bar charts:

childBarChart.xAxisLength = compositeChart.xAxisLength;

@jesper998
Copy link

Also ran into this today. Anyone have a workaround?

@gordonwoodhull
Copy link
Contributor

@jesper998, I take it @paloteb's workaround does not work for you? What about manually setting width and height (and margins, if needed) for the child charts? The problem is that they are only getting propagated when compose is first called.

Resizing was not part of the original design of dc.js, so there is a lot of troubleshooting to do.

@jesper998
Copy link

Thanks, got it working by setting height and width of children for events "preRender" and "preRedraw" on the composite chart!

@gordonwoodhull
Copy link
Contributor

Awesome!

@dahlbyk
Copy link
Contributor

dahlbyk commented Feb 23, 2018

Here's what I came up with for a more holistic patch for this. @gordonwoodhull thoughts? Would you be interested in a PR for this?

dc.override(dc, 'compositeChart', function (parent, chartGroup) {
    var _chart = dc._compositeChart(parent, chartGroup);

    dc.override(_chart, 'rescale', function () {
        _chart._rescale();

        var _children = _chart.children();
        _children.forEach(function (child) {
            child.rescale();
        });

        return _chart;
    });

    // properties originally passed through in compose()
    ['height', 'width', 'margins'].forEach(function (prop) {
        var _prop = '_' + prop;
        dc.override(_chart, prop, function (value) {
            if (!arguments.length) {
                return _chart[_prop]();
            }

            _chart[_prop](value);

            var _children = _chart.children();
            _children.forEach(function (child) {
                child[prop](value);
            });

            return _chart;
        });
    });

    return _chart;
});

@gordonwoodhull
Copy link
Contributor

This makes a lot of sense and it should be much simpler as part of the library. If you have time, please do file a PR!

@gordonwoodhull
Copy link
Contributor

Fixed by #1365 in 3.1.5

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

Successfully merging a pull request may close this issue.

6 participants