2
\$\begingroup\$

I made a simple list with drag and drop ordering function using JavaScript, HTML and CSS. Although I tried to do this as efficient as possible, I'm sure there are some improvements to be made.

What would be considered bad practice in my code below and what could be improved or are there any suggestions for another approach?

Working demo:

// code by Friso NL - frisog at gmail .com

// events to create according ot: http://www.html5rocks.com/en/tutorials/dnd/basics/
//
// dragstart
// drag
// dragenter
// dragleave
// dragover
// drop
// dragend



var listItems = document.querySelectorAll('.listItem');




var dragSrcEl = null;

function handleDragStart(e) {
  this.className += " dragStartClass";
  dragSrcEl = this;

  e.dataTransfer.effectAllowed = 'move';
  e.dataTransfer.setData('text/html', this.innerHTML);
  e.dataTransfer.setDragClass("dataTransferClass");

}

function handleDragOver(e) {
  // if (e.preventDefault) { not needed according to my question and anwers on : http://stackoverflow.com/questions/36920665/why-if-statement-with-e-preventdefault-drag-and-drop-javascript
  e.preventDefault();
  // }
  e.dataTransfer.dropEffect = 'move'; // sets cursor
  return false;

}

function handleDragEnter(e) {
  // this / e.target is the current hover target.
  this.classList.add('over');
}

function handleDragLeave(e) {
  this.classList.remove('over'); // this / e.target is previous target element.
}

function handleDrop(e) {

  var listItems = document.querySelectorAll('.listItem');
  e.stopPropagation(); // stops the browser from redirecting.
  dragSrcOrderId = parseInt(dragSrcEl.getAttribute("order-id"));
  dragTargetOrderId = parseInt(this.getAttribute("order-id"));
  var tempThis = this;


  // Don't do anything if dropping the same column we're dragging.
  // and
  // check if only one difference and then do not execute
  // && ((Math.abs(dragSrcOrderId - dragTargetOrderId)) != 1)
  if (dragSrcEl != this) {
    // Set the source column's HTML to the HTML of the column we dropped on.
    var tempThis = this;

    function makeNewOrderIds(tempThis) {
      // check if up or down movement

      dragSrcEl.setAttribute("order-id", dragTargetOrderId);
      tempThis.setAttribute("order-id", dragTargetOrderId);

      //  find divs between old and new location and set new ids - different in up or down movement (if else)
      if (dragSrcOrderId < dragTargetOrderId) {
        for (i = dragSrcOrderId + 1; i < dragTargetOrderId; i++) {
          listItems[i].setAttribute("order-id", i - 1);
          // set new id src
          dragSrcEl.setAttribute("order-id", dragTargetOrderId - 1);
        }
      } else {
        for (i = dragTargetOrderId; i < dragSrcOrderId; i++) {
          listItems[i].setAttribute("order-id", i + 1);
          // set new id src
          dragSrcEl.setAttribute("order-id", dragTargetOrderId);

        }
      }

    };
    makeNewOrderIds(tempThis);


    dragSrcEl.classList.remove("dragStartClass");

    reOrder(listItems);




  } else {

    dragSrcEl.classList.remove("dragStartClass");
    return false;

  }

};

function handleDragEnd(e) {

  for (i = 0; i < listItems.length; i++) {
    listItem = listItems[i];
    listItem.classList.remove('over');
  }
  dragSrcEl.classList.remove("dragStartClass");


}



for (i = 0; i < listItems.length; i++) {
  listItem = listItems[i];


  listItem.setAttribute("order-id", i);



  listItem.addEventListener('dragstart', handleDragStart, false)
  listItem.addEventListener('dragenter', handleDragEnter, false)
  listItem.addEventListener('dragover', handleDragOver, false)
  listItem.addEventListener('dragleave', handleDragLeave, false)
  listItem.addEventListener('drop', handleDrop, false)
  listItem.addEventListener('dragend', handleDragEnd, false)
}

