2
\$\begingroup\$

I'm trying to figure out how to correctly hover over an image to fade in another image and when you move your mouse out, it would go fade back to the normal image.

$(document).ready(function(){

$('img').hover(
    function(){
        $(this).stop().fadeOut(function(){
        var $temp = $(this).attr('src');
        $(this).attr('src', $(this).attr('data-alt-src'));
        $(this).attr('data-alt-src', $temp);
        });

        $(this).fadeIn();
    },
    function(){
        $(this).stop().fadeOut(function(){
        var $temp = $(this).attr('data-alt-src');
        $(this).attr('data-alt-src', $(this).attr('src'));
        $(this).attr('src', $temp);
        });

        $(this).fadeIn();

    }
);
});
#main-wrapper {
width: 970px;
margin: 0 auto;
text-align: center;
}

img {
width: 200px;
}
<!DOCTYPE html>
<html>
<head>
    <title>Hover In/Out</title>
    <meta charset="UTF-8">
    <link rel="stylesheet" href="styles.css">
    <script src= 'http://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js'></script>
    <script src="script.js"></script>
</head>
<body>
    <div id="main-wrapper">
        <img src="frontside.jpg" data-alt-src="backside.jpg" alt="front side picture">
        <img src="frontside.jpg" data-alt-src="backside.jpg" alt="front side picture">
        <img src="frontside.jpg" data-alt-src="backside.jpg" alt="front side picture">
        <img src="frontside.jpg" data-alt-src="backside.jpg" alt="front side picture">
    </div>
</body>
</html>

So far, it works as it should with one minor defect. If I were to quickly hover my mouse over the image and out, it would stay in the second image. I initially wanted to hover over the image to show the other image and then when I hover out from the image it would go back to its initial image.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. Code Review is not the site to ask for help in fixing or changing what your code does. Once the code does what you want, we would love to help you do the same thing in a cleaner way! Please see our help center for more information. \$\endgroup\$ – Mast Jun 17 '15 at 12:34
  • 1
    \$\begingroup\$ it works as it should with one minor defect I would leave this open, just we can't help you fix that minor defect. \$\endgroup\$ – Caridorc Jun 17 '15 at 12:50
  • \$\begingroup\$ It is not clean enough, what you want to do? \$\endgroup\$ – outoftime Jun 17 '15 at 15:46
  • \$\begingroup\$ Can you wrap image with additional div container and set background-image for it? \$\endgroup\$ – outoftime Jun 17 '15 at 15:53
  • \$\begingroup\$ I can't understand the functioning of this without an image. Can you please use images from placehold.it or placekitten.com or even lorempixel.com ? \$\endgroup\$ – Ismael Miguel Jul 5 '15 at 19:58
3
\$\begingroup\$

You need to fix your indentation.

Code inside blocks gets one more level of indentation.

Here is what your indented JavaScript would look like:

$(document).ready(function () {

    $('img').hover(
    function () {
        $(this).stop().fadeOut(function () {
            var $temp = $(this).attr('src');
            $(this).attr('src', $(this).attr('data-alt-src'));
            $(this).attr('data-alt-src', $temp);
        });

        $(this).fadeIn();
    },

    function () {
        $(this).stop().fadeOut(function () {
            var $temp = $(this).attr('data-alt-src');
            $(this).attr('data-alt-src', $(this).attr('src'));
            $(this).attr('src', $temp);
        });

        $(this).fadeIn();

    });
});

Your CSS also has poor indentation.

Here is your indented CSS:

#main-wrapper {
    width: 970px;
    margin: 0 auto;
    text-align: center;
}

img {
    width: 200px;
}

In the two functions that you are passing to $('img').hover, you are doing pretty much the same thing. I recommend creating a separate function that takes two parameters for the data to swap.

Here is what I came up with:

