Allow specifying the time axis via t attribute

[benmccann]

For time series charts it may make more sense to specify the horizontal axis using the variable t

This change will make it much easier to use the time scale with the financial chart, which takes in the data points { t, o, h, l, c }

[simonbrunel]

helpers.isObject(value) seems a better choice since typeof null === 'object'

[benmccann]

done

[benmccann]

I had to revert this change. See #4538

[simonbrunel]

I guess .isValid verifies if rawValue is a moment object? if so, can we explicitly check for it (rawValue instance moment) instead.

[benmccann]

Hmm. We do this same check in core.scale.js. I'm not sure I want to change it to rawValue instance moment because then we'd be introducing a dependency on moment in core

[simonbrunel]

I agree that core.scale.js should not depend of moment, but I realize that we should remove these checks from getRightValue in core.scale.js and keep only: object ? horizontal ? value.x : value.y. There is no reason to have them in the base class, it's time scale specific and this new override is the way to go.

Since there is already a dependency of moment in scale.time.js, I prefer an explicit check which is more robust and readable (I'm also changing it in parseTime).

[simonbrunel]

And to be correct, it should be something like that in core.scale.js :

var me = this;

// ...

if (helpers.isObject(rawValue)) {
    if (me.isHorizontal()) {
        if (rawValue.x !== undefined) {
            return me.getRightValue(rawValue.x);
        }
    } else if (rawValue.y !== undefined) {
        return me.getRightValue(rawValue.y);
    }
}

// ...

return rawValue;

[benmccann]

Good call on moving the date handling from core to time. I've done that. I didn't make the other changes. partially because helpers.isObject seems bugged. #4538

[simonbrunel]

helpers.isObject doesn't handle Date as an object. I'm still thinking that my previous comment is valid: only handle defined object.x and object.y else return rawValue. If rawValue is a date, then it works as before, no need for a special case. Also, it should not call me.getRightValue if rawValue doesn't define x (or y) properties.

[benmccann]

done

[simonbrunel]

I think we should fallback to x/y only if t === undefined. Also t should be considered for vertical scale:

this.getRightValue(helpers.valueOrDefault(rawValue.t, this.isHorizontal() ? rawValue.x : rawValue.y));

[simonbrunel]

Can we also fallback to the base implementation for x/y:

// ...
if (rawValue.t !== undefined) {
    return this.getRightValue(rawValue.t);
}
// ...

[benmccann]

done