function reOrder(listItems) {


  var tempListItems = listItems;
  tempListItems = Array.prototype.slice.call(tempListItems, 0);

  tempListItems.sort(function(a, b) {
    return a.getAttribute("order-id") - b.getAttribute("order-id");
  });



  var parent = document.getElementById('checklist');
  parent.innerHTML = "";

  for (var i = 0, l = tempListItems.length; i < l; i++) {
    parent.appendChild(tempListItems[i]);
  }
};
/* Prevent the text contents of draggable elements from being selectable. */

[draggable] {

  -moz-user-select: none;

  -khtml-user-select: none;

  -webkit-user-select: none;

  user-select: none;

  /* Required to make elements draggable in old WebKit */

  -khtml-user-drag: element;

  -webkit-user-drag: element;

}

.listItem {

  padding: 10px 10px 10px 10px;

  margin: 3px;

  background-color: red;

  color: white;

  width: 30px;

  border-top: thick solid white;

  border-top-width: 1px;

  -webkit-transition: all 0.2s ease-out;

  -moz-transition: all 0.2s ease-out;

  -ms-transition: all 0.2s ease-out;

  -o-transition: all 0.2s ease-out;

  transition: all 0.2s ease-out;

}

.dataTransferClass {

  background-color: brown;

}

.dragStartClass {

  opacity: 0;

  -webkit-transition: all 0.2s ease-out;

  -moz-transition: all 0.2s ease-out;

  -ms-transition: all 0.2s ease-out;

  -o-transition: all 0.2s ease-out;

  transition: all 0.2s ease-out;

}

.listItem.over {

  border-top: thick solid white;

  border-top-width: 50px;

  -webkit-transition: all 0.2s ease-out;

  -moz-transition: all 0.2s ease-out;

  -ms-transition: all 0.2s ease-out;

  -o-transition: all 0.2s ease-out;

  transition: all 0.2s ease-out;

}
<!DOCTYPE html>
<html>

<head>
  <link rel="stylesheet" href="css/style.css" media="screen" title="no title" charset="utf-8">
  <meta charset="utf-8">
  <title></title>
</head>

<body>

  <ul id="checklist">
    <li class="listItem" draggable="true">1</li>
    <li class="listItem" draggable="true">2</li>
    <li class="listItem" draggable="true">3</li>
    <li class="listItem" draggable="true">4</li>
    <li class="listItem" draggable="true">5</li>
    <li class="listItem" draggable="true">6</li>
  </ul>


</body>
<script type="text/javascript" src="app/js/main.js">
</script>

</html>

\$\endgroup\$
4
\$\begingroup\$

This is a surprisingly difficult problem, especially you want it to work reliably in mobile browsers. That's why there are quite a few plugins out there. Full disclosure, I'm the author of one of them (lithiumlist.com). When developing Lithium List, here are some of the issues I had to consider:

  • From a UX perspective, how will the sorting item be visualised? I
    found that a sizable drop shadow and small scale increase works well. Something like box-shadow: 0 5px 14px rgba(0,0,0,0.15), 0 6px 6px rgba(0,0,0,0.12) and transform: scale(1.02).
  • Will the non-sorting items slide up/down during sorting? Using CSS
    translations to move the items, together with transitions, can help to make this smooth.
  • When sorting finishes, will the item move smoothly to its final position? Again, CSS translations and transitions can help with this.
  • What happens if the sorting item reaches the top/bottom of the viewport?
    Ideally the list or page will automatically scroll.
  • Safari on MacOS (not iOS) produces strange jerky behaviour when a selection is
    dragged beyond the top/bottom of the browser window. Setting
    overflow: hidden on #checklist (even if only during sorting) will fix this.

By the way, you are right to apply the user-select: none classes to prevent selection during sorting. You probably also want -ms-user-select: none since you've included the 'ms' prefix elsewhere. You may also want to consider -webkit-touch-callout, which disables the callout in iOS browsers.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.