CODEWITHSUNDEEP
javascriptcallbackdom-events
Slater Victoroff
Slater Victoroff

Reputation: 21914

Javascript onclick fires on page load

Just starting out with front end development here, but despite my assuredly poor practices I've been enjoying things a lot. Basically I'm trying to add an onclick function to an element when the page loads. It was working fine as:

$("li a:contains('Search')").ready(function(){
    $("li a:contains('Search')").on("click", function(){
$("li a:contains('Search)").text("test")});
});

But my programmer spidey-senses were strongly activated by the quite frankly gross state of this code, so I tried to refactor and my thought for the easiest first-step refactoring was this:

function replaceWithSearch(element){
    element.text("test");
}

$("li a:contains('Search')").ready(function(){
    $("li a:contains('Search')").on("click", replaceWithSearch($("li a:contains('Search')")));
});

However, when I changed to that, the onclick function now launches immediately. Is there a way to force the function to only be evaluated when called as a callback? I would prefer to have the function be reusable, also if you have any other comments on refactoring my code or best practices I would be very glad to hear them. Again, just starting out with Javascript and I would love to build some good practices.

Upvotes: 0

Views: 781

Answers (2)

Shadow Wizard
Shadow Wizard

Reputation: 66389

Optimized code:

$(document).ready(function() {
    $("li a:contains('Search')").bind("click", function() {
        $(this).text("test");
    });
});

If you prefer to use a function, proper way is:

$(document).ready(function() {
    $("li a:contains('Search')").bind("click", replaceWithSearch);
});

And in the function you don't have to use argument:

function replaceWithSearch(){
    $(this).text("test");
}

Live test case.

If your elements are created after page load, use the .on() like this:

$(document).on("click", "li a:contains('Search')", replaceWithSearch);

Updated fiddle.

Upvotes: 3

Mike C
Mike C

Reputation: 3117

Wrap the handler in a function like:

$("li a:contains('Search')").ready(function(){
    $("li a:contains('Search')").on("click", function() {replaceWithSearch($("li a:contains('Search')"))});
});

That way, the handler is defined and associated with the click event, rather than executing immediately.

Upvotes: 1

Related Questions