CODEWITHSUNDEEP
jquery
Laramie
Laramie

Reputation: 5587

JQuery select children based on more than one class from parent element

I have just started with JQuery and I need to change the css class of two elements and remove their onclick event handlers.

When any of the three elements in the div with class upDown is clicked, a popup appears. When a button in that popup is clicked, vote_click() is called.

I need to disable the elements with classes "up" and "down", but no others.

<div class="upDown">
    <div class="up" id="ctl04_btnUp" onclick="lastClick=this;" ></div>
    <div class="com" id="ctl04_btnCom" onclick="lastClick=this;"></div>
    <div class="down" id="ctl04_btnDown" onclick="lastClick=this;"></div>
</div>

Here is my attempt

var lastClick; //will be set to either the "up" or "down"

    function vote_click() {
        if (lastClick && lastClick.className != "com") {
             $("#"+lastClick.parentElement.id + " > .up .down").each(
                function(index, e) {
                    alert('disabled ' + this.id);
                    this.onclick = null;
                    this.className += '-ya';
                }
            );

            lastClick = null;
        }
    }

I clearly have no idea what I'm doing so any general observations about my approach would be appreciated.

Upvotes: 1

Views: 641

Answers (1)

Tom Bartel
Tom Bartel

Reputation: 2258

I hope the following code does what you intend:

(function($) {
    var lastClick = null;

    $('div.upDown .up, div.upDown.com, div.upDown.down').bind('click', function(e) {
        lastClick = this;
    });

    function vote_click() {
        if (lastClick && (lastClick.hasClass('up') || lastClick.hasClass('down')) {
            $('div.upDown .up, div.upDown.down').each( function() {
                var $this = $(this);
                $this.unbind('click');
                var c = $this.attr('class');
                $this.removeClass(c).addClass(c + '-ya');
            });
        }
    }

})(jQuery);

Note:

  • The code gets rid of the onclick attributes in the HTML tags so that your JS is purely unobtrusive.
  • Because everything is wrapped in an anonymous function that is executed right away, it introduces no global variables, which is good for interoperability. Previously, the lastClick variable was global.
  • In your code, $("#"+lastClick.parentElement.id + " > .up .down") would select all elements with class down that are descendants of an element with class up that is a child of the element with id lastClick.parentElement.id. That is not what you want. You could write what you want more easily as $(lastClick).parent().find('.up, .down').

EDIT A word on why onclick is often undesirable: Personally, I don't like globals in JavaScript because all the JS code running in the page (possibly from several developers who don't communicate) share one global namespace. Event handlers declared by means of onclick etc. often refer to globals (as in your case), so this is one reason for me to avoid them.

The second is that the code specified in an onclick attribute runs in a special scope (or scope chain, to be precise). It does not run in the global namespace directly, but in a scope chain consisting of

  • The element that triggered the event handler (here, the div element)
  • The document object
  • Maybe anything in between (this is not specified)
  • Finally, of course, the global object (window)

This can lead to unintended misunderstandings, like you call open() and mean window.open(), but what you get is document.open(). In short, I prefer knowing exactly what my event handlers can and cannot see, and I think in bigger projects, strict separation of code parts and avoidance of global variables pays off. For smaller pieces of code and as long as you don't write novels in the onclick attributes, using onclick etc. is fine.

P.S.: Yes, it is shamefully obvious that I am using an example from Flanagan's "JavaScript - The Definitive Guide".

Upvotes: 1

Related Questions