0

I have an image, and when I click on it I want it to change to a different image and change its ID as well. Then when I click on this new image, it reverts back.

$(document).ready(function(){
    $("#name_edit").click(function(){
        $(this).attr("src", "img/tick.png");
        $(this).attr("id","name_confirm");
    });
    $("#name_confirm").click(function(){
        $(this).attr("src", "img/edit.png");
        $(this).attr("id","name_edit");
    });
});

I have successfully done the first step, going from #name_edit to #name_confirm. However, not the reverse.

How do I go about solving this?

My suspicion is that since I'm using (document).ready, jQuery is preparing itself for elements already on the page. However, the element with the ID name_confirm does not exist until the image is clicked on.

Thanks.

Jack Paul
  • 219
  • 2
  • 10
  • I think Your suspicion is correct. Place both bindings, one for "name_edit" and one for "name_confirm" in separate functions (call them init_name_edit, for example). Call these functions at the end of .click function. – Krzysztof Witczak Apr 09 '16 at 10:11

5 Answers5

4

The element that you are working on is always the same...

$(document).ready(function(){

    // use just the first id value to find it in the DOM
    $("#name_edit").click(function(){
      var item = $(this);
      var id = item.attr('id');
      
      if(id === 'name_edit') {
        return item
          .attr("src", "img/tick.png")
          .attr("id","name_confirm")
        ;
      }
    
      
      return item
        .attr("src", "img/edit.png")
        .attr("id","name_edit")
      ;
    })
    ;
});
Hitmands
  • 13,491
  • 4
  • 34
  • 69
  • This worked, thanks a lot. And yes, I did make a small mistake in my title, the element itself is the same, only thing changing is the ID. Cheers. – Jack Paul Apr 09 '16 at 10:19
1

I think you have chosen bad solution for your problem. 1) Why your code doesn't work: You bind 2 events only 1 time, whne your document loaded. So, jquery finds #name_edit element and bind onclick event on it. But jquery cannot find #name_confirm element, because it doesn't exists on document ready) In your code you should bind 1 onclick event, but have some attr (for example class for checking your state). Something like:

 <img id="main_image" class="name_edit"/>
 <script>
 var img_paths = ["img/tick.png", "img/edit.png"]
 var img_index = 0;
 $("#main_image").click(function(){
    if($(this).attr("class") == "name_edit"){
       $(this).attr("src", "img/tick.png");
       $(this).attr("class","name_confirm"); 
    }
    else{   
       $(this).attr("src", "img/edit.png");
       $(this).attr("class","name_edit");
    }
 });
 </script>
  1. Other solutions: You can create 2 images and show/hide them. Or use styles with background attr. With pseudoclasses or classes. Also you can store image pathes in array and tick array index on click.

Something like:

 var img_paths = ["/content/img1.png", "/content/img2.png"]
 var img_index = 0;
 $("#main_image").click(function(){
    $(this).src = img_paths[img_index];
    img_index = !img_index;
 })
SirCapy
  • 295
  • 1
  • 12
0

It is not working because you are referencing the same elements, try this:

(function(window, document, $, undefined){
  $("#name_edit").on("click", function(){
    var self = $(this);
    if(self.attr("id") === "name_edit") {
      self.attr("src", "img/tick.png");
      self.attr("id", "name_confirm");
    } else {
      self.attr("src", "img/edit.png");
      self.attr("id", "name_edit");
    }
  });
})(this, this.document, jQuery);

Also for easier to understand code you could use classes like this:

(function(window, document, $, undefined){
  $(".name_edit").on("click", function(){
    var self = $(this);
    if(self.hasClass("name_edit")) {
      self.attr("src", "img/tick.png");
      self.removeClass("name_edit").addClass("name_confirm");
    } else {
      self.attr("src", "img/edit.png");
      self.removeClass("name_confirm").addClass("name_edit");
    }
  });
})(this, this.document, jQuery);

To simplify replacing classes you could even add your own $.fn.replaceClass(); like this:

jQuery.fn.replaceClass = function(classA, classB) {
  this.removeClass(classA).addClass(classB);
  return this;
};

Then use it like this:

(function(window, document, $, undefined){
  $(".name_edit").on("click", function(){
    var self = $(this);
    if(self.hasClass("name_edit")) {
      self.attr("src", "img/tick.png");
      self.replaceClass("name_edit", "name_confirm");
    } else {
      self.attr("src", "img/edit.png");
      self.replaceClass("name_confirm", "name_edit");
    }
  });
})(this, this.document, jQuery);
0

I can confirm what the others said.. the jquery gets run on document ready, but doesn't get updated subsequently - so it basically gets the correct element from the dom, and assigns the click event. It has no event for the name_confirm.

so this code does nothing...

$("#name_confirm").click(function(){
    $(this).attr("src", "img/edit.png");
    $(this).attr("id","name_edit");
});

See it not work in this instructive jsfiddle

Of course does the id need to change? is it possible to use for example a specific class for the img? then you could make the second click bind on the class instead... for example see this working example, which still changes the src and id...

Steinwolfe
  • 238
  • 2
  • 6
-2

Try On method:

$(document).on('click', '#name_edit', function(){
    $(this).attr("src", "img/tick.png");
    $(this).attr("id","name_confirm");
});
$(document).on('click', '#name_confirm', function(){
    $(this).attr("src", "img/edit.png");
    $(this).attr("id","name_edit");
});
Faisal Ashfaq
  • 2,545
  • 4
  • 28
  • 41
  • 2
    The problem is that `$('#name_confirm')` doesn't exists yet when you try to bind the click event. (`.on('click')` and `.click()` are the same functions, the second one is just an alias). – Hitmands Apr 09 '16 at 10:14
  • Nope, they're not the same functions. `on` binds events of the future elements as well which is the case here. It was previously named as `live` which is deprecated now. http://stackoverflow.com/questions/8191064/jquery-on-function-for-future-elements-as-live-is-deprecated – Faisal Ashfaq Apr 09 '16 at 10:17
  • you are misunderstanding it with the event delegation that you can obtain using a `filter selector` but you need to bind with its container and not with the element itself. `$('#name_confirm')` returns an empty jQuery collection. – Hitmands Apr 09 '16 at 10:19
  • your last edit makes more sense but isn't good in term of performances because you need to delegate the click event to the whole document object. Plus, if there's some `event.stopPropagation()` in the propagation chain you lost that event. Event delegation should be used only when you can't do something in another and more performant way. – Hitmands Apr 09 '16 at 10:25