tags:

views:

608

answers:

3

Hi There, I have a blog where each post doesn't have a detail page. Therefore each blog post has a footer link that loads comments and the comment form via jQuery ajax .load(); The code below doesn't work in IE6/7/8 but it does work in FF/Safari/Chrome. Also, I'm pretty new to jQuery and javascript, and the code just looks so bloated. Can it be slimmed down?

$("a.load-comments").livequery(function() {
    $(this).click(function(){ // when you click on the link
        var commentsWrapper = $(this).parent("div").parent("div").find(".comments-wrapper"); // find the right comments wrapper
        var commentsFormWrapper = $(this).parent("div").parent("div").find(".comments-form-wrapper"); // find the right comments form wrapper
        var commentsLoader = $(this).parent("div").parent("div").find(".comments-loader"); // find the right loader image
        $(".comments-form-wrapper").fadeOut("fast"); // hide the comments form wrapper
        if(!$(this).hasClass("current")) { // check if link is currently opened
            $("a.load-comments, a.load-comments-form").removeClass("current"); // remove the 'current' class from all other links
            $(this).addClass("current"); // add current class to this link
            commentsLoader.fadeIn("fast"); // fade in the loader icon

            commentsWrapper.load($(this).attr("href"), function() { // load the comments
                commentsLoader.fadeOut("fast", function() { // fade out the loader image
                    commentsWrapper.fadeIn("fast"); // fade in the comments
                });
            });

        } else
        if($(this).hasClass("current")) { // if the link does have the class 'current'
            $(this).removeClass("current"); // remove the class 'current'
            commentsWrapper.fadeOut("fast"); // fade out comments
        }
        return false; // prevent following the link
    });
});
+1  A: 

You could make it more efficient by doing some lookups only once and reusing them. Also, you may want to have the effects chained using callbacks so that the next action doesn't fire until the effect is complete. It's hard to know if this is what is tripping you up since you don't define what "not working" means.

$("a.load-comments").livequery(function() { 
    $(this).click(function(){ // when you click on the link
        var $this = $(this); 
        var $grandpa = $this.parent("div").parent("div");
        var commentsWrapper = $grandpa.find(".comments-wrapper"); // find the right comments wrapper 
        var commentsFormWrapper = $grandpa.find(".comments-form-wrapper"); // find the right comments form wrapper 
        var commentsLoader = $grandpa.find(".comments-loader"); // find the right loader image 
        commentsFormWrapper.fadeOut("fast", function() { // hide the comments form wrapper 
            if(!$this.hasClass("current")) { // check if link is currently opened 
                $("a.load-comments, a.load-comments-form").removeClass("current"); // remove the 'current' class from all other links 
                $this.addClass("current"); // add current class to this link 
                commentsLoader.fadeIn("fast", function() { // fade in the loader icon 

                    commentsWrapper.load($this.attr("href"), function() { // load the comments 
                        commentsLoader.fadeOut("fast", function() { // fade out the loader image 
                            commentsWrapper.fadeIn("fast"); // fade in the comments
                        });
                    }); 
                }); 
            } else {  // we know that this has class "current"
                $this.removeClass("current"); // remove the class 'current' 
                commentsWrapper.fadeOut("fast"); // fade out comments 
            }
        });
        return false; // prevent following the link 
    });
});
tvanfosson
A: 

Personally I'd refactor a lot of that to a helper method, I'm not sure how efficient it is to use a prototype method within a live query for a bind. If you add an ID to a div that surrounds each blog post you can move from .parent to an ID lookup which is always faster, then you can get rid of the .finds which slow things down. Plus I'd just have the link call the function instead of using a jQuery.live lookup to bind it. So assuming your HTML for the blog looks like this:

<div id="blog12" class="blogpost">
    ....
    <a href="/url/to/comments" onclick="return ToggleComments('blog12');" class="comments-link">
</div>

Then the javascript would be:

function ToggleComments(blogPostId) {
    var blogPost = $("#" + blogPostId);
    var commentsWrapper = $("#" + blogPostId + " .comments-wrapper");
    var commentsFormWrapper = $("#" + blogPostId + " .comments-form-wrapper");
    var commentsLoader = $("#" + blogPostId + " .comments-loader");
    var commentsLink = $("#" + blogPostId + " .comments-link");

    commentsFormWrapper.fadeOut("fast", function() {
        if(!blogPost.hasClass("current")) {
            blogPost.addClass("current");
            $("div.blogpost:not(id=" + blogPostId + ")").removeClass("current");
            commentsLoader.fadeIn("fast");
            commentsWrapper.load(commentsLink.attr("href"), function() {
                commentsLoader.fadeOut("fast", function() {
                    commentsWrapper.fadeIn("fast");
                });
            });
        } else {
            blogPost.removeClass("current");
            commentsWrapper.fadeOut("fast");
        }
    });

    return false;
}

I tend to avoid livequeries if I can, unless you're adding blog posts to the page using an ajax.load too then I wouldn't bother with it.

As for fixing the IE problem I'd wonder if it's something to do with the livequery + event bind or something. Hopefully this will resolve it.

Parrots
A: 

I'm not sure what part wasn't working for you, but I got this working in IE6:

$("a.load-comments").live('click', function(e){ // when you click on the link
    e.preventDefault(); // prevent following the link
    var link = $(this),
        wrapper = link.parent('div').parent('div'),
        commentsWrapper = wrapper.find(".comments-wrapper"),
        commentsLoader = wrapper.find(".comments-loader");
    $(".comments-form-wrapper").fadeOut("fast"); // hide the comments form wrapper
    if(!link.hasClass("current")) { // check if link is currently opened
        $("a.load-comments, a.load-comments-form").removeClass("current"); // remove the 'current' class from all other links
        link.addClass("current"); // add current class to this link
        commentsLoader.fadeIn("fast"); // fade in the loader icon
        commentsWrapper.load(link.attr("href"), function() { // load the comments
            commentsLoader.fadeOut("fast", function() { // fade out the loader image
                commentsWrapper.fadeIn("fast"); // fade in the comments
            });
        });
    } else { // if the link does have the class 'current'
        link.removeClass("current"); // remove the class 'current'
        commentsWrapper.fadeOut("fast"); // fade out comments
    }
});

my test html looked like this:

<div>
    <div>
        <a class="load-comments" href="/url">click it</a>
        <div class="comments-wrapper">comments wrapper</div>
        <div class="comments-loader">loading....</div>
    </div>
</div>

Like the previous posters said, if you don't specifically need the livequery() (which I changed to live()), you should just attach the events once using click()

You might also run your HTML through a validator--some IE bugs I've struggled with end up being caused by a markup error that the other browsers ignored.

good luck!

honeybuzzer