2
\$\begingroup\$

If one menu-item is clicked then the description with the same data-filter attribute should be shown.

I added a class to the description elements to associate each one with the affiliated menu item.

What do you think?

$(".button").click(function() {
  $(".button").removeClass("active-filter");
  $(this).addClass("active-filter");

  var button = $(this).attr("data-filter");
  var active_element = $('.descr[data-filter="' + button + '"]');

  $(".descr").removeClass("active-element");
  active_element.addClass("active-element");
});
.element {
  display: inline-block;
  background-color: lightgreen;
  width: 220px;
}

.filter {
  display: inline-block;
  margin: 10px;
  background-color: orange;
}

.button {
  width: 60px;
  height: 60px;
  padding: 10px;
  margin: 10px;
  background-color: lightgrey;
  float: left;
}

.descr {
  display: block;
  max-height: 0;
  visibility: hidden;
  background-color: lightgrey;
  padding: 10px;
  position: absolute;
  top: 0;
  left: 0;
  right: 0;
  margin: 10px;
}

.data-element {
  height: 60px;
  display: block;
  background-color: orange;
  margin: 10px;
  position: relative;
}

.active-filter {
  background-color: yellow;
  display: block;
}

.active-element {
  background-color: yellow;
  max-height: 100%;
  visibility: visible;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div class="element">
  <div class="filter">
    <div class="button active-filter" data-filter="3">data-filter 1</div>
    <div class="button" data-filter="4">data-filter 2</div>
  </div>
  <div class="data-element">
    <div class="descr active-element" data-filter="3">data-element 1</div>
    <div class="descr" data-filter="4">data-element 2</div>
  </div>
</div>

\$\endgroup\$
4
  • \$\begingroup\$ Just to clarify, you are talking about this line being not so elegant right? var active_element = $('.d[data-filter="' + b + '"]'); \$\endgroup\$
    – 3limin4t0r
    Commented Jan 3, 2019 at 13:33
  • 1
    \$\begingroup\$ @JoopEggen The code at the question does work as expected. \$\endgroup\$ Commented Jan 3, 2019 at 16:37
  • \$\begingroup\$ @guest271314 sorry, "I don't get it right" and some code that looked to me not so okay made me add the comment. Thanks very much for preventing the OP to be stalled. \$\endgroup\$
    – Joop Eggen
    Commented Jan 3, 2019 at 16:52
  • \$\begingroup\$ @JoopEggen It appears OP at some attempted to use .find() or .filter() to chain jQuery methods, instead of declaring multiple variables. \$\endgroup\$ Commented Jan 3, 2019 at 16:59

2 Answers 2

1
\$\begingroup\$

One purpose for using jQuery is the ability to chain methods. You can first remove the "active-element" class from .d, then utilize .filter() with a single line passed as parameter within a template literal which removes the "active-filter" class from .b elements, filters and adds the same class to this element, and concludes by returning the .data() of the current element, which is the attribute value selector to match the corresponding .d element where the "active-element" class is added.

$(".b").click(function(e) {
  $(".d").removeClass("active-element")
  .filter(`[data-filter='${
    $(".b").removeClass("active-filter")
    .filter(this).addClass("active-filter")
    .data().filter
   }']`)
  .addClass("active-element");
});
.b {
  width: 60px;
  height: 60px;
  padding: 10px;
  margin: 10px;
  background-color: lightgrey;
  float: left;
}

.c {
  width: 60px;
  height: 60px;
  padding: 10px;
  margin: 10px;
  background-color: lightgrey;
  float: left;
}

.element {
  float: left;
  display: block;
}

.active-filter {
  background-color: pink;
}

.active-element {
  background-color: yellow;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div class="element">
  <div>
    <div class="b active-filter" data-filter="3">data-filter</div>
    <div class="b" data-filter="4">data-filter</div>
  </div>
  <div>
    <div class="c d" data-filter="3">data-element</div>
    <div class="c d" data-filter="4">data-element</div>
  </div>
</div>

\$\endgroup\$
0
0
\$\begingroup\$

The code looks okay. The Javascript code looks somewhat simple but then again it is jQuery code. There are some inefficiencies - for example, the DOM elements are not cached so on every click there are at least three DOM queries. This could be improved by storing those references in variables when the DOM is ready - typically done with the jQuery DOM ready callback - $(function() { ... }) (formerly $(document).ready(function() { ... }); but as of version 3 that is deprecated1. Once the DOM elements are stored after the DOM is ready, the description elements can later be filtered using the .filter() method.

The .data() method could also be used to simplify the lookup of the dataset attributes.

The name button for the variable to store the value of the attribute data-filter feels a little misleading.

var button = $(this).attr("data-filter");

A more appropriate name would be filterValue or something along those lines.

A rewrite

The re-written code utilizes the advice above.

I tried to find a way to utilize .toggleClass() to simplify adding and removing classes but I couldn't find anything that was any simpler.

$(function() {//DOM ready
  var buttons = $('.button');
  var descriptions = $('.descr');
  buttons.click(function() {
    buttons.removeClass("active-filter");
    $(this).addClass("active-filter");
    var filterValue = $(this).data('filter');
    var active_element = descriptions.filter('[data-filter="' + filterValue + '"]');

    descriptions.removeClass("active-element");
    active_element.addClass("active-element");
  });
});
.element {
  display: inline-block;
  background-color: lightgreen;
  width: 220px;
}

.filter {
  display: inline-block;
  margin: 10px;
  background-color: orange;
}

.button {
  width: 60px;
  height: 60px;
  padding: 10px;
  margin: 10px;
  background-color: lightgrey;
  float: left;
}

.descr {
  display: block;
  max-height: 0;
  visibility: hidden;
  background-color: lightgrey;
  padding: 10px;
  position: absolute;
  top: 0;
  left: 0;
  right: 0;
  margin: 10px;
}

.data-element {
  height: 60px;
  display: block;
  background-color: orange;
  margin: 10px;
  position: relative;
}

.active-filter {
  background-color: yellow;
  display: block;
}

.active-element {
  background-color: yellow;
  max-height: 100%;
  visibility: visible;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div class="element">
  <div class="filter">
    <div class="button active-filter" data-filter="3">data-filter 1</div>
    <div class="button" data-filter="4">data-filter 2</div>
  </div>
  <div class="data-element">
    <div class="descr active-element" data-filter="3">data-element 1</div>
    <div class="descr" data-filter="4">data-element 2</div>
  </div>
</div>

1https://api.jquery.com/ready/

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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