[RESOLVED] Improving my jQuery Code

Dear JS Ninjas,

I wrote this for my website in Webflow.

$('#select-generic').click(function() {
	$('#info-generic').addClass('unhide').siblings().removeClass('unhide');
});
$('#select-survey').click(function() {
	$('#info-survey').addClass('unhide').siblings().removeClass('unhide');
});
$('#select-reg').click(function() {
	$('#info-reg').addClass('unhide').siblings().removeClass('unhide');
});
$('#select-exam').click(function() {
	$('#info-exam').addClass('unhide').siblings().removeClass('unhide');
});
$('#select-template').click(function() {
	$('#info-template').addClass('unhide').siblings().removeClass('unhide');
});

Basically I have 2 columns, first one being a <ul> and its <li> and the other a bunch of <div> wrapped inside a master <div>.

What I am trying to do is that, when user clicks <li#Apple>, it will add class ā€œunhideā€ to <div#Apple> and remove ā€œunhideā€ from all its siblings.

The code works well, but Iā€™m wondering if anyone here could help me improve my code. Iā€™m still learning now, and not really a code guy. But I would love to know how it can be improved.

Without a link to your page we canā€™t suggest anything.

Hey Sam,

Due to NDA, I canā€™t share any links to the site. The code piece is my only option. As you could see the way it was written is very redundant, Iā€™m wondering any possibilities to make it more efficient or shorter.

To me that code is fine, thereā€™s a saying that goes around in Software Development - KISS (Keep it Simple Stupid)

I can see exactly what itā€™s doing, and thereā€™s nothing fancy so no need to bother with improving performance.
Thumbs up dude and good luck with your journey into the programming world! :smile:

NOTE: What does ā€˜unhideā€™ mean? (rhetorical)
I would rather use a convention that states the classā€™s intention explicity like ā€˜showā€™ or ā€˜visibleā€™. By saying ā€˜unhideā€™ you are using a double negative

NOTE #2: There is a JQuery function called .show() and .hide(), check them out if you can

Then at least post HTML structure of the elements that are referenced here, without any text.

<div class="w-clearfix gm-new-form-content">
      <div class="gm-new-form-choice">
        <ul class="w-list-unstyled gm-new-form-list">
          <li id="select-generic" class="gm-new-form-item"></li>
          <li id="select-survey" class="gm-new-form-item"></li>
          <li id="select-reg" class="gm-new-form-item"></li>
          <li id="select-exam" class="gm-new-form-item"></li>
          <li id="select-template" class="gm-new-form-item"></li>
        </ul>
      </div>
      <div class="gm-new-form-info">
        <div class="gm-new-form-empty">
          <img src="https://d3e54v103j8qbb.cloudfront.net/img/image-placeholder.svg" class="gm-new-form-empty-thumb">
          <div class="gm-empty-title">Some gibberish here</div>
        </div>
        <div class="gm-absolute-block">
          <div id="info-survey" class="gm-new-form-info-overlay"></div>
          <div id="info-generic" class="gm-new-form-info-overlay"></div>
          <div id="info-reg" class="gm-new-form-info-overlay"></div>
          <div id="info-exam" class="gm-new-form-info-overlay"></div>
          <div id="info-template" class="gm-new-form-info-overlay"></div>
        </div>
      </div>
    </div>

I hope this suffice. Let me know if this helps. Thank you.

Removed IDs from individual lis and divs. Added IDs to parent containers.

<div class="w-clearfix gm-new-form-content">
  <div class="gm-new-form-choice">
    <ul class="w-list-unstyled gm-new-form-list" id="links">
      <li class="gm-new-form-item"></li>
      <li class="gm-new-form-item"></li>
      <li class="gm-new-form-item"></li>
      <li class="gm-new-form-item"></li>
      <li class="gm-new-form-item"></li>
    </ul>
  </div>
  <div class="gm-new-form-info">
    <div class="gm-new-form-empty">
      <img src="https://d3e54v103j8qbb.cloudfront.net/img/image-placeholder.svg" class="gm-new-form-empty-thumb">
      <div class="gm-empty-title">Some gibberish here</div>
    </div>
    <div class="gm-absolute-block" id="targets">
      <div class="gm-new-form-info-overlay"></div>
      <div class="gm-new-form-info-overlay"></div>
      <div class="gm-new-form-info-overlay"></div>
      <div class="gm-new-form-info-overlay"></div>
      <div class="gm-new-form-info-overlay"></div>
    </div>
  </div>
</div>

jQuery is now this:

$('#links').on('click', 'li', function() {
  $('#targets').children().removeClass('unhide')
    .eq($(this).index()).addClass('unhide');
});

Iā€™m not sure why you need to use the class ā€˜unhideā€™, if there is no animation involved, you can do away with adding and removing the classes, and use jQuery .show() and .hide()

1 Like

Another way to simplify it based on your initial method of using individual IDs:

$(.gm-new-form-item').click(function() {
  $('#info-'+ $(this).attr('id').match(/[a-z]+$/))
    .addClass('unhide').siblings().removeClass('unhide');
});
1 Like
$('#links').on('click', 'li', function() {
     $('#target').children().hide().eq($(this).index()).show();
});
1 Like

Wow awesome! @samliew and @Aidz, thanks a bunch guys! The code works, tested all of them. Besides some minor typo. :smiley: Iā€™ll rename the class as your suggested @Aidz.

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.