-
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 sheet transitions (iOS 18) #849
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
Added sharedElement prop to sheet and setPreferredTransition to zoom if provided. Same idea as already in place for scenes
Thought was gonna have to setPreferredTransition in didSetProps as well as didMoveToWindow. But didMoveToWindow seems to be enough - it's almost always the place where the sheet is first shown because have to wait until it's in window; and because it's removed from tree when dismissed (so close then open adds it back to window). The only time could get didSetProps to do the opening is when non-modal open then navigate and then gesture back. This fires didMoveToWindow before dismissed cleared so didSetProps handles it - but there's no need to setPreferredTransition because it was already set when it was opened
Open non-modal sheet with zoom transition on scene A. Then navigate to scene B and quickly navigate back to A using a custom button. With the new fluid zoom the onNavigated in javascript would fire before the onDismissed from native. That meant the sheet didn't reopen (and wouldn't open again). The timing on zoomed controllers is different to non-zoomed. They stay around longer - take longer to fire the didDisappear events etc. - so other things can happen while they're still open/animating. Changed to dismiss without animation if the sheet is fluidly zoomed - this fixes the timing. Also it's just better. It looked v odd to animate the closing of the sheet on scene B because it would try to zoom back down to an element that isn't there anymore. For now have assumed there's only one sheet open but will investigate what happens if there's multiple.
Shared elements with nested sheets didn't work because the nested sheet couldn't access the scene. The scene wasn't an ancestor of the sheet because it was a descendant of the BottomSheetController not the SceneController. So moved the shared elements onto the Controller so can access them using reactViewController - in scene and first sheet this will return SceneController; in nested sheet it will return BottomSheetController
The timing with zoomed controllers is very different - they're much slower to complete/dismiss then traditional controllers. So with non-modal open and navigate away and back quickly the onNavigated fires before the onDismiss and then the sheet doesn't reopen. So turned off the animation for zoomed sheets - it was better without because otherwise the sheet zooms back to nothing because it's different scene. But this it's too tricky to do this when there are multiple sheets open - checking if any of them are zoomed is hard - and there's no gain. Better to disable the animation completely because it's consistent. Plus avoids potential timing issues with even non-zoomed sheets if the back happens super fast or the dismiss is slow
Created protocol NVSharedElement so can store both old and new shared element views on the controller
These imports are old arch so don't want them included
Also removed the android platform restriction on shared elements - should've done this when adding shared elements for scenes on ios 18
It did work before but just by luck
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.
Follow up to #845. The new fluid zoom transitions in iOS 18 work when presenting any
UIViewController
so hooked them up to theSheet
component.There's no target
SharedElement
inside theSheet
because iOS fluid transitions target theUIViewController
(the target isn't even needed inside the destinationScene
).Can even share elements when opening a Sheet B from Sheet A. The
SharedElement
must come from Sheet A. Shared element names only have to be unique within a Scene or Sheet. For example, can use the same name for a shared element in the Scene, when opening Sheet A, and for an element in Sheet A, when opening Sheet B.