Since I'm still learning jQuery, I would like to have some feedback on my code:
var showImgage = function()
{
var selection = $(this).val();
switch(selection)
{
case 'Monica Bellud':
$('#Belluci').removeClass().addClass('show');
$('#Andres').removeClass().addClass('hide');
$('#Seydoux').removeClass().addClass('hide');
break
case 'Lea Seydoux':
$('#Seydoux').removeClass().addClass('show');
$('#Belluci').removeClass().addClass('hide');
$('#Andres').removeClass().addClass('hide');
break
case 'Ursula Andress':
$('#Andres').removeClass().addClass('show');
$('#Belluci').removeClass().addClass('hide');
$('#Seydoux').removeClass().addClass('hide');
break
default:
selection.removeClass();
}
}
<div>
<label for = "bondgirl">Wie van de onderstaande dames is géén Bond Girl in de nieuwe film 'SPECTRE'?</label>
<input type="radio" value="Monica Bellud" name="girlchoice" id="choicemonica" checked /><label for="choicemonica" class="sidelabel">Monica Bellud</label>
<input type="radio" value="Lea Seydoux" name="girlchoice" id="choicelea" /><label for="choicelea" class="sidelabel">Lea Seydoux</label>
<input type="radio" value="Ursula Andress" name="girlchoice" id="choiceursula" /><label for="choiceursula" class="sidelabel">Ursula Andress</label>
</div>
I simply want to change the image (which is in my div
of my HTML) for each radiobutton. It works, but it looks like I'm duplicating my code 3 times. How can I make this easier?
2 Answers 2
I would probably add a class/something to use as a selector (in this case I added girls
) to make lookup easier. You can do this in other ways such as getting the values of the actors map so do it as you will.
Also your default
case is calling removeClass
on a string as selection
is the current contents of whatever the event is hooked to.
// Actor name -> the corresponding element selector on the page
var actorsIdMap = {
'Monica Bellud': '#Bellud',
'Lea Seydoux': '#Seydoux',
'Ursula Andress': '#Andres'
}
var showImage = function() {
var selection = $(this).val();
if (selection in actorsIdMap) {
// Hide the other actors
$('.girls').removeClass('hide show');
$(actorsIdMap[selection]).addClass('show');
} else {
// Default case what do?
}
}
The first thing that you can do will reduce the code written but slightly increase the number of images that you hide. Before the switch, hide all the images, like so:
$('#Belluci').removeClass().addClass('hide');
$('#Seydoux').removeClass().addClass('hide');
$('#Andres').removeClass().addClass('hide');
Or
$('input[name=girlchoice].show').removeClass('show').addClass('hide');
Consider changing the value of the radio buttons like so:
<input type="radio" value="Belluci" name="girlchoice" id="choicemonica" checked /><label for="choicemonica" class="sidelabel">Monica Bellud</label>
<input type="radio" value="Seydoux" name="girlchoice" id="choicelea" /><label for="choicelea" class="sidelabel">Lea Seydoux</label>
<input type="radio" value="Andres" name="girlchoice" id="choiceursula" /><label for="choiceursula" class="sidelabel">Ursula Andress</label>
Then you can just say
$('#' . selection).removeClass().addClass('show');
So the whole function would be
var showImgage = function()
{
var selection = $(this).val();
$('input[name=girlchoice].show').removeClass('show').addClass('hide');
$('#' . selection).removeClass('hide').addClass('show');
}
You lose the default
case, but I'm not sure that you really needed it.
Note: it might be
$('input.show[name=girlchoice]').removeClass('show').addClass('hide');
instead. I'm not set up for testing at the moment.
-
\$\begingroup\$ Doesn't work for me :o \$\endgroup\$Awa– Awa2015年01月16日 14:51:36 +00:00Commented Jan 16, 2015 at 14:51