Ajax .load calls before the click event that is used to call it

JavaScript
1

So I’m doing some basic href functionality replacement for users that have JS enabled. I admit I’m more of a PHP guy so I may have some syntax issues in my JQuery, please point those issues out.

This will replace all my href attributes in my nav links to remove their functionality and then replace with an click event listener…


function replaceLinks() {
    $('#nav_ribbon li a').each(function() {
        $(this).attr({'href': 'javascript:void(0)'}).on('click', navLinkClick(this));
    });
}
function navLinkClick(selector) {
    var pageValue = $(selector).attr('title').toLowerCase();
    var page = pageValue + '.php';
    $('#content').load(page);
}

So on document ready I run replaceLinks but when my page loads it automatically activates the handler without the event taking place… so I end up with loaded content instead of the home page content

sample of the navigation html:


<div id="nav_ribbon"> <a class="header_logo" href="index.php"><img title="Home" alt="Logo" src="img/logo.png" /></a>
<ul>
   <li><a title="page0" class="nav_item" href="index.php">page0</a></li>
   <li><a title="page1" class="nav_item" href="index.php?page=page1">page1</a></li>
   <li><a title="page2" class="nav_item" href="index.php?page=page2">page2</a></li>
   <li><a title="page3" class="nav_item" href="index.php?page=page3">page3</a></li>
   <li><a title="page4" class="nav_item" href="index.php?page=page4">page4</a></li>
   <li><a title="page5" class="nav_item" href="index.php?page=page5">page5</a></li>
</ul>
</div>
2

That’s because you’re calling it on that line. You should be passing a function reference.

3

Thank you Logic Ali!

Oh, so like this:


$(this).attr({'href': 'javascript:void(0)'}).on('click', function() {
     navLinkClick(this);
});

Can someone explain what’s going on here so I have a better understanding :inspector:
What is happening differently between these calls?

.on('click', function() {navLinkClick(this);} );

AND

[COLOR=#000000].on('click', navLinkClick(this));[/COLOR]

I’m eager to learn

4

Here, you’re executing navLinkClick immediately, and the return value is used for the second argument to .on().

Here, you’re passing .on() a function reference, which when executed will call navLinkClick.

5

And although what you proposed will fix the problem, there are also other ways to improve the code quality, such as removing the “javascript:void(0)” thing (event.preventDefault is cleaner), and passing a reference to navLinkClick itself rather than wrapping it in an anonymous function.

function replaceLinks() {
    $('#nav_ribbon li a').each(function() {
        [COLOR="#FF0000"]$(this).on('click', navLinkClick);[/COLOR]
    });
}
function navLinkClick([COLOR="#FF0000"]event[/COLOR]) {
    [COLOR="#FF0000"]event.preventDefault();[/COLOR]
    var pageValue = $([COLOR="#FF0000"]event.target[/COLOR]).attr('title').toLowerCase();
    var page = pageValue + '.php';
    $('#content').load(page);
}
6

Thank you Jeff Mott, I appreciate your explanation and clean code suggestions