Is there any way to improve my sort of code? I'm trying to add / remove classes of active or check in a menu item. My menus are like steps with a prev and next button.

My only goal here is to reduce and improve the jQuery code.

Here's my code:

HTML

<nav class="test-sidebar">

    <ul class="menus list-unstyled components">
        <li class="active">
            <a href="#" data-target="test_1"><span>Test 1</span></a>
        </li>
        <li>
            <a href="#" data-target="test_2"></i><span>Test 2</span></a>
        </li>
        <li>
            <a href="#" data-target="test_3"><span>Test 3</span></a>
        </li>
        <li>
            <a href="#" data-target="test_4"><span>Test 4</span></a>
        </li>
        <li>
            <a href="#" data-target="test_5"><span>Test 5</span></a>
        </li>
        <li>
            <a href="#" data-target="test_6"><span>Test 6</span></a>
        </li>
        <li>
            <a href="#" data-target="test_7"><span>Test 7</span></a>
        </li>
    </ul>
</nav>

<button data-target="test_2" class="prev-page-btn w-100">Back</button>

JS

$(document).ready(function () {
    // load the function
    prevPageButtons();
  });

// function for Previous buttons
function prevPageButtons() {
    // setting target from data attributes
    var target, container;
  
    $("body").on("click", ".prev-page-btn", function (e) {
      target = $(this).attr('data-target'),
        container = $('#get_content');
  
      container.load(target   '.php');
  
      if (target == "test_1") {
        $(".test-sidebar ul li:first-child").removeClass('check').addClass('active');
        $(".test-sidebar ul li:nth-child(2)").removeClass('active');
      }
  
      if (target == "test_2") {
        $(".test-sidebar ul li:nth-child(2)").removeClass('check').addClass('active');
        $(".test-sidebar ul li:nth-child(3)").removeClass('active');
      }
  
      if (target == "test_3") {
        $(".test-sidebar ul li:nth-child(3)").removeClass('check').addClass('active');
        $(".test-sidebar ul li:nth-child(4)").removeClass('active');
      }
  
      if (target == "test_4") {
        $(".test-sidebar ul li:nth-child(4)").removeClass('check').addClass('active');
        $(".test-sidebar ul li:nth-child(5)").removeClass('active');
      }
  
      if (target == "test_5") {
        $(".test-sidebar ul li:nth-child(5)").removeClass('check').addClass('active');
        $(".test-sidebar ul li:nth-child(6)").removeClass('active');
      }
  
      if (target == "test_6") {
        $(".test-sidebar ul li:nth-child(6)").removeClass('check').addClass('active');
        $(".test-sidebar ul li:nth-child(7)").removeClass('active');
      }
  
    });
  }

I'm trying to reduce the nth-childs / by not using nth-childs of my menu items in my jQuery. Is there anyway to achieve this by not using nth-childs for removing and adding classes. I'm thinking of using prevAll() function or any sorts, but how do I achieve it?

Just want to focus on improving the jQuery.

CodePudding user response：

From the data-target, you can get to the element to add active to, then use .next to get the next sibling.

It would also be good to use a different data attribute - don't use the same one for the button and the <a>s. Maybe call the <a>s the data-name instead, eg

<a href="#" data-name="test_1"><span>Test 1</span></a>

Then, replace all of the

if (target == "test_1") {

with only:

$("body").on("click", ".prev-page-btn", function(e) {
  const target = $(this).attr('data-target');
  const container = $('#get_content');
  const li = $(`[data-name="${target}"]`).parent();
  li.removeClass('check').addClass('active');
  li.next().removeClass('check');
});

Live demo:

$("body").on("click", ".prev-page-btn", function(e) {
  const target = $(this).attr('data-target');
  const container = $('#get_content');
  const li = $(`[data-name="${target}"]`).parent();
  li.removeClass('check').addClass('active');
  li.next().removeClass('check');
});

.active {
  background-color: yellow;
}
.check {
  background-color: orange;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<nav class="test-sidebar">

  <ul class="menus list-unstyled components">
    <li>
      <a href="#" data-name="test_1"><span>Test 1</span></a>
    </li>
    <li class="check">
      <a href="#" data-name="test_2"></i><span>Test 2</span></a>
    </li>
    <li class="check">
      <a href="#" data-name="test_3"><span>Test 3</span></a>
    </li>
    <li>
      <a href="#" data-name="test_4"><span>Test 4</span></a>
    </li>
    <li>
      <a href="#" data-name="test_5"><span>Test 5</span></a>
    </li>
    <li>
      <a href="#" data-name="test_6"><span>Test 6</span></a>
    </li>
    <li>
      <a href="#" data-name="test_7"><span>Test 7</span></a>
    </li>
  </ul>
</nav>

<button data-target="test_2" class="prev-page-btn w-100">Back</button>

<iframe name="sif1" sandbox="allow-forms allow-modals allow-scripts" frameborder="0"></iframe>

Depending on how the <button>s are arranged, you might be able to get rid of the data attributes altogether and instead use the index of the button clicked to navigate to the same indexed <li>.