Skip to content
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

[v4] | [v2] BottomSheetModal failing to present on iOS #1560

Open
walterholohan opened this issue Oct 4, 2023 · 40 comments
Open

[v4] | [v2] BottomSheetModal failing to present on iOS #1560

walterholohan opened this issue Oct 4, 2023 · 40 comments
Labels
bug Something isn't working

Comments

@walterholohan
Copy link

Bug

We just recently updated RN to 0.72.5 and react-native-reanimated to 3.5.4 and released it to production over the weekend. And then we started to get reports from our users that the modals were failing to appear. I will attach some videos below but we were unable to reproduce it on a simulator or a real device however we have ~150,000 MAU so after 24 hours we had over 10 users who were affected.

We had users on iOS 16.x and iOS 17.x who were affected

To unblock our users I had to set animateOnMount to false which makes me believe the issue is with the opening animation of the modal and reanimated

Environment info

Library Version
@gorhom/bottom-sheet 4.4.7
react-native 0.72.5
react-native-reanimated 3.5.4
react-native-gesture-handler 2.13.1

Steps To Reproduce

  1. User tries to press on Link Activity and you can see the modal briefly flashes
  2. User tries to press on Referral Code and again the modal flashes and closes
RPReplay_Final1696360061.mov

Reproducible sample code

can attach an example of code if necessary but I was just following to docs for BottomSheetModal
@walterholohan walterholohan added the bug Something isn't working label Oct 4, 2023
@walterholohan walterholohan changed the title [v4] | [v2] Issue title [v4] | [v2] BottomSheetModal failing to present on iOS Oct 4, 2023
@SrAnthony
Copy link

Same happening to us, affecting at least 15 users so far.
We couldn't get a repro yet either.

Bottom sheet 4.5.1
React native 0.72.5
Reanimated 3.5.4

@LouisKraemer
Copy link

Same here, animateOnMount to false does fix the issue but really low number of users affected.

@walterholohan
Copy link
Author

Thanks @SrAnthony and @LouisKraemer good to hear that you are also experiencing the same issue for your users. I haven't been able to deepdive into the animation implementation but hopefully @gorhom can give some insight or potential avenue's we can explore for a fix

@karbone4
Copy link

karbone4 commented Oct 5, 2023

Same here, we are also using expo sdk 49. It also happens on android

@enchorb
Copy link

enchorb commented Oct 5, 2023

Same issue here, more and more users are starting to complain about this - also unable to reproduce for us on simulator or our testing devices. This is happening for our users on both v4 and v5.

@hatem-72
Copy link

hatem-72 commented Oct 6, 2023

Some of our users are affected as well.
I think I managed to reproduce it, at least on IOS: Activate the Reduce Motion accessibility setting in your Iphone, restart your app then bottomsheets are not opening anymore.

Reanimated recently added support for this setting -> https://docs.swmansion.com/react-native-reanimated/docs/guides/accessibility/

@walterholohan
Copy link
Author

wow great spot @hatem-72 , did you manage to create a patch for this lib?

@hatem-72
Copy link

hatem-72 commented Oct 6, 2023

For the moment, I've only disabled the opening animation for users with reduced motion :

import { useReducedMotion } from 'react-native-reanimated';
import { BottomSheetModal } from '@gorhom/bottom-sheet';

// ...

function MyComponent() {
    const reducedMotion = useReducedMotion();

    return <BottomSheetModal
        // ...
        animateOnMount={!reducedMotion}
    >
        // ...
    </BottomSheetModal>
}

@enchorb
Copy link

enchorb commented Oct 6, 2023

@hatem-72 great find! reached out to 2 users who were having this issue and they both had it enabled.

@walterholohan
Copy link
Author

Can we buy @hatem-72 a coffee?

@karbone4
Copy link

karbone4 commented Oct 6, 2023

Thanks @hatem-72 for the fix.

I have another bug, I can't dismiss the bottom modal 2 times. If I dismiss, open it again, I can't dismiss it. I found a workaround by overriding ref :

const reducedMotion = useReducedMotion();
useImperativeHandle(
    bottomModalRef,
    () => ({
        dismiss: () => {
            if (reducedMotion) {
                ref?.current?.snapToPosition(0);
            } else {
                ref?.current?.dismiss();
            }
        }
    }),
    [reducedMotion]
);

@walterholohan
Copy link
Author

@karbone4 I am unable to reproduce your bug. It works for me

@leymytel
Copy link

leymytel commented Oct 10, 2023

Hello everyone!

