WDFN-67 - Create a symlog-like graph, using a "compound scale". #89
Conversation
|
There is a corresponding d3-scale PR here: d3/d3-scale#134 |
![accesslint[bot]](https://avatars.githubusercontent.com/in/1502?s=60&v=4){kind=small}
| // If all values are above zero, make zero the lower bound | ||
| if (domainExtent[0] >= 0 && domainExtent[1] >= 0) { | ||
| return [ | ||
| Math.max(0, domainExtent[0] - padding), |
There was a problem hiding this comment.
Do we have a preference for using Math.max over d3.max? I have been using d3.max in the tooltip code for basically the same purpose as here where I'm finding the max of two numbers. I used d3.max because I was already using it in the package to find the maximum of an array.
There was a problem hiding this comment.
I personally don't have a preference, though I've been using Math methods recently (e.g. Math.abs), but that's because I couldn't find a d3 absolute value method.
There was a problem hiding this comment.
I wasn't aware of d3.max - curious if it does something different than the standard one, which I would assume would be pretty optimized.
There was a problem hiding this comment.
In d3-scale, only Math.maxis used... From the docs, I assume there's specific cases whered3.max` is useful but I don't think we need it here.
| ]; | ||
| } | ||
|
|
||
| // If we have values less than zero, keep 20% padding around both sides. |
There was a problem hiding this comment.
Style thing, but I think having one return statement is cleaner.
There was a problem hiding this comment.
I like that when there's cleanup to do before returning, but otherwise I tend to prefer the "return early" notion of weeding out the edge cases first.
There was a problem hiding this comment.
In this case though there really are only have two paths, in which case I think it's clearer to have an else clause.
| @@ -0,0 +1,47 @@ | |||
| export default function compound() { | |||
| var scales = [].slice.call(arguments); | |||
There was a problem hiding this comment.
I'm just curious about the var vs let here?
Also, it seems that this stylistically a bit different from the rest of the javascript... 🤔
There was a problem hiding this comment.
This is a copy-paste of the code I submitted to d3-scale, so I wrote it in the D3 style.
| }; | ||
|
|
||
| scale.scales = function(_) { | ||
| return arguments.length ? (scales = _, scale) : scales; |
There was a problem hiding this comment.
I'm not understanding this syntax. Can you describe what this does?
There was a problem hiding this comment.
If the function is called without arguments, it returns the scales array. Otherwise, it sets the array to the passed in parameters and then returns the scale object, enabling function chaining. That clause is using the comma operator: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Comma_Operator
There was a problem hiding this comment.
Thanks. Really need to read up on ES6 syntax.
This introduced an issue with tooltips on sites that have 100% masked data. @mbucknell, since you've been working in there, I'll wait for the merge to see if the issue needs resolution.