Skip to content
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

Merged
merged 319 commits into from
Aug 10, 2024
Merged

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Feb 21, 2024

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. 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: Fix ScrollView interactions with Android's CoordinatorLayout facebook/react-native#44099, no other workaround found. There is one approach suggested by grahammendick, 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

Copy link
Member

@tboba tboba left a 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/Screen.kt Outdated Show resolved Hide resolved
android/src/main/java/com/swmansion/rnscreens/Screen.kt Outdated Show resolved Hide resolved
android/src/main/java/com/swmansion/rnscreens/Screen.kt Outdated Show resolved Hide resolved
ios/RNSScreen.mm Show resolved Hide resolved
ios/RNSScreen.mm Outdated Show resolved Hide resolved
ios/RNSScreen.mm Outdated Show resolved Hide resolved
ios/RNSScreen.mm Outdated Show resolved Hide resolved
native-stack/README.md Outdated Show resolved Hide resolved
@diamont1001
Copy link

Looking forward to the new release

Copy link
Member Author

@kkafar kkafar left a 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 {
Copy link
Member Author

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.

Comment on lines +364 to +392
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")
}
Copy link
Member Author

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.

@kkafar kkafar merged commit 85a4196 into main Aug 10, 2024
9 checks passed
@kkafar kkafar deleted the @kkafar/custom-detents-pg branch August 10, 2024 19:02
@kkafar
Copy link
Member Author

kkafar commented Aug 10, 2024

For every one interested: I've released 4.0.0-beta.0 #2045

Testing & any feedback is very very welcome. Thanks!

@terrysahaidak
Copy link

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

@shovel-kun
Copy link

shovel-kun commented Oct 8, 2024

Same behavior on Xiaomi Note 10X running Android 13

ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
## 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
kkafar added a commit that referenced this pull request Oct 21, 2024
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants