Make comments.js indenpendant of jQuery

[madskristensen]

https://github.com/madskristensen/MiniBlog/blob/master/Website/scripts/comments.js shouldn't use jQuery but native JavaScript instead. It's the only .js file loaded on a post-page and if it didn't use jQuery we could eliminate including jQuery for other than when logged in.

[jodydonetti]

which browser support level you have in mind?

[am11]

Here is the (untested) draft. Except for slideup and slideDown animations, the rest is independent of jQuery:

//#region Helpers

var AsynObject = AsynObject ? AsynObject : new Object();

AsynObject.ajax = function (url, callback) {
    var ajaxRequest = AsynObject.getAjaxRequest(callback);
    ajaxRequest.open("GET", url, true);
    ajaxRequest.setRequestHeader('X-Requested-With', 'XMLHttpRequest');
    ajaxRequest.send(null);
};

AsynObject.postAjax = function (url, callback, data) {
    var ajaxRequest = AsynObject.getAjaxRequest(callback);
    ajaxRequest.open("POST", url, true);
    ajaxRequest.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
    ajaxRequest.setRequestHeader("Connection", "close");
    ajaxRequest.send("data=" + encodeURIComponent(data));
};

AsynObject.getAjaxRequest = function (callback) {

    var ajaxRequest;

    try {
        ajaxRequest = new XMLHttpRequest();
    } catch (e) {
        try {
            ajaxRequest = new ActiveXObject("Msxml2.XMLHTTP");
        } catch (e) {
            try {
                ajaxRequest = new ActiveXObject("Microsoft.XMLHTTP");
            } catch (e) {
                return null;
            }
        }
    }

    ajaxRequest.onreadystatechange = function () {
        callback(ajaxRequest.readyState, ajaxRequest.status, ajaxRequest.responseText);
    };

    return ajaxRequest;

};

function hasClass(elem, className) {
    return new RegExp(' ' + className + ' ').test(' ' + elem.className + ' ');
}

function addClass(elem, className) {
    if (!hasClass(elem, className)) {
        elem.className += ' ' + className;
    }
}

function removeClass(elem, className) {
    var newClass = ' ' + elem.className.replace( /[\t\r\n]/g, ' ') + ' ';
    if (hasClass(elem, className)) {
        while (newClass.indexOf(' ' + className + ' ') >= 0 ) {
            newClass = newClass.replace(' ' + className + ' ', ' ');
        }
        elem.className = newClass.replace(/^\s+|\s+$/g, '');
    }
}

function toDOM(htmlString) {
    var wrapper = document.createElement('div');
    wrapper.innerHTML = htmlString;
    return wrapper.firstChild;
}

function getElementsByAttribute(attr, value, root) {
    var arr = new Array();

    root = root || document.body;
    if (root.hasAttribute(attr) && root.getAttribute(attr) == value) {
        arr.push(root);
    }
    var children = root.children;

    for (var i = children.length; i--;) {
        if (children[i].hasAttribute(attr) && children[i].getAttribute(attr) == value) {
            arr.push(children[i]);
        }
    }
    return arr;
}

function getAllParents(element) {
    var arr = [];
    while (element) {
        arr.unshift(element);
        element = element.parentNode;
    }
    return arr;
}

function getParentsByAttribute(element, attr, value) {
    var arr = new Array();

    var parents = getAllParents(element);

    for (var i = 0; i < parents.length; i++) {
        if (parents[i].hasAttribute(attr) && parents[i].getAttribute(attr) == value) {
            arr.push(parents[i]);
        }
    }
    return arr;
}

function bindEvent(el, eventName, eventHandler) {
    if (el.addEventListener) {
        el.addEventListener(eventName, eventHandler, false);
    } else if (el.attachEvent) {
        el.attachEvent('on' + eventName, eventHandler);
    }
}

//#endregion

window.onload = function () {
    var endpoint = "/comment.ashx";

    function deleteComment(commentId, postId, element) {

        if (confirm("Do you want to delete this comment?")) {
            AsynObject.postAjax(endpoint, function(state, status, data) {
                if(state === 4 && status === 200) {
                    element.slideUp(function () { element.remove(); });
                    return;
                }
                alert("Something went wrong. Please try again");
            }, { mode: "delete", postId: postId, commentId: commentId });
        }
    }

    function saveComment(name, email, website, content, postId, callback) {

        if (localStorage) {
            localStorage.setItem("name", name);
            localStorage.setItem("email", email);
            localStorage.setItem("website", website);
        }

        AsynObject.postAjax(endpoint, function (state, status, data) {

            if (state === 4 && status === 200) {
                var elemStatus = document.getElementById("status");
                elemStatus.innerText("Your comment has been added");
                removeClass(elemStatus, "alert-danger");
                addClass(elemStatus, "alert-success");

                document.getElementById("commentcontent").value = "";

                AsynObject.ajax(data, function (html) {
                    if (state === 4 && status === 200) {
                        var comment = toDOM(html);
                        comment.style.display = "none";
                        var elemComments = document.getElementById("comments");
                        elemComments.appendChild(comment);
                        comment.slideDown();
                        callback(true);
                    }
                });

                return;
            }

            addClass(elemStatus, "alert-danger");
            elemStatus.innerText(data.statusText);
            callback(false);

        }, { mode: "save", postId: postId, name: name, email: email, website: website, content: content })

    }

    function initialize() {

        var postId = getElementsByAttribute("itemprop", "blogPost")[0].getAttribute("data-id");
        var email = document.getElementById("commentemail");
        var name = document.getElementById("commentname");
        var website = document.getElementById("commenturl");
        var content = document.getElementById("commentcontent");
        var commentForm = document.getElementById("commentform");

        commentForm.onsubmit =  function (e) {
            e.preventDefault();
            var button = e.target;
            button.setAttribute("disabled", true);

            saveComment(name.value, email.value, website.value, content.value, postId, function () {
                button.removeAttribute("disabled");
            });
        };

        var elementsDeleteComments = document.getElementsByClassName('deletecomment');

        for (i = 0, len = elementsDeleteComments.length; i < len; i++) {
            bindEvent(elementsDeleteComments[i], 'click', function (e) {
                e.preventDefault();
                var button = e.target;
                var element = getParentsByAttribute(button, "itemprop", "comment");
                deleteComment(element.getAttribute("data-id"), postId, element);
            });
        }

        if (localStorage) {
            email.value = localStorage.getItem("email");
            website.value = localStorage.getItem("website");

            if (name.value.length === 0)
                name.value = localStorage.getItem("name");
        }
    }

    if (document.getElementById("commentform"))
        initialize();

};

[docluv]

the slideup and slidedown animations should not be done in JavaScript they should be CSS animations instead.

[am11]

@docluv, if the application is supposed to target only the latest browsers, then CSS3 animation makes sense. . Also, in that case, we can get rid of the getElememtsByAttribute() and instead use document.querySelector("[itemprop=blogpost]") in initialize function.

[docluv]

You should target current browsers, which all support CSS animations. Obsolete browsers should not force it to do things it was not designed to do. So maybe just show and hide the element if the browser is obsolete. My rule, don't bother going out of your way for them anymore.

[madskristensen]

I think IE9 and above is fair at this point

[JimBobSquarePants]

I know this is done and dusted and closed etc but I'm not convinced it was a wise move.

• Firstly, you've removed JavaScript consistency within the project. That makes it much less easy to maintain and provide bug fixes for. Increasing developer complexity is a definite no-no, especially when you are targeting the engine as a responsive one which can be viewed on multiple devices each with their specific quirks.
• Secondly, jQuery is generally cached by browsers especially if you are sourcing it from a cdn, the 27kb you are saving (jQuery gzipped and minified minus the increased file size of the new file) will most likely be offset by a single image within a post or font file within a theme. You're already setting headers that will reduce subsequent requests so why not use them?
• Thirdly, the JavaScript that has replaced the original code is well... a bit clunky. Extending the Element and Nodelist prototypes in the way they have been would be considered extremely poor practise, encodeURIComponent should be used in the method that encodes object properties to a url, and the slide code also messes about with the DOM too much and could be achieved by setting the elements height to auto, reading the height using getComputedStyle() then forcing a repaint by querying the offsetHeight. Is the slide code really necessary if you want the codebase to be lean?

[am11]

@JimBobSquarePants, the idea was to seamlessly replace jQuery. I tried my best to NOT change anything on the server-side, which attracted the poor code. Yes encodeURIComponent should be used. But jQuery doesn't use it that way. I dived into jQuery code to figure it out. Finally the custom encoding was the way to go.

Thanks for pointing out the issues. Honestly, I never got time to revisit it. If you can improve the existing scrip, PR is most welcome. But please try to contain the fixes at JS side (of course unless its necessary).

[JimBobSquarePants]

Thanks for the response,

JQuery does actually use encodeURIComponent in that way, it's in the add() function in jQuery.param() that's after it has already ran checks to sanitize the data.

That you had to dive into the jQuery source in order to try to replicate its behaviour proves my point. You've added vastly increased development complexity to the project in order to save a few kb.

If saving a few kb really is that important I would have a good dig through Bootstraps source code. It's littered with inefficient, bloated gems like this.

I'll see if I can get some time together to go through the code and do a PR. I am crazy busy at the moment though so it could be a few weeks before I get the chance.

[madskristensen]

jQuery adds nothing to the table for the simple operation of adding comments. It was an unnecessary overhead. The only reason it remains in the project is because the WYSIWYG editor uses it.

[docluv]

I agree with Mads. The gap jQuery bridged and bridged well between old browsers has been filled and the browsers now natively implement just about everything jQuery offers. 27kb gzipped is actually a lot, I build most apps with < 10kb gzipped in total now thanks to browser advancements. With performance being a big deal it adds a lot of value. Plus as the power of browsers continues to improve you do not want to be explicitly tied to a version of jQuery, etc. I understand you wanting to keep it, but as someone who has gone down the road try life without jQuery and other large libraries and see how easy it actually is.

[JimBobSquarePants]

Then strip out Bootstrap, strip out the editor, the blog could be written without either of them.

@docluv I'm not sure you and I are speaking the same language. When I talk about performance I talk about frame rates, DOM manipulation, jank etc, An initial download of a non blocking, probably cached framework is not a performance concern especially in a website that will most likely contain images.

I'm very comfortable writing apps, full websites even without jQuery, I only queried the thread because the framework was already present and because the JavaScript was overly complex with some poorly written code.

If it were that easy, the replacement code would not be riddled with bugs, polyfills wouldn't exist and jQuery 2+ would not contain normalising code to iron out differences between browsers.

Also, natively implement just about everything jQuery offers? I invite you to look through the source.

[docluv]

I look through the source code all the time. I can reduce the 'needed' functionality to < 8kb. jQuery is not good for performance FWIW. I don't want to hijack the thread, but DOM manupilation is on the order of 30-50,000 times faster with the DOM API than jQuery for example. Animations using CSS3 are far smoother than using JavaScript timers for example. I am a performance 1st developer. If it can't load in 1 second on Sprint 3G it is not acceptable. If there is jank it is not done. As for the quality of the code, you maybe right, that's ok, that why development is an iterative cycle. Make it better.

I hear the argument for cached libraries, but the honest truth is which version of jQuery is cached? remember jQuery is very fragemented right now and caching does vary. A lot of anitviruses effectively negate the benefits of HTTP cache. so the argument there falls apart quicker than we would like when checked in the real world. I wish I had the link to some studies done around this to share.