update scaling api #249
update scaling api #249
Conversation
…, so was causing issues)
|
@narenranjit, would you mind reviewing this PR when you get a chance? No rush. Its related to your comments on #247 . |
| axisType = 'smart'; | ||
| } | ||
|
|
||
| if (axisType === 'linear' && (options.yAxis.centeredAxis || options.yAxis.scaling.centeredAxis)) { | ||
| if (axisType === 'linear' && (options.yAxis.centeredAxis || options.yAxis.scaling.type === 'centered')) { |
There was a problem hiding this comment.
I don't think we should support . centeredAxis -- we should still support smartAxis for legacy reasons, but going forward mandate everything else should be set through scaling.type.
| @@ -17,7 +17,7 @@ | |||
| /*jshint eqnull:true */ | |||
| var options = this.options.yAxis; | |||
| var domain = this.domain; | |||
| var zeroAnchor = options.zeroAnchor || options.scaling.zeroAnchor; | |||
| var zeroAnchor = (typeof options.zeroAnchor !== 'undefined') ? options.zeroAnchor : options.scaling.options.zeroAnchor; | |||
There was a problem hiding this comment.
This catches undefineds, but not nulls. I think options.zeroAnchor != null may be a better check
| @@ -117,22 +118,23 @@ | |||
|
|
|||
| _getYScaledDomain: function (domain, options) { | |||
| var opts = this.options.yAxis; | |||
| var absMin = (opts.zeroAnchor || opts.scaling.zeroAnchor) && domain && domain[0] > 0 ? 0 : undefined; | |||
| var zeroAnchor = (typeof opts.zeroAnchor !== 'undefined') ? opts.zeroAnchor : opts.scaling.opts.zeroAnchor; | |||
There was a problem hiding this comment.
see above for null check comment
| } else if (opts.centeredAxis || opts.scaling.centeredAxis) { | ||
| } else if (opts.smartAxis || opts.scaling.type === 'smart') { | ||
| return d3.extent(zeroAnchor || opts.min != null ? [min].concat(domain) : domain); | ||
| } else if (opts.centeredAxis || opts.scaling.type === 'centered') { |
There was a problem hiding this comment.
see above for removing centered axis comment
|
Changes from @narenranjit's review have been pushed. PR is ready for merge. |
|
This looks good, but has merge conflicts. Can you resolve repush? |
Related to #247 - updated structure of scaling option for y-axis (and updated tests). The following jsfiddle shows the use of this option (affects zeroAnchor, smartAxis and centeredAxis) - http://jsfiddle.net/a5L7x0c5/4/