views:

183

answers:

5

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!

+3  A: 

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).

Crescent Fresh
+3  A: 

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.

Parrots
From the sounds of the question the `em` is a child of the `button`, and `closest()` looks *up* the tree, not down.
Crescent Fresh
If the em is a child of the button then you could just substitute children('em.green') for closest('em.green').
Parrots
+2  A: 

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.

Matthew Crumley
+1  A: 

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...')
});
redsquare
A: 

(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.

altCognito