chore: upgrade react-native-gesture-handler to v2 #3440
Conversation
📏 Size AnalysisTotal install size 60.3 MB | This change: ⬆️ +103.4 kB (+0.172%)🗂 See size breakdown
🔎 See the full size analysis (ab529f8) merging into main (c717cb7) |
Codecov Report
@@ Coverage Diff @@
## main #3440 +/- ##
=======================================
Coverage 81.08% 81.08%
=======================================
Files 650 650
Lines 22888 22885 -3
Branches 4126 4126
=======================================
- Hits 18559 18557 -2
+ Misses 4267 4266 -1
Partials 62 62
Continue to review full report at Codecov.
|
| @@ -48,13 +48,6 @@ export default offRamps = () => { | |||
| await waitForElementId('RNWebView') | |||
| await expect(element(by.text('Bidali'))).toBeVisible() | |||
| }) | |||
|
|
|||
| // TODO(tomm) debug why this is failing on Android | |||
| it(':ios: Then should not be able to spend CELO', async () => { | |||
There was a problem hiding this comment.
I removed this test because it was failing and i don't think it works as intended - i was surprised to find that it's not straighforward to test if a button is disabled using detox because detox only has access to native elements (not JS ones). wix/Detox#3203
this was previously passing because celoRadioButton returns a native element which doesn't have any property called enabled. i think that e2e tests are better for testing flows rather than display state, so i didn't feel too bad about moving this test into the FiatExchangeCurrency unit test. wdyt @MuckT ?
There was a problem hiding this comment.
Moving this to a unit test is the right choice in my opinion. Anything we can test on a lower test level we should. LGTM!
There was a problem hiding this comment.
Awesome! 👍
I'm curious what @MuckT thinks about the e2e test case.
### Description This was added to fix a vulnerability (#418). This package is sub-dependency of react-native-gesture-hander@v1. Since upgrading v2 (#3440), this is no longer needed. ### Test plan CI ### Related issues N/A ### Backwards compatibility Yes Co-authored-by: Tom McGuire <[email protected]>
Description
We mostly don't need to use this dependency directly. I think that we were using some components from there because of auto import suggestions, in this PR i updated the touchables to be used from either the Touchable component or from react native directly. Also simplified some styles along the way.
This is a good one to upgrade because it removes the dependency on RNGestureHandlerEnabledRootView which was causing conflicts while upgrading react native.
Test plan
Tested all the changes manually. Relying on CI to check the rest.
Related issues
Backwards compatibility
Y