Skip to content

Commit

Permalink
fix: ensure screens that are bottom sheets scroll correctly (valora-i…
Browse files Browse the repository at this point in the history
…nc#5062)

### Description

Context
https://valora-app.slack.com/archives/C025V1D6F3J/p1709855852378389

The bottom sheet screens were not scrolling correctly, this was probably
a regression but unnoticed because most of these screens do not require
scrolling. Root cause is a known issue that is documented in a comment
in the Navigator - we need to use the react-native-gesture-handler
components to scroll inside bottom sheet screens.

This PR adds a prop to the BottomSheetScrollView component to use the
correct scrollview component.

### Test plan

Scrolling content:


https://github.com/valora-inc/wallet/assets/20150449/349c2628-017c-46b1-af88-06d20bcb19ce



Short content:


https://github.com/valora-inc/wallet/assets/20150449/5929f8f9-08f1-495f-8131-211cd055fbdb



Tested also on the WC flow.


### Related issues

- n/a

### Backwards compatibility

Y

### Network scalability

Y
  • Loading branch information
kathaypacific authored and shottah committed May 15, 2024
1 parent 60f738f commit 493a552
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 13 deletions.
3 changes: 2 additions & 1 deletion src/components/BottomSheet.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import GorhomBottomSheet from '@gorhom/bottom-sheet'
import React, { useRef } from 'react'
import { ScrollView, StyleSheet, Text, TextStyle, View } from 'react-native'
import { StyleSheet, Text, TextStyle, View } from 'react-native'
import { ScrollView } from 'react-native-gesture-handler'
import BottomSheetBase from 'src/components/BottomSheetBase'
import BottomSheetScrollView from 'src/components/BottomSheetScrollView'
import fontStyles from 'src/styles/fonts'
Expand Down
31 changes: 27 additions & 4 deletions src/components/BottomSheetScrollView.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,50 @@
import { BottomSheetScrollView as GorhomBottomSheetScrollView } from '@gorhom/bottom-sheet'
import React, { useState } from 'react'
import { LayoutChangeEvent, ScrollView, StyleProp, StyleSheet, View, ViewStyle } from 'react-native'
import { LayoutChangeEvent, StyleProp, StyleSheet, View, ViewStyle } from 'react-native'
import { ScrollView } from 'react-native-gesture-handler'
import { useSafeAreaInsets } from 'react-native-safe-area-context'
import { Spacing } from 'src/styles/styles'
import variables from 'src/styles/variables'

interface Props {
containerStyle?: StyleProp<ViewStyle>
testId?: string
forwardedRef?: React.RefObject<ScrollView>
isScreen?: boolean // should be set to true if using this component directly from a component that is registered as a native bottom sheet screen on the navigator
children: React.ReactNode
}

function BottomSheetScrollView({ forwardedRef, containerStyle, testId, children }: Props) {
function BottomSheetScrollView({
forwardedRef,
containerStyle,
testId,
isScreen,
children,
}: Props) {
const [containerHeight, setContainerHeight] = useState(0)
const [contentHeight, setContentHeight] = useState(0)

const insets = useSafeAreaInsets()
const scrollEnabled = contentHeight > containerHeight

// the BottomSheetScrollView component from @gorhom/bottom-sheet does not
// scroll correctly when it is the root component of a native bottom sheet on
// the navigator (i.e. a bottom sheet screen). This is a workaround to enable
// scrolling using a ScrollView from react-native-safe-area-context when the
// content is large enough to require scrolling. The downside of using this
// ScrollView is the bottom sheet gestures are not enabled (so you cannot
// overscroll to dismiss)
const ScrollViewComponent = isScreen ? ScrollView : GorhomBottomSheetScrollView
// use max height simulate max 90% snap point for screens. when bottom sheets
// take up the whole screen, it is no longer obvious that they are a bottom
// sheet / how to navigate away
const maxHeight = isScreen ? variables.height * 0.9 : undefined

return (
<GorhomBottomSheetScrollView
<ScrollViewComponent
ref={forwardedRef}
scrollEnabled={scrollEnabled}
style={{ maxHeight }}
onLayout={(event: LayoutChangeEvent) => {
setContainerHeight(event.nativeEvent.layout.height)
}}
Expand All @@ -40,7 +63,7 @@ function BottomSheetScrollView({ forwardedRef, containerStyle, testId, children
>
{children}
</View>
</GorhomBottomSheetScrollView>
</ScrollViewComponent>
)
}

Expand Down
2 changes: 1 addition & 1 deletion src/dappkit/DappKitAccountScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const DappKitAccountScreen = ({ route }: Props) => {
}

return (
<BottomSheetScrollView>
<BottomSheetScrollView isScreen>
<RequestContent
type="confirm"
onAccept={handleAllow}
Expand Down
2 changes: 1 addition & 1 deletion src/dappkit/DappKitSignTxScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const DappKitSignTxScreen = ({ route }: Props) => {
}

return (
<BottomSheetScrollView>
<BottomSheetScrollView isScreen>
<RequestContent
type="confirm"
onAccept={handleAllow}
Expand Down
2 changes: 1 addition & 1 deletion src/dapps/DappShortcutTransactionRequest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function DappShortcutTransactionRequest({ route: { params } }: Props) {
}

return (
<BottomSheetScrollView>
<BottomSheetScrollView isScreen>
{pendingAcceptShortcut?.transactions?.length ? (
<RequestContent
type="confirm"
Expand Down
2 changes: 1 addition & 1 deletion src/fiatExchanges/FiatExchangeCurrencyBottomSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function FiatExchangeCurrencyBottomSheet({ route }: Props) {
}

return (
<BottomSheetScrollView containerStyle={{ padding: undefined }}>
<BottomSheetScrollView isScreen containerStyle={{ padding: undefined }}>
{/* padding undefined to prevent android ripple bug */}
<Text style={styles.selectDigitalCurrency}>{t('sendEnterAmountScreen.selectToken')}</Text>
{tokenList.length &&
Expand Down
9 changes: 5 additions & 4 deletions src/navigator/Navigator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,11 @@ const mainScreenNavOptions = () => ({
})

function nativeBottomSheets(BottomSheet: typeof RootStack) {
// Note: scrolling views inside bottom sheet screens should use the relevant
// components from react-native-gesture-handler instead of directly from
// react-native
// https://github.com/osdnk/react-native-reanimated-bottom-sheet/issues/264#issuecomment-674757545

return (
<>
<BottomSheet.Screen name={Screens.WalletConnectRequest} component={WalletConnectRequest} />
Expand Down Expand Up @@ -698,10 +703,6 @@ function RootStackScreen() {
[]
)

// Note: scrolling views inside bottom sheet screens should use the relevant
// components from react-native-gesture-handler instead of directly from
// react-native
// https://github.com/osdnk/react-native-reanimated-bottom-sheet/issues/264#issuecomment-674757545
return (
<RootStack.Navigator
screenOptions={{
Expand Down
1 change: 1 addition & 0 deletions src/walletConnect/screens/WalletConnectRequest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ function WalletConnectRequest({ route: { params } }: Props) {

return (
<BottomSheetScrollView
isScreen
containerStyle={
params.type === WalletConnectRequestType.Loading ||
params.type === WalletConnectRequestType.TimeOut
Expand Down

0 comments on commit 493a552

Please sign in to comment.