0

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')
});
3
  • Does it increment by 1 each time you click the button to fire the event?
    – Alfabravo
    Commented Aug 8, 2012 at 16:33
  • Seems that in bindEvents method your'e binding events not to the one element, which needs to be changed, but to all elements. Maybe that causes an issue.
    – tijs
    Commented Aug 8, 2012 at 16:38
  • It does increment by 1, and also the events are being fired for the same element over and over again. Thank you anyway Commented Aug 8, 2012 at 16:58

2 Answers 2

2

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

1
  • Thanks for your answer, this solves the issue and improves the code. Commented Aug 8, 2012 at 16:59
2

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.

1
  • Thanks for pointing that out, it does solve the issue. I forgot to add that one and added only to the input. Commented Aug 8, 2012 at 16:59

Not the answer you're looking for? Browse other questions tagged or ask your own question.