Skip to content
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

D3 Zoom on Selections? #59

Closed
canyon289 opened this issue Sep 26, 2016 · 2 comments
Closed

D3 Zoom on Selections? #59

canyon289 opened this issue Sep 26, 2016 · 2 comments

Comments

@canyon289
Copy link

canyon289 commented Sep 26, 2016

Maybe not an issue but I have a question of whether this is meaningful or not. Consider the example below

<!DOCTYPE html>
<meta charset="utf-8">
<script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/d3/4.2.6/d3.min.js"></script>
<body>
</body>
<script>
var zoom
var svg
var initial_transform

zoom = d3.zoom()
initial_transform = d3.zoomIdentity.translate(50,50)

svg = d3.select("body").append("svg")
 svg.call(zoom.transform, initial_transform)

console.log(d3.zoomTransform(svg))
console.log(d3.zoomTransform(svg.node()))
</script>
</html>

ea {k: 1, x: 0, y: 0}
ea {k: 1, x: 50, y: 50}

The first line is the d3.zoomTransform object on the selection, the second is the zoom transform on the element.

My twofold question is
Is passing a selection to zoomTransform meaningful? I especially ask because all the documentation shows node as an argument.

If not would it be better for d3 to raise an error if I try passing a selection to d3.zoomTransform instead of a node? I spent a couple of hours scratching my head trying to figure out why the initial_transform was not working and it took a while for me to realize I needed to pass the svg node, not the svg selection.

also thanks for the great library!

mbostock added a commit that referenced this issue Sep 26, 2016
@mbostock
Copy link
Member

As documented, d3.zoomTransform takes a node, not a selection. This is because a selection may consist of multiple nodes, and d3.zoomTransform only returns a single transform. I’ve added some clarifying text to the README to make this more obvious.

Regarding the implementation: perhaps it could be changed to use the first node from the selection if passed a selection, similar to selection getter methods, but I’d rather avoid overloading the method signature and have usage be consistent. Often this method is used from within an event handler, where you want to pass in this, being the active element.

Or it could be changed to throw an error if node is a selection, but that seems tailored to the exact error here. Wouldn’t it be better if it throw an error if the given node is not a node? Yet there isn’t a strict definition of what a “node” is in the context of selections, though node instanceof Element would likely be reasonable here. There are some cases where it’s valid to create a selection of non-element objects; see tomwanzek/d3-v4-definitelytyped#2. D3 selections use duck typing: if a node is an object that defines the needed methods, then it’s fine to create a selection from it.

More generally, D3 does not try very hard to protect against incorrect usage. This is partly a language consequence: JavaScript is a weakly-typed language. Some operations enforce types, while others do not. For example, consider the following abuse of function.apply, which takes an array of arguments as the second argument:

function log(message) {
  console.log(message);
}

log.apply(null, "hello"); // TypeError: CreateListFromArrayLike called on non-object(…)

Yet JavaScript is perfectly happy to take other questionably-array inputs:

log.apply(null, {0: "hello"}); // logs "undefined"
log.apply(null, {0: "hello", length: 1}); // logs "hello"

As another example, d3.dispatch throws an error if you try to register a callback listener that is not a function. This is so because callbacks are typically invoked asynchronously, and so it’s much easier to debug if there error is thrown when the callback is registered rather than when it is invoked. But if you pass the wrong sort of thing to d3.zoomTransform, the result is synchronous: the returned transform will always be the default identity transform. If you use a debugger, you can step through the code and see why (node.__zoom is undefined).

So, I’m not convinced it’s worthwhile to add an assertion here, though if people continue to run into this confusion, then I’ll consider it again in the future.

Another option for stronger protection is a language enhancement such as TypeScript; see tomwanzek/d3-v4-definitelytyped and DefinitelyTyped/DefinitelyTyped#9936.

@canyon289
Copy link
Author

Thank you for the long and informative response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants