-
-
Notifications
You must be signed in to change notification settings - Fork 611
fix(Android, FormSheet): Fix dynamic height change for fitToContents #3484
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
Conversation
|
NOW it's feels like Christmas. 🎉 |
kligarski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks clean, I'm only worried about interactions with different animation system.
Some comments to the assumptions section:
Visual styles such as backgroundColor should be applied via contentStyle on the Screen level. In fitToContents mode, the height of the ScreenContentWrapper should always match the height of the content.
Something I thought about now is that we won't be able to support background image/gradient then but I guess that's not possible on iOS either and it would require more changes. Maybe something to consider in the future.
Unmounting of components remains the responsibility of the application code. We do not include default enter/exit animations at the component level.
I wonder what would happen if there was a component with animation that would change the height? Would this still work correctly? But this also might be something to handle in the future.
kligarski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, not sure if this is a real concern but I was able to make the form sheet not adapt to content by changing the content quickly:
Screen.Recording.2025-12-18.at.09.04.29.mov
I used TestFormSheet with fitToContents and just added some text at the bottom.
checking.. |
Should be fixed, tested with import React, { useRef, useState } from 'react';
import {
Button,
View,
Text,
StyleSheet,
Animated,
} from 'react-native';
import { NavigationContainer } from '@react-navigation/native';
import {
createNativeStackNavigator,
NativeStackScreenProps,
} from '@react-navigation/native-stack';
import Colors from '../shared/styling/Colors';
type StackParamList = {
Home: undefined;
FormSheet: undefined;
};
const Stack = createNativeStackNavigator<StackParamList>();
const HomeScreen = ({ navigation }: NativeStackScreenProps<StackParamList>) => (
<View style={styles.screen}>
<Text style={styles.title}>Home Screen</Text>
<Button
title="Open Form Sheet"
onPress={() => navigation.navigate('FormSheet')}
/>
</View>
);
const FormSheetScreen = () => {
const [isTextVisible, setIsTextVisible] = useState(true);
React.useEffect(() => {
const timer = setTimeout(() => {
setIsTextVisible(false);
}, 1000);
return () => clearTimeout(timer);
}, []);
React.useEffect(() => {
const timer = setTimeout(() => {
setIsTextVisible(true);
}, 1016);
return () => clearTimeout(timer);
}, []);
return (
<View style={styles.formSheetContainer}>
<Text style={styles.formSheetTitle}>Form Sheet Content</Text>
{isTextVisible && (
<Text>
Lorem, ipsum dolor sit amet consectetur adipisicing elit. Velit deserunt
qui fugit unde assumenda. Non id facere mollitia sequi aliquid nostrum,
tenetur eligendi quos at natus eius, corporis quidem eaque libero
reiciendis, labore sed minima doloremque temporibus! Veritatis harum
provident molestias incidunt at temporibus quod ipsam totam optio,
quisquam amet quae molestiae. Exercitationem animi facere iure, nobis
nulla repudiandae aut nam natus! Qui, id? Nobis quia recusandae
repellendus illum voluptas, ipsa, doloribus reprehenderit nulla quas
facere eos! Alias, accusantium voluptatibus doloremque unde eius totam
et officia asperiores? Voluptate totam provident, quas inventore
doloribus autem nihil quisquam soluta eum, dolorum nobis.{' '}
</Text>
)}
</View>
);
};
export default function App() {
return (
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen name="Home" component={HomeScreen} />
<Stack.Screen
name="FormSheet"
component={FormSheetScreen}
options={{
presentation: 'formSheet',
sheetAllowedDetents: 'fitToContents',
contentStyle: {
backgroundColor: Colors.YellowLight40,
},
// TODO(@t0maboro) - add `sheetContentDefaultResizeAnimationEnabled` prop here when possible
}}
/>
</Stack.Navigator>
</NavigationContainer>
);
}
const styles = StyleSheet.create({
screen: {
flex: 1,
alignItems: 'center',
justifyContent: 'center',
padding: 20,
},
title: {
fontSize: 24,
marginBottom: 20,
},
formSheetContainer: {
alignItems: 'center',
justifyContent: 'center',
padding: 20,
gap: 20,
},
formSheetTitle: {
fontSize: 20,
fontWeight: 'bold',
marginBottom: 10,
},
text: {
fontSize: 16,
marginBottom: 8,
color: Colors.White,
},
rectangle: {
width: '100%',
backgroundColor: Colors.NavyLight80,
height: 200,
alignItems: 'center',
justifyContent: 'center',
},
input: {
width: 100,
height: 20,
borderColor: Colors.BlueDark100,
borderWidth: 1,
},
}); |
|
Thank you very much for this PR — looking forward to seeing it merged. :) |
|
My formSheet doesn't appear at all in Android with fitToContents, maybe this will help 🙏 😄 . Thanks! ❤️ |
kligarski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks solid. I've tested on Test2560 & it also looks solid.
I've checked also TestFormSheet & haven't spotted a regression.
Really great job here!
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before merging I asked internally for clarification what is absoluteWoBottom style useful for and we still have some discussion about timing of component unmount. Will progress shortly.
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One additional remark regarding naming.
|
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates!
Description
This PR introduces improved support for
fitToContentsin BottomSheet by modifying its style toabsoluteWithNoBottom. This style provides a reference to the current top position of the sheet, making it easier to handle dynamic content within FormSheet.Having improved control over
maxHeight(added in previous PRs), we are now able to implement a smooth resizing animation for the BottomSheet.Important
Key assumptions (these should be carefully reviewed)
backgroundColorshould be applied viacontentStyleon the Screen level. In fitToContents mode, the height of the ScreenContentWrapper should always match the height of the content.Note
Note:
bottom: 0is intentionally excluded from these styles for two reasons:Omitting the bottom constraint ensures the Yoga layout engine does not dynamically recalculate the Screen and content size during animations.
Including
bottom: 0with 'position: absolute' would force the component to anchor itself to an ancestor's bottom edge. This creates a dependency on the ancestor's size, whereas 'fitToContents' requires the FormSheet's dimensions to be derived strictly from its children.bottom-0.mov
empty-style-object.mov
This PR also introduces a prop that allows overriding the default enter/exit animation, enabling users who use the Animated/Reanimated API to handle animations manually. When the
sheetContentDefaultResizeAnimationEnabledprop is set, we skip performing our resize + translate animation and only adjust the screen size to match the current content.Fixes: #2560
Changes
absoluteWithNoBottomfor androidfitToContentssheetContentDefaultResizeAnimationEnabledproperty for disabling default animationScreenshots / GIFs
Here you can add screenshots / GIFs documenting your change.
You can add before / after section if you're changing some behavior.
Before
before.mov
animation-before.mov
After
after.mov
animated.mov
Test code and steps to reproduce
Test2560
Checklist