-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
[v6] First-class animation primitives #7946
Comments
The problem is that it does not replace |
Also, if react-router wants to gain traction in react-native land then you'll need to be able to support stack navigation. Please see #7943 for details. |
Hi, I hadn't seen this discussion and created a PR for adding |
I'm not sure Maybe a more explicit name would be clear: The point of the |
This update feels wrong. The |
@sebastian-nowak |
As far as I can tell, animation is the only real use case for the The modal gallery that some here have pointed out is actually busted. If you refresh the page when the modal is open, you go to the actual page for that color without the modal. We should probably rebuild that example using |
Refreshing the page to have the modal go away is actually the intended behavior. It's not busted. It's the exact same behavior used on Reddit when opening a comment thread. Within the same page load, it pops a full height modal to show the thread, leaving the thread list behind it. If you refresh the page, it's just the comment thread and no modal overlay. This is important to browsing threads, as the order of the thread list changes rapidly. If you lose the state of the thread list when going back you would end up having threads disappear on you regularly. Retaining prior state while navigating is super important for their UX. They use React Router and I'm not sure how they'd upgrade to v6 without a location prop. |
@mjackson I hope I’m not being a nuisance but I have a good bit of experience in react-native and I have concerns that v6 won’t be a viable option for react-native without the location prop. With mobile apps, users expect to be able to use operating system gestures (or emulated gestures) to navigate around the app. The most common and important of these gestures is the swipe-back gesture. Below is a GIF I found that hopefully illustrates this gesture clearly. As you can see, after I navigate to a new screen I need to be able to swipe that screen away and see the previous screen underneath. People call this stack-navigation because it looks like you’re swiping away cards in a stack of cards. The tricky thing here is you should be able to have an arbitrary number of cards in the stack. So if I go from screen A -> B -> C then the stack should be [A, B, C]. As an optimization, I only need to render the two most recent cards [B, C]. However, if I swipe away C then I need to be able to render A again. I’m not sure AnimatedRoutes with its key-based solution would be able to inject the correct location back into A after it has unmounted. It may be clearer to see code so I included a WIP implementation of this gesture in issue #7943. Otherwise, I love the v6 API so far! |
My use case is similar to the modal example but a bit more complex, and related to the discussion. I have not been able to implement the current functionality in V6, since I am missing the location prop. There might be another way but I haven't found it, so excuse me if that's the case. You can check it if you visit our demo project (you can just skip de login)
If you open the same product url in a new window you get the same side dialog, but the base collection is now I could not replicate this without overriding the location prop in routes. For the rest, the V6 api is much better in my opinion and allows us to provide better routes customization to our users. |
That's exactly why I've used I.e if you're opening a details page in modal, you're seeing it on top of feed (without losing feed position, etc) but if you send the same link to your friend, they do not need to see the feed you were scrolling, post could have been in any/personal feed. I can't think of a better idea to build that functionality but if there's any, I'm all ears. |
We use react-router in Todoist for the settings, which are provided in a modal UI, and the modal routing works great. What we do is that we internally set a default background URL for when the modal was not triggered by the user, but the URL was visited directly. In this case, the users get the view with the settings modal open right from the start, and the background renders the default view we set. When the users close the settings modal, they land on this view as the initial one (which would've been the page they'd get anyway if they start by visiting https://todoist.com/app). Here's a demo: CleanShot.2021-08-28.at.18.54.58.mp4I trigger the modal from a view called "upcoming", the URL changes fully to not encode anything about the upcoming view being visible underneath. But the upcoming view is still underneath, and all its state preserved if the user closes the settings. In the video, however, I did not close the settings, but I refreshed. You can see that in that case I no longer get the Upcoming view underneath, but the Today view instead (with the settings modal open on top). This is the view I'd get if I visited the app's root path anyway. This is all working, inspired by the modals example in v5 documentation. So I would not call it busted. And the approach is great because it allows us to have users open the settings for a quick tweaking, without loosing any transient state in the page underneath (e.g. they may be in the middle of adding a task that's not yet saved, etc). |
@timdorr No it isn't :) Sorry, I shouldn't have just said the modal example was busted without digging a bit further. It turns out the example code isn't actually busted, just the Codesandbox browser is. My original point stands though; the modal should still be open after a page refresh. This is because Here's how it should work (running in Chrome): Notice how after the page refresh the modal is still open. Here's the broken behavior I mentioned, where the modal is not still open after the page refresh (running in the Codesandbox browser in our docs): Notice how after the page refresh the modal isn't open anymore. That's busted. My guess is the Codesandbox browser doesn't support
@gitcatrat Yes, we agree the modal should not be there in a new window (i.e. your friend's). But if all you're doing is refreshing the existing page (same user, not your friend) you should still see the modal in our example. But anyway, getting back to the original discussion: it's fairly easy to mimic the current modal example without the
I put some state in the So again, I don't think that modals are a good use case for maintaining the |
What if what I need to render in the background is not always the same, but actually driven by an actual location value that's not just boolean? Imagine I have a grid (gallery) view at How can we know here what background to render: |
@gnapse Use a string instead of a boolean. <Route
path="/img/:id"
children={
background === "gallery" ? (
<>
<Gallery />
<Modal />
</>
) ? background === "list" ? (
<>
<List />
<Modal />
</>
) : (
<ImageView />
)
}
/> |
So this solution is similar to what I have implemented while adapting to V6 in https://demo.firecms.co/ Will all due respect @mjackson, I fail to understand the resistance to implement a feature, that basically involves adding a prop and connecting it to an internal API, and that is fixing so many issues (which are there because the prop was removed) In my opinion, adding the
I was really excited with all the new v6 routes system, it is well thought and it allows developers using my library to expand it and customise it ways that were not possible before, especially regarding nested routes. But unfortunately this little detail is breaking it for me. |
Ok, so you're basically telling me to replicate what I've already encoded into routes as an ugly and potentially huge if/else expression in my jsx. A few questions:
It seems you're pretty determined to getting rid of this feature. I really hope you folks reconsider. |
I'm all about simplicity but I can only see pain when looking at my routes. So many different cases..
This would be a nightmare without <>
<Header />
<div className={styles.wrapper}>
<Switch location={background || location}>
<Route exact path="/"><Home /></Route>
<Route path="/login"><Login /></Route>
<Route path="/user/:id"><UserProfile /></Route>
<Route path="/post/:id"><CommunityPost /></Route>
<Route path="/blog/:topic/:sort?"><Blog /></Route>
<Route path="/blog"><Blog /></Route>
<Route><NotFound /></Route>
</Switch>
<Footer />
{background &&
<Modal previousPath={background.pathname}>
<Switch>
<Route path="/login"><Login /></Route>
<Route path="/user/:id"><UserProfile /></Route>
<Route path="/post/:id"><CommunityPost /></Route>
</Switch>
</Modal>
}
</div>
</> |
@gnapse All I did was show you how to do what everyone was saying is impossible to do without the
@fgatti675 I can see why you'd think that for a modal, but I also feel like it's kind of an awkward solution for animation (which is the topic of this issue). For example, maintaining a separate @gitcatrat Thank you for posting your code. I think this example much more clearly illustrates the need for the However, I still think |
Again, this isn't actually more elegant. You still need to get the location from let location = useLocation();
return <TransitionGroup>
<CSSTransition key={location.key}>
<Routes location={location}>
// vs
let location = useLocation();
return <TransitionGroup>
<CSSTransition key={location.key}>
<AnimatedRoutes> And, more importantly, there are now two entirely different sets components/hooks to use in different situations. It becomes unclear if they can be used interchangeably or what their different props imply. Does Maybe it would be helpful to understand the motivations behind this API change. It seems to be to avoid some kind of common footgun that users are running into, although I'm not sure what that is exactly. Can you describe that more or give some other context? You also mentioned v6 being hook-based, but I'm not sure I understand how that factors in to a component API. I feel like there's some common ground amongst the stakeholders here, we just need to find it. |
One idea I just had: <TransitionGroup>
<AnimatedRoutes animator={CSSTransition}> Just |
@timdorr The We use |
@mjackson, it is not that it is ugly. It's the fact that I'd be replicating what react-router already does really well: mapping a location to something that should be rendered for that location. Not just that, there are things that I suspect are impossible. Because react-router's algorithm to map locations to elements goes beyond a mere
What if the location that's used to decide whether to render Do you have a suggestion on how to solve that in user land without the location prop? Even if it's ugly? |
Why
|
@CooCooCaCha I just remembered that you had said this, and I wanted to let you know that React Native support is not a goal for us in v6. We have supported React Router on React Native for several years now, but haven't seen very much engagement from the React Native community, unfortunately. So we are not planning on shipping v6 for React Native. |
That's quite unfortunate to hear. If I'm being honest, I've been following react-router for awhile and I actually got the impression that you all never wanted to support react-native in the first place. Perhaps the community picked up on that too and went a different route. Anyways, best of luck. |
Coming from React DOM world I find out extremely useful to be able to use react-router based components in both web and native version of the app I'm developing. Saves me a ton of time. Do you think there's anything we could do to help releasing RR6 for RN as well? Including financially. |
@ryanflorence I like your solution, but why don't we add a function Outlet({ animated = false }) {
const currentOutlet = useOutlet();
const prevOutlet = useState( currentOutlet );
return (
animated
? prevOutlet
: currentOutlet
);
} |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
What is the new or updated feature that you are suggesting?
React Router v6 should include first-class primitives for supporting animations on route transitions.
A bit of history here: in v4/5, we added the
<Switch location>
prop. This prop provided a way for you to keep the<Switch>
rendering an old location when it is rendered inside another element with akey
that is used to perform an animation, such as a<CSSTransition>
from react-transition-group. It looked something like this:This API worked fine for v4 and v5, but it's not entirely clear outside of the context of animations that the
<Switch location>
prop is to be used for animations. It was proposed in #7117 that we add a<Routes location>
prop that could be used in v6. However, since v6 is built entirely on React hooks, I think we can do better.Instead of a
location
prop, we can provide "animated" versions of all components and hooks that give you elements to render. So, for example, the animated version of<Routes>
would be<AnimatedRoutes>
. You could use it exactly like<Switch location>
, except it's much more clear what it's for.In addition to
<AnimatedRoutes>
we can also provide:<AnimatedOutlet>
useAnimatedRoutes()
useAnimatedOutlet()
Each of these elements and hooks would ofc work exactly like the non-animated version but with one important difference: they don't subscribe to location updates from context. Instead, they must be used inside another element with a
key
that will be unmounted in the transition.AFAICT all animation libraries for React work this way. They rely on a
key
changing on an element to know when an element is going to transition out/in.Why should this feature be included?
Providing animation primitives should make it easier for people to do animations between routes.
The text was updated successfully, but these errors were encountered: