1
\$\begingroup\$

I'm adding an onclick listener to the whole document then checking the source of the click. If the source of the click contains the attribute data-widget I want to go to the appropriate action. I did it but I'm hoping for a more efficient way of doing things and if I can do without the self invoking function. 

<div data-widget="myaction"> Click here </div>

document.addEventListener("click", function(e) {
    var widgets = new Widgets();
    var w = e.target.getAttribute("data-widget");
    if(w){
        e.preventDefault();
        widgets[w];
    }
});

function Widgets(){
    var myaction = (function myaction(){
        console.log('indeed');
    })();
}
\$\endgroup\$
8
  • \$\begingroup\$ The widgets[w] line does nothing but access a member of that object. It doesn't appear to call a function. Is that the intent? \$\endgroup\$
    – Greg Burghardt
    Apr 7, 2016 at 12:54
  • \$\begingroup\$ @GregBurghardt reading the variable evaluates the lambda and executes it. \$\endgroup\$
    – Pimgd
    Apr 7, 2016 at 15:34
  • \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Pimgd
    Apr 7, 2016 at 15:51
  • \$\begingroup\$ @Pimgd: No it doesn't. The "myaction" variable is just that - a local variable to the Widgets constructor and not publically available using dot or array syntax. Invoking new Widgets is actually calling the "myaction" function. This is a bug since the "data-widget" attribute does not even need to exist to execute that function. \$\endgroup\$
    – Greg Burghardt
    Apr 7, 2016 at 19:27
  • \$\begingroup\$ @Pimgd It's sad because I received an answer that I think I would have benefited more if my edit was available, as I think what he does is close to what I had in my edit. I feel like opening a new question would polute the forum \$\endgroup\$
    – Ced
    Apr 7, 2016 at 20:02

2 Answers 2

Reset to default
1
\$\begingroup\$

  1. For readability, place space characters around the parenthesis in if statements:

        if (condition) {

    Instead of:

        if(condition){

  2. The line of code:

        if(w) {
        widgets[w];

    Doesn't actually do anything. It accesses a property and nothing more. To invoke a function, maybe you want:

        // Pass in the event object in case someone needs it
    widgets[w](e);

  3. Creating a new Widgets object on each click can be a tad wasteful. Since they are so disposable, create one instance and hide it from the global context using an auto invoked function expression:

        (function(widgets) {
        document.addEventListener("click", function(e) {
            var w = e.target.getAttribute("data-widget");

            if (w) {
                e.preventDefault();
                widgets[w](e);
            }
        });
    })(new Widgets());

  4. This also necessitates changes to the Widgets class:

        function Widgets() {
    }

    Widgets.prototype = {
        constructor: Widgets,

        myAction: function(event) {
            // do stuff
        },

        anotherAction: function(event) {
            // do more stuff
        }
    };

  5. You don't need to attach the handler to the document object. You have a couple of options:

    1. Attach it to the document.body object. This works well because "click" events sure can't from the <head> element. The disadvantage is that you must wait for the <body> element to exist before attaching the event handler

    2. Attach event handlers to document.documentElement which is the <html> tag. Yes, the event must bubble up two whole more DOM nodes than it needs to, but this property exists as soon as JavaScript begins executing. No need to wait for the "DOM ready" event. Just attach the handler and be on your merry way.

  6. Consider removing the call to e.preventDefault() and let the widget methods decide to do that. This way you can respond to an event, but not change its default behavior.

You've got the right idea. It's a pretty good attempt at a simple event delegation framework. Clean and concise.

\$\endgroup\$
0
2
\$\begingroup\$

The way constructors work is that the engine creates an object, and calls the constructor passing the object as context. Essentially, the constructor is being run like a function.

console.log('indeed'); runs because you created an instance of Widget, which calls the constructor and runs your iife. The following code alone works in the same way.

document.addEventListener("click", function(e) {
  var widgets = new Widgets();
});

function Widgets(){
  console.log('indeed');
}

Now your code creates a Widgets instance regardless of the presence of data-widget. If you only deal business when it has data-widget, I suggest you create the instance within the if block.

I'm not sure what your goal with widgets[w] is, but if you mean storing w's value as a property of an instance of Widgets, I it's best if you define the object's properties in the constructor in advance. While this sacrifices dynamicity, it eliminates guesswork as to what properties an instance of Widgets contain.

Additionally, delegating all the way up to document is slow. That's one reason jQuery deprecated live and provided you on instead. live bound all handlers to document, while on gives you flexibility to where you want the delegate target bound.

\$\endgroup\$
8
  • \$\begingroup\$ The live method in jQuery queried the document every so many seconds to update collections of DOM nodes. The on method is the very same event delegation posted in the question. The only difference is you can call the on function using any element. Adding the event listener on the document object is not nearly as slow as you say. \$\endgroup\$
    – Greg Burghardt
    Apr 7, 2016 at 12:55
  • \$\begingroup\$ @GregBurghardt sitepoint.com/on-vs-live-review \$\endgroup\$
    – Joseph
    Apr 7, 2016 at 13:12
  • \$\begingroup\$ My intent with widgets[w] is to call a function with the same name as my attribute so I don't have a switch inside my condition. About your last paragraph I've read some questions on SO where the answers basically said it could be slow. However I've quite an huge amount of click events with a comment section similar to reddit's. I checked the source of reddit and as far as I can tell they use delegation all the way to document. I didn't notice huge delays when using their website. \$\endgroup\$
    – Ced
    Apr 7, 2016 at 13:34
  • \$\begingroup\$ Could you give me a codified example for what you'd do for widgets ? Basically I wanted to have my functions for widgets being encapsulated there and call them on the fly depending on the value of the argument data-widget. I marked it as answered as you have reviewed my question already, further insight would be appreciated though \$\endgroup\$
    – Ced
    Apr 7, 2016 at 14:48
  • \$\begingroup\$ @JosephtheDreamer: Correct. But under the hood, the live jQuery method is polling the document with the given selector to attach event handlers directly to the DOM nodes, vs. on which utilizes event delegation, albeit closer to the event target element than using the document element. \$\endgroup\$
    – Greg Burghardt
    Apr 7, 2016 at 19:13

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.