Add createScrollBehavior hook

[taion]

Tests require taion/scroll-behavior#125 to pass.

[taion]

cc @satazor, @vasco-santos

[vasco-santos]

Hello @taion

Sorry, for the late answer, but I have not been available in the previous days. Regarding your solution, it does not allow a specific behavior configuration for a specific route.

In spite of providing the
getCurrentLocation, I would provide the whole routerProps, such as in the shouldUpdateScroll callback. With this, we can override a generic solution for the whole app in specific routes.

For example, we use in the shouldUpdateScroll with some specifications regarding the route. Something like the following example:

function shouldUpdateScroll(prevRouterProps, routerProps) {
    const matchedRoute = [... routerProps.routes].pop();

    if (typeof matchedRoute.shouldUpdateScroll === 'function') {
        return matchedRoute.shouldUpdateScroll(prevRouterProps, routerProps);
    }

    return routerProps.location.action !== 'REPLACE';
}

Can you provide the routerState as well? Even the prevRouterState would be nice to be available.

Thanks

[taion]

I'm not sure what you mean. Provide routerState where?

[taion]

Your shouldUpdateScroll hook will still get the router state. That part is controlled by

react-router-scroll/src/ScrollBehaviorContext.js

Line 28 in 5cf4d36

shouldUpdateScroll: this.shouldUpdateScroll,

and

react-router-scroll/src/ScrollBehaviorContext.js

Line 48 in 5cf4d36

this.scrollBehavior.updateScroll(prevRouterProps, routerProps);

, which haven't changed.

[vasco-santos]

Ok, I will explain in a better way.

The shouldUpdateScroll callback receives the prevRouterProps and routerProps. So, it is possible for the consumer of the library to add logic according to the routerProps, or even override the default shouldUpdateScroll defined, as I presented in my example:

if (typeof matchedRoute.shouldUpdateScroll === 'function') {
     return matchedRoute.shouldUpdateScroll(prevRouterProps, routerProps);
}

Therefore, the same logic should be available to the scrollBehavior. If it receives also the routerProps, in addition to the addTransitionHook, stateStorage, getCurrentLocation and shouldUpdateScroll , we can override the scrollBehavior in specific routes of the app, such as we can do with the shouldUpdateScroll.

[taion]

Should be available where in the scroll behavior? You can still pass in shouldUpdateScroll to useScroll, and it will still work as before. I thought you just needed to override the method that does the actual scrolling.

[vasco-santos]

Yes, in the scroll behavior. In the shouldUpdateScroll is ok, but consider that I want a smooth scroll behavior in web pages with a small height, but in pages with a big height, it may be strange to scroll slowly during several seconds, until arriving the same position as before. If you provide routerProps to the scrollBehavior, as you do to the shouldUpdateScroll, some pages of the router may customize their scroll behavior. For this, they would provide a prop and it would be possible to override the method, as I am doing in the provided example for the shouldUpdateScroll.

[taion]

You should be able to use something like a 3-tuple in the return value of shouldUpdateScroll to communicate that to scrollToTarget, something like [left, top, { extraData }]. I don't think we need to directly plumb that data through to scrollToTarget.

[vasco-santos]

That solution does not work when it is intended to use the saved left and top coordinates value.

It would be ok for scrolling to a specific position, but when the scrollToTarget aims to use the stateStorage values, it is not possible to custom a specific route without the routerProps being provided to the createScrollBehavior.

[taion]

The saved target will include anything else you set, so you can just add extra data there. It has no fixed significance other than being the argument to scrollToTarget. It's roughly the same constraints as you would have had with the "extra argument for callback" approach.

[ThaNarie]

For anyone else reading this later; although this was published as a patch release 0.4.3 > 0.4.4, and marked as a feature, it's actually a breaking change if you used the ScrollBehaviorContext directly.

Added propType in this PR:
createScrollBehavior: PropTypes.func.isRequired

To solve this in your project:

• install & import ScrollBehavior from 'scroll-behavior';
• add this prop to your ScrollBehaviorContext component: createScrollBehavior={config => new ScrollBehavior(config)}

Although the release note says for creating a custom scroll behavior, it's also needed for the default usage.

[taion]

That component is not part of the public API and is not intended to be used directly. Why do you need to use it directly?

[ThaNarie]

Ah, apologies for that, it's indeed not part of the intended API, so no need for me to mention the breaking changes!
Still useful for people doing the same thing as we did I hope :)

The reason we are using that component, is because we can't use the <Router> directly. In our app we have to use the match, RouterContext and useRouterHistory ourselves, and because of that, also use the <ScrollBehaviorContext> as a provider.

[taion]

Well, okay. Can you submit a PR to make that a default prop, then use undefined in the middleware (with a comment explaining why)?