6
\$\begingroup\$

I'm developing a publishing engine and trying to find better ways to optimize the code. The end result is to have an open-source alternative to publish content on mobile devices.

For now, though, I have a touch-scroll-slide system I have created from a modified scroll system found elsewhere. There is a peculiar hesitation while doing slides (horizontally) on a mobile device (mostly iPad 1) while in portrait, yet the landscape version works very well. It is jQuery-based.

I realize there are other portions of the larger project which may be dragging the behaviour down, but don't want to overwhelm people with code at this time. The end result (so far) is viewable here.

The code function for scroll-slide is here:

    (function($){
 $.fn.jScrollTouch = function () {
     return this.each(function() {
                      // The two concepts for each page
                      // scroller = text scrolling area
                      // slider = full-screen page
                      //previous = the page before the current
                      // next = the page after the current

              var scroller =    $(this).find('.article-scroller');
              var slider =      $(this);
              var previous =    slider.prev();
              var next =        slider.next();

              var slideOn = true;
                if (slider.attr('id')=='overlayed'){ slideOn = false;
                      //console.log(slider.attr('id'))
                      };

              scroller.css({'overflow': 'auto','position':'relative'});               
              /*
               if (mobile) {
               //console.log('touch only');
               scroller.css({'overflow': 'hidden','position':'relative'});
               } else {
               //console.log('desk only');
               scroller.css({'overflow': 'auto','position':'relative'});
               }*/

              var height = 0;
              var cpos = scroller.scrollTop()
              scroller.scrollTop(100000);
              height = scroller.scrollTop();
              scroller.scrollTop(cpos);
              var fullheight = height + scroller.outerHeight();
              var scrollbarV_length = scroller.innerHeight()*(scroller.innerHeight()/fullheight)+2;

              var width = 0;
              minWidth=($("body").width())*0.15;
              var lpos = scroller.scrollLeft();
              scroller.scrollLeft(100000);
              width = scroller.scrollLeft();
              scroller.scrollLeft(lpos);
              var fullwidth = width + scroller.outerWidth();
              var scrollbarH_length = scroller.innerWidth()*(scroller.innerWidth()/fullwidth)+2;

              if(mobile){
                  var scrollbarV = $('<div></div>');
                  scrollbarV.css({  'display':'none',
                                    'position':'absolute',
                                    'width':'5px',
                                    'height':scrollbarV_length+'px',
                                    'left':width-10+'px',
                                    'top':0,'background':'black',
                                    'border':'1px white solid',
                                    '-webkit-border-radius':'5px',
                                    'opacity':'0.9' });

                  var scrollbarH = $('<div></div>'); //not used
                      scrollbarH.css({  'display':'none',
                                        'position':'absolute',
                                        'height':'5px',
                                        'width':scrollbarH_length+'px',
                                        'top':scroller.innerHeight()-7+'px',
                                        'left':0,'background':'black',
                                        'border':'1px white solid',
                                        '-webkit-border-radius':'5px',
                                        'opacity':'0.9'});

                  if(height) scroller.append(scrollbarV);
                  if(width) scroller.append(scrollbarH);
              }
                      // these two chceks are to prevent sliding if
                      // slider is animated or it is overlayed image
        currentPage = JSON.parse(localStorage.getItem('currentPage'));
        $remover =  $('.article-wrapper:eq(' + currentPage + ')');
                      $remover.stop();
          if (!$remover.is(':animated')) {

              slider.bind('mousedown touchstart',function(e){


                  width=$("body").width();
                  height=$("body").height();

                  if(mobile){
                      e = e.originalEvent.touches[0];
                      scrollbarV.show();
                      scrollbarH.show();
                  }

                  if (slideOn==true){         
                      $('.active').toggleClass('active');
                      slider.stop(false, true).addClass('active').removeClass('hidder');
                  }

                  $('.article-wrapper').not('.active').addClass('hidder');
                          if(previous.is(':animated')){
                  previous.stop(true, true).removeClass('hidder');
                          }
                          if (next.is(':animated')){
                  next.stop(true, true).removeClass('hidder');
                          }
                          //  previous.css('style','');
                          //      next.css('style','');

                  var initX = e.pageX;
                  var sX = e.pageX;
                  var sWX = 0;

                  var initY = e.pageY;
                  var sY = e.pageY;
                  var sWY = 0;

                  var prevsWX = 0;  //stores the previous sWX

                  var scrollDirection = 0;
                  var nextpage = 0;

                  var display = false;
                  var displayed = false;

                  cpos = scroller.scrollTop();

                  slider.bind('mousemove touchmove ',function(ev){
                      if(mobile){
                          ev.preventDefault();
                          ev = ev.originalEvent.touches[0];
                      } 

                              //      console.log('started');
                              //    currentPage=parseInt(slider.attr("id"));
                      var top = cpos-(ev.pageY-sY);

                      var left =  lpos-(ev.pageX-sX);

                      sWX = sWX-(sX-ev.pageX);
                      sX = ev.pageX;

                      sWY = sWY-(sY-ev.pageY);
                      sY = ev.pageY;

                      var horDistance = Math.abs(sWX);
                      var verDistance = Math.abs(sWY);

                      if (scrollDirection ==0){ // haven't checked direction yet
                          if ( verDistance < horDistance) {
                              scrollDirection = 1; // moving horizontally
                          } else if ( verDistance > horDistance) {                    
                              scrollDirection = 2; // moving vertically
                          }
                      }
                      if (scrollDirection == 2 ){                       //set up the scrolling movement
                          //set up the scroll bars
                          scroller.scrollTop(top);
                          cpos = scroller.scrollTop(); 
                          sY = ev.pageY;

                          if(mobile){
                          scrollbarV.css({  'left':Math.min(scroller.innerWidth()-7+lpos,fullwidth)+'px',
                                            'top':Math.min(cpos+cpos*scroller.innerHeight()/fullheight,fullheight-scrollbarV_length)+'px'});
                          scrollbarH.css({  'top':Math.min(scroller.innerHeight()-7+cpos,fullheight)+'px',
                                            'left':Math.min(lpos+lpos*scroller.innerWidth()/fullwidth,fullwidth-scrollbarH_length)+'px'});
                          }
                      }

                      if (scrollDirection == 1 && slideOn==true){                       //set up the sideways movement

                              //  console.log('continuing');
                              slider.css('left',sWX+'px');              //page follows finger
                              //    unused for now
                              //    previous.css('left',(sWX-width)+'px');  //moves prev with the top
                              //    next.css('left',(sWX+width)+'px');      //moves next with the top

                              if (!((Math.abs(prevsWX)/prevsWX) == (Math.abs(sWX)/sWX))) {
                              //    checking to see if we are going in the same direction still

                                  if (sWX<0) {                          //moving right
                                      display = true;
                                      nextPage = currentPage + 1;
                                      if (nextPage > maxPage+1){        //don't display any new pages
                                          nextPage = maxPage+1;
                                          display = false;
                                      } else {          
                                        //      display right page under the current page
                                        //      hide the previous page, show the next page

                                        previous.addClass('hidder');
                                        next.css('style','').removeClass('hidder');
                                        display = true;                      
                                        } 
                                    } 

                                  if (sWX>0) {                          //moving Left
                                      display = true;
                                      nextPage = currentPage - 1;
                                      if (nextPage< 0 ){              
                                      nextPage = 0;
                                          display = false;
                                      }  else{          
                                            //      display left page under the current page
                                            //      hide the next page, show the previous page

                                        next.addClass('hidder');    
                                        previous.css('style','').removeClass('hidder');
                                        display = true;
                                        } 
                                    } 
                              }
                              prevsWX = sWX;
                        } //    end of same direction check

                    }); //end of sideways scroll check

                    slider.bind('mouseup touchend',function(ev){    
                        slider.unbind('mousemove touchmove mouseup touchend');
                        display = false;

                                //  console.log('finished');
                        if(mobile){
                          ev = ev.originalEvent.touches[0];
                          scrollbarV.fadeOut();
                          scrollbarH.fadeOut();
                        }
                        if (scrollDirection ==1 && slideOn==true){      // only perform horizontal changes
                              //      sX = ev.pageX;
                              //      var ultimate = initX-sX;
                            var distance = Math.abs(initX-sX);

                            if (nextPage < 1){      
                                //  can't go past beginning
                              distance = 0; 
                            }
                            if (nextPage > maxPage){    
                                //  can't go past end
                              distance = 0; 
                            }
                            if (sWX == 0){
                              // do nothing
                            } else if ( sWX>0 ) {           //are we moving the page to the left?
                                if(distance>minWidth){ 
                                    //  move to new page if we moved 25% of the width
                                    slider.stop(true,true).animate({'left':width+'px'},animTime/2,'easeOutBounce',function(){
                                        previous.css('left','0').addClass('active');
                                        next.addClass('hidder');
                                        slider.removeClass('active').addClass('hidder').attr('style','');
                                        currentPage = nextPage;
                                        localStorage.setItem('currentPage',JSON.stringify(currentPage));
                                    });   
                                }else{                  //move back into place. Rehide the next/prev page afterwards
                                  slider.stop(true,true).animate({'left':0+'px'},100,'easeOutBounce',function(){
                                });
                            } // end of left check

                            } else if ( sWX<0 ){            //are we moving the page to the right?
                                  if(distance>minWidth){    
                                    //  move to new page if we moved more than 25%
                                    slider.stop(true,true).animate({'left':-width+'px'},animTime/2,'easeOutBounce',function(){
                                        next.css('left','0').addClass('active');
                                        slider.removeClass('active').addClass('hidder').attr('style','');
                                        previous.addClass('hidder');
                                        currentPage = nextPage;
                                        localStorage.setItem('currentPage',JSON.stringify(currentPage));
                                    });
                            }else{                  //move back into place. Rehide the next/prev page afterwards
                                  slider.stop(true,true).animate({'left':0+'px'},100,'easeOutBounce',function(){

                                }); 
                            } //end of right check
                        }// finished horizontal checks
                    }
                }); //end mouse/touch end
        }); //end mouse/touch start
      } // end of two checks (remover:animated & slideOn)
    }); //end return each(?) function?
 } //end 'inner' function
 })(jQuery); //  end it all…

