Skip to content

"Scroll down - follow link - back - don't scroll - follow link" fails to scroll to top #15

@LDMFD

Description

@LDMFD

In Chrome:

1- scroll down a long page
2- click a link - page 2 will be correctly scrolled to the top
3- click Back
4- click the same or any other link without scrolling
5- observe the new page is not scrolled to the top

If after Step 3, a scroll of any distance is performed, the bug does not occur.

From remix-run/react-router#2469 (comment)

Activity

taion

taion commented on Nov 13, 2015

@taion
Owner

Thanks for filing this! I don't have anything remotely like a solution right now, but it's good to track this - it's definitely a problem.

taion

taion commented on Nov 13, 2015

@taion
Owner

What version of Chrome are you on? This might just be fixed on Chrome 46+ now.

LDMFD

LDMFD commented on Nov 13, 2015

@LDMFD
Author

Negative on 46.0.2490.86 (64-bit) (up-to-date)

mull

mull commented on Nov 16, 2015

@mull

I can confirm this happens. On latest Safari and Chome 46 as well. My reproduction steps:

  1. Scroll down on a long page
  2. pushState (react-router) to the next one
  3. window might not be scrolled up at this point
  4. go back (either through pushState or back button)
  5. window might or might not be scrolled up

The behaviour is super weird. I've tried increasing the timeout to 500ms and that seems to solve it. Chrome's manual scrollRestoration seems to be broken given that it's definitely restoring the scroll position. However, it doesn't work with Safari either.

useScrollToTop seems to not be working on all pushes/replaces either. After triggering the pop bugs, and then "moving forwards" to previously visited pages (using history.pushState) it seems to be scrolling down to previous positions again.

taion

taion commented on Nov 16, 2015

@taion
Owner

Well, Safari doesn't support scrollRestoration at all, so nothing I can do there. Definitely weird that Chrome scrolls you up even with scrollRestoration disabled, though.

How many of these bugs were present with RR 0.13? It's possible this one is just unfixable for now.

mull

mull commented on Nov 16, 2015

@mull

We used ignoreScrollBehavior on the one route this is causing issues on previously. I hadn't heard any complaints on any browser, which is why I assumed it works on Safari. Might be wrong. :-)

We had no issues at all on RR 0.13. It is possible that I'm using it wrong. This bug is on my list of things to look more at. It occurred during a ridiculous big feature called upgrade to React 0.14 and react-router@1.0.0 😁 I'll try and get back to you with more details later on, hopefully even a fix.

mmahalwy

mmahalwy commented on Jan 21, 2016

@mmahalwy

@cachvico any update on this? I am experiencing this too

LDMFD

LDMFD commented on Jan 21, 2016

@LDMFD
Author

I confess I never actually got scroll-behaviour integrated. I first used a snippet of code posted in one of the comments that precursed the creation of this package, and it worked more-or-less. When scroll-behaviour came into existence I tried to integrate it but had problems. I was going to ask for help/docs but ran out of time, so I stuck with the original hack - until a big package update broke the build and I had to look at it again. I again tried to integrate scroll-behavior but still had problems (this is probably just a documentation thing), and in the end I came up with a different (very simple) that seemed to do the job for me.

I was wondering, @taion et. al, what do you think about this approach?

import createHistory from 'history/lib/createBrowserHistory'

const history = createHistory();

history.listen(function(loc) {
    // Don't scroll to top if user presses back
    // - if (loc.action === 'POP' || loc.action === 'REPLACE') is an option
    if (loc.action === 'POP') {
        return;
    }

    // Allow the client to control scroll-to-top using location.state
    if (loc.state && loc.state.scroll !== undefined && !loc.state.scroll) {
        return;
    }

    // 200ms delay hack (for Firefox?)
    setTimeout(() => {
        window.scrollTo(0, 0);
    }, 200);
});


module.exports = history;

It is used like this:

var history = require('history');

var C = React.createClass({
    mixins: [ History ],

    buttonClicked: function() {
        // Prevent scroll-to-top in an event handler:
        this.history.replaceState({ scroll: false }, '/');
    },
    render: function() {
        return (
            // Prevent scroll-to-top with a <Link>:
            <Link to='/' state={{ scroll: false }}>
    },
});

// ...
<Router history={history}>
    <Route ...

LDMFD

LDMFD commented on Jan 21, 2016

@LDMFD
Author

n.b. @mmahalwy the 200ms delay hack is the actual workaround for this Issue in question

mmahalwy

mmahalwy commented on Jan 21, 2016

@mmahalwy

Awesome! Thanks for sharing! @cachvico

taion

taion commented on Jan 22, 2016

@taion
Owner

I'm probably going to rewrite this to leverage <Router render> when I get a chance.

mmahalwy

mmahalwy commented on Jan 28, 2016

@mmahalwy

@taion how can i help? I dont have a ton of knowledge of how react-router works but i want to help

taion

taion commented on Apr 14, 2016

@taion
Owner

Appears to be fixed now.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @mull@pc035860@LDMFD@taion@mmahalwy

        Issue actions

          "Scroll down - follow link - back - don't scroll - follow link" fails to scroll to top · Issue #15 · taion/scroll-behavior