Open tpitale opened 8 years ago
Thanks @tpitale for bringing this up, it was a fairly large assumption to default to 0 as the minimum for all non-ordinal scales. I'll update it to simply use the minimum shortly (with the existing option to override with domain
, if specified).
I'd be happy to submit a PR if you'd like one.
Or I could wait to try until after #43 is completed/merged.
I'm a little on the fence about the future of createScale
currently, it does too much and makes a few too many assumptions for my tastes. I have been thinking of breaking it into a few different approaches:
scaleBandSeries
that extends scaleBand in d3 v4. This covers the adjacent
, centered
, and series
optionsgetMinMaxDomain
and getOrdinalDomain
that handle data
and key
options for setting domains for linear-link and ordinal scales, respectively.The getMinMaxDomain
should cover this case, although I just looked at it and it has the same issue: vNext:src/mixins/xy (which in some ways is more egregious here since it has min in the name). I'll update it in that PR before it gets merged (which should be today or tomorrow).
I think the new approach is much more direct (with the tradeoff being slightly more to type), although I'm open to adding createScale
back as an add-on.
// Before
var xScale = {type: 'time', data: data, key: 'date'};
var x2Scale = {type: 'ordinal', domain: ['a', 'b', 'c'], adjacent: true, series: 2};
// After
var xScale = d3.time.scale()
.domain(getMinMaxDomain(data, 'date'));
var x2Scale = scaleBandSeries()
.domain(['a', 'b', 'c']).adjacent(true).seriesCount(2);
I'm glad to hear you're using the library and would love to have your feedback!
https://github.com/CSNW/d3.compose/blob/3e8a75f46f5488694bd3a96f772e10c7850bc714/src/helpers/create-scale.js#L137-L144
Does not work for
time
(and probably a lot of other scale types). The first time is always going to be > 0. But, we should still use the min value from the series.