patternjavascriptMinor
Jquery sortable style function
Viewed 0 times
stylejqueryfunctionsortable
Problem
This is a basic sorting function written in jQuery that moves items in the DOM around to create empty spaces for a droppable(). Since there is so much going on in a droppable over event, i'd love some feedback on how this could be done better.
This is all in a function called on the over event of the droppable()
I have an array of empty spots
cid is the droppable element that you're over top of, so I find the next open slot using
If there is a an empty slot further down the list, I use a for loop to loop through moving the items around the DOM to make the space needed to drop the element.
If there is not one further down the list, I do the same thing in reverse to check if there is a spot above the droppable to push items up into.
It's usable here: http://jsfiddle.net/mstefanko/LEjf8/9/
Any thoughts are extremely appreciated!
This is all in a function called on the over event of the droppable()
I have an array of empty spots
var emptyHolders = [];
$('.pipeline-holder').each(function(){
if($(this).hasClass('holder-empty')){
var eid = $(this).attr('id');
emptyHolders.push(eid.substring(14));
}
});cid is the droppable element that you're over top of, so I find the next open slot using
for (var i = 0; i cid) {
nextEmpty = currentEmpty;
i = emptyHolders.length;
} else {
prevEmpty = parseInt(currentEmpty);
}
}If there is a an empty slot further down the list, I use a for loop to loop through moving the items around the DOM to make the space needed to drop the element.
if (nextEmpty != null) {
var moveMe = nextEmpty -1;
for (var i = moveMe; i >= cid; i--) {
var nextcount = i + 1;
var me = $('#pipeline-rank-' + i);
var next = $('#pipeline-rank-' + i).parents('.pipeline-rank-row').next().find('.pipeline-holder');
var pid = $('#pipeline-rank-' + i).find('.content-wrapper').attr('id');
next.append($('#pipeline-rank-' + i).find('.content-wrapper'));
next.removeClass('holder-empty');
next.siblings('.remember-my-position-hover').html(pid);
}
$('#pipeline-rank-' + cid).addClass('holder-empty');
}If there is not one further down the list, I do the same thing in reverse to check if there is a spot above the droppable to push items up into.
It's usable here: http://jsfiddle.net/mstefanko/LEjf8/9/
Any thoughts are extremely appreciated!
Solution
Here you have some general thoughts:
-
First of all, cache jQuery elements (
-
You can also chain jQuery methods, it simplifies codes, and helps with above.
-
To reduce amount of code you can use
-
Instead of selecting
http://jsfiddle.net/tomalec/pXPdS/3/
-
To get id of empty/not empty holder ids, in my opinion,
http://jsfiddle.net/tomalec/pXPdS/4/
-
Pass dropped element to
http://jsfiddle.net/tomalec/pXPdS/5/
-
First of all, cache jQuery elements (
var $this = $(this); instead of multiple executions of $(this)). http://jsfiddle.net/tomalec/pXPdS/1/-
You can also chain jQuery methods, it simplifies codes, and helps with above.
-
To reduce amount of code you can use
toggleClass instead of addClass removeClass.http://jsfiddle.net/tomalec/pXPdS/2/-
Instead of selecting
$('#rankDrag') in start callback you can use reference given in arguments ui.helperhttp://jsfiddle.net/tomalec/pXPdS/3/
-
To get id of empty/not empty holder ids, in my opinion,
.map is more readable than .each. Moreover, you can move that function outside rearrange to avoid re-declaring it every time.function getTruncatedId(index, element){
return element.id.substring(14);
}
function rearrange(dropid) {
//..
emptyHolders = holders.filter('.holder-empty').map(getTruncatedId);
isRanked = holders.filter(':not(.holder-empty)').map(getTruncatedId);
//..
}http://jsfiddle.net/tomalec/pXPdS/4/
-
Pass dropped element to
rearrange, so you will not have to traverse through DOM again:rearrange( $(this).attr('id') );//dropId = $(this).attr('id');
//..
isEmpty = $('#'+dropId).hasClass('holder-empty');http://jsfiddle.net/tomalec/pXPdS/5/
Code Snippets
function getTruncatedId(index, element){
return element.id.substring(14);
}
function rearrange(dropid) {
//..
emptyHolders = holders.filter('.holder-empty').map(getTruncatedId);
isRanked = holders.filter(':not(.holder-empty)').map(getTruncatedId);
//..
}rearrange( $(this).attr('id') );//dropId = $(this).attr('id');
//..
isEmpty = $('#'+dropId).hasClass('holder-empty');Context
StackExchange Code Review Q#25546, answer score: 3
Revisions (0)
No revisions yet.