2
\$\begingroup\$

I need to add class is_active to all div's when I click on <div id="all-tags"> and remove class if I click again. Only every div have class is_active then main div (<div id="all-tags">) have class is_active.

Please tell me if this is ok or not. I need as fast of a solution as possible. I'm also not sure using .each() is a very good idea.

I need as fast of a solution as possible. I'm not sure using if .each() is a very good idea.

$("#all-tags").click(function() {
    if ($(this).hasClass("is_active")) {
        $(".tags_cloud div").each(function() {
            $(this).removeClass("is_active");
        });
    } else {
        $(this).addClass("is_active");
        $(".tags_cloud div").each(function() {
            $(this).addClass("is_active");
        });
    }
});

$(".tags_cloud div").click(function() {
    !$(this).is("#all-tags") ? $(this).toggleClass("is_active") : true;
    var c = 0;
    $(".tags_cloud div").each(function() {
        if ($(this).hasClass("is_active") && !$(this).is("#all-tags")) {
            // some my code
        } else if (!$(this).is("#all-tags")) {
            $("#all-tags").removeClass("is_active");
            c = 1;
        }
        c == 0 ? $("#all-tags").addClass("is_active") : true;
    })
});
\$\endgroup\$
5
  • \$\begingroup\$ this seems right, why your concern about using each ? \$\endgroup\$
    – porjolovsky
    May 4 '16 at 12:41
  • \$\begingroup\$ @porjolovsky is this fast enough?:) \$\endgroup\$
    – SimBioT
    May 4 '16 at 12:44
  • \$\begingroup\$ What you should consider is to store the result of repeating code into some variable, you are calling ''$("#all-tags")'' too many times. \$\endgroup\$
    – xxxmatko
    May 4 '16 at 12:46
  • \$\begingroup\$ @user6290619 yeah... It could be improved upon, depending on how many items you are going to be targeting in the final version, but for a small group as shown in the example, for a regular (average) website on an average, low-budget server, it serves the purpose magnificently \$\endgroup\$
    – porjolovsky
    May 4 '16 at 12:47
  • \$\begingroup\$ @porjolovsky I have a lot of items (tags) something about 25 so far \$\endgroup\$
    – SimBioT
    May 4 '16 at 12:52
1
\$\begingroup\$

I updated your code to remove the first click-handler and the each-loops. However I'm not quite sure if this is necessarily faster then your solution.

My suggestion relies on callbacks, css-selectors and conditions:

$(".tags_cloud div").click(function() {

  var $this = $(this),
    $tags = $(".tags_cloud div:not(#all-tags)"),
    $all = $(".tags_cloud div#all-tags");

  var all = $(this).is('#all-tags') ? true : false;
  if (all) {
    $this.toggleClass(function() {
      if (!$(this).hasClass('is_active')) {
        $tags.addClass('is_active');
      } else {
        $tags.removeClass('is_active');
      }
      return "is_active";
    });
  } else {
    $this.toggleClass('is_active');
    ($tags.not('.is_active').length === 0) ? $all.addClass('is_active') : $all.removeClass('is_active');
  }
});

Demo

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

I guess I would reduce it to this:

$(".tags_cloud div").click(function(){
  if ($(this).attr('id') == "all-tags" && !$(this).hasClass("is_active")) {
    $(".tags_cloud div").addClass("is_active");
  }
  else if ($(this).attr('id') == "all-tags" && $(this).hasClass("is_active")) {
    $(".tags_cloud div").removeClass("is_active");
  }
  else {
    $(this).toggleClass("is_active"); 
    $(".tags_cloud div#all-tags").removeClass("is_active");        
  }
});

It does exactly the same, as far as I can tell. But with half the code and without use of .each()...

\$\endgroup\$
1
  • \$\begingroup\$ Sorry, but you are wrong. If only 3 div's are "active" and I'm click on fourth div, then <div id="all-tags"> would not get class "is_active" \$\endgroup\$
    – SimBioT
    May 4 '16 at 12:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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