Javascript/jQuery onclick not working

Question

i made a test.html document to test a script. Somehow it is not working and i can't get why nothing is happening. The Script is in -tags and wrapped with -tag and the css also has it´s -tags. Why shouldn't it work? Here is the code

<!DOCTYPE html>
<html>

    <head>
        <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.11.3/jquery.min.js">
        </script>
        <script>
        $('#menu li a').on('click', function() {
            $('li a.current').removeClass('current');
            $(this).addClass('current');
        });
        </script>
        <style>
        .current {
            text-decoration: underline;
        }

        ul {
            list-style-type: none;
        }

        a {
            text-decoration: none;
        }
        </style>
    </head>

    <body>
        <div id="menu">
            <ul>
                <li><a id="about-link" class="current" href="#">ABOUT</a></li>
                <li><a id="events-link" href="#">EVENTS</a></li>
                <li><a id="reviews-link" href="#">REVIEWS</a></li>
                <li><a id="contact-link" href="#">CONTACT</a></li>
            </ul>
        </div>
    </body>

</html>

Tushar · Accepted Answer · 2015-09-02T05:50:11.197

12

Because your code for binding event handler is executed before the elements is present in DOM, the event is not bound on the element.

To solve this problem, you can either

Wrap your code in ready() callback, so that your code will be executed when DOM is finished loading
Move your script to the end of <body>, so all the elements are present in DOM and you can bind the events on it

Code:

$(document).ready(function () {
    $('#menu li a').on('click', function () {
        $('li a.current').removeClass('current');
        $(this).addClass('current');
    });
});

EDIT

You can also use load(not recommended) event callback to bind the event, but this will wait for all the resources to load completely.

If you want to use Vanilla Javascript, you can use DOMContentLoaded event on document.

edited Sep 02 '15 at 05:50

answered Aug 26 '15 at 09:31

Tushar

78,625
15
134
154

1

Note: The past-tense of *bind* is *bound* (not *binded*). i.e. "the event is not bound on the element" – Gone Coding Aug 26 '15 at 09:41
Better. That seems to be the most common grammatical error on SO :) +1 for also suggesting you can simply move the code to after the elements. – Gone Coding Aug 26 '15 at 09:44

score 8 · Answer 2 · answered Aug 26 '15 at 09:32

8

you are executing the code before the DOM content is loaded. Place your code in a ready block.

answered Aug 26 '15 at 09:32

atinder

2,002
12
15

1

*Or* simply place the code after the elements it references... (end of `body` being the most common place) – Gone Coding Aug 26 '15 at 09:46

score 0 · Answer 3 · edited May 23 '17 at 11:44

You are executing the JavaScript before the <body> tag loads, which contains your <li> elements.

To solve this problem, you have three choices :

1.You can insert the JavaScript code you have written inside a $(document).ready() callback. (see http://www.w3schools.com/jquery/event_ready.asp for more information)

Code :

$(document).ready(function () {
$('#menu li a').on('click', function () {
    $('li a.current').removeClass('current');
    $(this).addClass('current');
  });
});

2.You can use the JavaScript's built in window.onload function. (see https://thechamplord.wordpress.com/2014/07/04/using-javascript-window-onload-event-properly/ for more information)

Code:

window.onload = function () {
$('#menu li a').on('click', function () {
    $('li a.current').removeClass('current');
    $(this).addClass('current');
  });
}

3.Place your <script> tag before </body> tag in your page.

Code:

<head>
    <style>
    .current {
        text-decoration: underline;
    }

    ul {
        list-style-type: none;
    }

    a {
        text-decoration: none;
    }
    </style>
</head>

<body>
    <div id="menu">
        <ul>
            <li><a id="about-link" class="current" href="#">ABOUT</a></li>
            <li><a id="events-link" href="#">EVENTS</a></li>
            <li><a id="reviews-link" href="#">REVIEWS</a></li>
            <li><a id="contact-link" href="#">CONTACT</a></li>
        </ul>
    </div>
  <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.11.3/jquery.min.js">
    </script>
    <script>
    $('#menu li a').on('click', function() {
        $('li a.current').removeClass('current');
        $(this).addClass('current');
    });
    </script>
</body>

Note: Using $(document).ready() is a better option than using window.onload. (see window.onload vs $(document).ready() for more information)

score 0 · Answer 4 · edited Sep 10 '15 at 10:23

0

Your code is fine. Just wrap it in DOM ready and you are good to go.

$(document).ready(function(){
    $('#menu li a').on('click', function() {
        $('li a.current').removeClass('current');
        $(this).addClass('current');
    });
})

edited Sep 10 '15 at 10:23

Pang

8,605
144
77
113

answered Sep 10 '15 at 07:12

Deepak Sabharwal

9
10

Javascript/jQuery onclick not working

4 Answers4

Linked

Related