I have an online menu where I'd like to select either noodles or rice, but not both. When I select a button some CSS happens.
I tried using jquery to do this and it still allows me to select both and doesn't deselect the other.
Am I missing something glaringly obvious?
function selected(clicked) {
var ans=3 - clicked;
var noodles=$("1");
var rice=$("2");
var noodlesID=1;
var riceID=2;
switch (ans) {
case 1: document.getElementById(riceID).classList.toggle("select");
if (noodles.hasClass("select")) {
document.getElementById(noodlesID).classList.toggle("select");
}
break;
case 2: document.getElementById(noodlesID).classList.toggle("select");
if (rice.hasClass("select")) {
document.getElementById(riceID).classList.toggle("select");
}
break;
}
}
/* added by editor for demo purpose */
.selected {
border: 2px dashed red;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<button id="1" onclick="selected(this.id)">
<img src="images/plainnoodles.jpg" alt="">
<div >
Noodles
</div>
</button>
<button id="2" onclick="selected(this.id)">
<img src="images/plainrice.jpg" alt="">
<div >
Rice
</div>
</button>
CodePudding user response:
There's quite a few issues in your logic which need to be addressed:
- Don't mix native JS methods and jQuery. Stick to one or the other, otherwise you end up with some confused code which is more difficult to maintain.
- The id selectors in your JS code need to be prefixed with
#
- The class selectors in your CSS code need to be prefixed with
.
- The class name you apply in the JS is
select
yet in the CSS you define it asselected
- Don't use inline event handlers in your HTML, ie.
onclick
. This is bad practice. Use delegated event handlers, bound in JS code. You can then use thethis
keyword within the event handlers to reference the element which raised the event. - Use DOM traversal methods to relate content to each other to make the logic generic. Don't use things like
switch
or multipleif
conditions to change logic flow for multiple cases - otherwise you will have to update the JS every time the HTML changes, which is exactly what you need to avoid. - You need to
remove()
the class from the buttons which were not clicked, nottoggle()
it.
With all that said, here's a working example which caters for an infinite number of buttons:
let productButtons = document.querySelectorAll('button.productimg');
productButtons.forEach(productButton => {
productButton.addEventListener('click', e => {
let button = e.currentTarget;
productButtons.forEach(btn => btn !== button && btn.classList.remove('selected'));
button.classList.toggle('selected');
});
});
.selected { border: 2px dashed red; }
<button id="1">
<img src="images/plainnoodles.jpg" alt="">
<div >Noodles</div>
</button>
<button id="2">
<img src="images/plainrice.jpg" alt="">
<div >Rice</div>
</button>
<button id="3">
<img src="images/pizza.jpg" alt="">
<div >Pizza</div>
</button>
Note that the above is using plain JS. If you wanted to use jQuery instead, the code would look like this:
jQuery($ => {
let $productButtons = $('button.productimg').on('click', e => {
let $button = $(e.currentTarget);
$productButtons.not($button).removeClass('selected');
$button.toggleClass('selected');
});
});
CodePudding user response:
Just use the proper HTML element for this: two radio buttons.
<label><input type="radio" name="dish" value="noodles" /> Noodles</label>
<label><input type="radio" name="dish" value="rice" /> Rice</label>