wesnolte / jOrgChart

A jQuery plugin to draw tree-like structures such as OrgCharts.
992 stars 674 forks source link

Improved handling of none ID lists #17

Closed crash-dive closed 12 years ago

crash-dive commented 12 years ago

I wanted to use your plugin in a project, however I have lots of li elements where they would not have IDs and would have exactly the same contents as other elements in the list therefore the way you were searching for list elements based on the contents of the element was not working.

So I altered the source to use the HTML5 data- attributes to store a node count on each li and div.node in the form of data-tree-node=1 and then when the drop event is raised I can link the two elements by using the jquery attribute selector and data() methods like so:

var sourceLi = $this.find("[data-tree-node=" + ui.draggable.data("tree-node") + "]");

I also made one other minor change which is I switched draggable's revert option to 'invalid' which I think works a lot better, however you may disagree.

wesnolte commented 12 years ago

Love the hell out of the improvement but the html5 attributes means than html < 5 wouldn't be strictly valid. Could you change this to use the $(element).data('myVar', myVar) to store the information instead?

crash-dive commented 12 years ago

Hmm I had not considered validation. Yes you could change it to use data() but the only change you would have to make is you would have to use a filter function because jquery does not have a data selector. So instead of:

var sourceLi = $this.find("[data-tree-node=" + ui.draggable.data("tree-node") + "]");

you would have something like

var sourceID = ui.draggable.data("tree-node");
var sourceLi = $this.filter(function() { return $(this).data("tree-node") === sourceID } );

That will have a slight performance hit over the first approach but I doubt it would be meaningful. If you think that would be ok I will probably have a go at adding it over the weekend.

crash-dive commented 12 years ago

I have altered the commit to use the method described above and tested it on Chrome, FF and IE9 and it works fine.

wesnolte commented 12 years ago

A great piece of work, thanks for the effort.