João Gonçalves
João Gonçalves

Reputation: 4002

Jquery event being called multiple times

So I have a form to submit photos (to a total of 8), and I'm trying to apply a small effect: once you choose a photo, the button hides and the file name is displayed along with a 'X' to remove its selection.

However, when I add multiple photos and try to remove one, the event gets called multiple times, and the more I click, more multiple events are fired, all from the same element.

Can anyone figure it out?

var Upload =  {
    init: function ( config ) {
        this.config = config;
        this.bindEvents();
        this.counter = 1;
    },

    /**
     * Binds all events triggered by the user.
     */
    bindEvents: function () {
        this.config.photoContainer.children('li').children('input[name=images]').off();
        this.config.photoContainer.children('li').children('input[name=images]').on("change", this.photoAdded);
        this.config.photoContainer.children('li').children('p').children('a.removePhoto').on('click', this.removePhoto);
    },

    /**
     * Called when a new photo is selected in the input.
     */
    photoAdded: function ( evt ) {
        var self = Upload,
            file = this.files[0];
        $(this).hide();
        $(this).parent().append('<p class="photo" style="background-color: gray; color: white;">' + file.name + ' <a class="removePhoto" style="color: red;" href="#">X</a></p>');

        if(self.counter < 8) {  // Adds another button if needed.
            Upload.config.photoContainer.append( '<li><input type="file" name="images"></li>');
            self.counter++;
        } 
        Upload.bindEvents();
    },

    /**
     * Removes the <li> from the list.
     */
    removePhoto: function ( evt ) {
        var self = Upload;
        evt.preventDefault();

        $(this).off();
        $(this).parent().parent().remove();

        if(self.counter == 8) { // Adds a new input, if necessary.
            Upload.config.photoContainer.append( '<li><input type="file" name="images"></li>');
        }
        self.counter--;
        Upload.bindEvents();
    }
}

Upload.init({
    photoContainer: $('ul#photo-upload')
});

Upvotes: 0

Views: 837

Answers (2)

Laurent Perrin
Laurent Perrin

Reputation: 14881

From what I see, you are trying to attach/remove event handlers based on what the user selects. This is inefficient and prone to errors.

In your case, you are calling Upload.bindEvents() each time a photo is added, without cleaning all the previous handlers. You could probably debug until you don't leak event listeners anymore, but it's not worth it.

jQuery.on is very powerful and allows you to attach handlers to elements that are not yet in the DOM. You should be able to do something like this:

init: function ( config ) {
  this.config = config;
  this.counter = 1;
  this.config.photoContainer.on('change', 'li > input[name=images]', this.photoAdded);
  this.config.photoContainer.on('click', 'li > p > a.removePhoto', this.removePhoto);
},

You attach just one handler to photoContainer, which will catch all events bubbling up from the children, regardless of when they were added. If you want to disable the handler on one of the elements, you just need to remove the removePhoto class (so that it doesn't match the filter).

Upvotes: 2

obvdso
obvdso

Reputation: 92

You are doing a lot of: Upload.bindEvents();

You need to unbind events for those 'li's before you bind them again. Otherwise, you add more click events. That's why you are seeing more and more clicks being fired.

Upvotes: 2

Related Questions