How do I target event on variable selectors?

Question

When the drop-down box is set on "radio button" while another question has been added, all the .addrad buttons that should append input elements to their respective .q_ div only append the elements to the last .q_ div (as confirmed on line 25 of this JSFiddle). Each add button should append the cloned input elements only to the divs these buttons belong with. Should I add a variable to the selector for the click event on .addrad? What should the selector look like for this if I wouldn't need the variable? I'd appreciate any information and insights on how to go about this.

JQuery (rest of project found on JSFiddle):

$(document).ready(function () {
    var clonedForm = $('form').clone();
    var counter = 1;
    var counter1 = 1;
    var clone = $('#mp_0 div[name^="answer"]');
    var clone1 = $('#checkbox_0 div').children().clone();
    var clone2 = $('#radio_0 div').children().clone();
    var clone3 = $('#choose_0 div').children().clone();
    var newqid = $('#content form').length;

    $('.removecb').hide();
    $('.removeq').hide();
    $('.removerad').hide();
    $('.removechoose').hide();

    $(document).on("click", ".addrad", function(e) {
        var idx = $(this).siblings('div[id^="radio_"] div').find('.radio:last').length;
        var newRad = $(clone2).clone();
        counter++;

        $('div[id^="radio_"] div input[type="text"]:last').attr('name', 'radio_'+counter);

        $('.removerad').show();

        $('[id^="radio_"]:last div').append(newRad);
        e.preventDefault();
    });

    $(document).on("click", ".removerad", function(e) {

        if (counter !=1) {
            $('input[type="radio"]:last').remove();
            $('input[name^="radio_"]:last').remove(); 
            counter--;
        }
        if (counter == 1) {
        $('.removerad').hide();
        }

        if (counter < 1) {
            counter = 1;
        }
        e.preventDefault();
    });

    $(document).on("click", ".nextq", function () {
        var newForm = $(clonedForm).clone();
        counter = 1;
        counter1++;

        $('.done, .nextq, .removeAnswer, .removeq').hide();

        $('#content').append('');
        $('#content').append(newForm);
        $('[class^="q_"]:last b.qnum').empty().append(counter1 + '. ')

        $(newForm).children(".group").hide();
        $(newForm).children("#text").show();

        $('form:last').find('.select_0').attr('class', 'select_' + newqid);
        $('[class^="q_"]:last').attr('class', 'q_' + newqid);
        $('form:last').find('#mp_0').attr('id', 'mp_' + newqid);
        $('form:last').find('#checkbox_0').attr('id', 'checkbox_' + newqid);

        $('form:last').find('.done:last, .nextq:last, .removeq:last').show();
        return false;
    });

    $(document).on("click", ".removeq", function (e) {
        var newqid = $('form:last[name^="question"]:last').index() - 1;
        counter1--;

        $('hr:last').remove();
        $('form[name^="question"]:last').remove()

        $('[class^="q_"]:last input, [class^="q_"]:last textarea, [class^="q_"]:last select, [class^="q_"]:last button, [class^="q_"]:last option').prop('disabled', false);

        $('form:last').find('.done, .nextq, .removeq').show();
        newqid -1;

        $('[type="checkbox"]').prop('disabled', true);
        e.preventDefault();

        if (counter1 == 1) {
            $('.removeq').hide();
        }
    });

    $('.group').hide();
    $('#text').show();
    $(document).on("change", "select", function () {
        counter = 1;
        $(this).parent().parent().siblings('.group').hide();
        $(this).parent().parent().siblings('[id^="' + $(this).val() + '"]:last, [class^="' + $(this).val() + '"]').fadeIn();
    });
});

Pebbl · Accepted Answer

always know where your context is...

This is the age old problem of context. When building complicated interfaces that can contain repeating sections it is best to come up with a design that treats each of those sections as entire entities — creatable js/dom objects that represent and manage each new section internally — then you have one central "context" being this. That way you only need to instantiate those objects when needed, and add them to the page as a whole. You'll find it makes life much easier.

Sadly, with many JavaScript projects you don't see this. Mainly because things are constructed at the speed of thought, due to JS's highly sketchable nature. What you tend to see, is what you have got, which is a mixture of some context-based jQuery, some global-based jQuery, some id-based selectors, some global variables, and some stored dom objects. A mixture of contexts — which is a puzzle to read through, to understand and to extend.

recommendation

I would suggest that you modify your approach, and design an object to represent each section you wish to create i.e. RadioButtonSection, TextSection, MultipleChoiceSection and so on. Each object will be responsible for creating, managing and removing it's own sub-parts in a local way. By local I mean relying on class selectors, $(this), or stored DOM elements on this, rather than using ids, global jQuery selectors, global variables, or specific DOM structuring.

