Skip to content

Commit

Permalink
fix(Android): fix formSheet-keyboard interaction (#2511)
Browse files Browse the repository at this point in the history
## 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: <!-- 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
  • Loading branch information
kkafar authored Nov 18, 2024
1 parent 0ec5c56 commit 5cb6520
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ object ScreenWindowTraits {
InsetsObserverProxy.registerOnView(decorView)
InsetsObserverProxy.addOnApplyWindowInsetsListener(windowInsetsListener)
} else {
InsetsObserverProxy.unregisterOnView(decorView)
InsetsObserverProxy.removeOnApplyWindowInsetsListener(windowInsetsListener)
}
ViewCompat.requestApplyInsets(decorView)
Expand Down
4 changes: 0 additions & 4 deletions apps/Example.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ import { GestureHandlerRootView } from 'react-native-gesture-handler';

enableFreeze();

if (Platform.OS === 'android') {
StatusBar.setTranslucent(true);
}

const SCREENS: Record<
string,
{
Expand Down
102 changes: 102 additions & 0 deletions apps/src/tests/TestAndroidTransitions.tsx
Original file line number Diff line number Diff line change
@@ -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<RouteName extends keyof RouteParamList> = {
navigation: NativeStackNavigationProp<RouteParamList, RouteName>,
route: RouteProp<RouteParamList, RouteName>,
}

const Stack = createNativeStackNavigator<RouteParamList>();
// const Stack = createNativeStackNavigator();

function Home({ navigation }: RouteProps<'Home'>): React.JSX.Element {
return (
<View style={{ flex: 1, backgroundColor: 'lightsalmon', gap: 12 }}>
<View style={{ marginTop: 12 }}>
<Button title="Go Second" onPress={() => navigation.navigate('Second')} />
</View>
<View>
<Button title="Go FormSheet" onPress={() => navigation.navigate('FormSheet')} />
</View>
<View>
<Button title="Go Modal" onPress={() => navigation.navigate('Modal')} />
</View>
</View>
);
}

function Second({ navigation }: RouteProps<'Second'>): React.JSX.Element {
return (
<View style={{ flex: 1, backgroundColor: 'lightgreen', gap: 12 }}>
<View style={{ marginTop: 12 }}>
<Button title="Go Home" onPress={() => navigation.popTo('Home')} />
</View>
<View style={{}}>
<Button title="Go FormSheet" onPress={() => navigation.navigate('FormSheet')} />
</View>
</View>
);
}

function FormSheet({ navigation }: RouteProps<'FormSheet'> | RouteProps<'Modal'>): React.JSX.Element {
return (
<View style={{ backgroundColor: 'lightgreen' }}>
<View style={{ marginVertical: 12 }}>
<Button title="Go back" onPress={() => navigation.goBack()} />
</View>
<View style={{ alignItems: 'center' }}>
<TextInput style={{ marginVertical: 12, paddingVertical: 8, backgroundColor: 'lavender', borderRadius: 24, width: '80%' }} placeholder="Trigger keyboard..."></TextInput>
</View>
</View>
);
}

function Modal({ navigation }: RouteProps<'Modal'>): React.JSX.Element {
return (
<View style={{ flex: 1, backgroundColor: 'lightgreen' }}>
<View style={{ marginVertical: 12 }}>
<Button title="Go back" onPress={() => navigation.goBack()} />
</View>
<View style={{ alignItems: 'center' }}>
<TextInput style={{ marginVertical: 12, paddingVertical: 8, backgroundColor: 'lavender', borderRadius: 24, width: '80%' }} placeholder="Trigger keyboard..."></TextInput>
</View>
</View>
);
}

export default function App() {
return (
<NavigationContainer>
<Stack.Navigator screenOptions={{
statusBarTranslucent: false
}}>
<Stack.Screen name="Home" component={Home} />
<Stack.Screen name="Second" component={Second} options={{
headerShown: false,
}} />
<Stack.Screen name="FormSheet" component={FormSheet} options={{
presentation: 'formSheet',
sheetAllowedDetents: [0.5, 0.7],
contentStyle: {
backgroundColor: 'lightgreen'
},
}} />
<Stack.Screen name="Modal" component={Modal} options={{
presentation: 'modal',
headerShown: false,
}} />
</Stack.Navigator>
</NavigationContainer>
);
}
1 change: 1 addition & 0 deletions apps/src/tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,4 @@ export { default as TestHeaderTitle } from './TestHeaderTitle';
export { default as TestModalNavigation } from './TestModalNavigation';
export { default as TestMemoryLeak } from './TestMemoryLeak';
export { default as TestFormSheet } from './TestFormSheet';
export { default as TestAndroidTransitions } from './TestAndroidTransitions';

0 comments on commit 5cb6520

Please sign in to comment.