Merge pull request #1132 from jiji14/flashlist-scroll-bug

Fix blank screen issue when scrolling on trip list

www/js/components/LeafletView.tsx

@@ -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,
@@ -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>{`
@@ -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>
   );
 };

www/js/components/useLeafletCache.ts

@@ -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))),
+  };
+}

www/js/diary/cards/TripCard.tsx

@@ -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();
@@ -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 (
@@ -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}>

www/js/diary/list/TimelineScrollList.tsx

@@ -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')) {
@@ -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() : [];
@@ -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.
@@ -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 {