refactor(token-bottom-sheet): support bottom sheet screen and use in cico #5485
Conversation
src/components/TokenBottomSheet.tsx
Outdated
| return ( | ||
| <> | ||
| {isScreen ? ( | ||
| <View style={{ height: variables.height * 0.9 }}>{content}</View> |
There was a problem hiding this comment.
one implication of this is that the bottom sheet will always be 90% high regardless of the number of tokens. If this is not set and the token list exceeds the screen height, the bottom sheet occupies the full screen. The BottomSheetScrollView (which we use for other bottom sheet screens) specifies a maxHeight to the scroll view, but wasn't able to use that here. Open to other ideas.
Example:
There was a problem hiding this comment.
what about using just maxHeight here? you can remove the flex:1 on the container style to make that work (might need to bring the container into the render method and use it only if not isScreen)
There was a problem hiding this comment.
this worked. thanks!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5485 +/- ##
=======================================
Coverage 86.40% 86.40%
=======================================
Files 761 761
Lines 31365 31371 +6
Branches 5390 5397 +7
=======================================
+ Hits 27101 27107 +6
Misses 4033 4033
Partials 231 231
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
### Description Update `@th3rdwave/react-navigation-bottom-sheet` to the latest version and simplify some usage. It was updated to work with the latest version of `gorhom/react-native-bottom-sheet` with dynamic sizing. See https://github.com/th3rdwave/react-navigation-bottom-sheet/releases/tag/v0.3.0 I looked at the previous changes where we added workarounds for screen usage: - #5062 - #5485 These aren't needed anymore, but there's a very small patch needed again. I'll contribute it upstream. However, bonus is we now get scroll down to dismiss for bottom sheet screens! ### Test plan - Tests pass - Manually checked bottom sheets display with the correct size on both iOS and Android. And behave as expected (scroll to dismiss, etc) - Hopefully I haven't missed any special case 👀 ### Related issues N/A ### Backwards compatibility Yes ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [x] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added) --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Description
Refactors the TokenBottomSheet component so that it can be used as a bottom sheet screen for CICO flow. This will allow enabling filters without needing to duplicate much of the code
Test plan
Confirmed token bottom sheet works correctly on all the flows
Related issues
Backwards compatibility
Yes
Network scalability
If a new NetworkId and/or Network are added in the future, the changes in this PR will: