Playing with radio input

Question

I think my code has too many functions... can it be written better?

Please help reviewing my jQuery functions.

HTML

<div class="event_owner">
    <p>
        <label for="xxx">xxx</label>
        <input type="radio" name="event_owner" id="xxx">
    </p>

    <p>
        <label for="yyy">yyy</label>
        <input type="radio" name="event_owner" id="yyy">
    </p>
<input type="text" name="_event_owner[name]" autocomplete="off" value="">
</div>

jQuery

$(".event_owner").click(function(){
    $(".event_owner input[type='text']").val($('.event_owner :radio:checked').attr("id"));
    $('.event_owner p').css({"opacity":"1"});
    $('.event_owner input').not(":checked").parent("p").css({"opacity":"0.3"});
});

// when page loaded and input(radio) saved

if($(".event_owner input[type='text']").length) {
    $("input[id="+$(".event_owner input[type='text']").val()+"]").attr("checked","checked");
    $('.event_owner input').not(":checked").parent("p").css({"opacity":"0.3"});

}

Playground

http://jsfiddle.net/Zf4UE/

Answer 1

There are a few points worth mentioning:

1. Your code does not actually function the way you want it to.
   $(".event_owner input[type='text']").length checks for the existence of that element, not whether its value is set.

2. Your radio buttons don't have a value attribute on them. They really should.

3. You should cache your selectors. You're querying for .event_owner a total of 7 times in your code, and an additional 4 times on every click.

4. Instead of listening for every click inside .event_owner, you should instead only listen for the change event of the radio buttons.

5. On page load, if there's a value in the input text field, all we have to do is check the correct radio button. The change event listener will take care of the rest.

With the above in mind, here's some sample code you might consider:

var $wrapper = $('.event_owner'),
    $textInput = $wrapper.find('input[type="text"]'),
    $radios = $wrapper.find('input[type="radio"]'),
    owner = $textInput.val();

$radios.on( 'change', function () {
    $textInput.val( $radios.filter(':checked').val() );
    $radios.each(function () {
        $(this).closest('p').css( 'opacity', this.checked ? 1 : .3 );
    });
});

if ( owner ) $radios.filter('#' + owner).prop('checked', true);

Here's the fiddle: http://jsfiddle.net/Zf4UE/8/