From 5cb65208d1f8033e03fd6799380ff2d4fe79aafb Mon Sep 17 00:00:00 2001 From: Kacper Kafara Date: Mon, 18 Nov 2024 19:19:35 +0100 Subject: [PATCH] fix(Android): fix formSheet-keyboard interaction (#2511) ## Description I've investigated the regression on form sheet keyboard behaviour. It has stopped respecting allowed detents when keyboard was opened. https://github.com/user-attachments/assets/42d393c8-a50b-40f4-a8cc-41bca122b4aa I tracked down the regression to #2371. I do not understand the interactions completely, as the regression happens only when `statusBarTranslucent` is explicitly set to `false`. `true` or `undefined` make the formsheet-keyboard interaction work correctly. This is especially baffling that I'm testing this on Fabric, therefore `false` is always provided What I understand is that *setting insets observer* to `null` in `ScreenWindowTraits.setTranslucent` effectively removes **any** listener currently attached to decor view, thus preventing other functionalities from work (such as `StatusBar.setTranslucent`). Right now all inset listeners on decor view in our lib are supposed to be managed via `InsetObserverProxy` and I want to keep it that way. The "invalid behaviour" #2371 was supposed to fix, was caused by call to `StatusBar.setTranslucent` in Example app, which started to work after I've removed "nulling any listener" in `ScreenWindowTraits.setTranslucent`. I am not sure what is the intended interaction, but earlier we simply prevented `StatusBar` api to work correctly (it could work if user had a lucky timing...). I guess we'll be better off with what I'm proposing now. Bottomline is I want to get rid of these props from the lib anyway. @WoLewicki I'm gonna proceed, but I want you to provide me with your opinion, especially in case your assessment is different from mine. ## Changes * Clearing only our observer if it has been registered with proxy, * Removed the call to `StatusBar.setTranslucent` in Example app making it work correctly. I don't know why call to `setTranslucent` was there in first place, most likely some left over after `statusBarTranslucent` prop had been introduced. This is how this works now: https://github.com/user-attachments/assets/baa5ed34-11ab-4549-b35b-b86d5ef0b471 ## Test code and steps to reproduce `TestAndroirdTransitions` ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: - [ ] 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 --- .../rnscreens/InsetsObserverProxy.kt | 11 +- .../swmansion/rnscreens/ScreenWindowTraits.kt | 1 - apps/Example.tsx | 4 - apps/src/tests/TestAndroidTransitions.tsx | 102 ++++++++++++++++++ apps/src/tests/index.ts | 1 + 5 files changed, 108 insertions(+), 11 deletions(-) create mode 100644 apps/src/tests/TestAndroidTransitions.tsx diff --git a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt index dd5468284f..8da1f52ed1 100644 --- a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt +++ b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt @@ -23,10 +23,7 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener { ): WindowInsetsCompat { var rollingInsets = if (shouldForwardInsetsToView) { - WindowInsetsCompat.toWindowInsetsCompat( - v.onApplyWindowInsets(insets.toWindowInsets()), - v, - ) + ViewCompat.onApplyWindowInsets(v, insets) } else { insets } @@ -57,8 +54,10 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener { } } - fun unregisterOnView(view: View) { - ViewCompat.setOnApplyWindowInsetsListener(view, null) + fun unregister() { + getObservedView()?.takeIf { hasBeenRegistered }?.let { + ViewCompat.setOnApplyWindowInsetsListener(it, null) + } } private fun getObservedView(): View? = eventSourceView.get() diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt b/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt index c56ede1482..37aefe87b5 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt @@ -163,7 +163,6 @@ object ScreenWindowTraits { InsetsObserverProxy.registerOnView(decorView) InsetsObserverProxy.addOnApplyWindowInsetsListener(windowInsetsListener) } else { - InsetsObserverProxy.unregisterOnView(decorView) InsetsObserverProxy.removeOnApplyWindowInsetsListener(windowInsetsListener) } ViewCompat.requestApplyInsets(decorView) diff --git a/apps/Example.tsx b/apps/Example.tsx index 49381ab308..ba27c3579e 100644 --- a/apps/Example.tsx +++ b/apps/Example.tsx @@ -39,10 +39,6 @@ import { GestureHandlerRootView } from 'react-native-gesture-handler'; enableFreeze(); -if (Platform.OS === 'android') { - StatusBar.setTranslucent(true); -} - const SCREENS: Record< string, { diff --git a/apps/src/tests/TestAndroidTransitions.tsx b/apps/src/tests/TestAndroidTransitions.tsx new file mode 100644 index 0000000000..0d98e720e6 --- /dev/null +++ b/apps/src/tests/TestAndroidTransitions.tsx @@ -0,0 +1,102 @@ +import { NavigationContainer, RouteProp } from "@react-navigation/native"; +import { NativeStackNavigationProp, createNativeStackNavigator } from "@react-navigation/native-stack"; +// import { NativeStackNavigationProp, createNativeStackNavigator } from "react-native-screens/native-stack"; +import React from "react"; +import { Button, TextInput, View } from "react-native"; + +type RouteParamList = { + Home: undefined; + Second: undefined; + FormSheet: undefined; + Modal: undefined; + NestedStackHost: undefined; +} + +type RouteProps = { + navigation: NativeStackNavigationProp, + route: RouteProp, +} + +const Stack = createNativeStackNavigator(); +// const Stack = createNativeStackNavigator(); + +function Home({ navigation }: RouteProps<'Home'>): React.JSX.Element { + return ( + + +