current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I have made some jQuery Fallback Support for the CSS Property "background-attachment: local" and I am mainly a CSS developer. I do some jQuery, but I thought I would check to see if I am doing this in the more efficient way, as I may have multiple versions of this on one page.

CODEPEN DEMO

HTML

<div class="container">
  <div class="data-holder">
    <div id="shadowtop"></div>
    <div id="shadowbottom"></div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
    <div class="block">hello</div>
  </div>
</div>

CSS

@import "http://fonts.googleapis.com/css?family=Open+Sans:400,600,700";
* {
    -moz-box-sizing: border-box;
    -webkit-box-sizing: border-box;
    box-sizing: border-box;
}
body {
    background: none repeat scroll 0 0 #fff;
    font-family: "Open Sans";
    line-height: 26px;
    margin: 20px;
}

.container {
  position: relative;
  margin-top: 30px;
}

.data-holder {
  border-top: 1px solid #eee;
  border-bottom: 1px solid #eee;
  width: 200px;
  height: 300px;
  overflow: auto;
}

#shadowtop,
#shadowbottom {
  position: absolute;
  left: 0;
  height: 12px;
  width: 180px;
  z-index: 9999;
  display: none;
  background-size: 100% 12px;

}
#shadowtop {
  top: 0;
  background:   radial-gradient(farthest-side at 50% 0, rgba(0,0,0,.15), rgba(0,0,0,0)) 0 100%;
}
#shadowbottom {
  bottom: 0;
  background:   radial-gradient(farthest-side at 50% 100%, rgba(0,0,0,.15), rgba(0,0,0,0)) 0 100%;
}

.block {
  background: #fff;
  border-bottom: 1px solid #f4f4f4;
  padding: 10px;
}
.block:last-child {
  border-bottom: 0 none;
}

jQuery

$(document).ready(function(){
  $('.data-holder').scroll(function(){
    var scrollValue = $(this).scrollTop();
    var elementHeight = $(this).height();
    if(scrollValue == 0){
      $("#shadowtop").fadeOut(200);
      console.log("at top");  //should remove
    } else if (scrollValue == ($(this).get(0).scrollHeight - $(this).height())){
      $("#shadowbottom").fadeOut(200);
      console.log("at bottom"); //should remove
    } else {
      console.log("in middle"); //should remove
      $("#shadowtop").fadeIn(200);
      $("#shadowbottom").fadeIn(200);
    }
  });
  var scrollValue = $('.data-holder').scrollTop();
  if(scrollValue < ($('.data-holder').get(0).scrollHeight - $('.data-holder').height())){
     $("#shadowbottom").show();
  }
});
share|improve this question
add comment

1 Answer

up vote 2 down vote accepted
+50

This code annoyed me for a while, the part outside of the initialization does not belong and it seemed silly to add it to the listener. Plus, the code outside of the listener will mess up when you have multiple data-holder elements.

Then I realized that I could take a different approach, do this in 2 steps:

  1. Determine whether I should hide or show the top
  2. Determine whether I should hide or show the bottom

Then, I do not need the piece of code outside of the listener, and it becomes cleaner. Also, the 200 has to be a constant, also I would take only the #shadowbottom and #shadowtop belonging to the .data-holder.

So, in all, I would counter propose:

$(document).ready(function(){
  var fadeSpeed = 200;
  $('.data-holder').scroll(function(){
    var scrollValue = $(this).scrollTop(),
        elementHeight = $(this).height(),
        $top = $(this).children('#shadowtop'),
        $bottom = $(this).children('#shadowbottom'),
        atTop = (scrollValue == 0),
        atBottom = (scrollValue == ( this.scrollHeight - elementHeight));
    (atTop)   ? $top.fadeOut( fadeSpeed )    : $top.fadeIn( fadeSpeed );
    (atBottom)? $bottom.fadeOut( fadeSpeed ) : $bottom.fadeIn( fadeSpeed );
  }).scroll();
});

I was mildly worried that with each scroll event the top / bottom might flicker because of the repeated fadeIn() calls, but I can tell from your CodePen that does not happen, so there is nothing to worry about there.

Finally, I did not look at your CSS as you actually are a CSS developer and also I did not test my code, but I am confident you get where I am going with this.

share|improve this answer
    
Very nice answer. Thank you. codepen.io/2ne/pen/gELai works here with 2 elements and it is a lot easier to see how your code is done. More universal –  Parallel 2ne Jan 24 at 20:34
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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