-
Notifications
You must be signed in to change notification settings - Fork 42
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
Supported shared element transitions (iOS 18) #845
Merged
Merged
Conversation
This file contains 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
Folows the Android approach. When added to the window the shared elements adds itself to the scene so it's accessible from the NavigationStackView
Matches with the HashSet approach on Android
Setting the preferred transition to zoom and returning the shared element does the trick. (Changing to different shared element on details not working)
In the zoom sample, change color on the details scene then press/swipe back. This didn't work on ios because the shared elements didn't update until the back navigation happened - that was ok for android because the navigation happens on js side first then native. But on ios the navigation back happens native first then js. So updated the shared elements on refresh navigation, too. This is handy for Android with predictive back, too. But shared elements don't work with predictive back.
In the zoom example navigate to red then back then to quickly tap on crimson. This would do nothing. And no matter how many times tapping crimson did nothing still. The fast crimson navigation happened before the red scene was removed from the stack on the React side. Because of #838 both red and crimson get the same key so on native side the scene is already added to the stack because it's the same key. So nothing happens. The same scenario would sometimes give an error instead of doing nothing. If the pop from native red scene fired at just the right time then the scene would be removed and the view on the native crimson scene (same as the red scene) would be null. Then findNavigationBar would throw an error trying to append crumb to string. Realised that #838 had actually caused a regression of #585. Giving scenes the same key meant that funny things could happen when navigating fast. The native pop could happen and remove scenes that should be there. It’s actually hard to recreate #585 error scenario normally - but shared element navigation made it easy for some reason. Guess that popping keeps the popped scene around a bit longer so it’s easier to navigate again before the scene destructor is called by iOS. Anyway put back the count from #585 and the problem is fixed. Will check later whether #838 still works.
Matches android check
Matches android behaviour. If there's a shared element name but no matching view then don't do zoom
This was referenced Dec 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #809
Hooked the
SharedElement
component up to the new fluid zoom transition in iOS 18.Also fixed a regression of #585 by restoring count in scene keys. On native, it’s essential that all forward navigations get unique keys. Quick navigation back then forward in zoom sample on iOS shows the problem with non-unique keys in easier way than outlined in #585.