Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix blank screen issue when scrolling on trip list #1132

Merged
merged 11 commits into from
Mar 4, 2024
90 changes: 72 additions & 18 deletions www/js/components/LeafletView.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,55 @@
import React, { useEffect, useRef, useState } from 'react';
import { View } from 'react-native';
import React, { useEffect, useMemo, useRef } from 'react';
import { View, ViewProps } from 'react-native';
import { useTheme } from 'react-native-paper';
import L, { Map } from 'leaflet';
import { GeoJSONStyledFeature } from '../types/diaryTypes';
import L, { Map as LeafletMap } from 'leaflet';
import { GeoJSONData, GeoJSONStyledFeature } from '../types/diaryTypes';
import useLeafletCache from './useLeafletCache';

const mapSet = new Set<any>();

// open the URL in the system browser & prevent any other effects of the click event
window['launchURL'] = (url, event) => {
window['cordova'].InAppBrowser.open(url, '_system');
event.stopPropagation();
return false;
};
const osmURL = 'http://www.openstreetmap.org/copyright';
const leafletURL = 'https://leafletjs.com';

export function invalidateMaps() {
mapSet.forEach((map) => map.invalidateSize());
}

const LeafletView = ({ geojson, opts, ...otherProps }) => {
type Props = ViewProps & {
geojson: GeoJSONData;
opts?: L.MapOptions;
downscaleTiles?: boolean;
cacheHtml?: boolean;
};
const LeafletView = ({ geojson, opts, downscaleTiles, cacheHtml, ...otherProps }: Props) => {
const mapElRef = useRef<HTMLDivElement | null>(null);
const leafletMapRef = useRef<Map | null>(null);
const geoJsonIdRef = useRef(null);
const leafletMapRef = useRef<LeafletMap | null>(null);
const geoJsonIdRef = useRef<string | null>(null);
const { colors } = useTheme();
const leafletCache = useLeafletCache();

// unique ID for map element, like "map-5f3e3b" or "map-5f3e3b-downscaled"
const mapElId = useMemo(() => {
let id = 'map-';
// non-alphanumeric characters are not safe for element IDs
id += geojson.data.id.replace(/[^a-zA-Z0-9]/g, '');
if (downscaleTiles) id += '-downscaled';
return id;
}, [geojson.data.id, downscaleTiles]);

function initMap(map: Map) {
L.tileLayer('https://tile.openstreetmap.org/{z}/{x}/{y}.png', {
attribution: '&copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>',
function initMap(map: LeafletMap) {
map.attributionControl?.setPrefix(
`<a href="#" onClick="launchURL('${leafletURL}', event)">Leaflet</a>`,
);
const tileLayer = L.tileLayer('https://tile.openstreetmap.org/{z}/{x}/{y}.png', {
attribution: `&copy; <a href="#" onClick="launchURL('${osmURL}', event)">OpenStreetMap</a>`,
opacity: 1,
detectRetina: true,
detectRetina: !downscaleTiles,
}).addTo(map);
const gj = L.geoJson(geojson.data, {
pointToLayer: pointToLayer,
Expand All @@ -30,30 +60,44 @@ const LeafletView = ({ geojson, opts, ...otherProps }) => {
geoJsonIdRef.current = geojson.data.id;
leafletMapRef.current = map;
mapSet.add(map);
return tileLayer;
}

useEffect(() => {
// if a Leaflet map is cached, there is no need to create the map again
if (cacheHtml && leafletCache.has(mapElId)) return;
// if a Leaflet map already exists (because we are re-rendering), remove it before creating a new one
if (leafletMapRef.current) {
leafletMapRef.current.remove();
mapSet.delete(leafletMapRef.current);
}
if (!mapElRef.current) return;
const map = L.map(mapElRef.current, opts || {});
initMap(map);
}, [geojson]);
const tileLayer = initMap(map);

if (cacheHtml) {
new Promise((resolve) => tileLayer.on('load', resolve)).then(() => {
// After a Leaflet map is rendered, cache the map to reduce the cost for creating a map
const mapHTMLElements = document.getElementById(mapElId);
leafletCache.set(mapElId, mapHTMLElements?.innerHTML);
leafletMapRef.current?.remove();
});
}
}, [geojson, cacheHtml]);

/* If the geojson is different between renders, we need to recreate the map
(happens because of FlashList's view recycling on the trip cards:
https://shopify.github.io/flash-list/docs/recycling) */
if (geoJsonIdRef.current && geoJsonIdRef.current !== geojson.data.id && leafletMapRef.current) {
if (
!leafletCache.has(mapElId) &&
geoJsonIdRef.current &&
geoJsonIdRef.current !== geojson.data.id &&
leafletMapRef.current
) {
leafletMapRef.current.eachLayer((layer) => leafletMapRef.current?.removeLayer(layer));
initMap(leafletMapRef.current);
}

// non-alphanumeric characters are not safe for element IDs
const mapElId = `map-${geojson.data.id.replace(/[^a-zA-Z0-9]/g, '')}`;

return (
<View {...otherProps} role="img" aria-label="Map">
<style>{`
Expand Down Expand Up @@ -96,13 +140,23 @@ const LeafletView = ({ geojson, opts, ...otherProps }) => {
/* glyph for 'flag' from https://pictogrammers.com/library/mdi/icon/flag/ */ ''
}
}
.leaflet-tile-loaded {
opacity: 1 !important;
}
`}</style>

<div
id={mapElId}
ref={mapElRef}
data-tap-disabled="true"
aria-hidden={true}
style={{ width: '100%', height: '100%', zIndex: 0 }}></div>
style={{ width: '100%', height: '100%', zIndex: 0 }}
dangerouslySetInnerHTML={
/* this is not 'dangerous' here because the content is not user-generated;
it's just an HTML string that we cached from a previous render */
cacheHtml && leafletCache.has(mapElId) ? { __html: leafletCache.get(mapElId) } : undefined
}
/>
</View>
);
};
Expand Down
10 changes: 10 additions & 0 deletions www/js/components/useLeafletCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { useState } from 'react';
export default function useLeafletCache() {
const [cachedMaps, setCachedMaps] = useState(new Map());

return {
has: (key: string) => cachedMaps.has(key),
get: (key: string) => cachedMaps.get(key),
set: (key: string, value: any) => setCachedMaps((prev) => new Map(prev.set(key, value))),
};
}
22 changes: 13 additions & 9 deletions www/js/diary/cards/TripCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import { useGeojsonForTrip } from '../timelineHelper';
import { CompositeTrip } from '../../types/diaryTypes';
import { EnketoUserInputEntry } from '../../survey/enketo/enketoHelper';

type Props = { trip: CompositeTrip };
const TripCard = ({ trip }: Props) => {
type Props = { trip: CompositeTrip; isFirstInList?: boolean };
const TripCard = ({ trip, isFirstInList }: Props) => {
const { t } = useTranslation();
const { width: windowWidth } = useWindowDimensions();
const appConfig = useAppConfig();
Expand All @@ -54,7 +54,7 @@ const TripCard = ({ trip }: Props) => {
navigation.navigate('label.details', { tripId, flavoredTheme });
}

const mapOpts = { zoomControl: false, dragging: false };
const mapOpts = { attributionControl: isFirstInList, zoomControl: false, dragging: false };
const showAddNoteButton = appConfig?.survey_info?.buttons?.['trip-notes'];
const mapStyle = showAddNoteButton ? s.shortenedMap : s.fullHeightMap;
return (
Expand Down Expand Up @@ -113,13 +113,17 @@ const TripCard = ({ trip }: Props) => {
</View>
<View style={{ flex: 1, paddingBottom: showAddNoteButton ? 8 : 0 }}>
{/* left panel */}
<LeafletView
geojson={tripGeojson}
opts={mapOpts}
/* the map should be at least as tall as it is wide
{tripGeojson && (
<LeafletView
geojson={tripGeojson}
opts={mapOpts}
downscaleTiles={true}
cacheHtml={true}
/* the map should be at least as tall as it is wide
so it doesn't look squished */
style={[{ minHeight: windowWidth / 2 }, mapStyle]}
/>
style={[{ minHeight: windowWidth / 2 }, mapStyle]}
/>
)}
<ModesIndicator trip={trip} detectedModes={detectedModes} />
{showAddNoteButton && (
<View style={cardStyles.notesButton}>
Expand Down
33 changes: 26 additions & 7 deletions www/js/diary/list/TimelineScrollList.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import React from 'react';
import { FlashList } from '@shopify/flash-list';
import TripCard from '../cards/TripCard';
import PlaceCard from '../cards/PlaceCard';
import UntrackedTimeCard from '../cards/UntrackedTimeCard';
import { View } from 'react-native';
import { View, FlatList } from 'react-native';
import { ActivityIndicator, Banner, Text } from 'react-native-paper';
import LoadMoreButton from './LoadMoreButton';
import { useTranslation } from 'react-i18next';
import { Icon } from '../../components/Icon';

const renderCard = ({ item: listEntry }) => {
const renderCard = ({ item: listEntry, index }) => {
if (listEntry.origin_key.includes('trip')) {
return <TripCard trip={listEntry} />;
return <TripCard trip={listEntry} isFirstInList={index == 0} />;
} else if (listEntry.origin_key.includes('place')) {
return <PlaceCard place={listEntry} />;
} else if (listEntry.origin_key.includes('untracked')) {
Expand Down Expand Up @@ -40,6 +39,7 @@ const TimelineScrollList = ({
isLoading,
}: Props) => {
const { t } = useTranslation();
const listRef = React.useRef<FlatList | null>(null);

// The way that FlashList inverts the scroll view means we have to reverse the order of items too
const reversedListEntries = listEntries ? [...listEntries].reverse() : [];
Expand Down Expand Up @@ -82,11 +82,11 @@ const TimelineScrollList = ({
} else if (listEntries) {
/* Condition: we've successfully loaded and set `listEntries`, so show the list */
return (
<FlashList
inverted
<FlatList
ref={listRef}
nativeID="timelineScrollList"
data={reversedListEntries}
renderItem={renderCard}
estimatedItemSize={240}
keyExtractor={(item) => item._id.$oid}
/* TODO: We can capture onScroll events like this, so we should be able to automatically
load more trips when the user is approaching the bottom or top of the list.
Expand All @@ -97,6 +97,25 @@ const TimelineScrollList = ({
}
ListFooterComponent={isLoading == 'prepend' ? smallSpinner : footer}
ItemSeparatorComponent={separator}
/* use column-reverse so that the list is 'inverted', meaning it should start
scrolling from the bottom, and the bottom-most item should be first in the DOM tree
This method is used instead of the `inverted` property of FlatList, because `inverted`
uses CSS transforms to flip the entire list and then flip each list item back, which
is a performance hit and causes scrolling to be choppy, especially on old iPhones. */
style={{ flexDirection: 'column-reverse' }}
contentContainerStyle={{ flexDirection: 'column-reverse' }}
/* Workaround for iOS Safari bug where a 'column-reverse' element containing scroll content
shows up blank until it's scrolled or its layout changes.
Adding a temporary 1px margin-right, and then removing it on the next event loop,
is the least intrusive way I've found to trigger a layout change.
It basically just jiggles the element so it doesn't blank out. */
onContentSizeChange={() => {
const list = document.getElementById('timelineScrollList');
list?.style.setProperty('margin-right', '1px');
setTimeout(() => {
list?.style.setProperty('margin-right', '0');
});
}}
/>
);
} else {
Expand Down
Loading