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

use xUnits for axis padding #1230

Closed
gordonwoodhull opened this issue Dec 1, 2016 · 1 comment
Closed

use xUnits for axis padding #1230

gordonwoodhull opened this issue Dec 1, 2016 · 1 comment
Milestone

Comments

@gordonwoodhull
Copy link
Contributor

@mtraynham noted in #892 (comment)

So the coordinate-grid chart currently has xUnits which could potentially be reused for this purpose. xUnits supports all the d3.time[t].range functions (i.e. d3.time.minutes, d3.time.hours), which are aliases to the range functions of intervals (i.e. d3.time.minute.range, d3.time.hour.range).

Maybe instead, we have xUnits just take the top level interval functions (i.e. d3.time.minute, d3.time.hour). It's typically the same thing that is passed in for scales. For brushing, we'd call the range function where appropriate and for padding we would call the offset function.

To support that, we'd have to update the dc xUnits variants for integer/float/ordinal. Maybe something like:

dc.units.integers = {
    range: function (start, end, step) {
        if (!step) {
            step = 1;
        }
        return Math.abs((end - start) / step);
    },
    offset: function (num, step) {
        return num + step;
    }
};

dc.units.ordinal = function (domain) {
    return {
        range: function (start, end, step) {
            return domain;
        },
        offset: function (value, step) {
            return value;
        }
    };
};

dc.units.float = function (precision) {
    return {
        range: function (start, end, step) {
            if (!step) {
                step = 1;
            }
            var d = Math.abs((end - start) / (precision * step));
            if (dc.utils.isNegligible(d - Math.floor(d))) {
                return Math.floor(d);
            } else {
                return Math.ceil(d);
            }
        },
        offset: function (num, step) {
            return num + precision * step;
        }
    };
};

We then could reuse all these functions for padding, although, we may need to support yUnits as well. As you said, this doesn't address the percentage padding problem, but these solutions don't necessarily have to be done together.

@gordonwoodhull gordonwoodhull added this to the v2.1 milestone Dec 1, 2016
@gordonwoodhull gordonwoodhull changed the title use xUnits for padding use xUnits for axis padding Dec 1, 2016
gordonwoodhull added a commit that referenced this issue Jan 5, 2017
doing regular math on dates results in invalid dates
ref: #1026

this is a little suspicious but at least we are putting the special case
into utils.add/subtract where it will be noticed e.g. for #1230

i'm surprised we haven't needed extent arithmetic before
@gordonwoodhull
Copy link
Contributor Author

gordonwoodhull commented Apr 19, 2018

I don't know why I didn't see this before: it's trivial just to make dc.utils.add / dc.utils.subtract / coordinateGridMixin.xAxisPaddingUnits take the d3 interval object instead of the name of it.

There was no need to change xUnits since this is already what xAxisPaddingUnits does.

Fixed in 3.0 alpha 11. Commit (with dyslexic reference to this issue) 5776594

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

No branches or pull requests

1 participant