Bug with anchors

[MoOx]

Simple to reproduce

• go https://phenomic.io/docs/usage/plugins/
• click on a title anchor link (# when you over a link)
• scrolling to top ! (should not)
• click again to get the correct behavior

You can reproduce locally by doing

git clone https://github.com/MoOx/phenomic.git
cd phenomic
npm install
npm run docs-start

The doc will start & open locally.

Tell me if you need more (I made this issue really quickly as I am busy, sorry).

[MoOx]

Note: maybe that not the fault of react-router-scroll since we are catching links in the page body, but we just update the browser history so react-router can handle the updates.

[taion]

If you're handling the link yourself, use shouldUpdateScroll and return false in the hook to prevent the scroll update.

Eventually there should be support for hash links here, but there isn't yet.

[MoOx]

In our case, we are not especially "handling the link ourselves". We are letting react-router change the component if relevant.
Maybe the default shouldUpdateScroll should be like this

const shouldUpdateScroll = (prevProps, props)=> {
  return !prevProps || prevProps.location.pathname !== props.location.pathname
}

(no need to test the origin, hostname etc as if it's on the app runtime it means it is already the same origin etc.)

What do you think about this simple method?

[taion]

That wouldn't work for e.g. query or search changes, which might need to update the scroll position.

[MoOx]

Good catch haha. Will provide a better simple default then.

[taion]

What I mean is – if you're using hash links, you can set something up... I'm not sure that it's generic enough that I want to pull it in here. Edge cases and edge cases, you know.

[MoOx]

Oh, I see. Maybe we can add something to the readme then?

[taion]

Sure. Care to PR a note

[oliviertassinari]

@MoOx Thanks for opening that thread and sharing your solution.
Here is mine for handling a simple page with anchor links with Material-UI.

function shouldUpdateScroll(prevProps, props) {
  const hash = props.location.hash;

  // We have no hash to take into account.
  // We let react-router handling it.
  if (hash === '') {
    return true;
  }

  // It's our first navigation.
  if (!prevProps) {
    // Push onto callback queue so it runs after the DOM is updated,
    // this is required when navigating from a different page so that
    // the element is rendered on the page before trying to getElementById.
    setTimeout(() => {
      const element = document.querySelector(hash);
      if (element) {
        element.scrollIntoView();
      }
    }, 0);
  }

  return false;
}

[taion]

btw this is natively supported now since https://github.com/taion/scroll-behavior/releases/tag/v0.9.3

[oliviertassinari]

@taion I confirm, thanks for raising that point 😄 ! (We were using v0.9.2)

[ArthurClemens]

Each page view I am scrolling to the top of the page, to prevent showing the new page halfway. This conflicts with anchor links that obviously should not scroll the page. Somehow only Edge has a problem with this situation.

So I am using a solution (simpler than mentioned above) that works for Edge and other browsers:

const maybeScrollToTop = () => {
  const hash = window.location.hash;
  if (!hash) {
    window.scrollTo(0, 0);
  }
};

export const Routes = () => (
  <Router onUpdate={maybeScrollToTop} history={browserHistory}>
    <Route>
      // ...
    </Route>
  </Router>
);