skip to Main Content

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 class="productimg" id="1" onclick="selected(this.id)">
<img src="images/plainnoodles.jpg" alt="">
<div class="productName">
Noodles
</div>
</button>

<button class="productimg" id="2" onclick="selected(this.id)">
<img src="images/plainrice.jpg" alt="">
<div class="productName">
Rice
</div>
</button>

2

Answers


  1. 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 as selected
    • 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 the this 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 multiple if 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, not toggle() 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 class="productimg" id="1">
      <img src="images/plainnoodles.jpg" alt="">
      <div class="productName">Noodles</div>
    </button>
    
    <button class="productimg" id="2">
      <img src="images/plainrice.jpg" alt="">
      <div class="productName">Rice</div>
    </button>
    
    <button class="productimg" id="3">
      <img src="images/pizza.jpg" alt="">
      <div class="productName">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');
      });
    });
    
    Login or Signup to reply.
  2. 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>
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search