Jquery click firing on all elements

Question

I am attempting to use Bootstrap's Collapse feature with custom icons from font-awesome. I am able to get the collapse to work but the problem I am having is that all of the icons are being triggered with Jquery's click, I want to scale this because at any given time the amount of "containers" can change. Any suggestions are appreciated.

$(document).ready(function () {
    $faChevronDown = $('.fa-chevron-down');
    var z = 0;
    $faChevronDown.click(function () {
        if (z == 0) {
            turnUp();
            z++;
        } else {
            turnDown();
            z = 0;
        }
    });
});

function turnUp() {
    $faChevronDown.removeClass('fa-chevron-down');
    $faChevronDown.addClass('fa-chevron-up');
};

function turnDown() {
    $faChevronDown.removeClass('fa-chevron-up');
    $faChevronDown.addClass('fa-chevron-down');
};

JS Fiddle

Thank you Edit : Thank you for the great answers!

I've simplified it for you: jsfiddle.net/74t2jLcu/8

Mark Hughes
– Mark Hughes

2015-03-31 16:32:22 +00:00
Commented Mar 31, 2015 at 16:32 — Mark Hughes
– Mark Hughes, Commented Mar 31, 2015 at 16:32

taxicala · Accepted Answer · 2015-03-31 16:28:33Z

2

You are clicking only one element, but your function is changing all icons, you have use $(this) instead in order to only change the icon you are clicking:

function toggleClass() {
    $(this).toggleClass('fa-chevron-down fa-chevron-up');
};

and then use only one function:

$faChevronDown.click(toggleClass);

With this you avoid the use of Ifs and elses and the code is much simplier and small.

answered Mar 31, 2015 at 16:28

taxicala

21.8k7 gold badges41 silver badges67 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Timur Osadchiy · Accepted Answer · 2015-03-31 16:39:08Z

2

Set click handler on the parent element of a .fa-chevron-down element or if the parent element is not known on body element:

$(document).ready(function () {
    var z = 0;
    $("body").on("click", ".fa-chevron-down", function () {
        if (z == 0) {
            turnUp.call(this);
            z++;
        } else {
            turnDown.call(this);
            z = 0;
        }
    });
});

function turnUp() {
    $(this).removeClass('fa-chevron-down');
    $(this).addClass('fa-chevron-up');
};

function turnDown() {
    $(this).removeClass('fa-chevron-up');
    $(this).addClass('fa-chevron-down');
};

If you are using z variable only for switching classes fa-chevron-down and fa-chevron-up, the code could be simplified to:

$(document).ready(function () {
    $("body").on("click", ".fa-chevron-down", function () {
        $(this).toggleClass('fa-chevron-down fa-chevron-up');
    });
});

edited Mar 31, 2015 at 16:39

answered Mar 31, 2015 at 16:26

Timur Osadchiy

6,2994 gold badges28 silver badges29 bronze badges

2 Comments

user3196599 Over a year ago

Thank you for your input, wouldn't this cause a problem when the turnDown function gets called? the click might stop working because .fa-chevron-down is selected and is no long available.

Timur Osadchiy Over a year ago

@Dnwebdev always welcome! Please feel free to accept my answer:)

ferr · Accepted Answer · 2015-03-31 16:35:56Z

1

You can pass in the element to perform granular toggling,

$(document).ready(function () {
    $fa= $('.fa');
    var z = 0;
    $fa.click(function () {
        if (z == 0) {
            turnUp($(this));
            z++;
        } else {
            turnDown($(this));
            z = 0;
        }
    });
});

function turnUp(el) {
    el.removeClass('fa-chevron-down');
    el.addClass('fa-chevron-up');
};

function turnDown(el) {
    el.removeClass('fa-chevron-up');
    el.addClass('fa-chevron-down');
};

edited Mar 31, 2015 at 16:35

answered Mar 31, 2015 at 16:28

ferr

1,2473 gold badges14 silver badges30 bronze badges

2 Comments

user3196599 Over a year ago

Thank you for your input, this seems to work until you close one and then open another, I believe the selector is lost after that.

ferr Over a year ago

Ah I see, you are removing the fa-chevron-down class which is where the click event is bound. I would recommend binding with a different class from the one you are adding/removing. You can bind to .fa and continue to use the chevron classes elsewhere. I'll update.

j08691 · Accepted Answer · 2015-03-31 16:32:00Z

0

I'm not sure what the point of your z variable is, but you can reduce what you have, and fix the problem of not referencing the element by using this, by using just:

$(document).ready(function () {
    $('.fa-chevron-down').click(function () {
        $(this).toggleClass('fa-chevron-down fa-chevron-up')
    });
});

jsFiddle example

answered Mar 31, 2015 at 16:32

j08691

208k33 gold badges269 silver badges281 bronze badges

2 Comments

user3196599 Over a year ago

Thank you, works great! Edit - I was using .toggle until I went through the "why is everything hiding, oh it was deprecated!" loop. I will accept answer in 4 minutes, looks like the most efficient way to do it!!

user3196599 Over a year ago

Wouldn't this break if the chevron was up for some reason (a trigger from angular etc) on page load? Edit: Yes it does! Edit: Changed to .fa - fixes it. I will probably use an element selector because I am using chevrons in other locations too.

Collectives™ on Stack Overflow

Jquery click firing on all elements

4 Answers 4

Comments

2 Comments

2 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

2 Comments

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related