Skip to content

Commit 91400bc

Browse files
authored
fix(Android): formSheet with fitToContents does not have correct height after goBack (#2789)
## Description When fragment is reattached (on navigation back) `onCreateView` is called which tries to configure the form sheet (that has `fitToContents` set). On view recreation the subtree is not laid out (the flag is cleared) & therefore current code fails. ## Changes We now utilise the fact, that the Screen's subtree is retained by react-native and the components are not recreated themselves -> old height is stored in the member variable. We reuse it. ## Test code and steps to reproduce Added Test2789 ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
1 parent 0547b19 commit 91400bc

4 files changed

Lines changed: 104 additions & 9 deletions

File tree

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import com.google.android.material.shape.CornerFamily
2727
import com.google.android.material.shape.MaterialShapeDrawable
2828
import com.google.android.material.shape.ShapeAppearanceModel
2929
import com.swmansion.rnscreens.bottomsheet.isSheetFitToContents
30+
import com.swmansion.rnscreens.bottomsheet.useSingleDetent
3031
import com.swmansion.rnscreens.bottomsheet.usesFormSheetPresentation
3132
import com.swmansion.rnscreens.events.HeaderHeightChangeEvent
3233
import com.swmansion.rnscreens.events.SheetDetentChangedEvent
@@ -135,11 +136,7 @@ class Screen(
135136
val height = bottom - top
136137

137138
if (isSheetFitToContents()) {
138-
sheetBehavior?.let {
139-
if (it.maxHeight != height) {
140-
it.maxHeight = height
141-
}
142-
}
139+
sheetBehavior?.useSingleDetent(height)
143140

144141
if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
145142
// On old architecture we delay enter transition in order to wait for initial frame.

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,15 @@ class SheetDelegate(
130130
behavior.apply {
131131
val height =
132132
if (screen.isSheetFitToContents()) {
133-
screen.contentWrapper
134-
.get()
135-
?.height
136-
.takeIf { screen.contentWrapper.get()?.isLaidOut == true }
133+
screen.contentWrapper.get()?.let { contentWrapper ->
134+
contentWrapper.height.takeIf {
135+
// subtree might not be laid out, e.g. after fragment reattachment
136+
// and view recreation, however since it is retained by
137+
// react-native it has its height cached. We want to use it.
138+
// Otherwise we would have to trigger RN layout manually.
139+
contentWrapper.isLaidOut || contentWrapper.height > 0
140+
}
141+
}
137142
} else {
138143
(screen.sheetDetents.first() * containerHeight).toInt()
139144
}

apps/src/tests/Test2789.tsx

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { NavigationContainer, ParamListBase } from '@react-navigation/native';
2+
import { NativeStackNavigationProp, createNativeStackNavigator } from '@react-navigation/native-stack';
3+
import React from 'react';
4+
import { Button, View } from 'react-native';
5+
6+
type NavigationRouteProps<ParamList extends ParamListBase> = {
7+
navigation: NativeStackNavigationProp<ParamList>;
8+
}
9+
10+
type RootStackRouteParamList = {
11+
RootStackHome: undefined;
12+
};
13+
14+
type OneStackRouteParamList = RootStackRouteParamList & {
15+
OneHome: undefined;
16+
}
17+
18+
type TwoStackRouteParamList = OneStackRouteParamList & {
19+
TwoHome: undefined;
20+
TwoSecond: undefined;
21+
TwoSheet: undefined;
22+
}
23+
24+
type RootStackRouteNavProps = NavigationRouteProps<RootStackRouteParamList>;
25+
type TwoStackRouteNavProps = NavigationRouteProps<TwoStackRouteParamList>;
26+
27+
28+
const RootStack = createNativeStackNavigator<RootStackRouteParamList>();
29+
const TwoStack = createNativeStackNavigator<TwoStackRouteParamList>();
30+
31+
function RootStackHostComponent() {
32+
return (
33+
<RootStack.Navigator>
34+
<RootStack.Screen name="RootStackHome" component={RootStackHome} />
35+
</RootStack.Navigator>
36+
);
37+
}
38+
39+
function RootStackHome({ }: RootStackRouteNavProps) {
40+
return (
41+
<TwoStackHostComponent />
42+
);
43+
}
44+
45+
function TwoStackHostComponent() {
46+
return (
47+
<TwoStack.Navigator>
48+
<TwoStack.Screen name="TwoHome" component={TwoStackHome} />
49+
<TwoStack.Screen name="TwoSecond" component={TwoStackSecond} />
50+
<TwoStack.Screen name="TwoSheet" component={TwoStackSheet} options={{
51+
presentation: 'formSheet',
52+
sheetAllowedDetents: 'fitToContents',
53+
}} />
54+
</TwoStack.Navigator>
55+
);
56+
}
57+
58+
function TwoStackHome({ navigation }: TwoStackRouteNavProps) {
59+
return (
60+
<View style={{ flex: 1, backgroundColor: 'palevioletred' }}>
61+
<Button title="Open TwoSheet" onPress={() => navigation.navigate('TwoSheet')} />
62+
<Button title="Open TwoSecond" onPress={() => navigation.navigate('TwoSecond')} />
63+
</View>
64+
);
65+
}
66+
67+
function TwoStackSecond({ navigation }: TwoStackRouteNavProps) {
68+
return (
69+
<View style={{ flex: 1, backgroundColor: 'tomato' }}>
70+
<Button title="Go back" onPress={() => navigation.goBack()} />
71+
</View>
72+
);
73+
}
74+
75+
function TwoStackSheet({ navigation }: TwoStackRouteNavProps) {
76+
return (
77+
<View style={{ flex: undefined, backgroundColor: 'palegreen' }}>
78+
<View style={{ backgroundColor: 'peru', height: 400 }}>
79+
<Button title="Open TwoSecond" onPress={() => navigation.navigate('TwoSecond')} />
80+
</View>
81+
</View>
82+
);
83+
}
84+
85+
86+
export default function App() {
87+
return (
88+
<NavigationContainer>
89+
<RootStackHostComponent />
90+
</NavigationContainer>
91+
);
92+
}

apps/src/tests/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ export { default as Test2668 } from './Test2668';
124124
export { default as Test2675 } from './Test2675';
125125
export { default as Test2717 } from './Test2717';
126126
export { default as Test2767 } from './Test2767';
127+
export { default as Test2789 } from './Test2789';
127128
export { default as TestScreenAnimation } from './TestScreenAnimation';
128129
export { default as TestScreenAnimationV5 } from './TestScreenAnimationV5';
129130
export { default as TestHeader } from './TestHeader';

0 commit comments

Comments
 (0)