0

I'm adding a class with jQuery when a user click on a thumbnail to give an animation effect with the margin-top property but it doesn't seem to be working and i'm not sure why so i'm wondering someone could explain it to me. code below:

CSS:

.content {
  position: relative;
  width: 60%;
  height: auto;
  transition: all 200ms ease-in-out; }

  .content img {
    margin-top: -150px;
    width: 100%;
    height: auto;
    transition: all 900ms ease-in-out; }

    .content img.img-is-showing {
      margin-top: 0; }

JS:

$(document).ready(function(){

$('.lightbox-trigger').on('click', function(){
var image_href = $(this).attr('href');
$('.lightbox').addClass('is-showing');
$('.content img').addClass('img-is-showing');

$('.lightbox-image').attr('src', image_href);
$('.lightbox').show();

$('.lightbox').on('click', function(){
  $(this).removeClass('is-showing');
});

});
});

I have done the same type of animation using the opacity property on the ".lightbox" class and it's working fine but I'm not sure why the margin-top animation won't work.

The url for the site is Here

ironmike
  • 143
  • 2
  • 13

2 Answers2

3

I might be wrong here, but I feel the problem is related to the fact that you're adding the class .img-is-showing to the image before you're actually showing the lightbox.

lagboy
  • 94
  • 4
2

As stated in lagboy's answer, since the lightbox is hidden, adding .img-is-showing before the lightbox is shown won't visibly affect any positioning properties. Hence, no animation.

Additionally, you need to remove .img-is-showing when the image is dismissed if you want the animation to appear on subsequent clicks.

This gets it to work better:

$('.lightbox-trigger').on('click', function(){
  var image_href = $(this).attr('href');

  $('.lightbox').addClass('is-showing');
  $('.lightbox-image').attr('src', image_href);
  $('.lightbox').show();

  $('.content img').addClass('img-is-showing');

  $('.lightbox').on('click', function(){
    $(this).removeClass('is-showing');
    $('.content img').removeClass('img-is-showing');
  });
});

It definitely still needs some work, but at least you're getting your transition.

A few additional tweaks:

  • You keep on binding duplicate .lightbox click events with your removeClass handler. Consider cleaning them up by unbinding after triggering or binding outside of the body of the .lightbox-trigger click handler. It won't kill you at this scale, but it's messy and can cause trouble down the line.
  • Consider caching your more frequently used selectors (e.g. $('.lightbox')) outside of the event handler for a little performance boost. Again, small scale, but good habit.
Cole Kettler
  • 481
  • 4
  • 11
  • Thanks Cole! Works perfectly, I've learned what the problems is and you suggested future improvements, perfect answer to my question, I really appreciate it! I want to implement the tweaks you suggested however, I'm really new to jquery/JS so don't understand what you mean "binding duplicate .lightbox click events" and "Consider caching your more frequently used selectors" - could you possibly explain these to me a little further. Thanks again! – ironmike Sep 05 '15 at 16:39
  • 1
    @ironmike Sure - for caching, I just mean save the output to a variable in scope (i.e. within the `$(document).ready` callback), so you don't keep searching the DOM unnecessarily, and you can just refer to that variable. See [here](http://stackoverflow.com/a/24053931/2267428) for a quick explanation and a few pitfalls. For not binding duplicate events, see [`.one()`](http://api.jquery.com/one/), which aims to fix this sort of problem. Events bound with `.on()` stack. :) – Cole Kettler Sep 05 '15 at 18:54