6
\$\begingroup\$

I have an HTML list. The goal is that:

  1. When an li is clicked, the .my-list--selected class will be applied. And, since the inner div also has a border-bottom, the .border-bottom class must be removed (or else it appears as though there is a 2px wide border along the bottom if both classes are applied).

  2. When another li is clicked, the previously selected li's .my-list--selected is removed and the .border-bottom class is reinstated (there can only be 1 '.my-list--selected' at a time).

I've written the below code and it works, but it seems really messy and overly complicated to me.

Is there a better way to go about achieving this goal?

$("li").click(function() {
  //Remove the my-list--selected class from any elements that already have it
  $('.my-list--selected').removeClass('my-list--selected');
  //Add the .border-bottom class back to any element that is missing it
  $('.my-list--selected').addClass('border-bottom');
  //Add the my-list--selected class to the clicked element
  $(this).addClass('my-list--selected');
  //Remove the border-bottom class from the clicked element
  $(this).find('.border-bottom').removeClass('border-bottom');
});
li div {
  padding: 0 10px;
}

.border-bottom {
  border-bottom: 1px solid #ccc;
}

.my-list--selected {
  border: 1px solid #ccc;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<ul>
  <li>
    <div class="border-bottom">Content</div>
  </li>
  <li>
    <div class="border-bottom">Content</div>
  </li>
  <li>
    <div class="border-bottom">Content</div>
  </li>
</ul>

\$\endgroup\$
  • \$\begingroup\$ Please check this demo and demo 2. Let me know which one work for you. \$\endgroup\$ – Tushar Mar 6 '17 at 8:54
  • \$\begingroup\$ @Tushar hey, yes! Demo 2 is close to what I need. I'm sorry, I should have been clearer in my question - the div has padding-left on it making its border-bottom shorter than the li around it. I thin your Demo 2 is close though and can use some of it. \$\endgroup\$ – MeltingDog Mar 6 '17 at 22:35
  • \$\begingroup\$ @MeltingDog Please do not add or change the code in your question after you have received answers, as to not invalidate them. You are welcome to ask a new question with your improvements and/or new code. Thanks! \$\endgroup\$ – Phrancis Mar 7 '17 at 2:29
3
\$\begingroup\$

Are you required to have the .border-bottom class on the div elements? I'd move those up to the li elements so you don't have to bother with the add/remove .border-bottom and you could use first-child to cleanly add a top border for the selected element if it's first (and avoid a double border for the other items). See below snippet with the changes.

$("li").click(function() {
  //Remove the my-list--selected class from any elements that already have it
  $('.my-list--selected').removeClass('my-list--selected');
  //Add the my-list--selected class to the clicked element
  $(this).addClass('my-list--selected');
});
.border-bottom {
  border-bottom: 1px solid #ccc;
}

.my-list--selected {
  border-left: 1px solid #ccc;
  border-right: 1px solid #ccc;
}

.my-list--selected:first-child {
  border-top: 1px solid #ccc;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<ul>
  <li class="border-bottom">
    <div>Content</div>
  </li>
  <li class="border-bottom">
    <div>Content</div>
  </li>
  <li class="border-bottom">
    <div>Content</div>
  </li>
</ul>

\$\endgroup\$
  • \$\begingroup\$ Yes, unfortunately I am required to have border-bottom on the divs. In the project there is a padding-left and padding-right on the div, so the border-bottom actually appears shorter than the border around the li \$\endgroup\$ – MeltingDog Mar 6 '17 at 22:14
2
\$\begingroup\$

There's no reason for the jQuery code to mess around with the border-bottom class. There is no harm in letting the selected item have more than one class simultaneously.

$("li").click(function() {
  //Remove the my-list--selected class from any elements that already have it
  $('.my-list--selected').removeClass('my-list--selected');
  //Add the my-list--selected class to the clicked element
  $(this).addClass('my-list--selected');
});
.border-bottom {
  border-bottom: 1px solid #ccc;
}

.my-list--selected {
  border: 1px solid #ccc;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<ul>
  <li>
    <div class="border-bottom">Content</div>
  </li>
  <li>
    <div class="border-bottom">Content</div>
  </li>
  <li>
    <div class="border-bottom">Content</div>
  </li>
</ul>


I echo @P.Albert's recommendation not to put class="border-bottom" on each list item — it's unnecessarily tedious. Either style all li elements, or on children of certain ul elements.

Assuming that you might have multiple lists on a page, and that selections on lists should act independently, I recommend clearing the my-list--selected class on siblings of the clicked element, rather than all elements on the page with the my-list--selected class.

$("li").click(function() {
  $(this).siblings().removeClass('my-list--selected');
  $(this).addClass('my-list--selected');
});
ul.border-bottom > li {
  border-bottom: 1px solid #ccc;
}

.my-list--selected {
  border: 1px solid #ccc;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<ul class="border-bottom">
  <li>
    <div>Content</div>
  </li>
  <li>
    <div>Content</div>
  </li>
  <li>
    <div>Content</div>
  </li>
</ul>

\$\endgroup\$
  • \$\begingroup\$ Ah yes, I do know what you mean - but the problem here is the padding on the div element. The desired effect is the bottom border is shorter than the select border, hence why it's attached to a different element. Sorry, it's my fault for not making that clear in the first place. \$\endgroup\$ – MeltingDog Mar 7 '17 at 1:48

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.