-
Notifications
You must be signed in to change notification settings - Fork 41
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 Bottom Sheet Component (iOS) #744
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
Nothing was called when the sheet was animated - not viewDidLayoutSubviews. The frame size didn't even change - tried logging it out. But managed to get it on the presentationLayer! Got the [clue from react native screens ](https://github.com/software-mansion/react-native-screens/pull/1870/files#diff-ad35cf2dc9a86855afc1d2443a42a133ccf0dd8b17589719cb1aab4a81d20604R127)
Had to use a container view (got [the idea from React Native's Modal](https://github.com/facebook/react-native/blob/d077239fff600d41d093dfeca87f6744bd7f1cd3/packages/react-native/React/Views/RCTModalHostView.m#L38)). Without the container couldn't resize it - it worked when dragging but not when dropping. All the measurements went out. Guessing that resizing while it's being animated is bad. So used the container view as the animated view and resized the content. It works! but it's jumpy when dragging.
For smoothness, need the bounds did change resize for when dragging and the animated resize for when dropping. Need to find a way so they only run for their own use-case. Having them both running at the same time causes flickers
Don't want both the dragging and the dropping resize to run at the same time. Seems to cause extra flicker. Got the [detection code from react native screens](https://github.com/software-mansion/react-native-screens/blob/2de99533cd716f3757927e0b5a08a4f647a5d1e0/ios/RNSScreen.mm#L112). If there's an animation when frames layout called then it's dropping, otherwise it's dragging
Don't think it's any different if the resizes run together. But don't want the bounds resize when the sheet is dropped because that resizes to the destination detent (at the start of the drop). This causes the big glitch - it's hidden by the dropping resize so hard to see - but don't want it to be there
By default the collapsed detent is the built-in medium. But if peekHeight provided then defined a custom collapsed detent with that height and used it in place of the medium detent. This matches the behaviour of peekHeight on android
Tested dynamic peekHeight and ios automatically resizes
Keeps diff to a minimum
Used them to set a custom detent that acts as the large detent. The expanded height is the detent height and the offset is taken off the max detent height
These styles only apply on android
UISheetPresentationController not available in iOS < 15
Followed same pattern as done for collapsed and expanded detents. Created a custom detent and defaulted it to large. Only added if not the default
Prevented swipe to dismiss if not hideable
If not draggable added only the selected detent. So can't drag because only valid detent is the selected one. Checked can still programmatically change to another detent
Puts all set detents together
Also added a presented bool check because the window wasn't good enough. When navigating to another scene the didSetProps fired after didMoveToWindow but the reactSubView window wasn't set so tried to present twice and got error
Copied it from the react native modal but would rather wait to find it if it's needed. Maybe if background is undimmed so can close the scene without first dismissing?
Not done any bottom sheet stuff but this was needed to avoid runtime error
Made detent a string so it can be passed to android and ios. Will convert back to int in java managers - consistent with autoCapitalize prop of SearchBar
Can't resize a subview on new architecture, so changed old to check it worked
Saw fabric modal didn't use a container view so though maybe old arch didn't need it either
Saw fabric modal didn't use one so figured could remove. Removed and tested in old arch
It's not sizing correctly yet but it is showing as a sheet
Created the cpp state files etc. Copied from TitleBar/SearchBar
Seems a bit iffy on tapping r (reload) - sheet doesn't always come back?!
The sheet didn't reopen after tapping r. Dismissing the controller when recycling fixes this (worked out by comparing with fabric modal - although it does it in didMoveToSuperview - prefer recycle)
Copied the code from React Native modal
Made it a string so it's same on android and ios (ios doesn't use constants)
Made detent string because it's string on ios. Also added expandedHeight prop because forced to on fabric
Tried on ios 15 in the medley sample and the north button was pressable when sheet was hidden. iOS doesn't resize when it hides, so the height doesn't go below the collapsed height. On ios 15 this collapsed height blocked the north button
The north button still wasn't pressable using height on fabric on ios. Changing to display works on old and new arch
This was referenced Nov 10, 2023
Closed
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.
Wrapped the iOS UISheetPresentationController in the
BottomSheet
component. Implemented all the existing 'Android' props on iOS 16 using custom detents. For example, forpeekHeight
defined a custom detent with that height instead of the medium detent. Fordraggable
andskipCollapsed
props set the current detent as the only valid one. The bottom sheet works on iOS 15 but it doesn’t support all props.Tried to do the iOS bottom sheet a long time ago but the animation was janky when the sheet was dropped. Couldn’t find a way to track the animation and resize the view, for example, iOS doesn’t call
viewDidLayoutSubviews
. Finally worked out how to get the animated values from thepresentationLayer
.The animation still isn’t completely smooth because React Native resizes async. Won’t be able to smooth this out until the new React Native architecture supports synchronous render.