Mithril.js: Position should reset after route changes

[v4n]

I don't believe this was happening in v0.2 but in v1 when a route changes the page is opened from the previous page scroll position, that's bad UX.

Should probably be solvable with:

history.listen(_ => {
    window.scrollTo(0, 0)
})

[ycadaner-merkos302]

It's a rather opinionated decision on the router's part. There are times it's undesired, like when paning a sub-section of a page

[v4n]

@ycadaner-merkos302 would it be reasonable to say when a <a oncreate={m.route.link}></a> is clicked the behaviour should be just like a <a></a> with exception oncreate={m.route.link} causes a view change rather a full page load.

[ycadaner-merkos302]

@v4n If the url is saving state, say photo id in a gallery, this wouldn't be the desired behavior.

[barneycarroll]

This is a chronic problem. In traditional websites, getting back to the position where you left the last page was a very useful feature, but with modern single page applications where Javascript controls most aspects of the user experience it's less straightforward whether the behaviour is desirable or not.

When the HTML5 History API was implemented, Chrome and Firefox presented a difficulty in reliably preventing this behaviour because they triggered the scroll reset at different times, and it was impossible to reliably work out when this happened.

There's a host of Stackoverflow Q&As and libraries out there aiming to find solutions to this problem — Googling for "popstate prevent scroll" yields some promising results…

[v4n]

@ycadaner-merkos302 if a link with href="#image-4 and oncreate={m.route.link} acts exactly like a normal link then indeed scroll to top shouldn't happen (unless there is an element with image-4 id at the top). That's what I mean by my previous comment.

[v4n]

@barneycarroll imo about 99% of the cases when navigating to a new page it doesn't make sense for scroll to start from the previous page state. Reload yes. But '/home' to '/about' makes no sense.

It's like going through a book and each time you move to a new page you start reading at the middle of it.

Any thoughts on why this behaviour didn't occurred on v0.2?

[pygy]

There was a global.scrollTo(0, 0)hardcoded on route. Some people complained about it or picked another framework because it was unwanted.

If it bothers you you can wrap m.route.set and m.route.link to do it for you.

We may want to add a note in the migration guide about this.

[v4n]

@pygy what scenarios did people complained about the global.scrollTo(0, 0)?

I can only think of anchor links and updates to the url without causing a view change, both which can be achieved without the usage of m.route.link.

I imagine the scenarios are less common than navigating a page. If so I don't think Mithril should make people have to monkey patch to deal with a very common situation.

[tivac]

@v4n

567

747

989

[inta]

I don't think resetting the scroll position is that common for SPA. You might repeatedly want to change the URL and only parts of the app/page (I needed that several times and it was quite a pain with v0.2). Changing the URL without the router is not a nice option if you need some components to get initialized.

[v4n]

@tivac this is more subjective but feels like scroll position when navigating different pages is more important to handle off the bat than:

• back/forwards button scroll state is lost
• hash usage with infinite scrolling with two page sections that have different scrolling points

@inta interesting. So if I understand correctly you had two views where only part of the pages were different - componentB <-> componentC in the following example.

view1 = <div>
  <componentA></componentA>
  <componentB></componentB>
</div>
view2 = <div>
  <componentA></componentA>
  <componentC></componentC>
</div>

And when the route changed between view1 and view2 you wanted the scroll position to be in the same place?

[inta]

@v4n correct, the second component is something like an overlay with its own scroll area which does not entirely cover the first component. One oft those second components is for the detail view and another for the legal notice. The first one is the main content with a list things, where you don't want to loose focus while viewing some details. I think the user experience is quite nice, so there should be at least an option to route without scroll to top.

[v4n]

@inta aha makes sense.

In my case I use history push state to change the url when a modal is opened and since our main layout has <main>{window.modal ? window.modal : ''}</main> I simply add the modal component to window.modal when needed.

Your case might be more complex so I can understand the need of wanting to avoid scroll to top.

It would be nice having something like:

• m.route.link which is used when linking to new pages and scrolls to top
• m.route.update update existing page and don't scroll to top

If we don't provide fix out the box then we just increase the amount of things people need to do to get Mithril working reasonably on an app. Do you imagine navigating stackoverflow or twitter while the scroll position gets restored to the one of the previous page.

[ycadaner-merkos302]

I would expect a flag. Perhaps adding a boolean scrollToTop flag to m.route.link

[inta]

I think the right position to configurate this should be route setup, not the trigger/link. I can't think oft a situation where you sometimes want to scroll to top and sometimes not. That would be really confusing for the user.

[pygy]

An what happens when route resolution is deferred?

This IMO would best be handled as a routeResolver wrapper that injects a new- or wraps an existing onmatch hook.

