2
\$\begingroup\$

Following this question, I've rewritten my module as follows:

;(function(ns) { //anonymous and auto-executing function for the module definition
    //default options for addEditEvents()
    var defaults = {
        element: $("body"),
        selector: "li",
        url: "",
        idInUrl: "",
        idAttr: "",
        jsonAttr: ""
    };

    function putAjax(putUrl, putObj) {
        return $.ajax({
            method: "PUT",
            url: putUrl,
            data: JSON.stringify(putObj),
            contentType: "application/json; charset=utf-8",
        });
    };

    //function that will add events to elements 
    // element :    ancetor element                                             (e.g. $("sheets"))
    // selector :   sub-elements that will receive the events                   (e.g. "a[href^='/sheets']")
    // url :        url requested for the PUT request                           (e.g. "/sheets/ID/update/")
    // idInUrl :    what string to remplace in the url                          (e.g. "ID")
    // idAttr :     HTML attribute of selected elements where is stored the id  (e.g. "sheet_id")
    // jsonAttr :   attribute name in the JSON object of the request            (e.g. "label")
    ns.addEditEvents = function(options) {
        var mergedOptions = Object.assign({}, defaults, options);
        var element = mergedOptions.element;
        var selector = mergedOptions.selector;
        var url = mergedOptions.url;
        var idInUrl = mergedOptions.idInUrl;
        var idAttr = mergedOptions.idAttr;
        var jsonAttr = mergedOptions.jsonAttr;
        var DELAY = 700;
        var clicks = 0;
        var timer = null;
        var originElement;
        var editElement;



        // unset the double-click event
        element.on("dblclick", selector, function(event) {
            event.preventDefault();
        });

        // handle the click events
        element.on("click", selector, function(event) {
            event.preventDefault();
            clicks++; //count clicks
            var href = $(this).attr('href');
            if (clicks === 1) {
                timer = setTimeout(function() {
                    // here lives the single click event

                    // For some browsers, `attr` is undefined; for others,
                    // `attr` is false.  Check for both.
                    if (typeof href !== typeof undefined && href !== false) {
                        window.location = href // follow hlink
                    };
                    clicks = 0; //after action performed, reset counter
                }, DELAY);

            } else {
                clearTimeout(timer); //prevent single-click action
                //here lives the double-click action
                // change the DOM: <input> instead of child element + add a class to handle events
                originElement = $(this);
                editElement = $(document.createElement('input'));
                editElement.val(originElement.text()); // same text
                editElement.addClass("keyEvents"); // class for keyboard events
                editElement.attr(idAttr, originElement.attr(idAttr)); // pass the id through the DOM
                originElement.replaceWith(editElement);
                editElement.focus();

                clicks = 0; //after action performed, reset counter
            }

        });

        //handle the keypressed events on the edited elements
        element.on("keypress", ".keyEvents", function(event) {
            if (event.which == 13) {
                // form the url and the json object                
                var putUrl = url.replace(idInUrl, originElement.attr(idAttr)); //replace idInUrl in url by id                
                var putObj = {};
                putObj[jsonAttr] = $(this).val();

                // AJAX query as a promise
                console.log(putUrl);
                return putAjax(putUrl, putObj).then(function(data){
                    // DOM operation on success
                    // Very careful here : the RESTapi should return a JSON
                    // with the updated infos
                    originElement.text(data[jsonAttr]);
                    editElement.replaceWith(originElement);
                },function(error){
                    // DOM operation on error
                    // replace whith origin element
                    $(this).replaceWith(originElement);
                });

            } else if (event.which == 27) {
                // if Esc is pressed
                // replace whith origin element
                $(this).replaceWith(originElement);
            };
        });
    };
}(this.editable = this.editable || {}));

To add an "editable" behavior on some elements, I call:

$(document).ready(function() {        
    editable.addEditEvents({
        element: $("#sheets"),
        selector: "a[href^='/sheets']",
        url: "/api/v1/sheets/ID/update/",
        idInUrl: "ID",
        idAttr: "sheet_id",
        jsonAttr: "label"
    });
});

Everything works as expected : double-click on an anchor shows an , then press "enter" fires a PUT request (or escape to undo).

Now how can I optimize this code in terms of performance?

I will have around 1000 events added to one page with this method.

For instance, I'm wondering if the scoping of my variables is OK, in particular originElement and editElement.

Thanks

\$\endgroup\$
3
  • \$\begingroup\$ The previous answer gave an excellent advice: instead of attaching the event listeners to each added element, attach them just once to their common parent element. Even if it's document. \$\endgroup\$
    – woxxom
    Commented Sep 13, 2016 at 19:14
  • \$\begingroup\$ Have you tested a putAjax() error? Is $(this) in the error handler correct? \$\endgroup\$ Commented Sep 25, 2016 at 0:40
  • \$\begingroup\$ Have you tested the independence of editable elements from each other? For example, double-click A, then double-click B, then return focus to A and press Esc. A should revert to originElement, not B. Is this so? If not then scoping of variables is incorrect. \$\endgroup\$ Commented Sep 25, 2016 at 0:53

0

Your Answer

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

Browse other questions tagged or ask your own question.