ViewTransitions in Navigation#32028
Merged
Merged
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
|
Comparing: 98418e8902d6045e5138a2e765e026ce2e4de82d...70080fb34a0a135605859cf824255d09c7bc4216 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
sebmarkbage
force-pushed
the
viewtransitionnav
branch
from
8c0f119 to
6bb576b
Compare
…etes This ensures that we are measuring things in the viewport after scroll restoration happens. Otherwise we end up with the wrong assumptions about what will end up in the viewport.
In this case we flush the layout effects. We also used to incorrect flush after in the wrong order.
sebmarkbage
force-pushed
the
viewtransitionnav
branch
from
6bb576b to
143c7f8
Compare
acdlite
approved these changes
Jan 8, 2025
github-actions Bot
pushed a commit
that referenced
this pull request
Jan 9, 2025
This adds navigation support to the View Transition fixture using both `history.pushState/popstate` and the Navigation API models. Because `popstate` does scroll restoration synchronously at the end of the event, but `startViewTransition` cannot start synchronously, it would observe the "old" state as after applying scroll restoration. This leads to weird artifacts. So we intentionally do not support View Transitions in `popstate`. If it suspends anyway for some other reason, then scroll restoration is broken anyway and then it is supported. We don't have to do anything here because this is already how things worked because the sync `popstate` special case already included the sync lane which opts it out of View Transitions. For the Navigation API, scroll restoration can be blocked. The best way to do this is to resolve the Navigation API promise after React has applied its mutation. We can detect if there's currently any pending navigation and wait to resolve the `startViewTransition` until it finishes and any scroll restoration has been applied. https://github.com/user-attachments/assets/f53b3282-6315-4513-b3d6-b8981d66964e There is a subtle thing here. If we read the viewport metrics before scroll restoration has been applied, then we might assume something is or isn't going to be within the viewport incorrectly. This is evident on the "Slide In from Left" example. When we're going forward to that page we shift the scroll position such that it's going to appear in the viewport. If we did this before applying scroll restoration, it would not animate because it wasn't in the viewport then. Therefore, we need to run the after mutation phase after scroll restoration. A consequence of this is that you have to resolve Navigation in `useInsertionEffect` as otherwise it leads to a deadlock (which eventually gets broken by `startViewTransition`'s timeout of 10 seconds). Another consequence is that now `useLayoutEffect` observes the restored state. However, I think what we'll likely do is move the layout phase to before the after mutation phase which also ensures that auto-scrolling inside `useLayoutEffect` are considered in the viewport measurements as well. DiffTrain build for [800c9db](800c9db)
github-actions Bot
pushed a commit
that referenced
this pull request
Jan 9, 2025
This adds navigation support to the View Transition fixture using both `history.pushState/popstate` and the Navigation API models. Because `popstate` does scroll restoration synchronously at the end of the event, but `startViewTransition` cannot start synchronously, it would observe the "old" state as after applying scroll restoration. This leads to weird artifacts. So we intentionally do not support View Transitions in `popstate`. If it suspends anyway for some other reason, then scroll restoration is broken anyway and then it is supported. We don't have to do anything here because this is already how things worked because the sync `popstate` special case already included the sync lane which opts it out of View Transitions. For the Navigation API, scroll restoration can be blocked. The best way to do this is to resolve the Navigation API promise after React has applied its mutation. We can detect if there's currently any pending navigation and wait to resolve the `startViewTransition` until it finishes and any scroll restoration has been applied. https://github.com/user-attachments/assets/f53b3282-6315-4513-b3d6-b8981d66964e There is a subtle thing here. If we read the viewport metrics before scroll restoration has been applied, then we might assume something is or isn't going to be within the viewport incorrectly. This is evident on the "Slide In from Left" example. When we're going forward to that page we shift the scroll position such that it's going to appear in the viewport. If we did this before applying scroll restoration, it would not animate because it wasn't in the viewport then. Therefore, we need to run the after mutation phase after scroll restoration. A consequence of this is that you have to resolve Navigation in `useInsertionEffect` as otherwise it leads to a deadlock (which eventually gets broken by `startViewTransition`'s timeout of 10 seconds). Another consequence is that now `useLayoutEffect` observes the restored state. However, I think what we'll likely do is move the layout phase to before the after mutation phase which also ensures that auto-scrolling inside `useLayoutEffect` are considered in the viewport measurements as well. DiffTrain build for [800c9db](800c9db)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds navigation support to the View Transition fixture using both
history.pushState/popstateand the Navigation API models.Because
popstatedoes scroll restoration synchronously at the end of the event, butstartViewTransitioncannot start synchronously, it would observe the "old" state as after applying scroll restoration. This leads to weird artifacts. So we intentionally do not support View Transitions inpopstate. If it suspends anyway for some other reason, then scroll restoration is broken anyway and then it is supported. We don't have to do anything here because this is already how things worked because the syncpopstatespecial case already included the sync lane which opts it out of View Transitions.For the Navigation API, scroll restoration can be blocked. The best way to do this is to resolve the Navigation API promise after React has applied its mutation. We can detect if there's currently any pending navigation and wait to resolve the
startViewTransitionuntil it finishes and any scroll restoration has been applied.scroll-restoration.mov
There is a subtle thing here. If we read the viewport metrics before scroll restoration has been applied, then we might assume something is or isn't going to be within the viewport incorrectly. This is evident on the "Slide In from Left" example. When we're going forward to that page we shift the scroll position such that it's going to appear in the viewport. If we did this before applying scroll restoration, it would not animate because it wasn't in the viewport then. Therefore, we need to run the after mutation phase after scroll restoration.
A consequence of this is that you have to resolve Navigation in
useInsertionEffectas otherwise it leads to a deadlock (which eventually gets broken bystartViewTransition's timeout of 10 seconds). Another consequence is that nowuseLayoutEffectobserves the restored state. However, I think what we'll likely do is move the layout phase to before the after mutation phase which also ensures that auto-scrolling insideuseLayoutEffectare considered in the viewport measurements as well.