5
\$\begingroup\$

I just ran into this question on Stack Overflow and there was some discussion in the comments about whether or not it would fit better on Code Review.

On SO it doesn't seem to fit in its current form, perhaps it would be a better fit for SO if it were written as a self answered community wiki... But in its current form, asking more or less "How can I further improve this working code?" seems more on-topic on Code Review.

I wanted to check my thinking and see how the community here felt about it.

The question:

I was looking for a function that would scroll a given element into view with some smart behavior:

  • if an element is descendant of a scrollable element - that ancestor is scrolled rather than body.

  • if an element is descendant of a positioned element - body won't be scrolled.

I didn't find any suitable function, so I made one and wanted some expert opinion on it. Please check the plunkr http://plnkr.co/edit/DNGWLh5cH1Cr1coZbwpa?p=preview . There are problems with animated scroll in FF, so please use Chrome to check the logic.

To illustrate, what I'm looking for - here is the first update that came to mind - if we reached an element that can scroll, lets call it SC (Scroll Parent), we should not only scroll SC to make the target visible inside it, but also recursively scroll SC itself into view, since it may outside of the currently visible are of the page. Here is the update plunkr http://plnkr.co/edit/DNGWLh5cH1Cr1coZbwpa?p=preview
(also applied fix for FF scrolling problem).

And here is the code of the function

  function scrollTo(target){
  //Position delta is used for scrollable elements other than BODY
  var combinedPositionDelta = 0;  
  var previousParent = $(target);
  var parent = $(target).parent();

  while(parent){

    combinedPositionDelta += previousParent.position().top - parent.position().top;

    //If we reached body
    if(parent.prop("tagName").toUpperCase() == "BODY"){
      scrollBody(target.offset().top);
      break;
    }

    //if we reached an element that can scroll
    if(parent[0].scrollHeight > parent.outerHeight()){
      scrollElementByDelta(parent,combinedPositionDelta);
      //Recursively scroll parent into view, since it itself might not be visible
      scrollTo(parent);
      break;
    }

    //if we reached a apositioned element - break
    if(parent.css('position').toUpperCase() != 'STATIC'){
      console.log("Stopping due to positioned parent " + parent[0].outerHTML);
      break;
    }

    previousParent = parent;
    parent = parent.parent();

  }

}

var offsetSkin = 20;
function scrollElementByDelta(element,offsetDelta){
   $(element).animate({
        scrollTop: element.scrollTop() + (offsetDelta - offsetSkin) 
    }, 1000);
}

function scrollBody(offset){
     $('body,html').animate({
        scrollTop: offset - offsetSkin
    }, 1000);
}

The current bounty notice:

I was looking for a good implementation of a JS-HTML "scroll given element into view" function, but the ones I found on SO were lacking in different aspects, so I would like to communally improve the one I made and hopefully share it with the next seeker.

\$\endgroup\$
10
\$\begingroup\$

We have a checklist for migration to Code Review.

This description…

I didn't find any suitable function, so I made one and wanted some expert opinion on it. Please check the plunkr http://plnkr.co/edit/DNGWLh5cH1Cr1coZbwpa?p=preview . There are problems with animated scroll in FF, so please use Chrome to check the logic.

… and the bounty notice…

I was looking for a good implementation of a JS-HTML "scroll given element into view" function, but the ones I found on SO were lacking in different aspects, so I would like to communally improve the one I made and hopefully share it with the next seeker.

… certainly indicate that that the author is seeking a code review. This question would be welcome on Code Review with no modifications.

However, I'm not sure that it is suitable for migration, because:

  • There is an existing code-only answer that would not meet Code Review's standards.
  • There is an active bounty on the question.

Therefore, I recommend cross-posting the question to Code Review rather than migrating it.

\$\endgroup\$
  • 3
    \$\begingroup\$ I thought cross posting was generally frowned on \$\endgroup\$ – apaul Aug 12 '14 at 16:53
  • 2
    \$\begingroup\$ @apaul34208: It is, primarily when it's done on the OP's own accord. If approved by the community, then it's okay. That would be okay here if the target site doesn't want any of the answers. \$\endgroup\$ – Jamal Aug 12 '14 at 16:55
4
\$\begingroup\$

In addition to what @200_success has mentioned, the current bounty should also be canceled so that the question can be closed, and so that the bounty is not wasted on an off-topic question. A moderator flag will take care of that.

\$\endgroup\$

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .