[Bug]: Disabling animateOnMount - the animation still plays (+ visual jumps when it doesn't)

[kliakhovskii-brex]

Version

v5

Reanimated Version

v3

Gesture Handler Version

v2

Platforms

iOS, Android

What happened?

This is somewhat 2 distinct issues, but they seem to be related overall due to initial state of things with animateOnMount and a few other things, therefore reporting them as one for now.

tldr: We use bottom sheet inside of tabs, so each tab has a bottom sheet rendered, and therefore mount their own version, some tabs do not let bottom sheet to be collapsed state. Because it's predictable environment, we always know the height and do not require the dynamic sizing.

After 5.1.6 -> 5.2.6 upgrade surfaced two issues:

• animateOnMount={false} stops working when both height provided and handle is supplied as null.
• animateOnMount={false} (when it works) renders the initial frame in collapsed state, then jumps into position

Reproduction steps

Issue #1:

Description - supplying both containerLayoutState.height and handleComponent={null} breaks animateOnMount={false}

Steps:

The following code would start playing animation if handleComponent={null} is uncommented

const containerLayoutState = useSharedValue<ContainerLayoutState>({
    height: screenHeight,
    offset: {
      top: 0,
      left: 0,
      right: 0,
      bottom: 0,
    },
  });

...

<BottomSheet
        style={{ flex: 1 }}
        index={1}
        snapPoints={[104, 729, 792]}
        enableDynamicSizing={false}
        // handleComponent={null} // <- uncomment this line to see the animation start playing agoing.
        containerLayoutState={containerLayoutState}
        animateOnMount={false}
      >

Quick investigation:

I have looked at internals for quick investigation, and funny enough, this is very straightforward issue:

• We get this effect executed with original _providedIndex value. And because the animateOnMount is disabled, it'll go to handleSnapToIndex.
• Usually, the first call to handleSnapToIndex will bail out due to isLayoutReady condition.
• But in the case of having initial height: screenHeight and having null component for handleComponent, the layout will be marked as ready right away.
• Thus, it won't bail, and will just proceed to animateToPosition call. And because animatedPosition is initialized with sheet being collapsed in here, it'll play animation (even tho animateOnMount is disabled).

Issue #2:

Let's say I supply some 0-height handle component to avoid ready layout on the start. Then visually, the first frame of rendered bottom sheet will be collapsed (when we supply animateOnMount).

Steps:

• The same, but i will supply video later so it's easier to see. I render different random colors under sheet so it's easier to see.

Quick investigation:

As i mentioned in the prev issue, the animatedPosition is always initialized to be at screen height. If we fix the layout issue, it will auto-fix this position, but it won't happen on the first render right away, thus if you run some distinct background underneath -> you'll be able to see it for one frame.

The reason i feel those two issues are interconnected is that if this is initialized properly (with proper position if we know the sizes), then even the layout readiness would have not impacted as it would just animate to the point of initialization.

Note that if i change the initial value of animatedPosition to 0, now instead of color rendering underneath of what bottom sheet for a split second. We get the white sheet rendering on top of color in the top section. As well as i repro the bug with layout being ready -> it starts animating from the top.

Snack has the repro of the issue with animation, but really it's hard to see the flickering there due to perf, so i recorded all of it on my local simulator too (here's animation + content flicker):
https://github.com/user-attachments/assets/904a6b22-0a56-4933-8ad1-307fc0dadde5

And this one is what happens if the initial position is initialized as 0. Instead of sheet flickering, it starts to flicker on top now.
https://github.com/user-attachments/assets/e4ccd67a-8f12-4688-b09e-41dd56040e6e

So I feel like by fixing animatedPosition initial value for animateOnMount to be somewhat representable of the actual snap point when we know the sizing (e.g. height let's say), we should be in a good position to fix both of those issues. Even if layout is calculated and animateToPosition is called, it won't do much as long as values aren't changing.

Reproduction sample

https://snack.expo.dev/@kliakhovskii/f615a2

Relevant log output

[kliakhovskii-brex]

Ok, looking now at the possible patch locally (and see what can be a potential fix), i see that basically handleSnapToIndex will take detents if layout is ready and just animate to them. In case when Layout is initially ready (and animateOnMount is false), we can simply take the same approach to just initialize the animatedPosition with the correct values right away.

Switching from the following:

const animatedPosition = useSharedValue(INITIAL_POSITION);

to that ⬇️ will fix the issue for me (not sure i supposed to check the Layout ready tho, it's defined later, but we can prob bail to INITIAL_POSITION if detents are not defined):

const initialPosition = !animateOnMount
  ? animatedDetentsState.get().detents?.[_providedIndex]
  : undefined;
const animatedPosition = useSharedValue(initialPosition ?? INITIAL_POSITION);

Here's video of how that works in these settings (converted to gif, GH is annoying with .mov).