Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

I agree with the majority of what Michael KMichael K said. And I'd also do the following:

$('.footnote[id^="ret_"]').removeAttr('title').removeAttr('alt').mouseenter(function(e) {
    var footnote = $('#footnote_cont_' + this.id.substring(4)).html();  
    $footnoteTooltip.stop(true, false);

    //only completely hide and change text/position if we are hovering over a different footnote
    if($footnoteTooltip.html() != footnote) {
        $footnoteTooltip.hide().html(footnote).css({ left: e.pageX + 10, top: e.pageY + 15});
    }
    
    $footnoteTooltip.fadeTo(fadeTime, opacity);
}).mouseleave(function() {
    $footnoteTooltip.delay(fadeTime).fadeOut(fadeTime);
});
  • Personally I find the newlines on the chained removeAttr and mouseenter really interrupts the flow of the method, so I'd remove those.
  • I would also add braces { } after the single line if statement (that's more of a preference so that I don't get bit by subtle bugs.)
  • And like Michael K, I would line up the }).mouseLeave() call on the same line as the end of the block - however my treatment of the first line changes where that alignment occurs.

I agree with the majority of what Michael K said. And I'd also do the following:

$('.footnote[id^="ret_"]').removeAttr('title').removeAttr('alt').mouseenter(function(e) {
    var footnote = $('#footnote_cont_' + this.id.substring(4)).html();  
    $footnoteTooltip.stop(true, false);

    //only completely hide and change text/position if we are hovering over a different footnote
    if($footnoteTooltip.html() != footnote) {
        $footnoteTooltip.hide().html(footnote).css({ left: e.pageX + 10, top: e.pageY + 15});
    }
    
    $footnoteTooltip.fadeTo(fadeTime, opacity);
}).mouseleave(function() {
    $footnoteTooltip.delay(fadeTime).fadeOut(fadeTime);
});
  • Personally I find the newlines on the chained removeAttr and mouseenter really interrupts the flow of the method, so I'd remove those.
  • I would also add braces { } after the single line if statement (that's more of a preference so that I don't get bit by subtle bugs.)
  • And like Michael K, I would line up the }).mouseLeave() call on the same line as the end of the block - however my treatment of the first line changes where that alignment occurs.

I agree with the majority of what Michael K said. And I'd also do the following:

$('.footnote[id^="ret_"]').removeAttr('title').removeAttr('alt').mouseenter(function(e) {
    var footnote = $('#footnote_cont_' + this.id.substring(4)).html();  
    $footnoteTooltip.stop(true, false);

    //only completely hide and change text/position if we are hovering over a different footnote
    if($footnoteTooltip.html() != footnote) {
        $footnoteTooltip.hide().html(footnote).css({ left: e.pageX + 10, top: e.pageY + 15});
    }
    
    $footnoteTooltip.fadeTo(fadeTime, opacity);
}).mouseleave(function() {
    $footnoteTooltip.delay(fadeTime).fadeOut(fadeTime);
});
  • Personally I find the newlines on the chained removeAttr and mouseenter really interrupts the flow of the method, so I'd remove those.
  • I would also add braces { } after the single line if statement (that's more of a preference so that I don't get bit by subtle bugs.)
  • And like Michael K, I would line up the }).mouseLeave() call on the same line as the end of the block - however my treatment of the first line changes where that alignment occurs.
Source Link

I agree with the majority of what Michael K said. And I'd also do the following:

$('.footnote[id^="ret_"]').removeAttr('title').removeAttr('alt').mouseenter(function(e) {
    var footnote = $('#footnote_cont_' + this.id.substring(4)).html();  
    $footnoteTooltip.stop(true, false);

    //only completely hide and change text/position if we are hovering over a different footnote
    if($footnoteTooltip.html() != footnote) {
        $footnoteTooltip.hide().html(footnote).css({ left: e.pageX + 10, top: e.pageY + 15});
    }
    
    $footnoteTooltip.fadeTo(fadeTime, opacity);
}).mouseleave(function() {
    $footnoteTooltip.delay(fadeTime).fadeOut(fadeTime);
});
  • Personally I find the newlines on the chained removeAttr and mouseenter really interrupts the flow of the method, so I'd remove those.
  • I would also add braces { } after the single line if statement (that's more of a preference so that I don't get bit by subtle bugs.)
  • And like Michael K, I would line up the }).mouseLeave() call on the same line as the end of the block - however my treatment of the first line changes where that alignment occurs.