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

fix: [v5]fix backdrop gesture #1467

Closed
wants to merge 1 commit into from
Closed

fix: [v5]fix backdrop gesture #1467

wants to merge 1 commit into from

Conversation

itsramiel
Copy link
Contributor

Please provide enough information so that others can review your pull request:

Motivation

Fixes #1465

  1. Using the BottomSheetBackdrop component on web with latest v5 alpha 3 causes the following error:
    Uncaught TypeError: Cannot read properties of undefined (reading 'toString').
    This is due to an issue with useAnimatedGestureHandler and babel. You can read more here: [web] [babel plugin] TypeError: Cannot read properties of undefined (reading 'toString') when using useAnimatedGestureHandler  software-mansion/react-native-reanimated#2435

  2. You can avoid this issue by following this solution -> TypeError: Cannot read properties of undefined (reading 'toString') software-mansion/react-native-reanimated#2994 (comment)
    While this removes the error and backdrop shows fine, there is another issue:

  • the tap gesture does not work on web and the handleOnPress does not get called

Since we already call runOnJs and it is more effort I guess to go from the ui thread and call from js thread, we can just use an Animated Pressable and pass it on onPress.

This will fix the uncalled onPress and remove the error so users of the lib dont have to do workarounds

@itsramiel itsramiel changed the title fix: fix backdrop gesture fix: [v5]fix backdrop gesture Aug 2, 2023
@devoren
Copy link

devoren commented Aug 3, 2023

This may also fix #1446

@juanlarra1
Copy link

juanlarra1 commented Aug 22, 2023

Is this going to be merged or should we use it with a patch?

@gorhom
Copy link
Owner

gorhom commented Sep 6, 2023

thanks @itsramiel for submitting this PR, i will run multiple tests on your change and will update you

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

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

@devoren
Copy link

devoren commented Oct 6, 2023

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

it's not merged yet

@deeeed
Copy link

deeeed commented Oct 17, 2023

Is this supposed to also work with v4? I tried it but nothing is triggered onPress.

@devoren
Copy link

devoren commented Oct 17, 2023

@deeeed i think web support only in v5

@juanlarra1
Copy link

can we get an update on this?

Copy link

github-actions bot commented Dec 7, 2023

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

@devoren
Copy link

devoren commented Dec 7, 2023

still waiting for merge

@gorhom gorhom added the v5 label Jan 1, 2024
@gorhom gorhom self-assigned this Jan 1, 2024
@gorhom
Copy link
Owner

gorhom commented Jan 1, 2024

my only concern with this PR is that will block the layers behind backdrop when accessibility voice over is enabled ,, let me investigate it

@itsramiel itsramiel closed this by deleting the head repository Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants