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: change default gesture from Tap to Fling, fix failing CI #2023

Merged
merged 39 commits into from
Feb 7, 2024

Conversation

tboba
Copy link
Member

@tboba tboba commented Jan 31, 2024

Description

Unfortunately, the Tap gesture as the default gesture in ScreenGestureDetector breaks the back button on Android and iOS. Somehow, gesture detector while "tapping" just taps and holds the back button, resulting in breaking CI and our testing apps.
This PR changes this gesture to Fling, resulting in correct behavior. It cannot be changed to Manual gesture, since it also breaks the iOS CI.

Changes

  • Changed gesture from Tap to Fling
  • Updated detox from 20.11.4 to 20.17.0
  • Fixed Android E2E tests by building the bundle with .so files on x86_64 architecture
  • Changed scroll action that was opening background apps preview on Android 33 (and higher) levels
  • Adjusted CI parameters, fixed the typo of artifact names

Test code and steps to reproduce

You can go through Example app and try to click back button in Events. Right now it should work properly.

Checklist

  • Ensured that CI passes

@tboba tboba requested a review from WoLewicki January 31, 2024 15:41
@tboba tboba self-assigned this Jan 31, 2024
@tboba tboba changed the title fix: Change default gesture from Tap to Manual fix: change default gesture from Tap to Manual Jan 31, 2024
Co-authored-by: m-bert <michal.bert@swmansion.com>
@@ -8,7 +8,7 @@ describe('Events', () => {
await waitFor(element(by.id('root-screen-playground-Events')))
.toBeVisible()
.whileElement(by.id('root-screen-examples-scrollview'))
.scroll(200, 'down');
.scroll(100, 'down', NaN, 0.85);
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for changing this value?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see that on higher Android APIs with gestures as a device navigation, phone can accidentally go into the preview of apps in the background - that's why I've added the offset here

@WoLewicki
Copy link
Member

Also, I am not sure I understand what is going on exactly. Why does gesture-handler call the tap? Are we testing custom animations on CI?

@tboba
Copy link
Member Author

tboba commented Feb 1, 2024

@WoLewicki we're not testing them, but this change does not only limit to the case of the CI. Because of the default Tap option as an empty gesture, Android and iOS simulate that motion as holding the element (I don't specifically know why), resulting in unability to click "go back" button, because it detects the click as the hold motion. Fortunately, changing the empty gesture as Manual or Fling fixes that issue.
Also, unfortunately fixing this behavior does not fix the CI problem at all. Sorry for early requesting for code review!

@WoLewicki
Copy link
Member

Ok keep me updated 🚀

ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…re-mansion#2023)

## Description

Unfortunately, the `Tap` gesture as the default gesture in
ScreenGestureDetector breaks the back button on Android and iOS.
Somehow, gesture detector while "tapping" just taps and holds the back
button, resulting in breaking CI and our testing apps.
This PR changes this gesture to `Fling`, resulting in correct behavior.
It cannot be changed to `Manual` gesture, since it also breaks the iOS
CI.

## Changes

- Changed gesture from Tap to Fling
- Updated detox from 20.11.4 to 20.17.0
- Fixed Android E2E tests by building the bundle with .so files on
x86_64 architecture
- Changed scroll action that was opening background apps preview on
Android 33 (and higher) levels
- Adjusted CI parameters, fixed the typo of artifact names

## Test code and steps to reproduce

You can go through Example app and try to click back button in `Events`.
Right now it should work properly.

## Checklist

- [x] Ensured that CI passes

---------

Co-authored-by: m-bert <michal.bert@swmansion.com>
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.

3 participants