2
\$\begingroup\$

I was looking into removing multiple inputs and selects at once using jQuery, but I couldn't find a solution, so I ended up with the following code:

$(function() {

  $('a.add').click(function(evt) {
    //clone lists
    //is there any way to simplify this part --begin--
    $('#s0 option').clone().appendTo('#s'+obj);
    $('#i0 option').clone().appendTo('#i'+obj);
    $('#r0 option').clone().appendTo('#r'+obj);
    $('#ieu0 option').clone().appendTo('#ieu'+obj);
    $('#ied0 option').clone().appendTo('#ied'+obj);
    //--end--
    evt.preventDefault(); //prevents scrolling
  });

  //remove inputs/selects
  $('a.remove').click(function(evt) {
    //animation is not necessary. How can I simplify this code? --begin--
    $('#comp'+x).animate({opacity:"hide"}, "slow").remove();
    $('#ppt'+x).animate({opacity:"hide"}, "slow").remove();
    $('#ct'+x).animate({opacity:"hide"}, "slow").remove();
    $('#p'+x).animate({opacity:"hide"}, "slow").remove();
    $('#ipvs'+x).animate({opacity:"hide"}, "slow").remove();
    $('#s'+x).remove();
    $('#i'+x).remove();
    $('#r'+x).remove();
    $('#ipvs'+x).remove();
    $('#other'+x).remove();
    //--end--
  } 
  evt.preventDefault();
  });
});

How can the sections marked between --begin-- and --end-- be simplified/improved?

\$\endgroup\$
4
  • \$\begingroup\$ .each() \$\endgroup\$
    – zzzzBov
    Commented Aug 21, 2012 at 18:10
  • 1
    \$\begingroup\$ It would be hard to know how to best do this without seeing the HTML structure. \$\endgroup\$
    – Mike Brant
    Commented Aug 21, 2012 at 18:12
  • 2
    \$\begingroup\$ Just a hint, you can select elements using , to separate them. \$\endgroup\$
    – Ricardo Lohmann
    Commented Aug 21, 2012 at 18:12
  • \$\begingroup\$ Check this fiddle, it might help you. \$\endgroup\$
    – guy mograbi
    Commented Aug 21, 2012 at 19:50

2 Answers 2

3
\$\begingroup\$

I would go with ChrisThompson's method of adding classes and referring to them. But to answer your question straight and simple

the first block can look like

$.each( ['s','i','r','ieu','ied'], function(index,item){
        $('#' + item + '0 option').clone().appendTo('#' + item + obj );
    });

However, you did not specify what obj is

The second block can use the same technic to be simplified.

The entire code would look something like this

$(function() {
  $('a.add').click(function(evt) {
    //clone lists
    //is there any way to simplify this part --begin--
    $.each( ['s','i','r','ieu','ied'], function(index,item){
        $('#' + item + '0 option').clons().appendTo('#' + item + obj );
    });
    //--end--
    evt.preventDefault(); //prevents scrolling
  });

  //remove inputs/selects
  $('a.remove').click(function(evt) {
    //animation is not necessary. How can I simplify this code? --begin--
    $.each( ['comp','ppt','ct','p','ipvs'], function(index,item){ $('#' + item + x).animate({opacity:"hide"} ,"slow").remove();});
    $.each(['s','i','r','other'], function(index,item){ $('#'+item+x).remove()});
    //--end--
    evt.preventDefault();
  } 
  });
});

However, it seems to me you are using too complex HTML structure. you need to

  1. Stop using IDs, and start using classes for elements that have things in common For example :

  2. Instead of cloning each item, you can wrap all of the in a DIV and simple use html() to clone the entire HTML. When you inject the HTML to some other div, give the new wrapper div an ID.

For example

$(".my-form").append($("<div/>").addClass(".object-item").attr("id","newObject" + $(".my-form .object-item").length ).html( $(".items-to-clone-wrapper").html() );

You then have a unique identifier for every element on the page. for example $("#newObject1 .s") instead of $("#s1").

Here is a fiddle I made on the assumptions I made about your code. I hope it will be of use to you. please let me know if there's something I can do.

\$\endgroup\$
4
  • \$\begingroup\$ one though - I wouldn't necessarily use jQuery's each() function, as it tends to be slower (sometimes much slower) than JavaScript's native for statement \$\endgroup\$ Commented Aug 22, 2012 at 14:02
  • \$\begingroup\$ ok. then simply replace the each with a native for loop. - the answer still stands, it will still make the code simpler. \$\endgroup\$ Commented Aug 22, 2012 at 15:08
  • \$\begingroup\$ yes, I agree... I've also removed my answer, seeing you already have it in yours now :) \$\endgroup\$ Commented Aug 22, 2012 at 20:44
  • \$\begingroup\$ Exactly what I was looking for, thank you so much to everyone! I did put the cloned items inside a div tag as suggested, and to remove the whole thing I just used: <code> $('a.remove').click(function(evt) { if(i > 0) { $('#clonedItems'+i).remove(); i-- ; } evt.preventDefault(); }); </code> Not sure how to use the code block... I'm a noob here. Anyways, thank you all! \$\endgroup\$
    – R.D.
    Commented Aug 28, 2012 at 16:49
0
\$\begingroup\$

for the top part you might be able to add some logic to your html. it looks when something is clicked you move the options to another element. if you just specified in the HTML the source and target elements through a class name like: <select class="source" target="targetSelector"... and <select class="target"...

you could use a script like

$('a.add').click(function(){
    $('.source').each(function(){
        var source = $(this);
        var target = $(source.attr('target'));
        target.append(source.find('option'))
    })
})

Not really more simplified but a way to abstract your logic a bit.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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