maisano / react-router-transition

painless transitions built for react-router, powered by react-motion
http://maisano.github.io/react-router-transition/
MIT License
2.59k stars 138 forks source link

Route component mounting twice #54

Closed damianobarbati closed 7 years ago

damianobarbati commented 7 years ago

@maisano I can't figure out why my component appears before the transition and then it gets animated.

Here a simple repro: https://github.com/damianobarbati/react-app

git clone https://github.com/damianobarbati/react-app
cd react-app 
yarn install
yarn serve:dev
yarn build:dev

The user app running at http://localhost:8080/user/ shows this behaviour: clicking list and then home you can see:

  1. view/picture being at left
  2. view/picture disappears
  3. view is animated sliding right to left.

View container is position:fixed => https://github.com/damianobarbati/react-app/blob/master/apps/user/components/layout.js#L23

What am I doing wrong? Thanks in advance for your effort with this project, I'm trying to use it in a cordova app hoping for great performance!

jacobangel commented 7 years ago

I couldn't reproduce this. It looks like you fixed this by changing the css to absolutely positioned. Is this still reproducing?

damianobarbati commented 7 years ago

No @jacobangel I didn't fix it yet, I don't know how honestly :( Probably your eye is not catching it? (on mobile it's more evident because it is slower).

Try to record part of the screen with quicktime and go ahead frame by frame: you'll notice the behaviour I just said.

Thanks so much for helping :) out

damianobarbati commented 7 years ago

@jacobangel I did find the problem: the component is mounted thus rendered twice! If I remove the RouteTransition wrapping is works fine without mounting twice.

I don't get what I'm missing: Switch is outside of RouteTransition as stated in the Readme.

Relevant parts: . the Router components with the switch outside of Layout https://github.com/damianobarbati/react-app/blob/v2.0.0/apps/user/router.js . the Layout component which renders Transition component https://github.com/damianobarbati/react-app/blob/v2.0.0/apps/user/components/layout.js . the Transition aka RouteTransition component eventually rendering the route children https://github.com/damianobarbati/react-app/blob/v2.0.0/apps/user/components/transition.js

What's wrong with this setup? Why is the component mounted twice by react-router-transition? Help!

jasan-s commented 7 years ago

@damianobarbati did you ever solve this?

damianobarbati commented 7 years ago

@jasan-s I didn't manage to solve this: I'm at a dead end now and I'm afraid I'll have to fall back to css-transition-group. I tried to dig into the RouteTransition.jsx but I couldn't spot the problem!

@maisano can you please help us?

jasan-s commented 7 years ago

@damianobarbati the folowwing worked for me:

<RouteTransition
  pathname={this.props.location.pathname}
  atEnter={{ opacity: 0 }}
  atLeave={{ opacity: 2 }}
  atActive={{ opacity: 1 }}
  mapStyles={styles => {
    if(styles.opacity > 1){
      return { display: 'none'}
    }
    return { opacity: styles.opacity}
  }}
>
  {this.props.children}
</RouteTransition>
damianobarbati commented 7 years ago

@jasan-s I'm doing the same: https://github.com/damianobarbati/react-app/blob/master/apps/user/components/transition.js#L40

But no success so far. Double mounting always happens.

maisano commented 7 years ago

on holiday at the moment, but hoping to have some time to dig into this soon.

maisano commented 7 years ago

@damianobarbati + @jasan-s:

had a moment to look through the code and run the example repo. what causes the double render is the way RouteTransition is composed. you can fix it by rearranging the components in a manner similar to the following:

diff --git a/apps/user/router.js b/apps/user/router.js
index d73cf4e..c93789c 100644
--- a/apps/user/router.js
+++ b/apps/user/router.js
@@ -1,6 +1,6 @@
 import React from 'react';
 import { ConnectedRouter } from 'react-router-redux';
-import { Switch, Route, Redirect } from 'react-router-dom';
+import { Switch, Route, Redirect, withRouter } from 'react-router-dom';
 import createHistory from 'history/createBrowserHistory';
 import { ThemeProvider } from 'react-jss'

@@ -11,19 +11,23 @@ import List from './components/list';

 export const history = createHistory({ basename: window.cordova ? '/' : '/user' });

+const Body = withRouter(({ history }) => (
+  <Layout>
+    <Switch location={history.location}>
+      <Route exact path="/" component={Home} />
+      <Route exact path="/list" component={List} />
+    </Switch>
+  </Layout>
+));
+
 export default class Router extends React.Component {
     render() {
         return (
             <ConnectedRouter history={history}>
                 <ThemeProvider theme={theme}>
-                    <Switch>
-                        <Layout>
-                            <Route exact path="/" component={Home} />
-                            <Route exact path="/list" component={List} />
-                        </Layout>
-                    </Switch>
+                    <Body />
                 </ThemeProvider>
             </ConnectedRouter>
         );
     }
-}
\ No newline at end of file
+}

it basically just came down to:

  1. Switch needed a location provided to it (did this within Body)
  2. need to have Layout respond to route changes so the rendered route wouldn't get cached (pulling out the Body and wrapping it with withRouter).
maisano commented 7 years ago

it's also worth noting that i'll be publishing the v4 update soon which will make all of this easier by providing a couple higher-level rrv4 wrappers, AnimatedSwitch + AnimatedRoute.

damianobarbati commented 7 years ago

@maisano that made it, thank you so much!!!

damianobarbati commented 7 years ago

@maisano will the v4 update allow easier nested transition handling? I'd like to use this lib to animate transitions between nested routes as well always keeping outer wrapper components in place, it would be handy and nice!

/home => /calendar
     /calendar => /calendar/monday
         /calendar/monday => /calendar/monday/todo/123
maisano commented 7 years ago

@damianobarbati yup! i built it with nesting in mind.

damianobarbati commented 7 years ago

@maisano I didn't want to open another issue but: how do we choose the left or right sliding transition with new version? Because presets are not there anymore and there's no example. Thank :)

maisano commented 7 years ago

@damianobarbati here's what the presets file looked like: https://github.com/maisano/react-router-transition/blob/8e06859e2e640bc009ba351097adfd6451de318c/src/presets.js

but you raise a good point. i should probably add them back in.

damianobarbati commented 7 years ago

@maisano it was very handy indeed: even using it manually, how can we set a specific transition with AnimatedRoute? So we can choose between right/left sliding in (back/forward between pages)

damianobarbati commented 7 years ago

@maisano sorry to bother again: I was wondering, any chance to use the previous presets in current 1.0.0? 🤔