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\$
  • \$\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 Sep 13 '16 at 19:14
  • \$\begingroup\$ Have you tested a putAjax() error? Is $(this) in the error handler correct? \$\endgroup\$ – Roamer-1888 Sep 25 '16 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\$ – Roamer-1888 Sep 25 '16 at 0:53

Your Answer

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

Browse other questions tagged or ask your own question.