-
-
Notifications
You must be signed in to change notification settings - Fork 521
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
feat!: iOS custom detents & Android form sheets #2045
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We comin' home 😄 💪 💪
android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt
Outdated
Show resolved
Hide resolved
Looking forward to the new release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few remarks for future me.
I'm merging this PR & releasing the changes as beta version of the library to gather community feedback & bug reports.
@tboba @alduzy @maciekstosio @maksg I'll break our standard merging procedure by not first merging API changes to react-navigation
. Please be aware of that - we will have a custom patch for RN for 1,5 week until I'm back and tidying things up.
import androidx.core.view.WindowInsetsCompat | ||
import java.lang.ref.WeakReference | ||
|
||
object InsetsObserverProxy : OnApplyWindowInsetsListener { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is needed, because sheets do need to listen for keyboard appearance. Each sheet does need to register a listener to decor view, while Android allows for only single listener! Thus I've introduced a proxy, who can aggregate listeners and fan out the events.
override fun canDispatchLifecycleEvent(event: ScreenFragment.ScreenLifecycleEvent): Boolean { | ||
TODO("Not yet implemented") | ||
} | ||
|
||
override fun updateLastEventDispatched(event: ScreenFragment.ScreenLifecycleEvent) { | ||
TODO("Not yet implemented") | ||
} | ||
|
||
override fun dispatchLifecycleEvent( | ||
event: ScreenFragment.ScreenLifecycleEvent, | ||
fragmentWrapper: ScreenFragmentWrapper, | ||
) { | ||
TODO("Not yet implemented") | ||
} | ||
|
||
override fun dispatchLifecycleEventInChildContainers(event: ScreenFragment.ScreenLifecycleEvent) { | ||
TODO("Not yet implemented") | ||
} | ||
|
||
override fun dispatchHeaderBackButtonClickedEvent() { | ||
TODO("Not yet implemented") | ||
} | ||
|
||
override fun dispatchTransitionProgressEvent( | ||
alpha: Float, | ||
closing: Boolean, | ||
) { | ||
TODO("Not yet implemented") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are here, because they are present in ScreenWrapper
interface. Once they are moved out from here, we can safely remove them & reintroduce one by one as needed.
For every one interested: I've released 4.0.0-beta.0 #2045 Testing & any feedback is very very welcome. Thanks! |
So I tried it run the main branch, formsheet route on Pixel 8 pro looks like this for me (i made background of the screen red): screen-20240811-161216.mp4 |
Same behavior on Xiaomi Note 10X running Android 13 |
## Description This PR introduces series of features & changes: 1. possibility of specifying custom detents for form sheets on devices with iOS 16 or newer, 2. changes existing form sheet API of `Screen` component (namely types of values accepted), 3. Android form sheets (bottom sheets presented in current presentation context (in iOS terms) with dimming view with configurable interaction. The form sheet supports up to three detent levels with additional option of `fitToContents` 4. Android Footer component that works together with `formSheet` presentation style 5. 🚧 Android modal bottom sheet - similar to `formSheet`, however the sheet is mounted under separate window. 6. 🚧 iOS Footer component - similar to Android 7. Usage of Material 3 8. series of new props allowing for: a. controlling style of the `Screen` component (necessary workaround for issue with flickering on iOS, b. controlling whether the screen fragment of particular screen should be unmounted or not on Android when the screen is on JS stack but not visible (necessary to achieve "staying form sheet" when navigating back to a screen with presented form sheet), c. listening for `sheetDetentChange` events, in case of Android stable & dragging states are reported, in case of iOS only stable states d. todo: describe rest ## Changes ## Known issues 1. [x] ~After recent commits - iOS compilation on Fabric~ 2. [ ] Android: issue with nested scrollview - invalid behaviour when there is not enough content for scrollview to scroll (viewport is >= content size). Solvable by patching react-native: facebook/react-native#44099, no other workaround found. There is one approach [suggested by grahammendick](https://github.com/grahammendick/navigation/blob/916688d267bd3fc520e2e22328b6aa66124f52ed/NavigationReactNative/src/android/src/main/java/com/navigation/reactnative/CoordinatorLayoutView.java#L96-L148), however yet untested. 3. [ ] Android 'modal' presentation can crash randomly (unknown reason yet, can be deffered) ## Test code and steps to reproduce I've used & extended `Test1649` to present all capabilities of new API. ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: <!-- For adding new props to native-stack --> - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes
## Description I plant to extract most of the form sheet API related code out of `ScreenStackFragment`, so that it is not polluted that much and more readable. Having experienced the #2045, where big changes were rolled out at once, now I want to do it in couple of separate PRs. This one simplifies sheet behaviour configuration logic in ScreenStackFragment by implementing series of helper extension functions on BottomSheetBehavior. ## Test code and steps to reproduce Test1649 ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: <!-- For adding new props to native-stack --> - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes
Description
This PR introduces series of features & changes:
Screen
component (namely types of values accepted),fitToContents
formSheet
presentation styleformSheet
, however the sheet is mounted under separate window.a. controlling style of the
Screen
component (necessary workaround for issue with flickering on iOS,b. controlling whether the screen fragment of particular screen should be unmounted or not on Android when the screen is on JS stack but not visible (necessary to achieve "staying form sheet" when navigating back to a screen with presented form sheet),
c. listening for
sheetDetentChange
events, in case of Android stable & dragging states are reported, in case of iOS only stable statesd. todo: describe rest
Changes
Known issues
After recent commits - iOS compilation on FabricScrollView
interactions with Android'sCoordinatorLayout
facebook/react-native#44099, no other workaround found. There is one approach suggested by grahammendick, however yet untested.Test code and steps to reproduce
I've used & extended
Test1649
to present all capabilities of new API.Checklist