Some simple caveats:

currentPage is a variable used to hold where a person was when they last opened the publication (the publication can be stored for offline reading).

A typical page consists of a background graphic and scrollable region which contains content. The function is called in this manner, where article-wrapper is a class applied to a page. Each page is required to have a unique ID, article1-article'n':

$('.article-wrapper').jScrollTouch();

There is also an overlay which behaves differently. It contains scrollable content but it shouldn't move horizontally.

The end result for the project is an online publishing system that is free, similar to WordPress but dedicated to creating a more formal publication rather than a blog. The immediate next step is better layout options through CSS and inclusion of HTML5's canvas animations.

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Style belongs to CSS

Avoid styling code in JavaScript, especially large blocks like this one:

  scrollbarV.css({  'display':'none',
                    'position':'absolute',
                    'width':'5px',
                    'height':scrollbarV_length+'px',
                    'left':width-10+'px',
                    'top':0,'background':'black',
                    'border':'1px white solid',
                    '-webkit-border-radius':'5px',
                    'opacity':'0.9' });

Such styling details belong to a CSS file. In JavaScript (and HTML) you should use appropriate class attributes instead.

Use consistent indentation

The indentation here is pretty nutty:

  $('.article-wrapper').not('.active').addClass('hidder');
          if(previous.is(':animated')){
  previous.stop(true, true).removeClass('hidder');
          }
          if (next.is(':animated')){
  next.stop(true, true).removeClass('hidder');
          }

