CODEWITHSUNDEEP
javascriptjqueryrefactoringprototypejs
mwilliams
mwilliams

Reputation: 9978

Can this snippet of Javascript be simplified more with jQuery?

I have the following snippet of Javascript, which is just attaching an onclick to a button and then updating the text of an em tag underneath it. I'm slowly trying to educate myself and work with jQuery more as I've found to like the syntax a lot more and for some tasks, it's more of a pleasure to work with.

Some of the best examples I've found have come from Stackoverflow, so I come once again, to see how this could refactored and improved with jQuery; any thoughts?

$$('em.green').each(function(em) { 
  em.up('button').onclick = function() { 
    em.update('Saving...') 
  }; 
});

Thanks!

Upvotes: 2

Views: 239

Answers (5)

cgp
cgp

Reputation: 41391

(I can't edit, so I'll have to create a new answer but:)

Slightly shorter:

$('button').click(function() { 
  $(this).closest('em.green').html('Saving...');
});

Saves you from having to loop through every EM and then bind the onclick. Might also help to add a class to the button so you're not binding to every button on the page, just in case there are others.

It's unnecessary to include the function parameters, use the this variable to specify the context you want to find the closest parent of.

Upvotes: 0

redsquare
redsquare

Reputation: 78677

Matthew Crumley's answer is good but why attach multiple handlers when one will do.

The added advantage is that this will also work if you create any em.green elements later in the lifespan of the document.

$('button:has(em.green)').live('click', function(){
  $(this).find('em.green').html('Saving...')
});

Upvotes: 1

Matthew Crumley
Matthew Crumley

Reputation: 102745

This is a little shorter and might be easier to understand, but duplicates the "em.green" selector.

$('button:has(em.green)').click(function() {
    $(this).find('em.green').html('Saving...');
});

crescentfresh's answer is also good, and doesn't need search for the em element each time. The performance impact shouldn't be noticeable though, since you probably don't have a huge tree of elements under the button.

Upvotes: 2

Parrots
Parrots

Reputation: 26902

Try this, little bit shorter:

$('button').click(function(i, button) { 
  $(button).closest('em.green').html('Saving...');
});

Saves you from having to loop through every EM and then bind the onclick. Might also help to add a class to the button so you're not binding to every button on the page, just in case there are others.

Upvotes: 4

Crescent Fresh
Crescent Fresh

Reputation: 117008

Here is a line by line translation from your prototype code. Not a lot different:

$('em.green').each(function(i, em) {
    $(em).closest('button').click(function() { 
        $(em).html('Saving...') 
    })
});

IMO the prototype version looks just as nice if not nicer (without $ sprinkled everywhere).

Upvotes: 3

Related Questions