From 7002670c1ee036b9fffd3508d4924eee2917161b Mon Sep 17 00:00:00 2001 From: Ruxandra Machedon Date: Thu, 11 Apr 2024 19:02:23 -0400 Subject: [PATCH] fix: callouts no longer crash when selection is filtered out add helper functions to callout system to make initializing state more concise and mapping state to logic more consistent; as a result, if the user's selection is filtered out, the app gracefully hides the callout rather than crashing. further refactor SiteMapCallout for general clarity. --- .../src/screens/HomeScreen/HomeScreen.tsx | 15 +-- .../screens/HomeScreen/HomeScreenCallout.ts | 85 +++++++++++++--- .../components/SiteClusterCalloutListItem.tsx | 7 +- .../screens/HomeScreen/components/SiteMap.tsx | 46 +++++---- .../HomeScreen/components/SiteMapCallout.tsx | 97 ++++++++++++------- 5 files changed, 169 insertions(+), 81 deletions(-) diff --git a/dev-client/src/screens/HomeScreen/HomeScreen.tsx b/dev-client/src/screens/HomeScreen/HomeScreen.tsx index a0d48d880..58b607d06 100644 --- a/dev-client/src/screens/HomeScreen/HomeScreen.tsx +++ b/dev-client/src/screens/HomeScreen/HomeScreen.tsx @@ -48,7 +48,12 @@ import {selectSitesAndUserRoles} from 'terraso-client-shared/selectors'; import {ListFilterProvider} from 'terraso-mobile-client/components/ListFilter'; import {useGeospatialContext} from 'terraso-mobile-client/context/GeospatialContext'; import {Box} from 'terraso-mobile-client/components/NativeBaseAdapters'; -import {CalloutState} from 'terraso-mobile-client/screens/HomeScreen/HomeScreenCallout'; +import { + CalloutState, + noneCallout, + siteCallout, + locationCallout, +} from 'terraso-mobile-client/screens/HomeScreen/HomeScreenCallout'; type HomeScreenRef = { showSiteOnMap: (site: Site) => void; @@ -71,9 +76,7 @@ export const HomeScreen = memo(() => { const infoBottomSheetRef = useRef(null); const siteListBottomSheetRef = useRef(null); const [mapStyleURL, setMapStyleURL] = useState(Mapbox.StyleURL.Street); - const [calloutState, setCalloutState] = useState({ - kind: 'none', - }); + const [calloutState, setCalloutState] = useState(noneCallout()); const currentUserID = useSelector( state => state.account.currentUser?.data?.id, ); @@ -87,7 +90,7 @@ export const HomeScreen = memo(() => { const showSiteOnMap = useCallback( (targetSite: Site) => { mapRef.current?.moveToPoint(targetSite); - setCalloutState({kind: 'site', siteId: targetSite.id}); + setCalloutState(siteCallout(targetSite.id)); siteListBottomSheetRef.current?.collapse(); }, [setCalloutState], @@ -130,7 +133,7 @@ export const HomeScreen = memo(() => { const searchFunction = useCallback( (coords: Coords) => { - setCalloutState({kind: 'location', coords}); + setCalloutState(locationCallout(coords)); mapRef.current?.moveToPoint(coords); }, [setCalloutState, mapRef], diff --git a/dev-client/src/screens/HomeScreen/HomeScreenCallout.ts b/dev-client/src/screens/HomeScreen/HomeScreenCallout.ts index 5df1f535d..def69a21b 100644 --- a/dev-client/src/screens/HomeScreen/HomeScreenCallout.ts +++ b/dev-client/src/screens/HomeScreen/HomeScreenCallout.ts @@ -15,22 +15,83 @@ * along with state program. If not, see https://www.gnu.org/licenses/. */ -import { Coords } from 'terraso-mobile-client/model/map/mapSlice'; - +import {Site} from 'terraso-client-shared/site/siteSlice'; +import {Coords} from 'terraso-mobile-client/model/map/mapSlice'; export type CalloutState = | { - kind: 'site'; - siteId: string; - } + kind: 'site'; + siteId: string; + } | { - kind: 'location'; - coords: Coords; - } + kind: 'location'; + coords: Coords; + } | { - kind: 'site_cluster'; - siteIds: string[]; - coords: Coords; + kind: 'site_cluster'; + siteIds: string[]; + coords: Coords; + } + | {kind: 'none'}; + +export function siteCallout(siteId: string): CalloutState { + return {kind: 'site', siteId}; +} + +export function siteClusterCallout( + coords: Coords, + siteIds: Iterable, +): CalloutState { + return {kind: 'site_cluster', coords, siteIds: Array.from(siteIds)}; +} + +export function locationCallout(coords: Coords): CalloutState { + return {kind: 'location', coords}; +} + +export function noneCallout(): CalloutState { + return {kind: 'none'}; +} + +export function getCalloutCoords( + state: CalloutState, + sites: Record, +): Coords | null { + switch (state.kind) { + case 'site': + return getCalloutSite(state, sites); + case 'location': + return state.coords; + case 'site_cluster': + return state.coords; + case 'none': + return null; } - | { kind: 'none' }; +} +export function getCalloutSite( + state: CalloutState, + sites: Record, +): Site | null { + return state.kind === 'site' && state.siteId in sites + ? sites[state.siteId] + : null; +} + +export function getCalloutSites( + state: CalloutState, + sites: Record, +): Record { + switch (state.kind) { + case 'site': + const site = getCalloutSite(state, sites); + return site == null ? {} : Object.fromEntries([[state.siteId, site]]); + case 'site_cluster': + const presentSiteIds = state.siteIds.filter(siteId => siteId in sites); + return Object.fromEntries( + presentSiteIds.map(siteId => [siteId, sites[siteId]]), + ); + default: + return {}; + } +} diff --git a/dev-client/src/screens/HomeScreen/components/SiteClusterCalloutListItem.tsx b/dev-client/src/screens/HomeScreen/components/SiteClusterCalloutListItem.tsx index 89d4c5871..2b3c06dd6 100644 --- a/dev-client/src/screens/HomeScreen/components/SiteClusterCalloutListItem.tsx +++ b/dev-client/src/screens/HomeScreen/components/SiteClusterCalloutListItem.tsx @@ -18,7 +18,10 @@ import {useCallback} from 'react'; import {Pressable} from 'react-native'; import {Site} from 'terraso-client-shared/site/siteSlice'; -import {CalloutState} from 'terraso-mobile-client/screens/HomeScreen/HomeScreenCallout'; +import { + CalloutState, + siteCallout, +} from 'terraso-mobile-client/screens/HomeScreen/HomeScreenCallout'; import {useSelector} from 'terraso-mobile-client/store'; import { Column, @@ -41,7 +44,7 @@ export const SiteClusterCalloutListItem = ({ : state.project.projects[site.projectId], ); const onPress = useCallback(() => { - setState({kind: 'site', siteId: site.id}); + setState(siteCallout(site.id)); }, [site.id, setState]); return ( diff --git a/dev-client/src/screens/HomeScreen/components/SiteMap.tsx b/dev-client/src/screens/HomeScreen/components/SiteMap.tsx index 29243ac89..33ba16a58 100644 --- a/dev-client/src/screens/HomeScreen/components/SiteMap.tsx +++ b/dev-client/src/screens/HomeScreen/components/SiteMap.tsx @@ -28,7 +28,14 @@ import { import {useTheme} from 'native-base'; import {Keyboard, PixelRatio, StyleSheet} from 'react-native'; import {Site} from 'terraso-client-shared/site/siteSlice'; -import {CalloutState} from 'terraso-mobile-client/screens/HomeScreen/HomeScreenCallout'; +import { + CalloutState, + noneCallout, + siteCallout, + locationCallout, + siteClusterCallout, + getCalloutSite, +} from 'terraso-mobile-client/screens/HomeScreen/HomeScreenCallout'; import { coordsToPosition, positionToCoords, @@ -82,13 +89,10 @@ export const SiteMap = memo( })); const {filteredItems: filteredSites} = useListFilter(); - const sites = Object.fromEntries( filteredSites.map(site => [site.id, site]), ); - - const selectedSite = - calloutState.kind === 'site' ? sites[calloutState.siteId] : null; + const selectedSite = getCalloutSite(calloutState, sites); const {colors} = useTheme(); @@ -145,13 +149,14 @@ export const SiteMap = memo( 0, )) as GeoJSON.FeatureCollection; - setCalloutState({ - kind: 'site_cluster', - coords: positionToCoords( - (feature.geometry as GeoJSON.Point).coordinates, + setCalloutState( + siteClusterCallout( + positionToCoords( + (feature.geometry as GeoJSON.Point).coordinates, + ), + leafFeatures.features.map(feat => feat.id as string), ), - siteIds: leafFeatures.features.map(feat => feat.id as string), - }); + ); } }, [shapeSourceRef, setCalloutState], @@ -180,17 +185,12 @@ export const SiteMap = memo( paddingBottom: 0, cameraRef: cameraRef, }); - setCalloutState({kind: 'site', siteId: feature.id as string}); + setCalloutState(siteCallout(feature.id as string)); } }, [setCalloutState, handleClusterPress], ); - const onTempSitePress = useCallback( - () => setCalloutState(calloutState), - [calloutState, setCalloutState], - ); - const onPress = useCallback( async (feature: GeoJSON.Feature) => { if ( @@ -210,13 +210,12 @@ export const SiteMap = memo( paddingBottom: 320, cameraRef: cameraRef, }); - setCalloutState({ - kind: 'location', - coords: positionToCoords(feature.geometry.coordinates), - }); + setCalloutState( + locationCallout(positionToCoords(feature.geometry.coordinates)), + ); } else { Keyboard.dismiss(); - setCalloutState({kind: 'none'}); + setCalloutState(noneCallout()); } }, [calloutState, setCalloutState], @@ -313,8 +312,7 @@ export const SiteMap = memo( + shape={temporarySitesFeature}> ; @@ -34,45 +41,13 @@ type Props = { }; export const SiteMapCallout = ({sites, state, setState}: Props) => { - const closeCallout = useCallback(() => setState({kind: 'none'}), [setState]); - - if (state.kind === 'none') { + const coords = getCalloutCoords(state, sites); + if (!coords) { return null; } - const coords = state.kind === 'site' ? sites[state.siteId] : state.coords; - - let child: React.ComponentProps['children']; - - if (state.kind === 'site') { - child = ( - } - isPopover={true} - /> - ); - } else if (state.kind === 'site_cluster') { - child = ( - } - isPopover={true}> - id} - renderItem={({item: id}) => ( - - )} - ItemSeparatorComponent={() => } - /> - - ); - } else if (state.kind === 'location') { - child = ( - - ); - } else { + let child = CalloutChild(coords, {sites, state, setState}); + if (!child) { return null; } @@ -85,3 +60,51 @@ export const SiteMapCallout = ({sites, state, setState}: Props) => { ); }; + +const CalloutChild = (coords: Coords, {sites, state, setState}: Props) => { + const closeCallout = useCallback(() => setState(noneCallout()), [setState]); + + switch (state.kind) { + case 'site': + const site = getCalloutSite(state, sites); + if (!site) { + return null; + } + + return ( + } + isPopover={true} + /> + ); + case 'site_cluster': + const clusterSites = getCalloutSites(state, sites); + if (!clusterSites) { + return null; + } + + return ( + } + isPopover={true}> + id} + renderItem={({item: id}) => ( + + )} + ItemSeparatorComponent={() => } + /> + + ); + default: + return ( + + ); + } +};