0

I have a snippet of code that applies a highlighting effect to list items in a menu (due to the fact that the menu items are just POST), to give users feedback. I have created a second step to the menu and would like to apply it to any element with a class of .highlight. Can't get it to work though, here's my current code:

[deleted old code]

The obvious work-around is to create a new id (say, '#highlighter2) and just copy and paste the code. But I'm curious if there's a more efficient way to apply the effect to a class instead of ID?

UPDATE (here is my updated code):

The script above DOES work on the first ul. The second ul, which appears via jquery (perhaps that's the issue, it's initially set to hidden). Here's relevant HTML (sort of a lot to understand, but note the hidden second div. I think this might be the culprit. Like I said, first list works flawlessly, highlights and all. But the second list does nothing.)?

//Do something when the DOM is ready:

<script type="text/javascript">
$(document).ready(function() {

$('#foo li, #foo2 li').click(function() {
    // do ajax stuff
    $(this).siblings('li').removeClass('highlight');
    $(this).addClass('highlight');
});

//When a link in div is clicked, do something:
$('#selectCompany a').click(function() {
    //Fade in second box:

    //Get id from clicked link:
    var id = $(this).attr('id');


    $.ajax({
        type: 'POST',
        url: 'getFileInfo.php',
        data: {'id': id},
        success: function(msg){
            //everything echoed in your PHP-File will be in the 'msg' variable:
            $('#selectCompanyUser').html(msg)
            $('#selectCompanyUser').fadeIn(400);
        }
});
    });
});
</script>
<div id="selectCompany" class="panelNormal">
<ul id="foo">
<?
// see if any rows were returned 
if (mysql_num_rows($membersresult) > 0) { 
    // yes 
    // print them one after another 
    while($row  = mysql_fetch_object($membersresult)) { 
        echo "<li>"."<a href=\"#\""." id=\"".$row->company."\">".$row->company."</a>"."</li>";
    }  
} 
else { 
    // no 
    // print status message 
    echo "No rows found!"; 
} 

// free result set memory 
mysql_free_result($membersresult); 

// close connection 
mysql_close($link); 
 ?>

  </ul>
</div>


<!-- Second Box: initially hidden with CSS "display: none;" -->

<div id="selectCompanyUser" class="panelNormal" style="display: none;">
<div class="splitter"></div>

</div>
5
  • 1
    Drop that and use jQuery. Commented Aug 13, 2011 at 6:25
  • One improvement would be to use getElementsByClassName. While some browsers inheretly support it for the rest you can use a custom function. Check this link: ejohn.org/blog/getelementsbyclassname-speed-comparison Commented Aug 13, 2011 at 6:31
  • 1
    its not ajax, its plain old javascript Commented Aug 13, 2011 at 6:33
  • @Peter Of The Corn and Cybernate he asked why this is not working, If he asked what other method to use or what library your answers would be good, be he needs to solve the problem not to make it in other way. Commented Aug 13, 2011 at 6:35
  • 1
    @Mihai Certainly. there is a reason I posted it as a comment and not an answer. As for Cybernate, I think he actually has a valid suggestion. Commented Aug 13, 2011 at 6:38

3 Answers 3

1

You could just create #highlighter2 and make your code block into a function that takes the ID value and then just call it twice:

function hookupHighlight(id) {
    var context = document.getElementById(id);
    var items = context.getElementsByTagName('li');
    for (var i = 0; i < items.length; i++) {
        items[i].addEventListener('click', function() {
            // do AJAX stuff

            // remove the "highlight" class from all list items
            for (var j = 0; j < items.length; j++) {
                var classname = items[j].className;
                items[j].className = classname.replace(/\bhighlight\b/i, '');
            }

            // set the "highlight" class on the clicked item
            this.className += ' highlight';
        }, false);
    }
}

hookupHighlight("highliter1");
hookupHighlight("highliter2");

jQuery would make this easier in a lot of ways as that entire block would collapse to this:

$("#highlighter1 li, #highlighter2 li").click(function() {
    // do ajax stuff
    $(this).siblings('li').removeClass('highlight');
    $(this).addClass('highlight');
});

If any of the objects you want to click on are not initially present when you run this jQuery code, then you would have to use this instead:

$("#highlighter1 li, #highlighter2 li").live("click", function() {
    // do ajax stuff
    $(this).siblings('li').removeClass('highlight');
    $(this).addClass('highlight');
});
Sign up to request clarification or add additional context in comments.

5 Comments

Trying to implement the jquery. Error somewhere : /
Updated in my post. Got the jquery you provided to work (somewhat). Doesn't affect second list, however..
I don't see #foo2 in your HTML. Is it added dynamically after the jQuery that sets the click handler? If so, you will have to switch to from .click(function() {}) to .live("click", function() {}) because it isn't present for a normal click handler to be added at the time you run the code that sets the click handler. I updated my answer to show this form too.
You da man! That was it, and yes the data was added dynamically. But that change worked perfectly - thanks for the help man I would upvote your answer more if I could!
Cool. Glad it worked. This was a nice example of how much code jQuery can save.
0

change the replace in /highlight/ig, it works on http://jsfiddle.net/8RArn/

var context = document.getElementById('highlighter');
var items = context.getElementsByTagName('li');

for (var i = 0; i < items.length; i++) {
    items[i].addEventListener('click', function() {
        // do AJAX stuff

        // remove the "highlight" class from all list items
        for (var j = 0; j < items.length; j++) {
            var classname = items[j].className;
            items[j].className = classname.replace(/highlight/ig, '');
        }

        // set the "highlight" class on the clicked item
        this.className += ' highlight';
    }, false);
}

Comments

0

So all those guys that are saying just use jQuery are handing out bad advice. It might be a quick fix for now, but its no replacement for actually learning Javascript.

There is a very powerful feature in Javascript called closures that will solve this problem for you in a jiffy:

var addTheListeners = function (its) {
    var itemPtr;

    var listener = function () {
        // do AJAX stuff

        // just need to visit one item now
        if (itemPtr) {
            var classname = itemPtr.className;
            itemPtr.className = classname.replace(/\bhighlight\b/i, '');
        }

        // set the "highlight" class on the clicked item
        this.className += ' highlight';
        itemPtr = this;
    }

    for (var i = 0; i < its.length; i++) {
        its[i].addEventListener ('click', listener, false);
    }
}

and then:

var context = document.getElementById ('highlighter');
var items = context.getElementsByTagName ('li');
addTheListeners (items);

And you can call add the listeners for distinct sets of doc elements as many times as you want.

addTheListeners works by defining one var to store the list's currently selected item each time it is called and then all of the listener functions defined below it have shared access to this variable even after addTheListeners has returned (this is the closure part).

This code is also much more efficient than yours for two reasons:

  1. You no longer iterate through all the items just to remove a class from one of them
  2. You aren't defining functions inside of a for loop (you should never do this, not only for efficiency reasons but one day you are going to be tempted to use that i variable and its going to cause you some problems because of the closures thing I mentioned above)

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.