Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Window isn't scrolled to top when transitioning to the same route with a new query string #189

Closed
thomasboyt opened this issue Aug 11, 2014 · 2 comments · Fixed by #249
Closed

Comments

@thomasboyt
Copy link

If you transition from a route to the same route with a new query, window.scrollTo doesn't get called.

This is due to this line: https://github.com/rackt/react-router/blob/master/modules/components/Routes.js#L447 - match is undefined when transitioning to the same route. Removing that check fixes the issue, but I feel like it's likely there for a reason, so I don't just want to submit a PR deleting it ;)

@ryanflorence
Copy link
Member

Regarding the match check, on initial render of the route handlers you don't have a match, so that check needs to be there.

  • I can imagine many scenarios you want to scroll when QP change.
  • Its also easy to imagine scenarios where you don't.
  • there are also some scenarios where you'd want to scroll when you first show up to the page and then not when the params change.

@thomasboyt thoughts on how to appropriately opt in/out?

@bobeagan
Copy link
Contributor

Definitely can see cases for both of this: pagination being an example of when I would want it to scroll and a photo gallery being an example of when I wouldn't. Should <Link> have a way to override the default scroll behavior of the route?

mjackson referenced this issue in karlmikko/react-router Aug 19, 2014
[fixed] Initial render to null causing second render
[fixed] Query not passed to willTransitionTo
[fixed] ActiveState mixin causing second render
[added] Docs
[added] Tests

```js
Router.renderRoutesToString(routes, originalUrl).then(function (data) {
data.html; //string returned from React.renderComponentToString
}).catch(function (error){
//Redirect will set error.status and error.location
error.status; //302 for Redirect
error.location; //location to redirect to
});
@mjackson mjackson added this to the window scrolling milestone Aug 29, 2014
mjackson added a commit that referenced this issue Aug 29, 2014
The router now remembers the last window scroll position at various
paths and automatically scrolls the window to match after transitions
complete unless preserveScrollPosition=true is used.

This commit also introduces a flux-style architecture to the high-level
transitionTo/replaceWith/goBack methods.

Fixes #189
Fixes #186
@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants