nf-graph: better behavior when setting yMin, yMax to null

[jamesarosen]

Previously, setting yMin or yMax to undefined caused

Error: Invalid value for attribute transform="translate(520, NaN)"

There was no way to set yMin or yMax to "something if it's defined, or default behavior otherwise". This now treats the following as equivalent:

{{!-- no yMax specified: --}}
{{nf-graph}}

{{!-- yMax set to undefined: --}}
{{nf-graph yMax=somePropertyThatIsUndefined}}

Closes #77

[jamesarosen]

I also tried changing these to the new Object-style computed properties, but that won't work with Ember 1.11, which this library currently tests against.

I didn't see any relevant tests in the repo. I tried this out on my app and it seems to work as I expect. The axes rescale when I do $E.set('yMax', 100) and then $E.set('yMax', null).

[jamesarosen]

This doesn't quite work. The problem is that the default versions actively set _prop_. Since Handlebars bindings are currently two-way, that means the nf-graph simply causes a write back up to the parent, overriding the value it tried to send in.

For pre-Glimmer, the only idea I can come up with to solve this would be to create new _xMin, _xMax, _yMin, and _yMax properties that use their un-prefixed versions if set or the default calculation otherwise. That would mean every component in the library would have to rely on the prefixed versions.

If this library were 1.13+ only, there might be a way to use didReceiveAttrs instead of alternate property names.

[jamesarosen]

Ah, I'm actually wrong about the cause for the property getting overridden. It seems that something is causing the yMax to recompute even after I've set it manually. (Likely some dependency has changed.) I get a little closer with

if(value != null) {
  this[__Max_] = value;
} else if(this[__Max_] != null) {
  return this[__Max_];
} else {

but that causes a whole host of other problems.

[jamesarosen]

OK. After playing around a little more, I've come up with

var maxProperty = function(axis, defaultTickCount) {
  var _DataExtent_ = axis + 'DataExtent';
  var _Axis_tickCount_ = axis + 'Axis.tickCount';
  var _ScaleFactory_ = axis + 'ScaleFactory';
  var _MaxMode_ = axis + 'MaxMode';
  var __Max_ = '_' + axis + 'Max';
  var __didOverride_ = '_didOverride_' + axis + 'Max'; // <-- NEW
  var _prop_ = axis + 'Max';

  return Ember.computed(
    _MaxMode_,
    _DataExtent_,
    _ScaleFactory_,
    _Axis_tickCount_,
    function(key, value) {
      var mode = this.get(_MaxMode_);
      var ext;

      if(value != null) {                              // <-- NEW
        this[__didOverride_] = true;
        return this[__Max_] = value;
      }

      if(this[__didOverride_]) {                       // <-- NEW
        return this[__Max_];
      }

      var change = function(val) {
        this[__Max_] = val;                            // <-- CHANGED
      }.bind(this);

      if(mode === 'auto') {
        change(this.get(_DataExtent_)[1] || 1);
      }

      else if(mode === 'push') {
        ext = this.get(_DataExtent_)[1];
        if(!isNaN(ext) && this[__Max_] < ext) {
          change(ext);
        }
      }

      else if(mode === 'push-tick') {
        var extent = this.get(_DataExtent_);
        ext = extent[1];

        if(!isNaN(ext) && this[__Max_] < ext) {
          var tickCount = this.get(_Axis_tickCount_) || defaultTickCount;
          var newDomain = this.get(_ScaleFactory_)().domain(extent).nice(tickCount).domain();
          change(newDomain[1]);
        }
      }

      return this[__Max_];
    }
  );
};

Note that change no longer calls set so as to avoid causing the did-override logic to kick in.

[jamesarosen]

The whitespace-insensitive diff will help here.

[benlesh]

@jamesarosen can you rebase? I've made some changes in this area.

[jamesarosen]

Rebased