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

xAxisLabel / yAxisLabel modify margins #1324

Open
emiljas opened this issue Jul 25, 2017 · 2 comments
Open

xAxisLabel / yAxisLabel modify margins #1324

emiljas opened this issue Jul 25, 2017 · 2 comments
Labels
Milestone

Comments

@emiljas
Copy link

emiljas commented Jul 25, 2017

I think that this code cause a lot of trouble:

_chart.yAxisLabel = function (labelText, padding) {
        if (!arguments.length) {
            return _yAxisLabel;
        }
        _yAxisLabel = labelText;
        _chart.margins().left -= _yAxisLabelPadding;
        _yAxisLabelPadding = (padding === undefined) ? DEFAULT_AXIS_LABEL_PADDING : padding;
        _chart.margins().left += _yAxisLabelPadding;
        return _chart;
    };

In my code I set margin (e.g. left margin to 10px) and then set yAxisLabel (default padding 12px).
Then I do this again for some action, but in my model margin is still 10px but it isn't true.

I hope you understand my problem. Do you agree that this code smell?

@gordonwoodhull
Copy link
Contributor

I agree, that's disgusting!

Probably we should have a function that calculates the margins, that uses .margins() and _yAxisLabelPadding and whatever else. I think it could be a backward compatible change that way. (Except for changing the margins, which behavior does not need to be preserved.)

Any chance you'd like to contribute a fix?

@gordonwoodhull gordonwoodhull added this to the v2.0 milestone Jul 25, 2017
@emiljas
Copy link
Author

emiljas commented Sep 22, 2017

#1342 PR with fix

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

No branches or pull requests

2 participants