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>

Sᴀᴍ Onᴇᴌᴀ
29.5k16 gold badges45 silver badges201 bronze badges
asked Jan 3, 2019 at 12:32
\$\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\$ 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\$ 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>

answered Jan 3, 2019 at 16:30
\$\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/

answered Jan 11, 2019 at 20:58
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.