acdlite / redux-router

Redux bindings for React Router – keep your router state inside your Redux store
MIT License
2.3k stars 200 forks source link

Add `scroll-behavior` capability #261

Closed catamphetamine closed 8 years ago

catamphetamine commented 8 years ago

You can add a section in the README saying that if a user wants scroll-behavior then they should first install it (react-router-scroll has scroll-behavior bundled)

npm install react-router-scroll --save

And then pass scrollBehavior={true} prop to <ReduxRouter/>

ReduxRouter.js

...

class ReduxRouterContext extends Component {
    static propTypes = {
        location: PropTypes.object,
        RoutingContext: PropTypes.any, // PropTypes.element won't work
    }

    render() {
        ...

        let RoutingContext = this.props.RoutingContext || DefaultRoutingContext

        if (this.props.scrollBehavior) {
            const useScroll = require('react-router-scroll')

            // Add `scroll-bevahiour` for scroll position management
            RoutingContext = (props) =>
            {
                // https://github.com/taion/react-router-scroll/blob/master/src/index.js
                return useScroll().renderRouterContext(< DefaultRoutingContext {...props}/>, props)
            }
        }

        return <RoutingContext {...this.props} />
    }
}

I tested this code and it works as intended (waits for page to fetch data asynchronously, and then does the scrolling thing).

If someone tries using bare scroll-behavor using createHistory when creating Redux store then it won't work: the page scroll will be laggy when navigating through pages, not waiting for page data to load. This method I came up with solves the issue.

catamphetamine commented 8 years ago

Or maybe this library shouldn't incorporate scroll-behavior for simplicity's sake. Then this issue can be a how to for people coming from google.

import useScroll from 'react-router-scroll'
import { RouterContext } from 'react-router'

// Add `scroll-bevahiour` for scroll position management
const RoutingContext = (props) =>
{
    // https://github.com/taion/react-router-scroll/blob/master/src/index.js
    return useScroll().renderRouterContext(<RouterContext {...props}/>, props)
}

<ReduxRouter RoutingContext={RoutingContext}/>
Scarysize commented 8 years ago

Yep, would be better. You could also formulate a stackoverflow question and answer it yourself. This way it can be found more easily.

perrin4869 commented 8 years ago

Wouldn't something like this be better?

import { applyRouterMiddleware } from 'react-router'

const RouterContext = applyRouterMiddleware(useScroll());
<ReduxRouter RoutingContext={RouterContext}/>
catamphetamine commented 8 years ago

Oh, yes, Julian is a react-router contributor and he proposed this solution in react-router repo https://github.com/reactjs/react-router/issues/3641

@perrin4869 I've just tested your code and it seems to work. I agree that it can be considered a better solution.

perrin4869 commented 8 years ago

@halt-hammerzeit I'm not really a contributor to react-router, just a user that has had the chance to play extensively with middlewares lol