m.route(root, '/', {
  '/': scrollTop(RootComponent)
}

With logic in scrollTop to deal with components, routeResolvers with and without an onmatch hook.

[inta]

That's somehow what I meant with "route setup", though I'm not yet familiar with the v1 routing and did not know what the right hook would be.

[barneycarroll]

@pygy I'll defer to your experience on this but based on my attempts to paper over the differences between route components and route resolvers, the idea of a wrapper that patches consistent view lifecycle behaviour into any given route endpoint sounds terrifying if not impossible to write.

Besides, when @inta says

I think the right position to configurate this should be route setup

That chimes with me in that this behaviour should really be something that sits at an app-wide level, above individual routes (which are stateless inasmuch as they are ignorant of previous route resolutions). Besides, composing routes quickly gets cumbersome and difficult to rationalise as more concerns are forced into route endpoints, eg profile : ScrollReset( MainLayout( Authenticate( ProfileComponentOrResolver ) ).

I find a better pattern for execugting side effects in time with Mithril's lifecycle is to mount a component outside of the document and encapsulate the logic in a way that doesn't require patching orthogonal parts of the application:

m.mount(
  // Don't attach to the document
  document.createDocumentFragment(),
  {
    // We need a valid view for Mithril to behave
    view : () => '',

    // Will execute on the DOM ready phase of every draw
    onupdate(){
      const route = m.route.get()

      if(route != this.route)
        scrollTo(0, 0)

      this.route = route
    }
  }
)

// Route definitions are separate from routing behaviour configuration
m.route(/* ... */)

[pygy]

Something along this will go a long way (bring your own Object.assign(), and beware, this was typed in the GitHub text box, not tested at all).

// always return a new RouteResolver with both onmatch and render
function identity(vnode) {return vnode}
var noop = Function.prototype
function normalizeRoute(endpoint) {
  if (endpoint == null || typeof endpoint !== 'function' && typeof endpoint !== 'object') {
    throw new error ("Component or RouteResolver expected")
  }
  if (typeof endpoint === 'function' || typeof endpoint.view === 'function') {
    return {onmatch: function() {return endpoint}, render: identity}
  } else {
    return = Object.assign(Object.create(endpoint), {
      onmatch: typeof endpoint.onmatch === 'function' ? endpoint.onmatch : noop,
      render: typeof endpoint.render === 'function' ? endpoint.render : identity
    })
  }
}

// then

function scrollTop(endpoint) {
  endpoint = normalizeRoute(endpoint)
  var onmatch = endpoint.onmatch
  endpoint.onmatch = function() {
    return Promise.resolve(onmatch.apply(endpoint, arguments))
      .then(function(){window.scrollTo(0, 0)})
  }
  return endpoint
}

[v4n]

@barneycarroll based on your proposal if I had to add the 5 lines logic to our app's views (roughly 31) we are talking about 155 additional lines of code to handle an expected behaviour when navigating to a new page.

We could DRY things up and reduce it to 2-3 lines. But when creating views there is high likelihood handling this will be overlooked.

@pygy leaving it up to people to add a routeResolver for a behaviour expected for usable apps (which other libraries fail at helping with) feels like a bad way to resolve this.

Either people will have to come up with a similar code you wrote.
Or google, dig through docs/GH issues, find the code, and copy paste into their codebase. All of that just so that page navigation works as one would expect.

[ycadaner-merkos302]

This approach is maintaining that a url will _always_ either desire a scrollToTop or will not. However, sometimes the transition to the new url will scrollToTop and sometimes not. I think this should be at the discretion of the invoker of the request.

Truth be told, we can have all transitions scrollToTop, and augment m.route.set to m.route.set('url', 'preventScroll'). This will allow to attach an onclick event to an <a></a> and override the default for m.route.link.

[pygy]

@v4n these could become NPM packages (both normalizeRoute and scrollTop, and the latter could be mentioned in the migration guide).

@ycadaner-merkos302 You can modify the scrollToTop wrapper above to work conditionally. You'd need to wrap m.route.set and m.route.link as well and have all three wrappers communicate (a common variable in the parent closure would do).

[v4n]

@pygy would the RouteResolver solution force a view to _always_ start from the top including reload and going back?

Making the scroll decision when using m.route.link would make scrolling behaviour work as expected for all scenarios: new page, reload, back button and the more complex scenarios as mentioned in the previous comments.

[pygy]

For delayed resolution, the scrollTo(0, 0) call must happen after the resolution completes otherwise the old page will scroll up when you click the link and wait while the new page data is loading. During that time, the user would be free to scroll down again. Bad UX/UI.

You probably want something like this:

var shouldScrollToTop = false

function scrollTop(endpoint) {
  endpoint = normalizeRoute(endpoint)
  var onmatch = endpoint.onmatch
  endpoint.onmatch = function() {
    return Promise.resolve(onmatch.apply(endpoint, arguments))
      .then(function(){
        if(shouldScroll) {
          shouldScrollToTop = false
          window.scrollTo(0, 0)
        }
      })
  }
  return endpoint
}

m.route.linkTop = function() {
  shouldScrollToTop = true
  return m.route.link.apply(this, arguments)
}

m.route.setTop = function() {
  shouldScrollToTop = true
  return m.route.set.apply(this, arguments)
}

Possibly with wrappers around the real set and link that set shouldScrollToTop to false explicitly,

Possible caveat re. picking back the page position on reload/coming back to the page: the presence of onmatch on routeResolvers currently triggers an async first render (that's why the normalizeRoute helper here doesn't bother telling Promises and other values apart and just wraps the value of the previous onmatch in Promise.resolve().

I don't know if that allows the page to pick back its old position if the first paint occurs after DOMContentLoaded.

[isiahmeadows]

Closing, since this issue has not been updated in over a month. If you feel something in Mithril needs added or changed, please file a new issue.