4
\$\begingroup\$

I've just finished a week long adventure of creating an HTML5 Drag and Drop scheduler. As the prototype stands it works fine, but I'm curious if some of the code can be optimized. I mostly focused on it being readable and well documented, but I'm curious if someone can come up with any optimizations.

I've noticed that when Firefox is used there is some "twitching" that occurs when the cells are updated. Here's the code (minus the comments that are available on the fiddle):

var unscheduledTbody = null,
    scheduledTbody = null,
    isDroppable = true;

var dataTransferValue = null;

var Indecies = function (td) {
    var tr = td.parent(),
        tbody = tr.parent();

    var row = tbody.children().index(tr),
        column = tr.children().index(td);

    return [row, column];
};

var CheckCellRowspan = function (options) {
    var element = null;

    for (var i = 1; i < options.rows; i++) {
        element = scheduledTbody.find("tr:eq(" + (options.row + i) + ") td." + options.column);

        if (element.attr("id") != null) {
            isDroppable = false;

            return;
        };
    };

    element = null;

    isDroppable = true;
};

var ToggleCellVisibility = function (options) {
    var selectors = "",
        i = 1;

    for (i; i < options.rows; i++) {
        if (options.hide) {
            selectors = (selectors + ("tr:eq(" + (options.row + i) + ") td." + options.column + ","));
        } else {
            selectors = (selectors + ("tr:eq(" + (options.row + i) + ") td." + options.column + ":hidden,"));
        };
    };

    selectors = selectors.substring(0, (selectors.length - 1));

    if (selectors.length > 0) {
        scheduledTbody.find(selectors).css({
            display: (options.hide ? "none" : "table-cell")
        });
    };
};

$(function () {
    $("#Unscheduled tbody td,#Scheduled tbody td").prop("draggable", true);

    unscheduledTbody = $("#Unscheduled tbody");
    scheduledTbody = $("#Scheduled tbody");

    unscheduledTbody.find("td").data("scheduled", false);

    var scheduledTds = scheduledTbody.find("td");

    $("[draggable]").live("dragstart", function (e) {
        e.originalEvent.dataTransfer.effectAllowed = "move";
        e.originalEvent.dataTransfer.setData("text/plain", this.id);
    }).live("drag", function (e) {
    }).live("dragend", function (e) {
        var dropEffect = e.originalEvent.dataTransfer.dropEffect;

        switch (dropEffect) {
            case "copy":
            case "move":
                if (isDroppable) {
                    var source = $(this),
                        target = $("#" + dataTransferValue);

                    if (source.data("scheduled")) {
                        var sourceIndecies = Indecies(source),
                            sourceRow = sourceIndecies[0],
                            sourceColumn = sourceIndecies[1],
                            rows = target.data("rows");

                        ToggleCellVisibility({
                            rows: rows,
                            row: sourceRow,
                            column: sourceColumn,
                            hide: false
                        });

                        sourceIndecies = null;
                        sourceRow = null;
                        sourceColumn = null;
                        rows = null;
                    } else {
                        source.parent().remove();
                    };

                    dataTransferValue = null;
                    source = null;
                    target = null;
                };

                break;
            case "link":
            case "none":
            default:
                break;
        };
    });

    scheduledTds.each(function () {
        var td = $(this);

        td.addClass("" + Indecies(td)[1]);
    }).data("scheduled", true).live("dragenter", function (e) {
        e.originalEvent.dataTransfer.dropEffect = "move";

        $(this).addClass("Droppable");
    }).live("dragover", function (e) {
        e.preventDefault();
    }).live("dragleave", function (e) {
        $(this).removeClass("Droppable");
    }).live("drop", function (e) {
        e.preventDefault();

        dataTransferValue = e.originalEvent.dataTransfer.getData("text/plain");

        var source = $("#" + dataTransferValue),
            target = $(this),
            targetIndecies = Indecies(target),
            targetRow = targetIndecies[0],
            targetColumn = targetIndecies[1],
            rows = source.data("rows");

        CheckCellRowspan({
            rows: rows,
            row: targetRow,
            column: targetColumn
        });

        if (isDroppable) {
            var sourceIndecies = Indecies(source),
                sourceRow = sourceIndecies[0],
                sourceColumn = sourceIndecies[1],
                url = source.data("url");

            target.removeClass("Droppable")
                .html(source.html())
                .attr("id", source.attr("id"))
                .attr("rowspan", rows)
                .data("rows", rows)
                .data("url", url);

            source.html("")
                .removeAttr("id")
                .removeAttr("rowspan")
                .removeData("rows")
                .removeData("url");

            ToggleCellVisibility({
                rows: rows,
                row: targetRow,
                column: targetColumn,
                hide: true
            });

            sourceIndecies = null;
            sourceRow = null;
            sourceColumn = null;
        } else {
            $("b").text("The cell could not be dropped at the target location. It conflicted with an existing cell in its path.");

            target.removeClass("Droppable");
        };

        source = null;
        target = null;
        targetIndecies = null;
        targetRow = null;
        targetColumn = null;
        rows = null;
    }).find("a").live("click", function (e) {
        e.preventDefault();

        var source = $(this).closest("td"),
            sourceIndecies = Indecies(source),
            sourceRow = sourceIndecies[0],
            sourceColumn = sourceIndecies[1],
            rows = source.data("rows");

        unscheduledTbody
            .append($("<tr />")
                .append($("<td draggable=\"true\" data-scheduled=\"false\" />")
                    .html(source.html())
                    .attr("id", source.attr("id"))
                    .data("rows", rows)));

        source.html("")
            .removeAttr("id")
            .removeAttr("rowspan")
            .removeData("rows");

        ToggleCellVisibility({
            rows: rows,
            row: sourceRow,
            column: sourceColumn,
            hide: false
        });

        source = null;
        sourceIndecies = null;
        sourceRow = null;
        sourceColumn = null;
        rows = null;
    });

    scheduledTds = null;
});
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I don't really address the concrete questions you made; I hope someone else does. :)


As the prototype stands it works fine

Famous last words! You have at least two bugs:

  1. Drag D on top of B, and B is lost forever.
  2. Now drag A on top of D, and not only D disappears but the layout is ruined.

You will get some good tips by running your code through JSLint.
Some quick and non-exaustive remarks:

  • You can avoid escaping quotes by alternating between single and double quotes, which both JavaScript and HTML use interchangeably: use str = "<foo bar='nix' />" instead of str = "<foo bar=\"nix\" />";
  • Do not pollute the global namespace: encapsulate the code in a self-executing function;
  • Starting the code with "use strict"; will really help with identifying many problems early;
  • Use === to compare, and not ==;
  • indecies should be called indexes (actually, getIndexesOf);
  • Constructors start with a capital letter - all those methods are not constructors, so they should not start with a capital letter;
  • Use .on('event', ...) instead of live, and maybe even use the .on overload that takes an "event-map".

Note that .on will not bind to objects that are later loaded into the DOM. But you can bind to a parent element, and provide a selector to filter which children elements to catch the event on. This not only catches events on such children that might be later attached (same as live), but also attaches a single event handler instead of a million. Take heed not to use document to attach these, or similar elements high in the DOM tree - read the API: http://api.jquery.com/on/

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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