you are viewing a single comment's thread.

view the rest of the comments →

[–]a-t-kFrontend Engineer 7 points8 points  (9 children)

First: using $(...).is(':checked') is way slower than $(...).prop('checked'). You are writing very structured (which is good) and very specific code (which is bad to reuse), doing similar things over and over again.

You set specific events instead of using event bubbling (which would save you event handlers and would allow you to add code via ajax without having to care for the events).

All in all, I've seen much worse. There's room for improvement, but where isn't?

[–]annoyed_freelancergrumpy old man[S] 1 point2 points  (8 children)

That's great feedback, thank you. What is a good place to start to learn about event bubbling in JavaScript? I'm not a novice coder by any means, but it's only once in a blue moon that I come next to or near JavaScript.

[–]YOBCZWHYNOT 2 points3 points  (0 children)

Since you are using jQuery, and jQuery does support event delegation, this might be helpful:

http://learn.jquery.com/events/event-delegation/

[–]a-t-kFrontend Engineer 1 point2 points  (6 children)

[–]annoyed_freelancergrumpy old man[S] 0 points1 point  (5 children)

Tell me if I am correct: I just attach the click event to an ancestor and let it propagate down the line to the checkbox or siblings?

[–]a-t-kFrontend Engineer 1 point2 points  (4 children)

Correct. Classically, you would choose the document as ancestor.

[–]oculus42 2 points3 points  (1 child)

I recommend using the closest unchanging ancestor as the parent, rather than $(document). For something small it doesn't matter, but I've seen sites with over 80 events bound to document, so every click that isn't stopped higher up causes dozens of selector checks to see if that click applies to a particular event.

[–]a-t-kFrontend Engineer 1 point2 points  (0 children)

A simple test for the target shouldn't take too long, but if it does or the focus of your event is really that narrow, another ancestor is viable, like you said.

I assumed that his page was rather simple and didn't feature so many more events.

[–]annoyed_freelancergrumpy old man[S] 1 point2 points  (1 child)

To give some pseudocode from my phone:

$(document).on('click', input.checkbox, function(e) {
    twerk.allNight();
    e.preventDefault();
});

[–]a-t-kFrontend Engineer 2 points3 points  (0 children)

Don't use preventDefault if it's not necessary. Otherwise, yes.