Setting animateOnMount to false causes many weird issues (can't dismiss the modal, sometimes you need to click twice to get it open) . Another temporary fix is to add this patch (thanks to @efstathiosntonas) to override reduced motion to false in reanimated 3.5.4:

diff --git a/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts b/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts
index 9b3fcb1..0111380 100644
--- a/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts
+++ b/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts
@@ -49,5 +49,5 @@ export function isReducedMotion() {
       ? // @ts-ignore Fallback if `window` is undefined.
         !window.matchMedia('(prefers-reduced-motion: no-preference)').matches
       : false
-    : (global as localGlobal)._REANIMATED_IS_REDUCED_MOTION ?? false;
+    : false;
 }

@nihilenz
Copy link

Also observed on a device running Android 10 with reduce motion option enabled

@levibuzolic
Copy link

levibuzolic commented Oct 23, 2023

Also hitting this issue, rather than disabling the reduce motion detection for Reanimated globally we went with conditionally supplying our own animation config only when required. There are still some minor quirks when dismissing the sheet, but it does at least open and work correctly:

const reduceMotionEnabled = useReducedMotion();
const overrideConfig = useBottomSheetSpringConfigs({
  damping: 500,
  stiffness: 1000,
  mass: 3,
  overshootClamping: true,
  restDisplacementThreshold: 10,
  restSpeedThreshold: 10,
  reduceMotion: ReduceMotion.Never,
});

return (
  <BottomSheetModal
    /*
    There's a bug in BottomSheet which means that sheets will fail to open when reduce motion is enabled. Manually
    providing an animation config with `reduceMotion: ReduceMotion.Never` fixes this, but it does introduce a bit
    of jank when dismissing the sheet. This is a tradeoff we're willing to make for the sake of sheets at least
    working when reduce motion is enabled.

    @see https://github.com/gorhom/react-native-bottom-sheet/issues/1560
    */
    animationConfigs={reduceMotionEnabled ? overrideConfig : undefined}
    // other config here...
  />
);

The animation config was lifted from

const ANIMATION_CONFIGS_IOS = {
damping: 500,
stiffness: 1000,
mass: 3,
overshootClamping: true,
restDisplacementThreshold: 10,
restSpeedThreshold: 10,
};
to keep thre behaviour the same as before, I noticed that there's a platform fork and on Android the animation using a timing config, so you could probably use that based on platform, but we were happy enough with the spring config for both platforms.

@Gyogle
Copy link

Gyogle commented Oct 27, 2023

In fact, I think this problem is fundamentally a problem with the changes in Reanimated that were made when the BottomSheet version was upgraded, although BottomSheet itself handles reduceOption in a way.

Simple problem solving can be done in the other comment1 and comment2 posted above.
But I think the fundamental solution is to allow Reanimated itself to control the relevant settings.

So I posted my opinion and the cause I identified on Reanimated.(software-mansion/react-native-reanimated#5314 (comment))
We ask for your interest. Let’s solve this problem related to accessibility together!

@andac-ozcan
Copy link

andac-ozcan commented Oct 30, 2023

As I see, reduced motion is also enabled if low power mode is active and device battery is <= 10%. Almost all screenshots from our user reports shows that their device battery are <= 10%. Apple mentions this here, under Low Power Mode title. So it may be a wider issue than we think, not related with accessibility settings only.

@nguyenhoanganhdev
Copy link

nguyenhoanganhdev commented Nov 1, 2023

I think, reduced motion is also enabled if low power mode is active and device battery is <= 10%. Almost all screenshots from our user bug reports shows their device battery is <= 10%. Apple mentions this here, under Low Power Mode title. So it may be a wider issue than we think, not related with accessibility settings only.

Thank you very much. This is the root cause of my case

@CostasCF
Copy link

CostasCF commented Nov 5, 2023

Although I tested setting reducedMotion to false, it's not a viable solution because the problem still appears from time to time. I think it's a fundamental problem with how Reanimated and BottomSheet works as @Gyogle states.

@mattijsf
Copy link

mattijsf commented Nov 13, 2023

Inspired by the above answer, main difference being the import of ANIMATION_CONFIGS:

import { useBottomSheetSpringConfigs } from "@gorhom/bottom-sheet"
import { ANIMATION_CONFIGS } from "@gorhom/bottom-sheet/src/constants"
import { Platform } from "react-native"
import { ReduceMotion, WithTimingConfig, useReducedMotion } from "react-native-reanimated"

/**
 * https://github.com/gorhom/react-native-bottom-sheet/issues/1560
 *
 * Usage:
 * ```
 * const animationConfigs = useBottomSheetAnimationConfigsReducedMotionWorkaround()
 * <BottomSheetModal animationConfigs={animationConfigs} ...>
 * ```
 *
 * @returns
 */
export function useBottomSheetAnimationConfigsReducedMotionWorkaround():
  | WithTimingConfig
  | undefined {
  const reducedMotion = useReducedMotion()
  const iOSAnimationConfigWithoutReducedMotion = useBottomSheetSpringConfigs({
    ...ANIMATION_CONFIGS,
    reduceMotion: ReduceMotion.Never,
  })

  if (Platform.OS !== "ios" || !reducedMotion) return undefined

  return iOSAnimationConfigWithoutReducedMotion
}

@mattijsf
Copy link

@gorhom would the above be worth a v4 patch release?

@salman-ar-sar
Copy link

Tried the fix by @levibuzolic / @mattijsf. That works most of the time but got some weird issues when having multiple BottomSheets. Also tried disabling motion, that was very weird. Currently using the reanimated patch by @leymytel, that seems to be working fine.

@gorhom It would be really great if you can look into this.

@jlmosconi
Copy link

In my case, one of our users had the "Reduce Motion" option enabled and that caused the modals to not open. It was solved as follows

import {useReducedMotion} from 'react-native-reanimated'

...
const reducedMotion = useReducedMotion()

<BottomModal animateOnMount={!reducedMotion}

@hatem-72 Thanks!!

@marshallcool
Copy link

@leymytel this patch includes animation for users who have enabled motion reduction, I think not everyone will be happy with this behavior

@YaoHuiJi
Copy link

YaoHuiJi commented Dec 2, 2023

This issue will cause a lot of users to be completely unable to use the Apps that uses this library. If you have time, can you please have a look? @gorhom 💗

@keithluchtel
Copy link

I am experiencing the same issue with version 3.5.4, but am not having issues with 3.3.0

@henryteng07
Copy link

henryteng07 commented Dec 26, 2023

@karbone4 Same issue where I can't close modal the second time... I tried using your fix but it doesn't seem to be working. Any advice? Thanks!

`const reducedMotion = useReducedMotion();

useImperativeHandle(
bottomSheetModalRef,
() => ({
dismiss: () => {
if (reducedMotion) {
ref?.current?.snapToPosition(0);
} else {
ref?.current?.dismiss();
}
},
}),
[reducedMotion],
);

return (




<TouchableOpacity
onPress={() => {
bottomSheetModalRef.current?.dismiss();
}}>
`

@henryteng07
Copy link

@karbone4 I got it! Just use ref.current.close() instead of ref.current.dismiss()

@mMarcos208
Copy link

Is this working in the latest version? Same issue.

1 similar comment
@TomWq
Copy link

TomWq commented Jan 13, 2024

Is this working in the latest version? Same issue.

@jenskuhrjorgensen
Copy link

Hello everyone!

Setting animateOnMount to false causes many weird issues (can't dismiss the modal, sometimes you need to click twice to get it open) . Another temporary fix is to add this patch (thanks to @efstathiosntonas) to override reduced motion to false in reanimated 3.5.4:

diff --git a/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts b/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts
index 9b3fcb1..0111380 100644
--- a/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts
+++ b/node_modules/react-native-reanimated/src/reanimated2/PlatformChecker.ts
@@ -49,5 +49,5 @@ export function isReducedMotion() {
       ? // @ts-ignore Fallback if `window` is undefined.
         !window.matchMedia('(prefers-reduced-motion: no-preference)').matches
       : false
-    : (global as localGlobal)._REANIMATED_IS_REDUCED_MOTION ?? false;
+    : false;
 }

For me the solution was to disable reduced motion support with this patch, which to me seems like a fine solution for now. Before bumping reanimated to >=3.5.0 we didn't support reduced motion (at least not for our reanimated animations) and with this patch we still don't. Users are not going to notice any degradation in UX. When this has been properly fixed in bottomsheet and/or reanimated we can remove this patch and increase UX for accessibility users.

@obasille
Copy link

I'm having the same issue with my app. Reported by a user. Totally caught me by surprise.
This library is really a good one, I hope this issue will be addressed as it affects virtually every app using BottomSheetModal.

@pckz
Copy link

pckz commented Jan 23, 2024

Some of our users are having this problem. We could replicate activating "Reduce Motion" in a old device (14.7) and users reported the problem were using 16.6.1. In a device with 17.2 Reduce motion option not affect this bug.
So, the solutions proposed here were not implemented in last version of this lib? thanks!

@regalstreak
Copy link

@gorhom this is a P0 bug, is this being worked on?

@fobos531
Copy link

I've submitted a PR which should fix this in v5 of the library: #1743

@obasille
Copy link

FYI a simple workaround was posted in another issue about the same problem:
#1674 (comment)

yayvery pushed a commit to discord/react-native-bottom-sheet that referenced this issue Mar 18, 2024
gorhom#1674)

fix: bottom sheet not appearing for users that have reduced motion turned on (gorhom#1743)(by @fobos531)
yayvery pushed a commit to discord/react-native-bottom-sheet that referenced this issue Mar 18, 2024
gorhom#1674)

fix: bottom sheet not appearing for users that have reduced motion turned on (gorhom#1743)(by @fobos531)
@nhuesmann
Copy link

Thanks @hatem-72 for the fix.

I have another bug, I can't dismiss the bottom modal 2 times. If I dismiss, open it again, I can't dismiss it. I found a workaround by overriding ref :

const reducedMotion = useReducedMotion();
useImperativeHandle(
    bottomModalRef,
    () => ({
        dismiss: () => {
            if (reducedMotion) {
                ref?.current?.snapToPosition(0);
            } else {
                ref?.current?.dismiss();
            }
        }
    }),
    [reducedMotion]
);

Thank you for this, I was facing the same issue and this resolved it.

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@nihilenz
Copy link

not stale

@christophby
Copy link
Contributor

christophby commented Apr 22, 2024

The change from @hatem-72 should be included in the main code base of the bottom sheet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests