Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Turn on/off optional control elements with visibility property (vs. display property) #888

Closed
tsaltas opened this issue Mar 20, 2015 · 2 comments
Milestone

Comments

@tsaltas
Copy link

tsaltas commented Mar 20, 2015

I found it useful to create a feature that toggles the reset button similar to the examples in the annotated source, except using the visibility:hidden property instead of the display: none property.

I found that the appearance / disappearance of the reset button was impacting my page layout and making the charts jump around a bit: http://jsfiddle.net/6k8ytsfg/1/

I created a new CSS class for this feature (called "reset-hidden" instead of "reset") and added a few lines to the existing functions (noted with comments):

_chart.turnOnControls = function () {
    if (_root) {
        _chart.selectAll(".reset").style("display", null);
        // added by Shannon
        _chart.selectAll(".reset-hidden").style("visibility", null);
        _chart.selectAll(".filter").text(_filterPrinter(_chart.filters())).style("display", null);
    }
    return _chart;
};

_chart.turnOffControls = function () {
    if (_root) {
        _chart.selectAll(".reset").style("display", "none");
        // added by Shannon
        _chart.selectAll(".reset-hidden").style("visibility", "hidden");
        _chart.selectAll(".filter").style("display", "none").text(_chart.filter());
    }
    return _chart;
}; 

Here is what the result looks like: http://tsaltas.github.io/2014-cleantech-investments/

I thought I would share since it's only a few extra lines.

Thanks,
Shannon

@gordonwoodhull
Copy link
Contributor

I agree that's a better approach and I have also run into the problem of the layout getting jostled by the controls appearing/disappearing.

I'd be more inclined to change the behavior for 2.1 rather than adding another class. This is probably the original intent of the code.

Does anyone else have an opinion about this?

@gordonwoodhull gordonwoodhull added this to the v2.1 milestone Mar 20, 2015
@gordonwoodhull
Copy link
Contributor

Adding controlsUseVisibility feature on develop (2.1). Default is true, set to false to use the old display: none behavior.

gordonwoodhull added a commit that referenced this issue Nov 11, 2015
cherry-picking functionality from dev to master
and reversing the default for backward compatibility

fixes #888
fixes #1016

changing bar chart spec to use the new functionality;
pie chart spec uses the old functionality
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants