-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add type names to scales #274
Conversation
I added a test to check that the type for all scale functions matches |
@mbostock Just wanted to check if this was something you would consider as a PR. It would be very useful in the chart framework I maintain. |
Apologies but I don’t intend on supporting this for now. |
Thanks for letting me know |
@mbostock do you have any advice on differentiating between a |
@mhkeller Here’s a hack… const d = new Date;
d.getDay = () => console.log("time");
d.getUTCDay = () => console.log("utc");
const scale = d3.scaleUtc(); // or d3.scaleTime();
scale.tickFormat(0, "%a")(d); Basically, override a Date object, and see what methods are called when you try to format it. That way you can tell what the scale type is. |
Thanks that's much appreciated! |
For anyone coming across this in the future, I tweaked this slightly so my function would return and added a const d = new Date;
let s;
d.getDay = () => s = 'time';
d.getUTCDay = () => s = 'utc';
scale.copy().tickFormat(0, '%a')(d);
return s; |
Calling scale.tickFormat doesn’t mutate the scale, so no copy is necessary. It only affects the returned format function, not future calls to scale.tickFormat. |
Per #25 (comment), this proposal adds a
type
field to each scale that returns a string name. I called ittype
for now but have no opinion on what that would end up being. MaybescaleType
or something else is better.I added tests but, for now, only where the existing defaults were being tested. For example, I didn't immediately see where scales like
sequentialLog
were being tested for defaults. I can add those tests where most appropriate.Let me know if you think this is a worthwhile addition. It would definitely help my use case with a minimal footprint on the original library. Thanks for your time in reviewing it.