feat!: iOS custom detents & Android form sheets #2045

kkafar · 2024-02-21T11:58:52Z

Description

This PR introduces series of features & changes:

possibility of specifying custom detents for form sheets on devices with iOS 16 or newer,
changes existing form sheet API of Screen component (namely types of values accepted),
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
Android Footer component that works together with formSheet presentation style
🚧 Android modal bottom sheet - similar to formSheet, however the sheet is mounted under separate window.
🚧 iOS Footer component - similar to Android
Usage of Material 3
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

After recent commits - iOS compilation on Fabric
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.
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:
Ensured that CI passes

… fragment underneath is not removed

It is now implemented as extension method on `View` class

…standard modal

tboba

Great changes! Allow me to review this PR in parts 😄 (Part 1/3)

tboba · 2024-05-10T09:16:49Z

FabricTestExample/src/Test1649.tsx

+  // useFocusEffect(
+  //   React.useCallback(() => {
+  //     const handle = setInterval(() => {
+  //       console.log('Hellow from setInterval');
+  //       setBgColor(value => {
+  //         return value === 'firebrick' ? 'green' : 'firebrick';
+  //       });
+  //     }, 750);
+  //
+  //     return () => {
+  //       clearInterval(handle);
+  //     };
+  //   }, []),
+  // );
+
+  // React.useEffect(() => {
+  //   const handle = setInterval(() => {
+  //     console.log('Hellow from setInterval');
+  //     setBgColor(value => {
+  //       return value === 'firebrick' ? 'green' : 'firebrick';
+  //     });
+  //   }, 750);
+  //
+  //   return () => {
+  //     clearInterval(handle);
+  //   };
+  // }, []);


Is this commenting in the test intended? Do we want to leave this logic uncommented? 😄

tboba · 2024-05-10T09:20:53Z

FabricTestExample/src/Test1649.tsx

+              onSheetDetentChanged: (e: NativeSyntheticEvent<{ index: number }>) => {
+                console.log(`onSheetDetentChanged in App with index ${e.nativeEvent.index}`)
+              },


Why is this event defined like this? Don't we want to use listeners object in this case and define this event there?

tboba · 2024-05-10T09:23:37Z

TestsExample/App.js

