Coordinate mixin cleanup - xUnits

[kum-deepak]

This function took one of my longest time while I was working on the D3 v4 to comprehend about how it is working. Then I realized the code makes big distinction on whether Ordinal or Continuous scale is in use. This prompted me to make the logic explicit that what this function does in each case.

In this PR I have rearranged the code to make it more explicit about what it does.

[kum-deepak]

I also checked my notes from the days and I realized in case of non ordinal charts, having the 3rd parameter present causes test failure.

In D3v3 code _chart.x().domain() was always passed as the 3rd parameter - but the receiver (other than ordinal) was supposed to discard it. It seems in D3v4, some unit functions are impacted by the 3rd parameter. If we add the 3rd parameter at line 406, some tests fail.

[gordonwoodhull]

I was always confused by xUnits sometimes returning a number and sometimes an array, so in principal I'm in favor of this.

However, if d3v4 is forcing us to change our API for one of the more confusing corners of dc.js, that's kind of a big deal and we need to document it. In particular it means that a user can't pass their own function that acts like dc.units.ordinal - I doubt too many people did, but it was possible and now it's not.

What tests fail using the third argument? I bet we were misusing it and d3 got stricter about it.

[gordonwoodhull]

Actually I think this change means that dc.units.ordinal is never called at all - it's just a magic variable. That's not a great design but I'm not sure what to do about it.

[gordonwoodhull]

Alright I'm off to do weekend things. Good work! :)

[kum-deepak]

I agree that dc.units.ordinal is now only a marker, we can change it to a field, which indicates that it is an Ordinal chart. However not sure if that makes a better API for the users. For now we can leave it as is.

We should document it, in upgrade guide as well as in documentation for xUnits (I will do that now)

[gordonwoodhull]

I am documenting all of these in Changelog.md, and also in the API documentation. We could do a D3-style CHANGES summary as well

[gordonwoodhull]

Okay, I have now over-documented this, made the function throw, tested the throw, etc. Hopefully people will notice somehow.

[gordonwoodhull]

Thanks @kum-deepak, merged for 3.0 alpha 9

[kum-deepak]

I really think it can not be documented more :)