Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature proposal: Current animated transition is quite not good. #6283

Closed
orange4glace opened this issue Aug 9, 2018 · 2 comments
Closed

Comments

@orange4glace
Copy link

orange4glace commented Aug 9, 2018

First, I'll notice that since I'm not a native English speaker, there may be some awkward sentences that you may not understand well. If so, please let me posted :)

Currently, the animated-transition JSX template looks like this. (From example)

<TransitionGroup>
<CSSTransition key={location.key} classNames="fade" timeout={300}>
  <Switch location={location}>
    <Route exact path="/hsl/:h/:s/:l" component={HSL} />
    <Route exact path="/rgb/:r/:g/:b" component={RGB} />
    {/* Without this `Route`, we would get errors during
      the initial transition from `/` to `/hsl/10/90/50`
  */}
    <Route render={() => <div>Not Found</div>} />
  </Switch>
</CSSTransition>
</TransitionGroup>

After I tried to use this code, I found out that there is a big problem.

Problem

Nested route is useless when <Transition> is used

Since CSSTransition has location.key as key prop, whenever the path is changed, even subroute path changing, location.key value will be replaced with new value. This occurs CSSTransition and its descendants must be re-rendered with completely new DOM. This is a huge waste of performance. Besides, animation also occurs when subroute path is changed.

Example

honeycam 2018-08-09 17-45-59

Here is the basic behavior what I am going to propose.
As you can see, there is a first level route which are /A and /B with colored box component.
For each first level route, there's a nested second level route with HELLO! string, which is /{FIRST_ROUTE}/A

When first level route changes, the opacity of colored box will fade in or fade out.
When second level route changes, HELLO! string will wiped in or wiped out.
Finally, when first level route changes while second level route is activated, first level transition and second level transition will be fired simultaneously.

Proposal

To achieve this, some base logic should be changed.
First, for <Route> component, I propose a new prop which is always.
It always renders its component regardless of matching.
It is logically similar with children prop but some props passed to its component are different.

One of the problem of animated-transition is described below.

  1. Transition occurs when URL is changed. Now let's assume RouteA should disappear.
  2. RouteA, even though it is not matched anymore, should be rendered for some more 'animating' time.
  3. Since props related with router (eg: match, location) are passed by its ancestor route, RouteA receives its props but it is not valid anymore. Particularly, match prop will be NULL. This may crashes the application if there's a code something like using match.url because match is undefined and raises error eventually.

To solve this problem, <Route> will remember its latest matched match and location state. Now, even though <Route> is not matched anymore, <Route> knows its latest rendered state and its content can be rendered with it.

Now here's the difference between children prop and always prop.
children prop receives match and location prop according to its current history state.
always prop receives them according to its current history state when route is matched, if not, it receives them according to its latest matched state.(eg: when route is not matched) Additionally, there are additional props, rawMatch and rawLocation. They are based on current history state, just same with match and location props of children.

Now using always prop and <CSSTransition> component, implementing transition is fairly easy.

<Route path='/my_path' always={(props) => (
    <CSSTransition in={!!props.rawMatch} timeout={600} classNames='transition' unmountOnExit>
	{
		state => (<div>Content</div>)
	}
    </CSSTransition>
)}/>

By nesting them, nested-level transition can also be implemented.

<Route path='/my_path' always={props => (
    <CSSTransition in={!!props.rawMatch} timeout={600} classNames='transition' unmountOnExit>
	{
		state => (
			<div>
				<Route path={`${props.match.url}/nested`} always={props => (
					<CSSTransition in={!!props.rawMatch} timeout={600} className='nested-transition' unmountOnExit>
					{
						state => (
							<div>I'm nested!</div>
						)
					}
					</CSSTransition>
				)}/>
			</div>
		)
	}
    </CSSTransition>
)}/>

Of course it can be combined with HOC if needed.

I'll make PR if you guys are interested. Please feel free to have a discussion with me !

There's another animating problem with Switch and Parameter Route but I will separately describe it later if this current topic can be discussed well with others.

@timdorr
Copy link
Member

timdorr commented Aug 9, 2018

If you need to remember the previous state, you would do that from within a child component of Route. Route doesn't need to remember anything about the previous state; that's not its concern. You can compose in something that does retain state without affecting this library.

@timdorr timdorr closed this as completed Aug 9, 2018
@orange4glace
Copy link
Author

Oh, that's true. I didn't think about that!

@lock lock bot locked as resolved and limited conversation to collaborators Oct 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants