Added icons option to chart

[ghost]

I needed a timeline that allowed displaying timespans and your plugin fit the bill. But I also wanted the ability to show icons instead of text labels, so I added the option to your plugin. If you like it, feel free to accept this pull request. Or reject it if you don't. :-)

To use it, just add an "icon" key to each of your datapoint lists and then call "icons()" during the timeline instantiation. (Example added to examples.html)

Thanks!

[jiahuang]

Awesome feature. Thanks for adding it!

I'm wondering if there should both be an "icon" key and a .icons() call. It seems like "icon" would pretty effectively replace "id" (rendering-wise at least). Maybe it should just auto-icon if you have the icon key set for that data?

Thoughts?

[ghost]

No problem!

Auto-icon wouldn't be a bad idea. If we go that route, though, I'd want to replace "id" with "label" and then auto-label as well. Just for consistency. (We could use "index" instead of "datum.id," allowing us to get rid of the required "id" field.)

On Thu, Mar 7, 2013 at 7:58 AM, jiahuang notifications@github.com wrote:

Awesome feature. Thanks for adding it!

I'm wondering if there should both be an "icon" key and a .icons() call. It seems like "icon" would pretty effectively replace "id" (rendering-wise at least). Maybe it should just auto-icon if you have the icon key set for that data?

Thoughts?

— Reply to this email directly or view it on GitHubhttps://github.com/jiahuang/d3-timeline/pull/1#issuecomment-14568774 .

[ghost]

My last commit illustrates what I'm talking about.

EDIT: Of course, it may be a better idea to not make this change break backwards compatibility. In which case we can keep support for "id" and the "label" function, but auto-label if "label" is present on the datum.

[ghost]

Alright, I fixed backwards compatibility with that last commit. Let me know if everything looks kosher or not.

[jiahuang]

This looks great. I'll pull it in :)