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');
})();
}
2 Answers 2
For readability, place space characters around the parenthesis in
if
statements:if (condition) {
Instead of:
if(condition){
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);
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());
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 } };
You don't need to attach the handler to the
document
object. You have a couple of options: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 handlerAttach 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.
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.
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.
-
\$\begingroup\$ The
live
method in jQuery queried the document every so many seconds to update collections of DOM nodes. Theon
method is the very same event delegation posted in the question. The only difference is you can call theon
function using any element. Adding the event listener on thedocument
object is not nearly as slow as you say. \$\endgroup\$Greg Burghardt– Greg Burghardt2016年04月07日 12:55:51 +00:00Commented Apr 7, 2016 at 12:55 -
\$\begingroup\$ @GregBurghardt sitepoint.com/on-vs-live-review \$\endgroup\$Joseph– Joseph2016年04月07日 13:12:54 +00:00Commented 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– Ced2016年04月07日 13:34:30 +00:00Commented 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– Ced2016年04月07日 14:48:18 +00:00Commented 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 thedocument
element. \$\endgroup\$Greg Burghardt– Greg Burghardt2016年04月07日 19:13:19 +00:00Commented Apr 7, 2016 at 19:13
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\$Widgets
constructor and not publically available using dot or array syntax. Invokingnew 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\$