4
\$\begingroup\$

This little script makes list items selectable and draggable. I'm pretty new to programming in general, and even more so to Javascript. Most of my background is in PHP and some C++. On the large, I don't think this is the proper way to be writing code in JavaScript. I know global variables are bad, but I'm not used to dealing with event handling, I don't know how to maintain constants like that throughout various event calls that aren't otherwise attached to each other. I'm really not sure if I'm doing any of this right.

A more specific question - This script works as is, but if i change handleDragStart()'s variable dragSrc = this; into var dragSrc = this; as it should be, it breaks. When the handleDrop() event fires, it returns dragSrc is not defined. If anything, I would expect this behavior with the way it is written now, not after adding var. I'm a little lost.

var SELECTED_CLASS_CONST = 'selected';
var DROP_CLASS_CONST = 'drophover';

function handleDragStart(e) {

    dragSrc = this;

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

function handleDragEnter(e) {

    this.addClass(DROP_CLASS_CONST);
}

function handleDragLeave(e) {

    this.removeClass(DROP_CLASS_CONST);
}

function handleDragEnd(e) {

    [].forEach.call(draggables, function(drag) {

        drag.removeClass(DROP_CLASS_CONST);
    });
}

function handleDragOver(e) {

    if (e.preventDefault) {
        e.preventDefault();
    }
    e.dataTransfer.dropEffet = 'move';

    return false;
}

function handleDrop(e) {

    if (e.stopPropagation)
    {
        e.stopPropagation();
    }

    if ( dragSrc != this )
    {
        dragSrc.innerHTML = this.innerHTML;
        this.innerHTML = e.dataTransfer.getData('text/html');
        this.removeClass(DROP_CLASS_CONST);

        if (dragSrc.hasClass(SELECTED_CLASS_CONST) && this.hasClass(SELECTED_CLASS_CONST))
        { return; }
        else { swapClass(dragSrc, this, SELECTED_CLASS_CONST); }
    }
    return false;
}


function toggleClass(e) {

    if (e.preventDefault) {
        e.preventDefault();
    }
    this.toggleClass(SELECTED_CLASS_CONST);
}

function swapClass(elemA, elemB, c) {

    if (elemA.hasClass(c) || elemB.hasClass(c)) {
        elemA.toggleClass(c);
        elemB.toggleClass(c);
    }
    return;
}  

Element.prototype.toggleClass = function(name) {

    if (this.hasClass(name))
    {
        this.removeClass(name);
    }
    else
    {
        this.addClass(name);
    }
};

Element.prototype.hasClass = function(name) {

    return new RegExp("(?:^|\\s+)" + name + "(?:\\s+|$)").test(this.className);
};

Element.prototype.addClass = function(name) {

    if(!this.hasClass(name))
    {
        this.className = this.className ? [this.className, name].join(' '): name
    }
};

Element.prototype.removeClass = function(name) {

    if(this.hasClass(name))
    {
        var curClass = this.className;
        this.className = curClass.replace(new RegExp("(?:^|\\s+)" + name + "(?:\\s+|$)", "g"), "");
    }
};
var draggables = document.getElementsByClassName('drag');

[].forEach.call(draggables, function(drag) {
    drag.addEventListener('dragstart', handleDragStart, false);
    drag.addEventListener('dragenter', handleDragEnter, false);
    drag.addEventListener('dragover', handleDragOver, false);
    drag.addEventListener('dragleave', handleDragLeave, false);
    drag.addEventListener('dragend', handleDragEnd, false);
    drag.addEventListener('drop', handleDrop, false);
    drag.addEventListener('click', toggleClass, true);
});

I know I could just use jQuery or the myriad of other js libraries, but this is just a project to really learn Javascript thoroughly before moving to a library. At least how to properly structure my code (something I'm still learning how to do in PHP as well...) form closures when necessary, and how to recognize when that is.

Edit: JSFiddle (warning: I don't think preventDefault() works in the jsfiddle window, so it will probably redirect you, I didn't test with other browsers.) and another example with source. The second example has a similar code structure, but it's obviously a quick and dirty example; I want to know if there is a better way to do it. It mostly being event binding and maintaining a "const" between various unassociated event function calls.

\$\endgroup\$
  • \$\begingroup\$ On the var: a variable declared with var is in the local scope, one without is in the global scope. So when you assign to it in one function, it is available in the other as the variable is in the global scope; while when you use var dragSrc = this; then dragSrc is only available inside that function (... so while outside, you will access the global-scope variable with the same name, which is undefined). \$\endgroup\$ – ANeves Feb 13 '12 at 13:47
2
\$\begingroup\$

Edit: I missed that you were talking about the HTML5 drag and drop stuff, but I think this still kind of applies, so I'll leave it here for now. The idea is to "namespace" stuff into a singleton object.


The usual approach is to use a singleton object and attach all those functions and variables to it as properties.

You might try something like this, for example.

// mouse input stuff

var mouseInput = {};

// grab (start dragging) a node
mouseInput.grab = function(node, x, y) {
    this.activeNode = node;
    node.style.position = 'relative';
    node.style.left = '0px';
    node.style.top = '0px';
    node.style.zIndex = '10';
    this.lastX = x;
    this.lastY = y;
    this.lastLeft = 0;
    this.lastTop = 0;
};

// drag a node
mouseInput.drag = function(x, y) {
    if (!this.activeNode) return;
    this.lastLeft -= (this.lastX - x);
    this.lastTop -= (this.lastY - y);
    this.activeNode.style.left = this.lastLeft + 'px';
    this.activeNode.style.top = this.lastTop + 'px';
    this.lastX = x;
    this.lastY = y;
};

// drop (stop dragging) a node
mouseInput.drop = function(x, y) {
    if (!this.activeNode) return;
    this.activeNode.style.zIndex = null;
    this.activeNode.style.left = null;
    this.activeNode.style.top = null;
    this.activeNode.style.position = null;
    this.activeNode = null;
};

If you prefer, every this can be replaced with mouseInput.

\$\endgroup\$
  • \$\begingroup\$ I think you're misunderstanding a little. HTML5 implements drag and drop events that you can attach functions to with event handlers. No need to keep track of x,y and other things like that. I added a link and JSfiddle to my question. \$\endgroup\$ – Zack Feb 12 '12 at 9:12
  • \$\begingroup\$ Ah, my bad. But still, doesn't the same concept apply? i.e. instead of making dragSrc a global var, attach it to some kind of namespace object. \$\endgroup\$ – Dagg Feb 12 '12 at 9:23
  • \$\begingroup\$ dragSrc is not the global I was talking about. I meant SELECTD_CLASS_CONST and DROP_CLASS_CONST. Isn't dragSrc in the scope of handleDragStart()? \$\endgroup\$ – Zack Feb 12 '12 at 9:32
  • \$\begingroup\$ I did some digging on the singleton thing and now I get what you mean - having one singleton global variable, and binding all the other ones to it as properties. That definitely answers one question, thanks :) \$\endgroup\$ – Zack Feb 12 '12 at 10:38

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.