var RadioButtonSection = {
  create: function(){
    var obj = Object.create(this);
    obj.element = $('');
    return obj;
  },
  addRadio: function(){
    this.element.find('.radio-list').append('');
    /// ... and so on
    return this;
  },
  removeRadio: function(){
    /// ... and so on
    return this;
  },
  addToPage: function( target ){
    $(target).append( this.element );
    return this;
  },
  removeFromPage: function(){
    this.element.remove();
    return this;
  }
};

The reasons why this is better is that you compartmentalize your code, which makes it easier to read, you treat each new part of your page as a coherent object (rather than a smattering of elements) and also means you can start generalizing and reusing functionality.

For example, to create a new radio button area, you just call:

var rbs = RadioButtonSection.create();
rbs.addToPage( /* target-element-or-selector-here */ );

You can then use this object to manage your new area. Any GUI elements added to the page by this object should all work locally, so you don't have to worry about how many you create or destroy.

If you prefer the prototype approach you can use this model instead:

var RadioButtonSection = function(){
  if ( this instanceof RadioButtonSection ) {
    this.element = $('');
  }
  else {
    return new RadioButtonSection();
  }
};

RadioButtonSection.prototype.addRadio = function(){
  this.element.find('.radio-list').append('');
  /// ... and so on
  return this;
};

RadioButtonSection.prototype.removeRadio = function(){
  /// ... and so on
  return this;
};

RadioButtonSection.prototype.addToPage = function( target ){
  $(target).append( this.element );
  return this;
};

RadioButtonSection.prototype.removeFromPage = function(){
  this.element.remove();
  return this;
};

With which you would use:

var rbs = new RadioButtonSection()
  .addToPage( /* target-element-or-selector-here */ )
;

quicker option

Failing the above — as it would mean a substantial reworking — you should at the very least design your functions and event listeners to respect the current — jQuery managed — DOM context i.e. $(this) and work out from that. This way you won't run into the issues you have now.

Below is an example of rewriting your function to work using the clicked button as the context.

$(document).on("click", ".addrad", function(e) {
    counter++;
    e.preventDefault();
    var context = $(this);
    var group = context.closest('.group');
    var insert = group.find('div:first');
    var newRad = $(clone2).clone();
    newRad.find('input[type="text"]:last').attr('name', 'radio_'+counter);
    group.find('.removerad').show();
    insert.append(newRad);
});

For example, I have taken your $('.removerad') global selector and replaced it with group.find('.removerad').show(); which will only find the local remove button, rather than all of them page-wide. This is how you need to be thinking, at all times, when building complicated JavaScript UIs, local and not global.

With this "local" approach you can make things much easier by giving any wrapping

a specific grouping class, so that you don't have to rely on things like .find('div:first') as this is prone to breaking if you modify your GUI.

Until you have rewritten your methods to use local context-based thinking, you will find you have a number of other strange errors that will pop up, all because your code is mixing local and global behavior where it shouldn't be.

update

Good point with regard to the 'radio_' + counter not working. I hadn't spotted that your code elsewhere does the following:

var clone2 = $('#radio_0 div').children().clone();

This affects the newRad jQuery collection in a slightly unexpected way. .find() is designed to work from parent(s) on down to their children, but the .children() method will return a list of separate elements, broken away from their parent relationship.

So the one fix for the counter code would be to use .filter():

newRad.filter('input[type="text"]:last').attr('name', 'radio_'+counter);

But perhaps nicer would be to modify the var clone2 to use:

var clone2 = $('#radio_0 div').clone();

.. and insert the inputs along with their wrapping div, and continue to use .find(). Which leads me on to your next question.

Instead of using something like:

group.closest('[type="radio"]:last').remove()

Whenever inserting multiple elements it's always best to use the power of HTML and give them a wrapper. You can then remove them all by removing the wrapper. I would suggest that you modify the clone2 as I've stated above, give the wrapping div a class, and then target this for removal.

If you do this it does mean you also have to modify the way you are inserting the inputs, so the wrapping Divs get inserted one after the other, like so:

insert.after(newRad);

update x2

Yep, the global counter will cause you problems. In cases like these you are better off using the power of jQuery and the DOM to keep track of your counts. For example — taken from your .addrad method:

e.preventDefault();
var context = $(this);
var group = context.closest('.group');
var items = group.find('div'); // it might be best to give these DIVs a class
var insert = items.filter(':last');
var newRad = $(clone2).clone();
group.find('.removerad').show();
insert.after(newRad);
newRad.find('input[type="text"]:last').attr('name', 'radio_' + items.length);

In the above snippet of code I'm using the length of the items collection to work out the number to use for the new radio. You can do something similar for your delete functions too. The reason why this is better than a manual counter is because the DOM keeps track for you, and if some other code was — for whatever reason — to delete an item; your count would automatically be correct as it is re-calculated each time.

To make things easier I'd add a radio-items class here:

And then to find out how many radio-items there are, at any point, you can just run:

group.find('.radio-items').length