3

I was wondering if anyone has any idea on how I could rewrite this simple jquery code to be more efficient. It's of course working fine now but I imagine adding say 10 more items would make the code really big. I thought maybe I could add the classes to arrays and use some kind of loop? Not sure if that's the right approach though.

Here it is on jsfiddle: http://jsfiddle.net/QVS9X/42/

and here's a sample of it: JS:

$(".image1").mouseout(function() {
    $(".default").show();
    $(".cat1").hide();
}).mouseover(function() {
    $(".default").hide();
    $(".cat1").show();
});

$(".image2").mouseout(function() {
    $(".default").show();
    $(".cat2").hide();
}).mouseover(function() {
    $(".default").hide();
    $(".cat2").show();
});

HTML:

<div class="image1 image">
    <p>Hover for cat 1</p>
</div>
<div class="image2 image">
    <p>Hover for cat 2</p>
</div>
<div class="image3 image">
    <p>Hover for cat 3</p>
</div>
<div class="default">
    <p>Default Text</p>
</div>
<div id="cats">
    <p class="cat1">Category 1 text</p>
    <p class="cat2">Category 2 text</p>
    <p class="cat3">Category 3 text</p>
</div>
Richard Dalton
  • 35,513
  • 6
  • 73
  • 91
Alex Berg
  • 747
  • 3
  • 8
  • 17
  • What does your HTML structure look like? – John Strickler Sep 28 '11 at 12:36
  • Hi, I've pasted it all to jsfiddle for your convenience. It's the link up there :) – Alex Berg Sep 28 '11 at 12:38
  • Your class names really looks like ids. You should try to create a binding between the elements somehow. +1 to Richard D's answer. – Jørgen Sep 28 '11 at 12:42
  • 1
    I included your HTML so that incase jsfiddle isnt around in the future your question is still useful. – Richard Dalton Sep 28 '11 at 12:44
  • As has been mentioned, the smartest way to optimize your code is to replace those single-use CSS classes (image1, cat1, image2, default, etc.) with IDs. – Blazemonger Sep 28 '11 at 12:57
  • Too many answers, so I'll leave a comment. Use `.each()` and take advantage of the index argument to associate an `.image` with a `.cat-n`. Cache your DOM selections with `.default` outside the loop, and each `.cat-n` cached in each respective closure. [jsFiddle example.](http://jsfiddle.net/NszP7/1/) So now you're not doing repeated DOM selection, and you've no need to *calculate* the index number. – user113716 Sep 28 '11 at 13:06
  • @ Ӫ_._Ӫ : actually, I'm gonna go with your suggestion. It was doing some weird stuff at start until I realized I didn't have the latest jQuery ver. I can't accept your answer if you won't post it though. – Alex Berg Sep 29 '11 at 07:48

9 Answers9

4

If you can put a class of image on the divs that current have image1, image2 etc, then you can do this:

$(".image").hover(function() {
    $(".default").toggle();
    $("#cats p").eq($(this).index()).toggle();
}); 

This assumes that the image divs will be in the same order as the p tags inside #cats.

http://jsfiddle.net/QVS9X/44/

Example using data attributes:

JS:

$(".image").hover(function() {
    $(".default").toggle();
    $($(this).data('id')).toggle()    
});  

HTML:

<div class="image" data-id="#cat1">
    <p>Hover for cat 1</p>
</div>
<div id="cats">
    <p id="cat1">Category 1 text</p>
</div>

http://jsfiddle.net/QVS9X/55/

Richard Dalton
  • 35,513
  • 6
  • 73
  • 91
  • np, if your divs and p tags aren't guaranteed to be in the same order I would recommend using a data attribute like Andys answer. – Richard Dalton Sep 28 '11 at 12:50
2
var myDefault = $(".default");
var myCat1 =    $(".cat1");
var myCat2 =    $(".cat2");

and then

$(".image1").mouseout(function() {
    myDefault.show();
   myCat1.hide();
    }).mouseover(function() {
        myDefault .hide();
        myCat1 .show();
 });

$(".image2").mouseout(function() {
    myDefault.show();
    myCat2.hide();
    }).mouseover(function() {
       myDefault.hide();
        myCat2.show();
});

will reduce dom traversal, and improve performance

Community
  • 1
  • 1
NimChimpsky
  • 46,453
  • 60
  • 198
  • 311
  • Hi, well although that's neater since it is using variables, it doesn't really shorten the total length of the code. – Alex Berg Sep 28 '11 at 12:40
2

http://jsfiddle.net/QVS9X/45/ for example

<div class="image" data-id="1">
  <p>Hover for cat 1</p>
</div>
<div class="image" data-id="2">
  <p>Hover for cat 2</p>
</div>
<div class="image" data-id="3">
  <p>Hover for cat 3</p>
</div>
<div class="default">
  <p>Default Text</p>
</div>
<div id="cats">
  <p class="cat1">Category 1 text</p>
  <p class="cat2">Category 2 text</p>
  <p class="cat3">Category 3 text</p>
</div>