/*
    Sets the .stop().fadeOut of an element to swap two properties of that element
    @param(jQuery Object) elem -- the element
    @param(string) oldSrc -- one element to swap
    @param(string) newSrc -- another element to swap
*/
function fadeSwapSrc(elem, oldSrc, newSrc) {
    elem.stop().fadeOut(function() {
        var $temp = elem.attr(oldSrc);
        elem.attr('src', elem.attr(newSrc));
        elem.attr(newSrc, $temp);
    }
}

Now, your code simply becomes:

$(document).ready(function () {

    $('img').hover(
    function () {
        fadeSwapSrc('src', 'data-alt-src');
        $(this).fadeIn();
    },

    function () {
        fadeSwapSrc('data-alt-src', 'src');
        $(this).fadeIn();
    });
});
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Indentation has got absolutely nothing to do with the problem or solution and only detracts from the solution. \$\endgroup\$ – JBES Sep 11 '19 at 14:37
2
\$\begingroup\$

I feel I should point out that this can be done purely with CSS. Along with removing the need for javascript, this also has the benefit of fixing that "one minor defect".

Here's some explanation of what is going on here.

In the CSS, the lines about transition (with and without browser prefixes), handle the transition of the default image to the hover image, giving it the fade effect.

To have this happen smoothly on the first time they're hovered, I've added the hover images to the dom to pre-load them so that they will already be loaded and cached in the elements with the .imgpl class on them. These are hidden with CSS--they only serve to load the hover images. doing this is optional, but I feel it gives a better user experience. Without it, the browser has to load the image the first time it is hovered, which leads to a delay in the display.

The default image is defined as the background image,

.img1{background:url("http://www.placehold.it/350x150") no-repeat;}

And the hover images are defined with the :hover pseudo class:

.img1:hover{background:url("http://www.placehold.it/351x151")no-repeat;}

Run the snippet below to see it work.

.img{
    -webkit-transition: background 500ms ease-in-out;
    -moz-transition: background 500ms ease-in-out;
    -ms-transition: background 500ms ease-in-out;
    -o-transition: background 500ms ease-in-out;
    transition: background 500ms ease-in-out;
    width:350px;
    height:150px;
    overflow:hidden;/*just for the demo sake*/
}
/* these are just placeholders to preload the images */
.imgpl{
    display:none;
}

.img1{
    background:url("http://www.placehold.it/350x150") no-repeat;
}
.img1:hover, .img1.imgpl{
    background:url("http://www.placehold.it/351x151") no-repeat;
}
.img2{
    background:url("http://www.placehold.it/352x152") no-repeat;
}
.img2:hover, .img2.imgpl{
    background:url("http://www.placehold.it/353x153") no-repeat;
}
.img3{
    background:url("http://www.placehold.it/354x154") no-repeat;
}
.img3:hover, .img3.imgpl{
    background:url("http://www.placehold.it/355x155") no-repeat;
}
.img4{
    background:url("http://www.placehold.it/356x156") no-repeat;
}
.img4:hover, .img4.imgpl{
    background:url("http://www.placehold.it/357x157") no-repeat;
}
<div id="main-wrapper">
    <div class="img img1"></div>
    <div class="img img2"></div>
    <div class="img img3"></div>
    <div class="img img4"></div>
</div>

<!-- these are optional for pre-loading the hover images, they don't display -->
<div class="img1 imgpl"></div>
<div class="img2 imgpl"></div>
<div class="img3 imgpl"></div>
<div class="img4 imgpl"></div>

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ It would be better if you could further describe your implementation so that readers can more clearly understand why and how this is better than the OP's solution. \$\endgroup\$ – glampert Jul 5 '15 at 19:49
  • \$\begingroup\$ I would upvote this if it had an explanation built in (and yes, I know what it does myself, but it's just the way CR works). \$\endgroup\$ – user34073 Jul 5 '15 at 21:44
  • \$\begingroup\$ @glampert -- good point, I added some explanation. If there's more you think I should add, let me know. \$\endgroup\$ – Ted Jul 6 '15 at 15:20
  • \$\begingroup\$ @Hosch250 as mentioned, I added some explanation. If there's more you think I should add, let me know. \$\endgroup\$ – Ted Jul 6 '15 at 15:21
  • \$\begingroup\$ This is good now. \$\endgroup\$ – user34073 Jul 6 '15 at 15:51
0
\$\begingroup\$

You can use 2 custom attributes instead of one. In this case you should not swap src value, just replace it.

<img src="some/img.jpg" 
     data-foreground-src="some/img.jpg" 
     data-background-src="some/other/img.jpg">

UPD: With single data- attribute you should create singleton, which will handle synchronization. I had try to draft singleton, but it is not so easy for me. It should be responsible for removing duplicates from task queue and watching on data consistency.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I see your explanation, however is it bad practice to add more code to the html? \$\endgroup\$ – Tony Tai Nguyen Jun 18 '15 at 0:37
  • \$\begingroup\$ @TonyTaiNguyen otherwise you should add twice more code to js, which is worse, as became too sophisticated and hard to change and maintain. \$\endgroup\$ – outoftime Jun 18 '15 at 2:37
0
\$\begingroup\$

Try creating a parent element container for pairs of "front" , "back" image img elements ; set images within image container to position:abosolute ; call .fadeToggle() on last image within image container

$(document).ready(function() {

  var fx = function fx() {
    return $("img:last", this).fadeToggle();
  };

  $(".img-wrapper").hover(fx, fx);

});
#main-wrapper {
  width: 970px;
  margin: 0 auto;
  text-align: center;
}
img {
  width: 200px;
}
.img-wrapper {
  width: 200px;
  height: 200px;
  position: relative;
  display: inline-block;
}
.img-wrapper img {
  display: block;
  position: absolute;
}
.img-wrapper:hover {
  cursor: wait;
}
<!DOCTYPE html>
<html>

<head>
  <title>Hover In/Out</title>
  <meta charset="UTF-8">
  <script src='http://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js'></script>
</head>

<body>
  <div id="main-wrapper">
    <div class="img-wrapper">
      <img src="http://lorempixel.com/200/200/city" alt="front side picture">
      <img src="http://lorempixel.com/200/200/cats" alt="back side picture">
    </div>
    <div class="img-wrapper">
      <img src="http://lorempixel.com/200/200/nature" alt="front side picture">
      <img src="http://lorempixel.com/200/200/technics" alt="back side picture">
    </div>
  </div>
</body>

</html>

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.