jiahuang / d3-timeline

Simple JS timeline plugin for d3
1.04k stars 280 forks source link

Duplicate IDs #39

Closed blinskey closed 9 years ago

blinskey commented 9 years ago

If a time series contains more than one item, the plugin will produce rect elements with duplicate IDs. This can be seen in several of the timelines in the example timeline, such as timeline5. This is invalid HTML and also prevents us from using callbacks like mouseover and hover to uniquely identify a segment of the timeline.

I've implemented a simple fix for this in my fork of the project, but it will break any existing code that relies on the duplicate IDs to highlight an entire timeline. I'd imagine it would probably be best to use a class rather than IDs to provide that functionality.

I'd be happy to help if you're interested in fixing this, though I don't know how you'd want to approach the issue of maintaining compatibility with old code. Thanks for the great plugin!

jiahuang commented 9 years ago

Ugh, yeah I don't know why I did the ids per series of data elements in the first place...

I implemented a fix to this. As per your suggestion, the new per-dataset field is class which sets the class and each data element has an optional id field. If those aren't set, it falls back to the index of the dataset/data element.

blinskey commented 9 years ago

Awesome, thanks!