Add scroll position support about history

[elpete]

I'm trying to wrap my head around this project, so thanks in advance for being patient.

Is this issue saying that

1. We want to know the scroll position for the previous location object?
2. We want to provide the initial scroll position for the next location object?
3. Both?

[mjackson]

Hi @ericlynnpeterson! Thank you for taking a look :)

It may help to think about it like this: The location object tells you at any given point in time where you are. Currently, the pathname, search string, and state are all part of that object. They all answer the question "where am I?"

All we want to do here is add scroll information to the location object so it can better answer that question. This will most likely involve two parts:

1. Use getScrollPosition to record the scroll information immediately before we change the URL
2. Include the scrollX and scrollY properties in the createLocation function

As a side note, createLocation is getting quite a few arguments. We may want to consider slimming them down into an options object or something...

Does that help?

[elpete]

Thanks @mjackson. That does help. Maybe a followup question?

• If the location object answers the questions "where am I?", with regards to scrolling, do we need to update the current location object's scrollX and scrollY position on every scroll?

Maybe an example will help.

// Initial Location with scroll position

var history = createHistory({
  getScrollPosition() {
    return { x: 0, y: 200 };
  }
});

// Initial location
{
  pathname: '/',
  search: '',
  state: null,
  action: POP,
  scrollX: 0,
  scrollY: 200 // <-- from getScrollPosition()
}

// Scroll down a bit; updates along the way
{
  pathname: '/',
  search: '',
  state: null,
  action: POP,
  scrollX: 0,
  scrollY: 400
}

// Push a new location
history.pushState(null, '/home');
{
  pathname: '/home',
  search: '',
  state: null,
  action: PUSH,
  scrollX: 0,
  scrollY: 0
  // I was thinking that the only time we used the getScrollPosition()
  // value was for the initial location.  Correct?
}

// Scroll down in the new location a bit; updates along the way
{
  pathname: '/home',
  search: '',
  state: null,
  action: PUSH,
  scrollX: 0,
  scrollY: 300
}

// Programatically go back; old location and scroll position are loaded
history.goBack();
{
  pathname: '/',
  search: '',
  state: null,
  action: POP,
  scrollX: 0,
  scrollY: 400 // <-- Last know scroll position for the first location entry
}

Did I get the flow right?

Also, if there is a better place to discuss and ask these questions, point me there. Not sure if these are helpful or just noise inside the Github issue.

[gaearon]

I think the intent is to let you provide something like

var history = createHistory({
  getScrollPosition() {
    return { x: window.scrollX, y: window.scrollY };
  }
});

The reason it's customizable is for mocking, server rendering, and also more complicated setups a la “two different content areas with independent scrolls” or “one overflow: scroll area somewhere deep in the document”.

do we need to update the current location object's scrollX and scrollY position on every scroll?

You don't need to call getScrollPosition() and update location. Consider location immutable. Once it's created, nobody changes it.

When you call history.pushState() or similar method, internally, history calls createLocation:

  function pushState(state, path) {
    transitionTo(
      createLocation(path, state, PUSH, createKey())
    );
  }

The change is that, with this change, it will also call getScrollPosition() to remember where we were:

  function pushState(state, path) {
    transitionTo(
      createLocation(path, state, PUSH, createKey(), getScrollPosition())
    );
  }

This way, when location object is given to you in the change handler, it has scrollPosition.

[elpete]

@gaearon So in your above example, whenever we transition locations, the new location would have a scroll position equal to the result of the getScrollPosition function we defined in createHistory?

[mjackson]

the new location would have a scroll position equal to the result of the getScrollPosition function we defined in createHistory?

Almost. We actually want to save the scroll position on the previous location, most likely in sessionStorage. That way, when you navigate back to that page, we can restore scroll position.

Think about how a browser works. If you're scrolled halfway down a page, and then you navigate somewhere else, and then hit the back button, the browser scrolls down to where you were when you left that page. We want to enable the same functionality in single-page apps.

@gaearon Thanks for chiming in here!

[mjackson]

Just thinking about this a little more...

Maybe all we really need here is a way to update the current location.state. This is a more generic method that would allow people to store scroll position, among other things. It would work like React's setState method and do a shallow merge of the object you give it into the current location.state.

onScroll(function () {
  history.setState({ scrollX: 0, scrollY: 0 });
});

This API would skip transitions, so it would be fairly lightweight. If people are concerned about emitting too many location objects they can either a) limit the number of times they call history.setState (debounce) or b) perhaps pass a second falsy arg to setState that indicates that listeners should not be notified.

[agundermann]

Maybe all we really need here is a way to update the current location.state.

Yeah, that's also why I added updateState to the old branch (remix-run/react-router@a3bb271 followed by remix-run/react-router@b66737f). What's the reason behind using scroll listener instead of recording the position before the next transition though?

The major problem I see with this in general is that we don't get any information about the rendered location like we used to with the router. This can become problematic when rendering asynchronously.

That, and the fact that custom scroll management is inherently error-prone make me think it would be better to not include that functionality in the core. With location.key it shouldn't be too hard to come up with a standalone utility that provides something like restorePosition(location) and savePosition(location). That way, the user could decide himself when to save and restore the scroll position, depending on what he renders when.

[mjackson]

Yeah, that's why I added updateState

I missed that, @taurose! Sorry, you have to be patient with me. Either that or yell very loud when you do something awesome like that ;)

What's the reason behind using scroll listener instead of recording the position before the next transition though?

My thinking here is that we can be more accurate if we record the scroll position as the page scrolls instead of before the next transition. On popstate, we don't actually know the URL changed until after it already has. If we try to record the scroll position at that point, we may already be too late. (I'm not sure this is actually a problem. Just guessing...)

The major problem I see with this in general is that we don't get any information about the rendered location like we used to with the router. This can become problematic when rendering asynchronously.

I'm not suggesting we would try and restore scroll position with this module. All setState would do is give users (like the router) a way to record incremental state changes. It would still be up to the router to determine when to use that information to update the scroll position of the page.

[agundermann]

My thinking here is that we can be more accurate if we record the scroll position as the page scrolls instead of before the next transition.

@mjackson I forgot about that. IIRC, Firefox restores scroll before popstate event. However, I think that the browser scrolling will trigger scroll events as well. Not sure if there's a way to distinguish them from the rest.

I'm not suggesting we would try and restore scroll position with this module. All setState would do is give users (like the router) a way to record incremental state changes.

Oh, I see you said that in your previous comment. Sorry I didn't get that ;)

[mjackson]

I think that the browser scrolling will trigger scroll events as well. Not sure if there's a way to distinguish them from the rest.

Ya, I think we'll have to figure out a good high-level API for this in the router, @taurose. Maybe something like "this is the scrollable element, so only listen for scrolling here". Not sure yet.