tags:

views:

16

answers:

1

Hi guys, I'm bulding a brand new site for the disco I work for.

Please take a look on the guests page: http://www.jubileegroup.it/jubilee/guests.php

I've been able to get the exact effect I wanted when the pointer moves on a picture, but I think my code is really...bad... I'm sure there must be something cleaner.

I'm using this in jQuery ready():

$(".oneguest").bind("mouseenter", function () {
 //get the actual coords of the div
 var coord = $(this).position();
 //show guest's name
 $(".guestname", $(this)).show('slow');
 //I create another div with id = tempguest to make the other divs not to move!
 $(this).after("<div class='oneguest' id='tempguest'></div>");
 //check if this is the fifth element of the row (.norm = no right margin)
 if ($(this).hasClass('norm')) {
  $("#tempguest").addClass('norm');
 }
 //I convert the div in an absolute positioned div and place it perfectly centered with his older position
 $(this).css("width", "246px");
 $(this).css("height", "300px");
 $(this).css("border", "5px solid #ddd");
 $(this).css("top", (coord.top - 66) + "px");
 $(this).css("left", (coord.left - 39) + "px");
 $(this).css("position", "absolute");
 $(this).css("z-index", "100");
});
//on mouseleave I destroy the "placeholder" div (id=tempguest) and reconvert the div to non-absolute.
$(".oneguest").bind("mouseleave", function () {
 $("#tempguest").remove();
 $(".guestname", $(this)).hide('fast');
 $(this).css("width", "176px");
 $(this).css("height", "176px");
 $(this).css("border", "1px solid #ddd");
 $(this).css("top", "");
 $(this).css("left", "");
 $(this).css("position", "inherit");
 $(this).css("z-index", "1");
});

Like I said the effect works, but I think my solution isn't pretty at all! What do You think?

A: 

The main cleanup would be that .css() can take an object and .hover() as a shortcut for mouseenter and mouseleave, like this:

$(".oneguest").hover(function () {
    var coord = $(this).position();
    $(this).find(".guestname").show('slow');
    $(this).after("<div class='oneguest' id='tempguest'></div>");

    if ($(this).hasClass('norm')) 
       $("#tempguest").addClass('norm');

    $(this).css({ width: "246px", 
                  height: "300px", 
                  border: "5px solid #ddd", 
                  top:  (coord.top - 66) + "px", 
                  left: (coord.left - 39) + "px", 
                  position: "absolute", 
                  z-index: "100" });
}, function () {
    $("#tempguest").remove();
    $(this).find(".guestname").hide('fast');    
    $(this).css({ width: "176px", 
                  height: "176px", 
                  border: "1px solid #ddd", 
                  top:  "", 
                  left: "", 
                  position: "inherit", 
                  z-index: "1" });
});

There's not much to be done in terms of optimization here, but does make it a bit prettier.

Nick Craver
Thanks Nick, I'll fix that... Other than that, do you think the solution I'm following is goog? Shouldn't I create the abs positioned div instead of converting the existing one?
checcco
@checcco - If you can reuse the same `<div>` over and over (looks like you can), it would be quicker, since it's absolutely positioned you could shorten it down to `.css({ top: (coord.top - 66) + "px", left: (coord.left - 39) + "px", display: 'block' })` (no need to move it in HTML since it's `absolute`).
Nick Craver
Thanks for editing your answer, Nick... I noticed you changed $(".guestname", $(this)).show('slow'); to $(this).find(".guestname").show('slow'); is this better in anyway? faster maybe?
checcco
@checcco - It's equivalent to `$(".guestname", this)`, using a `$(selector, context)` internally is *really* `$(context).find(selector)`, not much difference either way, but flows much better I think. I find `$(".guestname", this)` equally readable, no need to wrap the `this` in a jQuery object :)
Nick Craver
Thanks for your time, Nick... I've already learned a lot from your advices... Thinking about reusing the same div over and over... Then I'll need to retrieve the guest name somehow from the original div.. Maybe trying to make things simpler I'm making them harder...
checcco
@checcco - You can set the HTML of that re-used div pretty easily, `$("#thatDiv").html($(this).html())` if that's what you're after, I'm not 100% clear from your comment.
Nick Craver