Struggling to abstract this block of code (jQuery)

JavaScript
1

Hi :slight_smile:

I wrote a simple piece of code that works as expected, but it’s helluva verbose! I’m a little stumped tho, how can I abstract it if i’m using multiple different selectors?

http://jsfiddle.net/s78oeq49/12/

Basically there’s only 4 divs with ‘previews’, and I was thinking of adding them to an array. The part i can’t get right is choosing the correct array index for the related button…sorry, that sounds super confusing, but if you test those buttons in the fiddle you’ll know exactly what’s going on :slight_smile:

Hope someone can give me tips on what to do to get the code shorter in any way.

2

You need to make more use of classes I think. And anchors are easier than buttons in this respect, as the anchor’s href can point at the id of the element you want. For example:

<a href="#shopping_cart_preview" class="preview-toggle">Show Cart</a>
<div class="preview" id="shopping_cart_preview">Cart</div>

<a href="#reputation_preview" class="preview-toggle">Show Points</a>
<div class="preview" id="reputation_preview">Points</div>

<a href="#notifications_preview" class="preview-toggle">Show Msgs</a>
<div class="preview" id="notifications_preview">Msgs</div>

<a href="#profile_preview" class="preview-toggle">Show Profile</a>
<div class="preview" id="profile_preview">Profile</div>

and in the CSS:

.preview-toggle {
  display: block;
}

.preview {
  display: none;
}

.preview.active {
  display: block;
}

This then allows you to do:

const $previews = $('.preview');

$(document).on('click', (e) => {
  // If a toggle has been clicked...
  if ($(e.target).hasClass('preview-toggle')) {
    // Prevent normal anchor behaviour:
    e.preventDefault();
    // Remove the .active class from all previews
    $previews.removeClass('active');
    // Get the target id
    const targetId = $(e.target).attr('href');
    // Add an .active class to the correct element:
    $(targetId).addClass('active');
  }
});

or in plain JS:

const previews = document.querySelectorAll(".preview");

document.addEventListener("click", (e) => {
  // If a toggle has been clicked...
  if (e.target.classList.contains("preview-toggle")) {
    // Prevent normal anchor behaviour:
    e.preventDefault();
    // Remove the .active class from all previews
    previews.forEach((preview) => preview.classList.remove("active"));
    // Get the target id
    const targetId= e.target.getAttribute("href");
    // Add an .active class to the correct element:
    document.querySelector(targetId).classList.add("active");
  }
});

And if you’d rather use JS to show/hide (so get rid of the display: none and .active rules in the CSS):

const $previews = $('.preview');
$previews.hide();

$(document).on('click', (e) => {
  if ($(e.target).hasClass('preview-toggle')) {
    e.preventDefault();
    $previews.hide();
    const targetId = $(e.target).attr('href');
    $(targetId).show();
  }
});
1 Like
3

I’m not a big fan of event delegation used in the JS example above.

They have their situations,. but it removes the ability to use stopPropagation(). I prefer to place the click event on the DOM element itself. Especially if the element wasn’t created dynamically.

4

Depends. I’d normally scope it to a containing element, but in the example the containing element is the document. I normally attach to the individual elements only if necessary (example I’m working on a the minute: I have a data table, and clicking on a row brings up more detailed info. In each row are some controls specific to the data in that row. So one event handler for the table, then attach handlers to the controls with stopPropagation to allow them to work)