Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Multi-value map support. #277

Closed
wants to merge 10 commits into
from

Conversation

9 participants
Owner

mbostock commented Aug 28, 2011

Current progress:

  • selection.attr - done
  • selection.style - done
  • selection.property - done
  • selection.classed - done
  • selection.on - done
  • transition.attr - done
  • transition.style - done

Also, we may want to include these, but I'm not 100% yet:

  • transition.attrTween?
  • transition.styleTween?
  • transition.tween?
Owner

mbostock commented Aug 29, 2011

There's probably a way to structure the code into two methods (a getter and a setter) per operator, and then write a generic wrapper that detects multi-valued maps and dispatches accordingly.

If transition.style was implemented without transition.styleTween-- does that mean all style transitions with a multi-value would happen instantaneously?

Owner

mbostock commented Aug 29, 2011

No, that's not what I meant. I meant that we could add multi-value map support for transition.style, but potentially transition.styleTween would still only support single values. Though it would be preferable for it to support a map for consistency with other operators.

mhalle commented Nov 5, 2011

Would it be possible to do this merge at this time?

Owner

mbostock commented Nov 5, 2011

Nope; still need to add support for transition.attr and transition.style, at the least. I might be okay deferring selection.classed and selection.on to a later release. Feel free to lend a hand if you want to see this happen more quickly!

Collaborator

jasondavies commented Nov 5, 2011

I did some work on this about a month ago: https://github.com/jasondavies/d3/tree/map. In @ba2b1537a18762d8ef4147c27ef3104d622ddc2e I didn't get round to adding tests, that's the next step for transition.attr. Almost there!

dandean commented Mar 15, 2012

Any idea if this will be merged in?

I'm working on my own much simpler attrs implementation which basically iterates a map and calls attr. But, if there are plans to merge this pull request in, I'd love to take advantage of its enhanced functionality.

Here's my branch, for reference:

https://github.com/dandean/d3/commit/a0754f6da6e035c7eb804253b42579713703a7d1

Owner

mbostock commented Mar 15, 2012

Shooting for the next minor release, 2.9. Hopefully that should be within the next couple of weeks.

dandean commented Mar 15, 2012

Cool - thanks @mbostock !

👍

This would be great to see in 2.9

This was referenced May 20, 2012

Owner

mbostock commented Aug 5, 2012

Related #55 #65

Owner

mbostock commented Aug 5, 2012

So far, I have added support for accepting both a map and a function that returns a map. When a map is specified (say to selection.attr), it can contain constants or functions. However if a function is specified, the map it returns can only specify constants. I was striving for consistency with other methods that allow either constants or functions, but I feel like this adds too many equivalent signatures and fails at parsimony. For example, these are all equivalent:

selection.attr("foo", 42); // constant
selection.attr("foo", function() { return 42; }); // function
selection.attr({foo: 42}); // map of constants
selection.attr({foo: function() { return 42; }}); // map of functions
selection.attr(function() { return {foo: 42}; }); // function returning a map of constants

In the earlier discussion for #55 and #65, it seemed like the last variant is probably overkill. It’s arguably the most powerful of the five signatures because you can share work across multiple attributes and compute attribute names dynamically. However, you can already structure your code this way using selection.each, though perhaps with a bit more typing and perhaps slightly worse performance:

selection.each(function() { d3.select(this).attr("foo", 42); });

Also consider this implies three signatures for selection.on:

selection.on("click", listener); // listener
selection.on({click: listener}); // map of listeners
selection.on(function() { return {click: listener}; }); // function returning a map of listeners

I’d like to kill support for the single-function signatures. That would leave four signatures for selection.attr (ignoring the fifth signature when used as a getter):

selection.attr("foo", 42); // constant
selection.attr("foo", function() { return 42; }); // function
selection.attr({foo: 42}); // map of constants
selection.attr({foo: function() { return 42; }}); // map of functions

And two for selection.on:

selection.on("click", listener); // listener
selection.on({click: listener}); // map of listeners

This preserves D3’s philosophy of encouraging only values (and not names) to be specified as functions.

