Document how to use scrollTop() properly #417

Open
jzaefferer opened this Issue Jan 13, 2014 · 18 comments

Comments

Projects
None yet
8 participants
@jzaefferer
Member

jzaefferer commented Jan 13, 2014

Via discussion in #14515.

This should be added to scrollTop():

Note that scrollTop can be used to read, but not write, the document scroll position. To scroll the document, animated or not, you have to call `scrollTop()` on the body element:

<pre><code>
// Without animation
$( document.body ).scrollTop( 500 );

// With animation
$( document.body ).animate({
  scrollTop: 500
});
</code></pre>

Any examples elsewhere that suggest $( "html, body" ).animate({ scrollTop: ... }); are wrong, since this will cause the animation callback to get triggered twice.
@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Jan 28, 2014

Member

I just reopened the Core ticket, see there for details: http://bugs.jquery.com/ticket/14515#comment:8

Member

jzaefferer commented Jan 28, 2014

I just reopened the Core ticket, see there for details: http://bugs.jquery.com/ticket/14515#comment:8

@gnarf

This comment has been minimized.

Show comment
Hide comment
@gnarf

gnarf Jan 28, 2014

Member

I left some comments in #425 about the message here, I think we need to re-word it a bit and also add a similar note to scrollLeft()

Member

gnarf commented Jan 28, 2014

I left some comments in #425 about the message here, I think we need to re-word it a bit and also add a similar note to scrollLeft()

@kswedberg

This comment has been minimized.

Show comment
Hide comment
@kswedberg

kswedberg Jan 28, 2014

Member

@jzaefferer I left a comment on the Core ticket for you.

Member

kswedberg commented Jan 28, 2014

@jzaefferer I left a comment on the Core ticket for you.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Mar 3, 2014

Member

Since Core won't try to normalize this, all we can do is document the canonical solution. Afaik this snippet works consistently and produces a single callback:

$( "html, body" ).animate({
  // scroll to end of page
  scrollTop: $(document).height()
}, 800).promise().then(function() {
  console.log("runs once!")
});

@leobalter would you update your branch and create a new PR? Thanks.

Member

jzaefferer commented Mar 3, 2014

Since Core won't try to normalize this, all we can do is document the canonical solution. Afaik this snippet works consistently and produces a single callback:

$( "html, body" ).animate({
  // scroll to end of page
  scrollTop: $(document).height()
}, 800).promise().then(function() {
  console.log("runs once!")
});

@leobalter would you update your branch and create a new PR? Thanks.

@gnarf

This comment has been minimized.

Show comment
Hide comment
@gnarf

gnarf Mar 3, 2014

Member

That snippet has other side effects...

Namely:

$( "html, body" ).animate({
  // scroll to end of page
  scrollTop: $(document).height()
}, 800).promise().then(function() {
  console.log("runs once!")
}).animate({
  scrollTop: 0
}, 800); // but only after this animation ALSO finishes

$("body").animate({opacity: 0}, 500); // oh this one too

The promise on a jQuery object only gets called when its default animation queue empties, not as an inline callback to animation completion.

Member

gnarf commented Mar 3, 2014

That snippet has other side effects...

Namely:

$( "html, body" ).animate({
  // scroll to end of page
  scrollTop: $(document).height()
}, 800).promise().then(function() {
  console.log("runs once!")
}).animate({
  scrollTop: 0
}, 800); // but only after this animation ALSO finishes

$("body").animate({opacity: 0}, 500); // oh this one too

The promise on a jQuery object only gets called when its default animation queue empties, not as an inline callback to animation completion.

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Mar 4, 2014

Member

As gnarf said, you probably want something akin to this:

$.Deferred(function( defer ) {
    $( "html, body" ).animate({
        // scroll to end of page
        scrollTop: $(document).height()
    }, 800, defer.resolve );
}).done(function() {
    console.log( "runs once!" )
});
Member

jaubourg commented Mar 4, 2014

As gnarf said, you probably want something akin to this:

$.Deferred(function( defer ) {
    $( "html, body" ).animate({
        // scroll to end of page
        scrollTop: $(document).height()
    }, 800, defer.resolve );
}).done(function() {
    console.log( "runs once!" )
});

@gnarf gnarf closed this Mar 4, 2014

@gnarf gnarf reopened this Mar 4, 2014

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Mar 5, 2014

Member

IMHO, some of these examples are looking way complex to be presented as examples.

I think we can use the suggestions made by @gnarf to mention scrollLeft and we can also add another example using $.Deferred as mentioned by @jaubourg.

It might be worth to mention that jQuery Core don't try to force a normalization on this and explain why it's needed to handle scrollTop both on html and body to get a crossbrowser solution.

Member

leobalter commented Mar 5, 2014

IMHO, some of these examples are looking way complex to be presented as examples.

I think we can use the suggestions made by @gnarf to mention scrollLeft and we can also add another example using $.Deferred as mentioned by @jaubourg.

It might be worth to mention that jQuery Core don't try to force a normalization on this and explain why it's needed to handle scrollTop both on html and body to get a crossbrowser solution.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Mar 7, 2014

Member

We can start with an example that doesn't have the callback, to keep things simple at first. Use that to talk about the underlying issue and lack of normalization. Then talk about the issue with animation callbacks and how to deal with that, along with another example.

Member

jzaefferer commented Mar 7, 2014

We can start with an example that doesn't have the callback, to keep things simple at first. Use that to talk about the underlying issue and lack of normalization. Then talk about the issue with animation callbacks and how to deal with that, along with another example.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Mar 7, 2014

Member

Can this be an article on learn perhaps, with a link from the api docs? It's starting to sound pretty big.

Member

dmethvin commented Mar 7, 2014

Can this be an article on learn perhaps, with a link from the api docs? It's starting to sound pretty big.

@kswedberg

This comment has been minimized.

Show comment
Hide comment
@kswedberg

kswedberg Mar 7, 2014

Member

👍 to linking to a learn article.

Member

kswedberg commented Mar 7, 2014

👍 to linking to a learn article.

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Mar 7, 2014

Member

I'll try to write something by the weekend.

challenge

Anyway, I don't want to prevent anyone else to write it as well. If we have other article to be posted on learn I can push mine to a blog, maybe.

Member

leobalter commented Mar 7, 2014

I'll try to write something by the weekend.

challenge

Anyway, I don't want to prevent anyone else to write it as well. If we have other article to be posted on learn I can push mine to a blog, maybe.

@arthurvr

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Feb 8, 2015

Member

I'll try to write something by the weekend.

@leobalter Still interested in taking this? Or did you already do?

Member

arthurvr commented Feb 8, 2015

I'll try to write something by the weekend.

@leobalter Still interested in taking this? Or did you already do?

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Feb 8, 2015

Member

I failed doing this. I may try it again but it shouldn't hold anyone else to fix it.

On Feb 8, 2015, at 11:23 AM, Arthur Verschaeve notifications@github.com wrote:

I'll try to write something by the weekend.

@leobalter Still interested in taking this? Or did you already do?


Reply to this email directly or view it on GitHub.

Member

leobalter commented Feb 8, 2015

I failed doing this. I may try it again but it shouldn't hold anyone else to fix it.

On Feb 8, 2015, at 11:23 AM, Arthur Verschaeve notifications@github.com wrote:

I'll try to write something by the weekend.

@leobalter Still interested in taking this? Or did you already do?


Reply to this email directly or view it on GitHub.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Apr 14, 2015

Member

@arthurvr interested in taking this over? Or finding someone to write the docs? Sad to see this still open more than a year later...

Member

jzaefferer commented Apr 14, 2015

@arthurvr interested in taking this over? Or finding someone to write the docs? Sad to see this still open more than a year later...

@arthurvr

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Apr 14, 2015

Member

interested in taking this over? Or finding someone to write the docs? Sad to see this still open more than a year later...

That's true for basically ever ticket open here :( One day I'll get to it for sure, but time just limits me.

Member

arthurvr commented Apr 14, 2015

interested in taking this over? Or finding someone to write the docs? Sad to see this still open more than a year later...

That's true for basically ever ticket open here :( One day I'll get to it for sure, but time just limits me.

@arthurvr

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Apr 14, 2015

Member

@jzaefferer Although I do definitely agree that many things in this repo are taking way too long. There are still APIs from back in 2012 undocumented, just because we lack the time and the resources.

Member

arthurvr commented Apr 14, 2015

@jzaefferer Although I do definitely agree that many things in this repo are taking way too long. There are still APIs from back in 2012 undocumented, just because we lack the time and the resources.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Apr 15, 2015

Member

@arthurvr the important thing is that we are making progress, thanks to you! This issue in particular seemed to get lots of people suggesting changes without a clear consensus on what to do. I think that's why it sat around so long, it is tedious to sort through these kinds of tickets and there is always concern you'll upset someone by ignoring their concerns. So go for the low-hanging fruit if it's easier.

Member

dmethvin commented Apr 15, 2015

@arthurvr the important thing is that we are making progress, thanks to you! This issue in particular seemed to get lots of people suggesting changes without a clear consensus on what to do. I think that's why it sat around so long, it is tedious to sort through these kinds of tickets and there is always concern you'll upset someone by ignoring their concerns. So go for the low-hanging fruit if it's easier.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer May 20, 2015

Member

Here's a good explanation of the underlying mess, along with a future-proof solution called document.scrollingElement: https://dev.opera.com/articles/fixing-the-scrolltop-bug/

If I read that correctly, we should document how to animate document.scrollingElement, along with a recommendation to load the polyfill.

Member

jzaefferer commented May 20, 2015

Here's a good explanation of the underlying mess, along with a future-proof solution called document.scrollingElement: https://dev.opera.com/articles/fixing-the-scrolltop-bug/

If I read that correctly, we should document how to animate document.scrollingElement, along with a recommendation to load the polyfill.

@AllThingsSmitty AllThingsSmitty referenced this issue in AllThingsSmitty/jquery-tips-everyone-should-know Nov 9, 2015

Closed

Crossbrowsing toTop #37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment