jQuery and Ajax comment edit system

[Fmdpa]

I am trying to implement an asynchronous comment edit system. It works just fine as far as the main back-end goes, but if you update the comment more than once without refreshing the page, I get some display issues...well, the comment actually doesn't display at all.

$('img#post_edit').click(function() {		$(this).parents('li.c_post').children('p').slideUp().next('textarea.update_comment').fadeIn().focus()			 .next('p#complete_edit').slideToggle(1000);		$(this).parents('li.c_post').attr('id', 'edit_status');		$("p#complete_edit").slideDown().text('Click outside the textbox to complete the edit.');				$(this).parents('li.c_post').children('textarea.update_comment').change(function() {		var id = $(this).parents('li.c_post').find('img#post_edit').attr('class');		var post = $(this).val();		$.post(			'/update_comment.php', {				'id' : id,				'post' : post			},				function(html){					$('li#edit_status textarea').slideUp();					$('li#edit_status p.comment_post').slideDown().html(html);					$("p#complete_edit").text('Comment updated successfully!');				});				}	   );});

It works perfectly the first time you edit the comment, but if you edit the comment more than once without refreshing the page, the textbox will slide up (like it should), but it will leave you with an blank space (where the edited comment should be displayed). I couldn't figure out what was going until I looked in firebug. I noticed that each time I made the update, it posted as many times as that comment had been updated without a page refresh.So if I had edited it 3 times, it would make 3 POSTs at the same time, the fourth time: 4 POST requests, and so on.How would I fix this?

[justsomeguy]

Every time that click event runs you're adding another change event to send another post request. You need to assign that change event once, not each time you click.

[Fmdpa]

How would I do that?Should I take the post edit function out of the function that displays the textbox?

$('img#post_edit').click(function() {		$(this).parents('li.c_post').children('p').slideUp().next('textarea.update_comment').fadeIn().focus()			 .next('p#complete_edit').slideToggle(1000);		$(this).parents('li.c_post').attr('id', 'edit_status');		$("p#complete_edit").slideDown().text('Click outside the textbox to complete the edit.');});				$(this).parents('li.c_post').children('textarea.update_comment').change(function() {		var id = $(this).parents('li.c_post').find('img#post_edit').attr('class');		var post = $(this).val();		$.post(			'/update_comment.php', {				'id' : id,				'post' : post			},				function(html){					$('li#edit_status textarea').slideUp();					$('li#edit_status p.comment_post').slideDown().html(html);					$("p#complete_edit").text('Comment updated successfully!');				});				}	   );

[justsomeguy]

You have 2 event handlers - one that fires when a certain element is clicked, and the other that fires when the children of a certain element are changed. You only need to assign each handler once, the way you had it before was that the first handler was assigning the second one, so every time the first one fired it would add another event handler to the same element, so it would keep stacking the handlers.If you're not sure which parts of the code are the event handlers, that's what you need to figure out first. With event-driven programming, you only need to assign any given handler to any given element once.

[Fmdpa]

I understand what you are saying, but I'm not sure how to fix it. How's this?

$('img#post_edit').click(function() {		$(this).parents('li.c_post').children('p').slideUp().next('textarea.update_comment').fadeIn().focus()			 .next('p#complete_edit').slideToggle(1000);		$(this).parents('li.c_post').attr('id', 'edit_status');		$("p#complete_edit").slideDown().text('Click outside the textbox to complete the edit.');				$('textarea.update_comment').change(function() {		var id = $(this).parents('li.c_post').find('img#post_edit').attr('class');		var post = $(this).val();		$.post(			'/update_comment.php', {				'id' : id,				'post' : post			},				function(html){					$('li#edit_status textarea').slideUp();					$('li#edit_status p.comment_post').slideDown().html(html);					$("p#complete_edit").text('Comment updated successfully!');				});				}	   );});

[justsomeguy]

That looks a lot like what you started with, it looks like you're just changing the target for the change handler, but it still gets attached by the click handler. You need to move the entire change handler function out of the click handler function.It looks to me like you've also got a misplaced bracket, I think the bracket that ends the change handler is in the wrong place. It looks like it's before the end of the post function.Moving the change handler out of the click handler will change the scope of it, so if that is relying on being inside the scope it's in now, then instead of moving it out you probably need to check if a change handler has already been defined and don't define another one if so. If necessary you can use a global variable to keep track of whether or not that handler has already been assigned.

[Fmdpa]

There...it works for me. Do you see any other problems?

$('img#post_edit').click(function() {		$(this).parents('li.c_post').children('p').slideUp().next('textarea.update_comment').fadeIn().focus().next('p#complete_edit').slideToggle(1000);		$(this).parents('li.c_post').attr('id', 'edit_status');		$("p#complete_edit").slideDown().text('Click outside the textbox to complete the edit.'); });		$('textarea.update_comment').change(function() {		var id = $(this).parents('li.c_post').find('img#post_edit').attr('class');		var post = $(this).val();		$.post(			'/update_comment.php', {				'id' : id,				'post' : post			},				function(html){					$('li#edit_status textarea').slideUp();					$('li#edit_status p.comment_post').slideDown().text(html);					$("p#complete_edit").text('Comment updated successfully!');				}	   );});

[Fmdpa]

BTW, thanks for your help, JSG. I appreciate you making me work for my solution. It sticks in my head better.

[justsomeguy]

No problem. I would also say to be consistent with your indenting, consistently indented code is easier to read:

$('img#post_edit').click(function() {  $(this).parents('li.c_post').children('p').slideUp().next('textarea.update_comment').fadeIn().focus().next('p#complete_edit').slideToggle(1000);  $(this).parents('li.c_post').attr('id', 'edit_status');  $("p#complete_edit").slideDown().text('Click outside the textbox to complete the edit.');});$('textarea.update_comment').change(function() {  var id = $(this).parents('li.c_post').find('img#post_edit').attr('class');  var post = $(this).val();    $.post(	'/update_comment.php',	{	  'id' : id,	  'post' : post	},	function(html){	  $('li#edit_status textarea').slideUp();	  $('li#edit_status p.comment_post').slideDown().text(html);	  $("p#complete_edit").text('Comment updated successfully!');	}  );});

When it's indented like that I think it's more obvious that you're defining a function for 2 events and what each one does.

[Fmdpa]

The thing I don't like about posting code on here is that I can't use TABs to indent it. So I just have to use spaces, which doesn't work as well.

[Synook]

You can always just write it in some other editor, then copy-and-paste it.

[Fmdpa]

That's what I usually end up doing.