I initially preferred the last variant (function returning a map of constants) over the second-to-last (map of functions), but after thinking on it for a few minutes I think this is a good compromise.

Using selection.each initially seems like a kludge, but within that function you can construct a map of constants for not only .attr(), but also .style(), .on(), etc. It would be quite verbose though.

The function returning a map of constants does look very pretty in coffeescript.

selection.attr (d) ->
  cx: x(d)
  cy: y(d)
  fill: color(d)
selection.attr(function(d) {
  return {
    cx: x(d),
    cy: y(d),
    fill: color(d)
  };
});

vs the map of functions

selection.attr
  cx: (d) -> x(d)
  cy: (d) -> y(d)
  fill: (d) -> color(d)
selection.attr({
  cx: function(d) { return x(d); },
  cy: function(d) { return y(d); },
  fill: function(d) { return color(d); }
});
Owner

mbostock commented Aug 5, 2012

A small point, but you can collapse the map of functions slightly in your example:

selection.attr({
  cx: x,
  cy: y,
  fill: color
});

Or in CoffeeScript:

selection.attr
  cx: x
  cy: y
  fill: color

This only works when the functions x, y and color take the standard signature (d, i). You wouldn’t derive this advantage from the function that returns a map, since you have to invoke the functions manually.

Ahh, very concise!

selection.attr({
cx: x,
cy: y,
fill: color
});

For the new user, reading the above, it's hard to tell that x, y and color are in-line declared functions because it's more typical to see a function declaration in this type of situation or else the assumption is that it's a function expression defined elsewhere or just a variable. Ugh, a little mind bending but any say I can't say I don't love it..

in-line declared was meant to read: implicitly declared

Owner

mbostock commented Aug 6, 2012

@idibidiart True, but that also applies to the previous syntax:

selection
    .attr("cx", x)
    .attr("cy", y)
    .attr("fill", color);

@mbostock

I didn't know that because I didn't expect it

The more I get into D3 the more my mind opens up to Design As Magic and further away from Design As Commodity

Not to be dramatic or anything but it certainly appeals to the magician/mathematician in me (in terms of its style) more than the typical "end user" The Principle Of Least Surprise appears far less important in the design of D3 than its non-trivial, almost magical design patterns :)

Sorry to go off topic. I just wanted to relay my subjective experience of the medium (D3) as I'm getting to know it

:)

Marc F

Owner

mbostock commented Aug 9, 2012

Merged into #756; staged for 2.10.0.

@mbostock mbostock closed this Aug 9, 2012

dribnet commented Feb 27, 2013

I’d like to kill support for the single-function signatures.

😢

I get it.

Nevertheless replacing wasteful code like:

      .enter().append("line")
        .attr("x1", function(d) { var p = project([d.lon, d.lat]); return p[0]; })
        .attr("y1", function(d) { var p = project([d.lon, d.lat]); return p[1]; })
        .attr("x2", function(d) { var p = project([d.lon, d.lat]); return p[0]+10; })
        .attr("y2", function(d) { var p = project([d.lon, d.lat]); return p[1]+10; });

with

      .enter().append("line")
        .each(function(d) {
            var p = project([d.lon, d.lat]);
            d3.select(this)
                .attr({ x1:p[0], y1:p[1], x2:(p[0]+10) y2:(p[1]+10) });
        });

instead of

      .enter().append("line")
        .attr(function(d) { 
            var p = project([d.lon, d.lat]);
            return { x1:p[0], y1:p[1], x2:(p[0]+10) y2:(p[1]+10) }; 
        });

makes me shed one tear... but maybe combinatorial explosion of method signatures would have made me shed two.

coli commented May 6, 2015

I really wish this is supported..

selection.attr(function() { return {foo: 42}; });
Owner

mbostock commented May 6, 2015

@coli This is discussed in #2109, and I think we can do this for 4.0 as selection.attrs (plural).

coli commented May 6, 2015

Thanks!

Using each + select as a workaround just turns functional code back to being procedural code again.

Edit: it breaks animation too.

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