Merge pull request #1257 from VEuPathDB/1227-marker-animation-fix

Marker animation mostly fixed

packages/libs/components/src/map/SemanticMarkers.tsx

@@ -64,7 +64,7 @@ export default function SemanticMarkers({
     // 2023-11: it does seem to be needed for zoom-in animation to work.
     const debouncedUpdateMarkerPositions = debounce(
       updateMarkerPositions,
-      1000
+      animation ? animation.duration : 0
     );
     // call it at least once at the beginning of the life cycle
     debouncedUpdateMarkerPositions();
@@ -106,13 +106,24 @@ export default function SemanticMarkers({
                 lnMin += 360;
                 recentered = true;
               }
+
+              // Is the new position inside the "viewport"?
+              // (strictly this is the un-greyed-out region in the middle when zoomed well out)
+              const inBounds =
+                lng <= bounds.northEast.lng &&
+                lng >= bounds.southWest.lng &&
+                lat <= bounds.northEast.lat &&
+                lat >= bounds.southWest.lat;
+
               return recentered
                 ? cloneElement(marker, {
                     position: { lat, lng },
                     bounds: {
                       southWest: { lat: ltMin, lng: lnMin },
                       northEast: { lat: ltMax, lng: lnMax },
                     },
+                    // to prevent "fly-bys" (see #628) disable animation for out-of-bounds markers
+                    ...(inBounds ? {} : { duration: -1 }),
                   })
                 : marker;
             })
@@ -126,27 +137,23 @@ export default function SemanticMarkers({
       ) {
         // get the position-modified markers from `animationFunction`
         // see geohash.tsx for example
-        const animationValues = animation.animationFunction({
-          prevMarkers: prevRecenteredMarkers,
-          markers: recenteredMarkers,
-        });
+        const { markers: oldAndNewRepositionedMarkers } =
+          animation.animationFunction({
+            prevMarkers: prevRecenteredMarkers,
+            markers: recenteredMarkers,
+          });
         // set them as current
         // any marker that already existed will move to the modified position
         if (
           !isEqual(
-            animationValues.markers.map(({ props }) => props),
+            oldAndNewRepositionedMarkers.map(({ props }) => props),
             consolidatedMarkers.map(({ props }) => props)
           )
         )
-          setConsolidatedMarkers(animationValues.markers);
-        // then set a timer to remove the old markers when zooming out
-        // or if zooming in, switch to just the new markers straight away
-        // (their starting position was set by `animationFunction`)
-        // It's complicated but it works!
-        timeoutVariable = enqueueZoom(
-          animationValues.zoomType,
-          recenteredMarkers
-        );
+          setConsolidatedMarkers(oldAndNewRepositionedMarkers);
+
+        // we used to set a timer to remove the old markers when zooming out
+        // but now we just let the next render cycle do it.
       } else {
         /** First render of markers **/
         if (
@@ -171,27 +178,6 @@ export default function SemanticMarkers({
       )
         setPrevRecenteredMarkers(recenteredMarkers);
     }
-
-    function enqueueZoom(
-      zoomType: string | null,
-      nextMarkers: ReactElement<BoundsDriftMarkerProps>[]
-    ) {
-      /** If we are zooming in then reset the marker elements. When initially rendered
-       * the new markers will start at the matching existing marker's location and here we will
-       * reset marker elements so they will animated to their final position
-       **/
-      if (zoomType === 'in') {
-        setConsolidatedMarkers(nextMarkers);
-      } else if (zoomType === 'out') {
-        /** If we are zooming out then remove the old markers after they finish animating. **/
-        return window.setTimeout(
-          () => {
-            setConsolidatedMarkers(nextMarkers);
-          },
-          animation ? animation.duration : 0
-        );
-      }
-    }
   }, [
     animation,
     map,

packages/libs/components/src/map/animation_functions/geohash.tsx

@@ -11,8 +11,8 @@ export default function geohashAnimation({
   prevMarkers,
   markers,
 }: geoHashAnimation) {
-  const prevGeoHash = prevMarkers[0].key as string;
-  const currentGeohash = markers[0].key as string;
+  const prevGeoHash = prevMarkers[0].props.id as string;
+  const currentGeohash = markers[0].props.id as string;
   let zoomType, consolidatedMarkers;
 
   /** Zoom Out - Move existing markers to new position
@@ -22,9 +22,9 @@ export default function geohashAnimation({
   if (prevGeoHash.length > currentGeohash.length) {
     zoomType = 'out';
     const hashDif = prevGeoHash.length - currentGeohash.length;
-    // Get a new array of existing markers with new position property
+    // Get array of old markers with new positions
     const cloneArray = updateMarkers(prevMarkers, markers, hashDif);
-    // Combine the new and existing markers
+    // Combine the new and old markers
     consolidatedMarkers = [...markers, ...cloneArray];
   } else if (prevGeoHash.length < currentGeohash.length) {
     /** Zoom In - New markers start at old position
@@ -33,10 +33,12 @@ export default function geohashAnimation({
      **/
     zoomType = 'in';
     const hashDif = currentGeohash.length - prevGeoHash.length;
-    // Get a new array of new markers with existing position property
-    // Set final render markers to the cloneArray which holds the new markers with
-    // their new starting location
-    consolidatedMarkers = updateMarkers(markers, prevMarkers, hashDif);
+    // Get array of new markers with old positions
+    const cloneArray = updateMarkers(markers, prevMarkers, hashDif);
+    // put them first - ideally underneath the old markers
+    // (though it seems impossible to get this to work)
+    // ((see also updateMarkers zIndexOffset failed attempt))
+    consolidatedMarkers = [...cloneArray, ...prevMarkers];
   } else {
     /** No difference in geohashes - Render markers as they are **/
     zoomType = null;

packages/libs/components/src/map/animation_functions/updateMarkers.tsx

@@ -8,12 +8,12 @@ export default function updateMarkers(
 ) {
   return toChangeMarkers.map((markerObj) => {
     // Calculate the matching geohash
-    const sourceKey = markerObj.key as string;
+    const sourceKey = markerObj.props.id as string;
     const sourceHash = sourceKey.slice(0, -hashDif);
 
     // Find the object with the matching geohash
     const matchingMarkers = sourceMarkers.filter((obj) => {
-      return obj.key === sourceHash;
+      return obj.props.id === sourceHash;
     });
 
     // Clone marker element with new position
@@ -22,6 +22,10 @@ export default function updateMarkers(
       // Clone marker element with new position
       markerCloneProps = {
         position: matchingMarkers[0].props.position,
+        icon: {
+          ...markerObj.props.icon,
+          className: 'bottom-marker', // doesn't seem to work :-(
+        },
       };
     }
 

packages/libs/components/src/map/styles/map-styles.css

@@ -71,6 +71,10 @@
   z-index: 1;
 }
 
+.bottom-marker {
+  z-index: -1000;
+}
+
 /* Replace leaflet styles*/
 
 /* general typography */