From 52d196dc0ebf64bd3d751615f6bea75b2b51b986 Mon Sep 17 00:00:00 2001 From: Steffen Kleinle Date: Wed, 12 Feb 2025 17:28:06 +0100 Subject: [PATCH] 1258: Refactor pois and bottom sheet --- native/src/components/BottomSheet.tsx | 70 --------- native/src/components/BottomSheetHandle.tsx | 28 +--- native/src/components/List.tsx | 2 +- native/src/components/MapView.tsx | 9 +- native/src/components/PoiListItem.tsx | 2 +- native/src/components/PoisBottomSheet.tsx | 117 +++++++++++++++ .../__tests__/PoisBottomSheet.spec.tsx | 82 +++++++++++ native/src/constants/dimensions.ts | 4 +- native/src/routes/Pois.tsx | 133 +++++------------- native/src/routes/PoisContainer.tsx | 11 +- native/src/routes/__tests__/Pois.spec.tsx | 5 +- 11 files changed, 249 insertions(+), 214 deletions(-) delete mode 100644 native/src/components/BottomSheet.tsx create mode 100644 native/src/components/PoisBottomSheet.tsx create mode 100644 native/src/components/__tests__/PoisBottomSheet.spec.tsx diff --git a/native/src/components/BottomSheet.tsx b/native/src/components/BottomSheet.tsx deleted file mode 100644 index 40328c4929..0000000000 --- a/native/src/components/BottomSheet.tsx +++ /dev/null @@ -1,70 +0,0 @@ -import RNBottomSheet, { - BottomSheetHandleProps, - BottomSheetScrollView, - BottomSheetScrollViewMethods, -} from '@gorhom/bottom-sheet' -import React, { memo, ReactElement, ReactNode, useCallback } from 'react' -import { Platform } from 'react-native' -import styled from 'styled-components/native' - -import BottomSheetHandle from './BottomSheetHandle' - -const StyledBottomSheet = styled(RNBottomSheet)<{ isFullscreen: boolean }>` - ${props => props.isFullscreen && `background-color: ${props.theme.colors.backgroundColor};`} -` - -type BottomSheetProps = { - children: ReactNode - snapPoints: (string | number)[] - title?: string - visible?: boolean - onChange?: (index: number) => void - initialIndex: number - snapPointIndex: number - setScrollPosition: (position: number) => void -} - -const BottomSheet = React.forwardRef( - ( - { - children, - title, - visible = true, - onChange, - snapPoints, - initialIndex = 0, - setScrollPosition, - snapPointIndex, - }: BottomSheetProps, - scrollRef: React.Ref, - ): ReactElement | null => { - const renderHandle = useCallback( - (props: BottomSheetHandleProps) => , - [title], - ) - - if (!visible) { - return null - } - - return ( - - setScrollPosition(event.nativeEvent.contentOffset.y)} - ref={scrollRef}> - {children} - - - ) - }, -) - -export default memo(BottomSheet) diff --git a/native/src/components/BottomSheetHandle.tsx b/native/src/components/BottomSheetHandle.tsx index 9ea1d8c6bb..4f0b21b542 100644 --- a/native/src/components/BottomSheetHandle.tsx +++ b/native/src/components/BottomSheetHandle.tsx @@ -1,18 +1,7 @@ import React, { ReactElement } from 'react' -import { View } from 'react-native' import styled from 'styled-components/native' -const SheetHeader = styled.View` - justify-content: center; - flex-direction: row; - margin-bottom: 20px; -` -const Title = styled.Text` - color: ${props => props.theme.colors.textColor}; - font-family: ${props => props.theme.fonts.native.decorativeFontBold}; - font-size: 18px; -` -const Indicator = styled.View` +const Handle = styled.View` width: 34px; border: 1px solid ${props => props.theme.colors.textSecondaryColor}; background-color: ${props => props.theme.colors.textSecondaryColor}; @@ -21,19 +10,6 @@ const Indicator = styled.View` margin: 20px 0; ` -type BottomSheetHandleProps = { - title?: string -} - -const BottomSheetHandle = ({ title }: BottomSheetHandleProps): ReactElement => ( - - - {!!title && ( - - {title} - - )} - -) +const BottomSheetHandle = (): ReactElement => export default BottomSheetHandle diff --git a/native/src/components/List.tsx b/native/src/components/List.tsx index 5e3db382ca..dec7b594b4 100644 --- a/native/src/components/List.tsx +++ b/native/src/components/List.tsx @@ -2,7 +2,7 @@ import React, { ReactElement } from 'react' import { FlatList, RefreshControl, ViewStyle } from 'react-native' import styled from 'styled-components/native' -const NoItemsMessage = styled.Text` +export const NoItemsMessage = styled.Text` color: ${props => props.theme.colors.textColor}; font-family: ${props => props.theme.fonts.native.contentFontRegular}; align-self: center; diff --git a/native/src/components/MapView.tsx b/native/src/components/MapView.tsx index 7b17a25091..49e8e80268 100644 --- a/native/src/components/MapView.tsx +++ b/native/src/components/MapView.tsx @@ -66,10 +66,9 @@ type MapViewProps = { selectedFeature: MapFeature | null userLocation: LocationType | null setUserLocation: (userLocation: LocationType | null) => void - iconPosition: string | number selectFeature: (feature: MapFeature | null) => void - setSheetSnapPointIndex: (index: number) => void bottomSheetHeight: number + bottomSheetFullscreen: boolean zoom: number | undefined Overlay?: ReactElement } @@ -78,13 +77,12 @@ const MapView = ({ boundingBox, features, selectedFeature, - iconPosition, userLocation, setUserLocation, selectFeature, - setSheetSnapPointIndex, Overlay, bottomSheetHeight, + bottomSheetFullscreen, zoom, }: MapViewProps): ReactElement => { const cameraRef = useRef(null) @@ -158,7 +156,6 @@ const MapView = ({ const feature = featureCollection?.features.find((it): it is MapFeature => it.geometry.type === 'Point') selectFeature(feature ?? null) - setSheetSnapPointIndex(1) zoomOnClusterPress(pressedCoordinates) } @@ -202,7 +199,7 @@ const MapView = ({ } onPress={onRequestLocation} - position={iconPosition} + position={bottomSheetFullscreen ? 0 : bottomSheetHeight} accessibilityLabel={t('showOwnLocation')} /> diff --git a/native/src/components/PoiListItem.tsx b/native/src/components/PoiListItem.tsx index 8dcd0b2fc6..14000cf5a0 100644 --- a/native/src/components/PoiListItem.tsx +++ b/native/src/components/PoiListItem.tsx @@ -30,7 +30,7 @@ const StyledPressable = styled(Pressable)<{ language: string }>` border-bottom-width: 1px; border-bottom-color: ${props => props.theme.colors.textDisabledColor}; flex-direction: ${props => contentDirection(props.language)}; - padding: 24px 0; + padding: 16px 0; ` const Description = styled.View` diff --git a/native/src/components/PoisBottomSheet.tsx b/native/src/components/PoisBottomSheet.tsx new file mode 100644 index 0000000000..b5d9704d8a --- /dev/null +++ b/native/src/components/PoisBottomSheet.tsx @@ -0,0 +1,117 @@ +import BottomSheet, { + BottomSheetFlatList, + BottomSheetFlatListMethods, + BottomSheetScrollView, +} from '@gorhom/bottom-sheet' +import React, { memo, ReactElement, Ref } from 'react' +import { useTranslation } from 'react-i18next' +import { Platform } from 'react-native' +import styled from 'styled-components/native' + +import { LocationType } from 'shared' +import { ErrorCode, PoiModel } from 'shared/api' + +import useCityAppContext from '../hooks/useCityAppContext' +import BottomSheetHandle from './BottomSheetHandle' +import Failure from './Failure' +import { NoItemsMessage } from './List' +import PoiDetails from './PoiDetails' +import PoiListItem from './PoiListItem' + +const StyledBottomSheet = styled(BottomSheet)<{ isFullscreen: boolean }>` + ${props => props.isFullscreen && `background-color: ${props.theme.colors.backgroundColor};`} +` + +const BottomSheetContent = styled.View` + flex: 1; + margin: 0 24px; +` + +const Title = styled.Text` + color: ${props => props.theme.colors.textColor}; + font-family: ${props => props.theme.fonts.native.decorativeFontBold}; + font-size: 18px; + font-weight: bold; +` + +type PoiBottomSheetProps = { + poiListRef: Ref + pois: PoiModel[] + poi: PoiModel | undefined + userLocation: LocationType | null + slug: string | undefined + selectPoi: (poi: PoiModel) => void + deselectAll: () => void + snapPoints: number[] + snapPointIndex: number + setSnapPointIndex: (index: number) => void + setScrollPosition: (position: number) => void + isFullscreen: boolean +} + +const PoisBottomSheet = ({ + poiListRef, + pois, + poi, + userLocation, + slug, + selectPoi, + deselectAll, + snapPoints, + snapPointIndex, + setSnapPointIndex, + setScrollPosition, + isFullscreen, +}: PoiBottomSheetProps): ReactElement | null => { + const { languageCode } = useCityAppContext() + const { t } = useTranslation('pois') + // ios has scrolling issues if content panning gesture is not enabled + const enableContentPanningGesture = Platform.OS === 'ios' || !isFullscreen + + const PoiDetail = poi ? ( + + ) : ( + + ) + + const renderPoiListItem = ({ item: poi }: { item: PoiModel }): ReactElement => ( + selectPoi(poi)} + distance={userLocation && poi.distance(userLocation)} + /> + ) + + return ( + + + {slug ? ( + {PoiDetail} + ) : ( + setScrollPosition(event.nativeEvent.contentOffset.y)} + showsVerticalScrollIndicator={false} + ListHeaderComponent={{t('listTitle')}} + ListEmptyComponent={{t('noPois')}} + /> + )} + + + ) +} + +export default memo(PoisBottomSheet) diff --git a/native/src/components/__tests__/PoisBottomSheet.spec.tsx b/native/src/components/__tests__/PoisBottomSheet.spec.tsx new file mode 100644 index 0000000000..4dd323b289 --- /dev/null +++ b/native/src/components/__tests__/PoisBottomSheet.spec.tsx @@ -0,0 +1,82 @@ +import { fireEvent } from '@testing-library/react-native' +import React from 'react' + +import { PoiModelBuilder } from 'shared/api' + +import TestingAppContext from '../../testing/TestingAppContext' +import renderWithTheme from '../../testing/render' +import PoisBottomSheet from '../PoisBottomSheet' + +jest.mock('../../components/Page') +jest.mock('@react-native-clipboard/clipboard', () => () => ({ setString: jest.fn() })) +jest.mock('react-i18next') +jest.mock('styled-components') +jest.mock('@gorhom/bottom-sheet', () => ({ + __esModule: true, + ...require('@gorhom/bottom-sheet/mock'), +})) + +describe('PoisBottomSheet', () => { + const pois = new PoiModelBuilder(3).build() + const poi0 = pois[0]! + const poi1 = pois[1]! + const poi2 = pois[2]! + const deselectAll = jest.fn() + const selectPoi = jest.fn() + + const renderPois = ({ slug = undefined }: { slug?: string; multipoi?: number; poiCategoryId?: number }) => + renderWithTheme( + + it.slug === slug)} + pois={pois} + snapPoints={[]} + snapPointIndex={0} + userLocation={null} + poiListRef={jest.fn()} + setScrollPosition={jest.fn()} + setSnapPointIndex={jest.fn()} + deselectAll={deselectAll} + selectPoi={selectPoi} + isFullscreen={false} + /> + , + ) + + it('should show failure if poi is not found', () => { + const { queryByText, getByText } = renderPois({ slug: 'invalid' }) + + expect(getByText('pageNotFound')).toBeTruthy() + expect(queryByText(poi0.title)).toBeFalsy() + expect(queryByText(poi1.title)).toBeFalsy() + expect(queryByText(poi2.title)).toBeFalsy() + + fireEvent.press(getByText('detailsHeader')) + + expect(deselectAll).toHaveBeenCalledTimes(1) + }) + + it('should show list', () => { + const { getByText } = renderPois({}) + + expect(getByText(poi0.title)).toBeTruthy() + expect(getByText(poi1.title)).toBeTruthy() + expect(getByText(poi2.title)).toBeTruthy() + + fireEvent.press(getByText(poi1.title)) + expect(selectPoi).toHaveBeenCalledTimes(1) + expect(selectPoi).toHaveBeenCalledWith(poi1) + }) + + it('should show poi', () => { + const { getByText, queryByText } = renderPois({ slug: poi2.slug }) + + expect(getByText(poi2.title)).toBeTruthy() + expect(getByText(poi2.category.name)).toBeTruthy() + expect(getByText(poi2.content)).toBeTruthy() + expect(getByText(poi2.category.name)).toBeTruthy() + expect(queryByText(poi0.title)).toBeFalsy() + expect(queryByText(poi1.title)).toBeFalsy() + }) +}) diff --git a/native/src/constants/dimensions.ts b/native/src/constants/dimensions.ts index b8ef1075c0..0c99acc5c3 100644 --- a/native/src/constants/dimensions.ts +++ b/native/src/constants/dimensions.ts @@ -11,7 +11,7 @@ export type DimensionsType = { */ fontScaling: number headerTextSize: number - bottomSheetHandler: { + bottomSheetHandle: { height: number } locationFab: { @@ -29,7 +29,7 @@ const dimensions: DimensionsType = { }, fontScaling: 0.04, headerTextSize: 20, - bottomSheetHandler: { + bottomSheetHandle: { height: 40, }, locationFab: { diff --git a/native/src/routes/Pois.tsx b/native/src/routes/Pois.tsx index 653634957c..89156d3ef7 100644 --- a/native/src/routes/Pois.tsx +++ b/native/src/routes/Pois.tsx @@ -1,93 +1,63 @@ -import { BottomSheetScrollViewMethods } from '@gorhom/bottom-sheet' +import { BottomSheetFlatListMethods } from '@gorhom/bottom-sheet' import React, { ReactElement, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' -import { ScrollView, useWindowDimensions } from 'react-native' +import { View, useWindowDimensions } from 'react-native' import { SvgUri } from 'react-native-svg' import styled from 'styled-components/native' -import { PoisRouteType, isMultipoi, LocationType, sortPois, MapFeature, preparePois, safeParseInt } from 'shared' -import { PoiCategoryModel, CityModel, PoiModel, ErrorCode } from 'shared/api' +import { PoisRouteType, isMultipoi, LocationType, MapFeature, preparePois, safeParseInt, sortPois } from 'shared' +import { PoiCategoryModel, CityModel, PoiModel } from 'shared/api' import { ClockIcon, EditLocationIcon } from '../assets' -import BottomSheet from '../components/BottomSheet' -import Failure from '../components/Failure' -import List from '../components/List' import MapView from '../components/MapView' -import PoiDetails from '../components/PoiDetails' import PoiFiltersModal from '../components/PoiFiltersModal' -import PoiListItem from '../components/PoiListItem' +import PoisBottomSheet from '../components/PoisBottomSheet' import ChipButton from '../components/base/ChipButton' import Icon from '../components/base/Icon' import { NavigationProps, RouteProps } from '../constants/NavigationTypes' import dimensions from '../constants/dimensions' import useOnBackNavigation from '../hooks/useOnBackNavigation' -const Container = styled.View` - flex: 1; - margin: 0 24px; -` - -const Title = styled.Text` - font-size: 16px; - font-weight: bold; -` - const StyledIcon = styled(Icon)` color: ${props => props.theme.colors.textSecondaryColor}; width: 16px; height: 16px; ` -const midSnapPointPercentage = 0.35 -const getBottomSheetSnapPoints = (deviceHeight: number): [number, number, number] => [ - dimensions.bottomSheetHandler.height, - midSnapPointPercentage * deviceHeight, - deviceHeight, -] +const SNAP_POINT_MID_PERCENTAGE = 0.35 type PoisProps = { pois: PoiModel[] cityModel: CityModel - language: string - refresh: () => void route: RouteProps navigation: NavigationProps } -const RESTORE_TIMEOUT = 100 - -const Pois = ({ pois: allPois, language, cityModel, route, navigation, refresh }: PoisProps): ReactElement => { +const Pois = ({ pois: allPois, cityModel, route, navigation }: PoisProps): ReactElement => { const { slug, multipoi, poiCategoryId, zoom } = route.params + const [deselectOnBackNavigation, setDeselectOnBackNavigation] = useState(slug === undefined && multipoi === undefined) const [poiCurrentlyOpenFilter, setPoiCurrentlyOpenFilter] = useState(false) const [showFilterSelection, setShowFilterSelection] = useState(false) const [userLocation, setUserLocation] = useState(null) - const [sheetSnapPointIndex, setSheetSnapPointIndex] = useState(1) + const [bottomSheetSnapPointIndex, setBottomSheetSnapPointIndex] = useState(1) const [listScrollPosition, setListScrollPosition] = useState(0) - const [deselectOnBackNavigation, setDeselectOnBackNavigation] = useState(slug === undefined && multipoi === undefined) - const scrollRef = useRef(null) - const deviceHeight = useWindowDimensions().height - const snapPoints = getBottomSheetSnapPoints(deviceHeight) + const poiListRef = useRef(null) const { t } = useTranslation('pois') + const { height } = useWindowDimensions() + const bottomSheetSnapPoints = [dimensions.bottomSheetHandle.height, SNAP_POINT_MID_PERCENTAGE * height, height] + const bottomSheetFullscreen = bottomSheetSnapPointIndex === bottomSheetSnapPoints.length - 1 + const bottomSheetHeight = bottomSheetSnapPoints[bottomSheetSnapPointIndex] ?? 0 const { pois, poi, mapFeatures, mapFeature, poiCategories, poiCategory } = preparePois({ pois: allPois, params: { slug, multipoi, poiCategoryId, currentlyOpen: poiCurrentlyOpenFilter }, }) - const scrollTo = (position: number) => { - setTimeout(() => { - if (scrollRef.current) { - scrollRef.current.scrollTo({ - y: position, - animated: false, - }) - } - }, RESTORE_TIMEOUT) - } + const scrollToOffset = (offset: number) => poiListRef.current?.scrollToOffset({ offset, animated: false }) const deselectAll = () => { navigation.setParams({ slug: undefined, multipoi: undefined }) - scrollTo(listScrollPosition) + scrollToOffset(listScrollPosition) } const deselect = () => { @@ -116,41 +86,22 @@ const Pois = ({ pois: allPois, language, cityModel, route, navigation, refresh } const selectMapFeature = (mapFeature: MapFeature | null) => { setDeselectOnBackNavigation(true) deselectAll() + setBottomSheetSnapPointIndex(1) const slug = mapFeature?.properties.pois[0]?.slug if (mapFeature && isMultipoi(mapFeature)) { navigation.setParams({ multipoi: safeParseInt(mapFeature.id), slug: undefined }) - scrollTo(0) + scrollToOffset(0) } else if (slug) { navigation.setParams({ slug, multipoi: undefined }) - scrollTo(0) } } const selectPoi = (poi: PoiModel) => { setDeselectOnBackNavigation(true) navigation.setParams({ slug: poi.slug }) - scrollTo(0) } - const setScrollPosition = (position: number) => setListScrollPosition(previous => (poi ? previous : position)) - - const PoiDetail = poi ? ( - - ) : ( - - ) - - const renderPoiListItem = ({ item: poi }: { item: PoiModel }): ReactElement => ( - selectPoi(poi)} - distance={userLocation && poi.distance(userLocation)} - /> - ) - const FiltersOverlayButtons = ( <> ) - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const currentBottomSheetHeight = snapPoints[sheetSnapPointIndex]! - return ( - + setShowFilterSelection(false)} @@ -195,39 +143,30 @@ const Pois = ({ pois: allPois, language, cityModel, route, navigation, refresh } - - - {!slug ? {t('listTitle')} : undefined} - {slug ? ( - PoiDetail - ) : ( - - )} - - - + + ) } diff --git a/native/src/routes/PoisContainer.tsx b/native/src/routes/PoisContainer.tsx index eba5e34abd..6ef455fb49 100644 --- a/native/src/routes/PoisContainer.tsx +++ b/native/src/routes/PoisContainer.tsx @@ -52,16 +52,7 @@ const PoisContainer = ({ navigation, route }: PoisContainerProps): ReactElement return ( - {data && ( - - )} + {data && } ) } diff --git a/native/src/routes/__tests__/Pois.spec.tsx b/native/src/routes/__tests__/Pois.spec.tsx index 9c6abb2b2e..2a2b691e7d 100644 --- a/native/src/routes/__tests__/Pois.spec.tsx +++ b/native/src/routes/__tests__/Pois.spec.tsx @@ -8,10 +8,13 @@ import Pois from '../Pois' jest.mock('../../components/MapView') jest.mock('../../components/Page') -jest.mock('../../components/BottomSheet') jest.mock('@react-native-clipboard/clipboard', () => () => ({ setString: jest.fn() })) jest.mock('react-i18next') jest.mock('styled-components') +jest.mock('@gorhom/bottom-sheet', () => ({ + __esModule: true, + ...require('@gorhom/bottom-sheet/mock'), +})) describe('Pois', () => { const pois = new PoiModelBuilder(3).build()