Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Let's straight to the point, I'm new in jQuery and played a bit with what can I do with radio button.

I want to create ONLY one input type when I click the "Notice" radio button, I already using many way to achieve this, but this is the only way I can do this, you can see the following Code...

HTML

<input type="radio" name="radio1" value="Immediate" /> Immediate
<br/>
<input type="radio" name="radio1" id="others" value="Notice" /> Notice me in...
<br />
<div id="inputplace"></div>

jQuery

$(function(){
    $("#others").click(function(){
        while($("#others").val()=="Notice")
        {
            var newElement = $(document.createElement('input')).attr({name:"noticeweeks", id:"noticeweeks"});   
            newElement.appendTo("#inputplace");

            $("#others").val("Notice me in...");
        }
    });
});

So... with that jQuery I changed the value of the radio button into "Notice me in...", so the while function will stop creating a new element since the value has been changed.

Is there any others way to achieve the same result? Thanks a bunch!

share|improve this question
add comment

2 Answers

  • There's no need to use a while loop, as you only want to add the element once, after which the condition won't hold anymore. That's what if is for.

  • There's a shorter way to create a new element

So I get to:

$(function() {
    $("#others").click(function() {
         if ($("#others").val() == "Notice") {
            var newElement = $('<input>')).attr({name:"noticeweeks", id:"noticeweeks"});   
            newElement.appendTo("#inputplace");    
            $("#others").val("Notice me in...");
        }
    });
});
share|improve this answer
    
Ah yeah, actually I already changed it to if already and it works, than you anyway. –  Xtrader Dec 9 '12 at 17:34
add comment

You can go about the "only once" part in a couple of ways:

  1. Remove the event-listener once the new input has been added
  2. Always have the extra input, but have it hidden, and show it when necessary
  3. Have a closure variable that holds the input; if it's defined, don't redefine it

And more. I'd personally go with #2, because it means that your inputs are actually in the markup, while the javascript only defines behavior. That is, keep elements in the markup, style and layout in CSS, and behavior in JS - don't mix it

In other words:

<input type="radio" name="radio1" value="Immediate" /> Immediate
<br/>
<input type="radio" name="radio1" id="others" value="Notice" /> Notice me in...
<br />
<!-- extra input, just hidden -->
<input type="text" name="noticeweeks" id="noticeweeks" value="" style="display: none" />

and the JS:

$(function(){
  // cache these!
  var radioButton = $("#others"),
      noticeWeeks = $("#noticeweeks");

  radioButton.change(function () { // listen for change - not click
    if( this.checked ) { // use the "raw" DOM property `checked`
      noticeWeeks.show();
    }
  });
});

Demo

Of course, I get the feeling, that what you really want is to show the extra input, when "Notice me in..." is selected, and hide it when "Immediate" is selected.

In that case, the JS could be

$(function(){
  // cache these!
  var radioButtons = $("[name=radio1]"), // get both radiobuttons
      noticeWeeks  = $("#noticeweeks");

  radioButtons.change(function () {
    if( $(this).val() === "Notice" ) { // this time, we do need to check the value
      noticeWeeks.show();
    } else {
      noticeWeeks.hide();
    }
  });
});

Demo

share|improve this answer
    
Ah no, actually if I select to Immediate using the JS I will remove it using .remove() But anyway, thanks a bunch I will try your way. –  Xtrader Dec 9 '12 at 17:35
    
@Xtrader In that case, I'd use noticeWeeks.prop('disabled', false).show(); and noticeWeeks.prop('disabled', true).hide(); to show/hide and enable/disable the input. That way it won't be visible or submitted to the server when it's "turned off". Again, keep you markup in your markup; don't add and remove trivial stuff like this with JS. –  Flambino Dec 9 '12 at 20:05
    
Okay, thanks a bunch! –  Xtrader Dec 10 '12 at 9:08
    
Ah sorry, if I use prop will it creating an memory leak in IE 9? Because the documentation said so, should I use the .data() method? –  Xtrader Dec 10 '12 at 16:07
    
@Xtrader No. disabled is a standard HTML attribute you're setting on the element, so data won't work. Use .prop() or .attr(). And read the documentation again: The memory leak only happens in IE below version 9, and it doesn't happen at all for a boolean values, which is what this is. –  Flambino Dec 10 '12 at 17:17
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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