Skip to content

Conversation

@t0maboro
Copy link
Contributor

@t0maboro t0maboro commented Dec 16, 2025

Description

This PR introduces improved support for fitToContents in BottomSheet by modifying its style to absoluteWithNoBottom. 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)

  • Unmounting of components remains the responsibility of the application code. We do not include default enter/exit animations at the component level.
  • 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.
  • When a component unmounts, the ScreenContentWrapper's size shrinks before we can start the transition. Since its size is necessary to calculate the animation delta, styles should be applied at the Screen level, where we have better control over when layout occurs.

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 sheetContentDefaultResizeAnimationEnabled prop is set, we skip performing our resize + translate animation and only adjust the screen size to match the current content.

Fixes: #2560

Changes

  • Updated style to absoluteWithNoBottom for android fitToContents
  • Added default expand/shrink animations
  • Added sheetContentDefaultResizeAnimationEnabled property for disabling default animation

Screenshots / 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

  • Included code example that can be used to test this change
  • Ensured that CI passes

@t0maboro t0maboro changed the title @t0maboro/fit to contents dynamic size fix(Android, FormSheet): Fix dynamic height change for fitToContents Dec 16, 2025
@RohovDmytro
Copy link

NOW it's feels like Christmas.

🎉

Copy link
Contributor

@kligarski kligarski left a 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.

Comment on lines 201 to 211
this.translationY = delta
this
.animate()
.translationY(0f)
.setDuration(sheetHeightAnimationDuration)
.withStartAction {
behavior.useSingleDetent(newHeight)
requestLayout()
}.withEndAction {
onSheetYTranslationChanged()
}.start()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't using ViewPropertyAnimator interfere with ValueAnimators used to animate enter/exit + IME animations? E.g. what would happen if content size changed during enter animation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the use case we should cover. I wouldn't consider such a case, as it would require changing content dynamically during enter/exit animation, what would look strange when the content both slides in/out and jumps

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

atm, I added a prop to disable this default animation, allowing to handle the content by developer and react only to the Screen size change on our side - having the combination: TextInput with autoFocus + unmounting another component at the same time (which is a pretty wild combo) - we may have some conflict which might be problematic to synchronize: textInput could want to move the formSheet up, and unmounted component might require to move the component down at the same moment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it works with the default one (this flicker is junky, I described the root cause for it above, but anyway, the final state is 'stable')
https://github.com/user-attachments/assets/e721e7d0-265d-41d2-8abf-41ed78c535c4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it could look with more advanced custom animator:

Screen.Recording.2025-12-18.at.17.38.04.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Therefore, I believe that (at least at the early stage, we can recommend this as the proper approach for handling such cases). I'm okay with continuing the development to synchronize it with our default animation, but as for now, I'd propose to add this support in the separate PR as this one already covers at least a few completely different scenarios. Let me know what do you think about the current state.

Copy link
Contributor

@kligarski kligarski left a 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.

@t0maboro
Copy link
Contributor Author

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..

@t0maboro
Copy link
Contributor Author

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.

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,
  },
});

@t0maboro t0maboro requested a review from kligarski December 18, 2025 16:41
@GaylordP
Copy link

Thank you very much for this PR — looking forward to seeing it merged. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FormSheet with sheetAllowedDetents: 'fitToContents' doesn't update height dynamically on Android

5 participants