Should have been:

  $('.article-wrapper').not('.active').addClass('hidder');
  if (previous.is(':animated')) {
      previous.stop(true, true).removeClass('hidder');
  }
  if (next.is(':animated')) {
      next.stop(true, true).removeClass('hidder');
  }

Btw the CSS class "hidder" looks odd too. Did you mean "hidden" instead?

The messy indentation style is rampant in the posted code, making it extremely difficult to read. Please review and correct everywhere.

Chaining with else-if

It seems these two condition cannot be true at the same time:

    if (nextPage < 1){      
        //  can't go past beginning
      distance = 0; 
    }
    if (nextPage > maxPage){    
        //  can't go past end
      distance = 0; 
    }

... in which case it would be better to use else if for the second.

Open-sourcing

An open-source project should be impeccable, to attract users and contributors. This code looks really messy, which gives the impression as if you don't care, and if you don't care, why should contributors care? Messy code is unlikely to attract supporters, but very easy to fix, so I urge you to do that.

\$\endgroup\$
0
3
\$\begingroup\$

First off, if you have lines of code that are commented out, you should probably either uncomment them, or remove them. Commented out code is an eyesore.

Secondly, some of your variable names are, not good. For example, I have no idea what the variable sX, sWX, or lpos do. Variable names should be not too long, but not too short, and be as descriptive as possible about the purpose of the variable.

A lot of your spacing is odd as well. For example, the block of code from your code below:

scrollbarH.css({  'display':'none',
                  'position':'absolute',
                  'height':'5px',
                  'width':scrollbarH_length+'px',
                                        'top':scroller.innerHeight()-7+'px',
                  'left':0,'background':'black',
                  'border':'1px white solid',
                  '-webkit-border-radius':'5px',
                  'opacity':'0.9'});

Would become the below block of code:

scrollbarH.css({  
    'display': 'none',
    'position': 'absolute',
    'height': '5px',
    'width': scrollbarH_length + 'px',
    'top': scroller.innerHeight() - 7 + 'px',
    'left': 0,
    'background': 'black',
    'border': '1px white solid',
    '-webkit-border-radius': '5px',
    'opacity': '0.9'
});

Generally, you should also have space between your operators. It usually makes expressions much easier to read as well.

Finally, I'd recommend finding a code style, and then writing in that style. I usually follow this style guide for how I write my Javascript code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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