$(document).ready(function() {

   $(".image").mouseout(function() {
        $(".default").show();
        $(".cat"+$(this).data('id')).hide();
   }).mouseover(function() {
            $(".default").hide();
            $(".cat"+$(this).data('id')).show();
   });

});
Andy
  • 29,707
  • 9
  • 41
  • 58
1

First, I'd wrap the target divs in a parent div if they aren't already contained - like such:

<div id="images">
  <div class="image1">
    <p>Hover for cat 1</p>
  </div>

   etc..
</div>

Then you can bind a single event handler to handle all your cases -

$('#images').delegate('div[class^=image]', 'mouseenter mouseleave', function(e) {
   $('.default').toggle(e.type === 'mouseleave');
   $('#cats p').eq($(this).index()).toggle(e.type !== 'mouseleave');
});

EDIT

You can actually clean it up much further by removing the "image1", "image2", "image3" and just name the class "image". You can also remove the class from the <p> in #cats

See here for the revised example - http://jsfiddle.net/QVS9X/50/

EDIT 2

Someday, you'll add a cat div dynamically and wonder why the event handler isn't working... then you'll revisit my answer :) And be like... John really had a 100% complete solution. =p

John Strickler
  • 25,151
  • 4
  • 52
  • 68
0

Something along the lines of this should work:

<div class="default">Content</div>
<img class="image" data-ref="1" />
<div class="cat" data-ref="1">Cat 1</div>
<img class="image" data-ref="2" />
<div class="cat" data-ref="2">Cat 2</div>

$('.image').hover(function() {
    $('.default, .cat[data-ref='+$(this).data('ref')+']').toggle();
});

http://jsfiddle.net/cugmj/2/

Petah
  • 45,477
  • 28
  • 157
  • 213
0

I would say you should make use of two things

  1. Use of data- attributes and jQuery's .data method
  2. A generic class used to identify all images that you want to have this behaviour

Then your code for all those items becomes:

$(".image").mouseout(function() {
        $(".default").show();
        $(".cat" + $(this).data('id')).hide();
        }).mouseover(function() {
            $(".default").hide();
            $(".cat" + $(this).data('id')).show();
           });

Live example: http://jsfiddle.net/sW8mZ/

Jamiec
  • 133,658
  • 13
  • 134
  • 193
0

Here is my code. http://jsfiddle.net/QVS9X/47/

examples:

$(document).ready(function() {
$('.image').hover(
    function(){
        $('.default p').html($(this).children('p').html());
    }, 
    function(){$('.default p').html('Default Text')}); 
});

Get rid of all the cat divs, have 1 div with default text, and then just update this text.

Tim B James
  • 20,084
  • 4
  • 73
  • 103
0

Here's my attempt, moving the category text to the div elements makes things a little easier.

http://jsfiddle.net/HV8vq/

* {
    font-size: 20px;
    text-align: center;
}
.image-cat-hover {
    float: left;
    width: 150px;
    border: thin solid #CCC;
    margin-bottom: 20px;
}
#cats, .default { 
    width: 450px;
}

<div class="image-cat-hover" data-category="Category 1 text">
  <p>Hover for cat 1</p>
</div>
<div class="image-cat-hover" data-category="Category 2 text">
  <p>Hover for cat 2</p>
</div>
<div class="image-cat-hover" data-category="Category 3 text">
  <p>Hover for cat 3</p>
</div>
<div class="default">
  <p id="placeholder">Default Text</p>
</div>

$(document).ready(function() {
    var defaultText = $(".default").text();
    $(".image-cat-hover").mouseout(function() {
        $(".default").text(defaultText);
        }).mouseover(function() {
            $(".default").text($(this).data("category"));
           });

});

Edit

I have to say I've learned alot answering and reading the other answers to this question. Here is an updated response:

http://jsfiddle.net/HV8vq/3/

<div>
  <p id="defaultText">Default Text</p>
  <p id="placeholder" style="display:none;"></p>
</div>

$(document).ready(function() {
    $(".image-cat-hover").hover(function() {
        $("#defaultText").toggle();
        $("#placeholder").toggle().text($(this).data("category"));
    });
});
asawyer
  • 17,642
  • 8
  • 59
  • 87
  • This looks really good as well. It's not validating against w3 though because of the custom div attribute. :( – Alex Berg Sep 28 '11 at 12:55
  • @Alex Berg I was unaware you could use `data-xxx` attributes until this question. That is awesome, and I'll edit here. Also about a dozen places in a recent project at work.... – asawyer Sep 28 '11 at 12:57
0

Something like this http://jsfiddle.net/QVS9X/51/

$(document).ready(function() {

    var defaultCat = $(".default");
    var cat1 = $(".cat1");
    var cat2 = $(".cat2");
    var cat3 = $(".cat3");

    $(".image").mouseout(function() {
        cat1.hide();
        cat2.hide();
        cat3.hide();
        defaultCat.show();
    });

    $(".i1").mouseover(function() {
        cat1.show();
        defaultCat.hide();
    });
    $(".i2").mouseover(function() {
        cat2.show();
        defaultCat.hide();
    });
    $(".i3").mouseover(function() {
        cat3.show();
        defaultCat.hide();
    });
});
Frederiek
  • 1,605
  • 2
  • 17
  • 32