-enableFreeze(true);
+// enableFreeze(true);
+enableFreeze(false);

 export default function App() {
-  return <Test42 />;
+  return <Test1649 />;


This shouldn't be Test1649 and enableFreeze should be still true

tboba · 2024-05-10T09:24:07Z

TestsExample/android/app/build.gradle

+
+    implementation("com.google.android.material:material:1.9.0")


Shouldn't this be defined inside the build.gradle of screens? Or moved to the separate PR? 😄

tboba · 2024-05-10T09:24:36Z

TestsExample/android/app/src/main/res/values/styles.xml

-    <style name="AppTheme" parent="Theme.AppCompat.DayNight.NoActionBar">
+    <style name="AppTheme" parent="Theme.Material3.DayNight.NoActionBar">


Same as above

tboba · 2024-05-10T12:46:49Z

android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt

                override fun runGuarded() {
                    // If the status bar is translucent hook into the window insets calculations
                    // and consume all the top insets so no padding will be added under the status bar.
-                    val decorView = activity.window.decorView
+//                    val decorView = activity.window.decorView


to be removed

tboba · 2024-05-10T12:47:09Z

android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt

                    if (translucent) {
+                        Log.w("ScreenWindowTraits", "setting onApplyWindowInsetsListener.Callback on $decorView")


To be removed :D

tboba · 2024-05-10T12:47:15Z

android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt

                        ViewCompat.setOnApplyWindowInsetsListener(decorView) { v, insets ->
+                            Log.w("ScreenWindowTraits", "onApplyWindowInsetsListener.Callback on $v")


to be removed

tboba · 2024-05-10T12:47:26Z

android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt

+//                        ViewCompat.requestApplyInsets(decorView)
                    } else {
+                        Log.w("ScreenWindowTraits", "Removing listener from $decorView")
                        ViewCompat.setOnApplyWindowInsetsListener(decorView, null)
+//                        ViewCompat.setOnApplyWindowInsetsListener(decorView) { view, insets ->
+//                            val isImeVisible = insets.isVisible(WindowInsetsCompat.Type.ime())
+//                            val prevInsets = insets.getInsets(WindowInsetsCompat.Type.navigationBars())
+//                            if (isImeVisible) {
+//                                WindowInsetsCompat
+//                                    .Builder(insets)
+//                                    .setInsets(
+//                                        WindowInsetsCompat.Type.navigationBars(),
+//                                        Insets.of(
+//                                            prevInsets.left,
+//                                            prevInsets.top,
+//                                            prevInsets.right,
+//                                            0
+//                                        )
+//                                    ).build()
+//                            } else {
+//                                insets
+//                            }
+//                        }
+//                        ViewCompat.setOnApplyWindowInsetsListener(decorView, decorView.getTag(androidx.core.R.id.tag_on_apply_window_listener) as? OnApplyWindowInsetsListener)
+                        hasAttachedWindowInsetsCallback = false
+//                        ViewCompat.requestApplyInsets(decorView)


Let's remove comments

tboba · 2024-05-10T13:17:02Z

android/src/main/java/com/swmansion/rnscreens/bottomsheet/BottomSheetDialogRootView.kt

+    private var jsPointerDispatcher: JSPointerDispatcher? = null
+
+    init {
+        // Can we safely use ReactFeatureFlags?


Let's for now remove this comment and assume we can 😄

^Nevermind, lemme check source code of RN to find out if we can 🤔

tboba

I was lying, that my review will take 3 parts 😄

Part 2/2

tboba · 2024-05-13T11:28:53Z

android/src/main/java/com/swmansion/rnscreens/bottomsheet/BottomSheetDialogRootView.kt

+    override fun handleException(throwable: Throwable?) {
+        // TODO: I need ThemedReactContext here.
+        // TODO: Determine where it is initially created & verify its lifecycle
+ //        reactContext?.reactApplicationContext?.handleException(RuntimeException(throwable))
+    }


Should we still keep this?

tboba · 2024-05-13T11:29:08Z

android/src/main/java/com/swmansion/rnscreens/bottomsheet/BottomSheetDialogScreen.kt

+package com.swmansion.rnscreens.bottomsheet
+
+ import android.content.Context
+ import android.util.Log


Optimize imports 😄

tboba · 2024-05-13T11:29:23Z

android/src/main/java/com/swmansion/rnscreens/bottomsheet/BottomSheetDialogScreen.kt

+    // to run, as this will lead to inconsistencies in ScreenStack state. Instead we intercept
+    // dismiss intention and run our logic.
+    override fun cancel() {
+        Log.d(TAG, "cancel")


Remove Log call

tboba · 2024-05-13T11:29:33Z

android/src/main/java/com/swmansion/rnscreens/bottomsheet/BottomSheetDialogScreen.kt

+    // dismiss intention and run our logic.
+    override fun cancel() {
+        Log.d(TAG, "cancel")
+        fragmentRef.get()!!.dismissFromContainer()


requireNotNull()? 🥺

tboba · 2024-05-13T11:29:42Z

android/src/main/java/com/swmansion/rnscreens/bottomsheet/DimmingFragment.kt

+import android.graphics.Color
+import android.os.Build
+import android.os.Bundle
+import android.util.Log


Optimize imports

tboba · 2024-05-13T12:25:55Z

src/native-stack/views/FooterComponent.tsx

+  // return (
+  //   <ScreenContentWrapper style={{ display: "flex", flexDirection: "column", justifyContent: "space-between" }}>
+  //     <View collapsable={false} />
+  //     <ScreenFooter
+  //       collapsable={false}>
+  //       {children}
+  //     </ScreenFooter>
+  //   </ScreenContentWrapper>
+  // );


tboba · 2024-05-13T12:26:18Z

src/native-stack/views/NativeStackView.tsx

+        // stackPresentation === 'formSheet' && Platform.OS === 'ios' ? styles.absoluteFillNoBottom : styles.container,
+        // styles.container,
+        stackPresentation === 'formSheet'
+          ? Platform.OS === 'ios'
+            ? styles.absoluteFillNoBottom
+            : null
+          : styles.container,
+        // stackPresentation === 'formSheet' && Platform.OS === 'ios' ? styles.absoluteFillNoBottom : styles.container,


Remove comments

tboba · 2024-05-13T12:26:49Z

src/native-stack/views/NativeStackView.tsx

@@ -319,31 +349,36 @@ const RouteView = ({
        });
      }}
      onWillAppear={() => {
+        console.log(`onWillAppear/transitionStart route: ${route.key}`);


Remove console.logs

tboba · 2024-05-13T12:27:10Z

src/types.tsx

@@ -256,20 +262,30 @@ export interface ScreenProps extends ViewProps {
   * - "landscape_right" – landscape-right orientation is permitted
   */
  screenOrientation?: ScreenOrientationTypes;
+  screenStyle?: StyleProp<ViewStyle>;


What does this prop do?

tboba · 2024-05-13T12:27:27Z

src/types.tsx

+  /**
+   * Integer value describing elevation of the sheet, impacting shadow on the top edge of the sheet.
+   *
+   * Not dynamic.


What does not dynamic mean?

tboba · 2024-05-13T12:29:43Z

Also, should we create an example of how does new sheets behave with new detent types on iOS/Android? Example of using ScreenFooter would be also helpful!

kkafar added 30 commits September 15, 2023 14:18

TOREVERT: Bump native deps & kotlin version in TestsExample & lib

449b2b9

Display modal

8554c41

Make StackPresentation.MODAL temporarily transparent to make sure the…

47b1bcf

… fragment underneath is not removed

Allow for dismissal

55d57cb

Refactor ScreenStack.rootScreen property impl

9d320c9

Go back button on Second screen in Test1649 (in TE)

81d6744

Attempt to dismiss modal via container dismiss method

75986e2

TOREVERT(manual): Modify Test1649

6dfbee1

TOREVERT: Log upon receiving native events on JS side

5b103b1

Bring ScreenModalFragment up to speed with main

8706e6b

Managed to present the fragment modally

2a3815b

TOREVERT: temporarily disable custom stack animations

9171cc3

Kotlinize implementation of ScreenStack.adapt method

e9ebb88

Play around with animation

e37055e

Stash changes in modal fragment

9ed55b7

Move recycleView method to a ext module

6e0036e

It is now implemented as extension method on `View` class

Recycle screen before adding it to modal layout

acac110

Another stash of changes 🤦

9c1ba79

Set behaviour on screen instead of using dedicated fragment type for …

7110e03

…standard modal

Some options for the modal

7c50989

Improve view extensions

8cb8d05

Rename dismiss -> dismissFromContainer

85df811

Add more comments into interfaces

256fa71

Present 'modal' modally & formSheet as standard bottom sheet

1d6b1fd

Stubs, tryGetContext / tryGetActivity, retrieve few props from JS

a64a400

Merge branch 'main' into @kkafar/android-modal-refactor-4

de0974b

MAKE DISMISSAL WORK ON MODAL SHEET

bd936c9

Throw when fragmentRef.get == null

49db846

Clip bottomsSheet to outline making corner shape visible

e5b8e95

Throw better error message on fragment restoration

541ae85

kkafar added 12 commits May 7, 2024 17:32

Next round of cleanup

cca0279

Restore modal code & cleanup

c566288

Unify examples

626319b

Setup onSheetDetentChangedEvent on Android

40c1abd

Emit events on sheet detent change

28efb25

Report detent change on iOS correctly

b4698d0

Unify examples

dcb97da

Add isStable info to sheet detent change event

a6ccf8d

Fix conversions on Fabric iOS

458af30

Fix event emitting on paper

d056286

Add dependency on jotai in FTE

5f14195

Update TE

809aee5

kkafar requested a review from tboba May 9, 2024 07:52

kkafar self-assigned this May 9, 2024

kkafar mentioned this pull request May 10, 2024

presentation="modal" not working on android #1650

Open

Attempt to fix iOS compilation

ed74c9f

tboba requested changes May 10, 2024

View reviewed changes

Add buttom for showing another sheet from current sheet

ffefdfc

tboba requested changes May 13, 2024

View reviewed changes

tboba requested a review from WoLewicki May 14, 2024 09:18

kkafar added 9 commits May 15, 2024 14:37

Merge branch 'main' into @kkafar/custom-detents-pg

9289f24

Add missing import

c03decb

cleanup registered callback on fragment removal

e8b6824

Change computation of maxHeight for sheet with keyboard visible

99f95a6

Update example

50adfc4

Update android layout

255e4c5

Use InsetsObserverProxy in place of regular onApplyWindowInsetsListener

42e86bb

Use rolling insets in observer proxy

14d960d

Fix jumping navigationBar inset on Android

9d2bcda

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: iOS custom detents & Android form sheets #2045

feat!: iOS custom detents & Android form sheets #2045

kkafar commented Feb 21, 2024 •

edited

tboba left a comment

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 10, 2024

tboba May 13, 2024

tboba left a comment

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba May 13, 2024

tboba commented May 13, 2024 •

edited


		implementation("com.google.android.material:material:1.9.0")

		<style name="AppTheme" parent="Theme.AppCompat.DayNight.NoActionBar">
		<style name="AppTheme" parent="Theme.Material3.DayNight.NoActionBar">

		if (translucent) {
		Log.w("ScreenWindowTraits", "setting onApplyWindowInsetsListener.Callback on $decorView")

		ViewCompat.setOnApplyWindowInsetsListener(decorView) { v, insets ->
		Log.w("ScreenWindowTraits", "onApplyWindowInsetsListener.Callback on $v")

feat!: iOS custom detents & Android form sheets #2045

Are you sure you want to change the base?

feat!: iOS custom detents & Android form sheets #2045

Conversation

kkafar commented Feb 21, 2024 • edited

Description

Changes

Known issues

Test code and steps to reproduce

Checklist

tboba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tboba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tboba commented May 13, 2024 • edited

kkafar commented Feb 21, 2024 •

edited

tboba commented May 13, 2024 •

edited