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 clipping children #1488

Closed
Frozenlock opened this issue Oct 3, 2018 · 3 comments
Closed

Composite chart not clipping children #1488

Frozenlock opened this issue Oct 3, 2018 · 3 comments

Comments

@Frozenlock
Copy link
Contributor

Individual charts

The bars and the datapoints are clipped when needed.
image
image

Composite

image

Potential cause

Wrong clipPath referred.
image

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Oct 8, 2018

Took me a while to repro this - turns out it only happens if the composite chart is initialized dynamically with a div, instead of being initialized with a selector.

I think the child charts should be referencing the clipPath URL of the parent. This works when the parent is named.

The problem is in anchorName:

    _chart.anchorName = function () {
        var a = _chart.anchor();
        if (a && a.id) {
            return a.id;
        }
        if (a && a.replace) {
            return a.replace('#', '');
        }
        return 'dc-chart' + _chart.chartID();
    };

In this case anchor a is a div with no id. So it misses the first two cases and instead generates a link to the chartID of the child (which has no clip path).

Would you please try this fix, in baseMixin.anchor? I am not sure if it's the cleanest fix but it works for me:

        if (dc.instanceOfChart(parent)) {
            _anchor = parent.anchor();
            if(_anchor.children) // is _anchor a div?
                _anchor = '#' + parent.anchorName();
            _root = parent.root();
            _isChild = true;
        } else if (parent) {

The idea is that, if we detect that parent.anchor() returns a div, we instead force the parent to generate its anchorName and we hold onto this anchor instead.

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Oct 8, 2018

Another possibility is to force the composite to have an id:

            if(_anchor.children && !_anchor.id)
                _anchor.id = parent.anchorName();

image

That's nice and clear, but maybe kind of rude if the user really doesn't want charts to have ids. (There is no escaping that the clip paths need unique ids.)

@Frozenlock
Copy link
Contributor Author

Eh... I'm an idiot, I pushed the fix to #1491
However I think it's trivial enough to leave it there.

(You were absolutely right btw, your small change fixed it.)

gordonwoodhull added a commit that referenced this issue Oct 9, 2018
this also tests dynamic, anonymous divs for #1488
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

2 participants