2
\$\begingroup\$

Is there a simple way to DRY this jQuery code?

  $('ul.icons li.size').click(function() {
    $('ul.icons li.size').not(this).removeClass('selected');
    $(this).toggleClass('selected');
  });

  $('ul.icons li.color').click(function() {
    $('ul.icons li.color').not(this).removeClass('selected');
    $(this).toggleClass('selected');
  });
\$\endgroup\$
1
  • 2
    \$\begingroup\$ As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$ – BCdotWEB May 20 '16 at 9:05
3
\$\begingroup\$

You need to have a group selector and just use this context to have the clicked list element:

  $('ul.icons li.size, ul.icons li.color').click(function() {
      $(this).addClass('selected')      // add to clicked li
             .siblings('li')           // get all other lis
             .removeClass('selected');  // remove the class
   });
\$\endgroup\$
2
\$\begingroup\$

Rather than .removeClass() and .toggleClass(), I suggest gathering all of the elements that need to be toggled, and calling .toggleClass() once.

It would also be beneficial to give this functionality a name. Since it seems like you want each group to have at most one item selected at any given time, I suggest radioGroup().

function radioGroup($set) {
  $set.click(function() {
    $set.filter('.selected').add(this).toggleClass('selected');
  });
}

$(function() {
  radioGroup($('ul.icons li.size'));
  radioGroup($('ul.icons li.color'));
});
.selected { background-color: black; color: white; }
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<ul class="icons">
  <li>Sizes:
    <ul>
      <li class="size">Small</li>
      <li class="size">Medium</li>
      <li class="size">Large</li>
    </ul>
  </li>
  <li>Color:
    <ul>
      <li class="color">Red</li>
      <li class="color">Green</li>
      <li class="color">Blue</li>
    </ul>
  </li>
</ul>

\$\endgroup\$
1
\$\begingroup\$

You basically want to assign the same piece of code on click on some $ selectors. Try something like this.

var selectors = ["ul.icons li.size","ul.icons li.color"];

selectors.forEach(function(item){  
  $(item).click(function() {
    $(item).not(this).removeClass('selected');
    $(this).toggleClass('selected');      
  });
});
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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