CODEWITHSUNDEEP
jqueryeach
Toki
Toki

Reputation: 587

Trouble converting a simple one off jQuery function to each

This should be a reasonably simple one - I have written a very basic script that detects when an image with specific class name appears on the screen via scroll. When the image appears, a class is added to it which triggers opacity and movement CSS animation.

This is all working fine, but I want this to work for several images on the same page. I have tried using each() both inside and outside of the scroll function and it breaks the script. I believe I am on the right track, but the variables used to detect when the image appears on screen are throwing me off I think, as I can't use $(this) as I normally would with an each() function.

jQuery Script is here:

  // Setup
  $(".appear").addClass("off");

  objectOffset = $(".appear").offset().top;
  winHeight = $(window).height();
  trigger =  objectOffset - winHeight;

  $(window).scroll(function() {
    if ($(window).scrollTop() > trigger) {

      $(".appear").removeClass("off");
      $(".appear").addClass("on");

    }

  });

JS Fiddle of the original, functioning script ( works only on the first image ):

https://jsfiddle.net/gv6qzxph/

Many thanks guys.

Edited to included broken attempt:

  // Setup
  $(".appear").addClass("off");

  $(".appear").each(function() {

    objectOffset = $(this).offset().top;
    winHeight = $(window).height();
    trigger =  objectOffset - winHeight;

    $(window).scroll(function() {
        if ($(window).scrollTop() > trigger) {

        $(this).removeClass("off");
        $(this).addClass("on");

        }

    });

  });

Upvotes: 0

Views: 42

Answers (2)

CreativeCreate
CreativeCreate

Reputation: 196

Not sure you really need this improvement. Either way, I had lil time and played with it :) and here it is.

  1. Use jQuery to display/hide w/o using classes.

  2. HIDE (back to opacity:0) when scroll up.

    $(window).scroll(function(){
    var winHeight = $(window).height();
    var scrollTop = $(window).scrollTop();

    $('.appear').css('opacity',function(){            // change opacity only on the 'appeared' items
        elmTop        = $(this).offset().top;
        scrollNeeded  = elmTop - winHeight;           // scrolling needed for $(this) to appear     
        return (scrollTop > scrollNeeded )? 1 : 0 ;   // if $(this) appear on the screen return 1 else 0
    });

    });

See Demo

Upvotes: 1

nizzik
nizzik

Reputation: 826

On your first attempt you have only defined trigger for your first <div>. In the second attemt you are trying to bind scroll within loop it would be rather killing the performance. What you need is to check if your element is visibile when scrolled. Please find working version below

  // Setup
  $(".appear").addClass("off");

  $(window).scroll(function() {

        $('.appear').each(function() {
            if(isVisible(this)){
                $(this).removeClass('off').addClass('on');
          }
      });

  });

  function isVisible(elem) {
    var $elem = $(elem);
    var $window = $(window);

    var elemTop = $elem.offset().top;
    var docViewTop = elemTop - $window.height();

    return $window.scrollTop() > docViewTop;
  };

And working jsfiddle here: https://jsfiddle.net/swnuro9z/1/

Cheers

Upvotes: 0

Related Questions