fix mapillary infinite loop and improve ui

index.html

@@ -30,7 +30,7 @@
           integrity="sha512-07I2e+7D8p6he1SIM+1twR5TIrhUQn9+I6yjqD53JQjFiMf8EtC93ty0/5vJTZGF8aAocvHYNEDJajGdNx1IsQ=="
           crossorigin=""/>
     <link
-      href="https://unpkg.com/mapillary-js@4.0.0/dist/mapillary.css"
+      href="https://unpkg.com/mapillary-js@4.1.2/dist/mapillary.css"
       rel="stylesheet"
     />
 

package.json

@@ -44,7 +44,7 @@
     "leaflet-vectoricon": "https://github.com/CollinBeczak/Leaflet.VectorIcon",
     "leaflet.markercluster": "^1.5.3",
     "lodash": "4.17.21",
-    "mapillary-js": "^4.1.0",
+    "mapillary-js": "^4.1.2",
     "normalize.css": "^7.0.0",
     "normalizr": "^3.6.0",
     "piwik-react-router": "^0.12.1",

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

@@ -1,14 +1,17 @@
-import { createRef, useState, useEffect, Component } from 'react'
+import React, { useState, useEffect, useRef } from 'react'
 import PropTypes from 'prop-types'
-import { LayerGroup, Marker, Popup } from 'react-leaflet'
+import { LayerGroup, Marker, Popup, useMap } from 'react-leaflet'
 import L from 'leaflet'
 import _map from 'lodash/map'
 import { Viewer } from 'mapillary-js'
 import resolveConfig from 'tailwindcss/resolveConfig'
 import { getAccessToken } from '../../../services/Mapillary/Mapillary'
 import tailwindConfig from '../../../tailwind.config.js'
+import { FormattedMessage } from 'react-intl'
+import messages from './Messages.js'
 
 const colors = resolveConfig(tailwindConfig).theme.colors
+const imageCache = new Map();
 
 /**
  * ImageMarkerLayer renders a layer of positioned image markers that, on hover,
@@ -25,20 +28,26 @@
  */
 const ImageMarkerLayer = props => {
   const [imageMarkers, setImageMarkers] = useState([])
+  const map = useMap()
+
+  const { images, markerColor = colors["blue-leaflet"], imageClicked, icon, mrLayerId, mrLayerLabel, style } = props
 
-  const {images, markerColor, imageAlt, imageClicked, icon, mrLayerId, mrLayerLabel, style} = props
   useEffect(() => {
+    if (!map.getPane('mapillaryPopups')) {
+      map.createPane('mapillaryPopups').style.zIndex = 700
+    }
+
     setImageMarkers(
       buildImageMarkers(
         images,
-        icon ? icon : circleIcon(markerColor),
+        icon || circleIcon(markerColor),
+        markerColor,
         imageClicked,
-        imageAlt,
         mrLayerId,
         mrLayerLabel
       )
     )
-  }, [images, markerColor, imageAlt, imageClicked, icon, mrLayerId, mrLayerLabel])
+  }, [images, markerColor, imageClicked, icon, mrLayerId, mrLayerLabel, map])
 
   return (
     <LayerGroup style={style}>
@@ -50,85 +59,113 @@
 ImageMarkerLayer.propTypes = {
   layerKey: PropTypes.string,
   images: PropTypes.arrayOf(PropTypes.shape({
-    key: PropTypes.string,
-    url: PropTypes.string,
-    position: PropTypes.object,
-  })),
-  imageClicked: PropTypes.func,
-  imageAlt: PropTypes.string,
-  buildIcon: PropTypes.func,
+    key: PropTypes.string.isRequired,
+    url: PropTypes.string.isRequired,
+  })).isRequired,
+  imageClicked: PropTypes.func.isRequired,
   markerColor: PropTypes.string,
 }
 
-class MapillaryViewer extends Component {
-  containerRef = createRef();
+const MapillaryViewer = ({ initialImageKey }) => {
+  const containerRef = useRef()
 
-  componentDidMount() {
-    this.viewer = new Viewer({
-      accessToken: getAccessToken(),
-      container: this.containerRef.current,
-      imageId: this.props.initialImageKey,
-      component: { cover: false },
-    })
-  }
+  useEffect(() => {
+    const accessToken = getAccessToken();
 
-  componentWillUnmount() {
-    if (this.viewer) {
-      this.viewer.remove();
-    }
-  }
+    if (!initialImageKey || !accessToken) return
 
-  shouldComponentUpdate(nextProps) {
-    return nextProps.initialImageKey !== this.props.initialImageKey
-  }
+    const viewer = imageCache.has(initialImageKey) ? imageCache.get(initialImageKey) : new Viewer({
+      accessToken,
+      container: containerRef.current,
+      imageId: initialImageKey,
+    });
+    imageCache.set(initialImageKey, viewer);
 
-  render() {
-    return (
-      <div className="mr-p-2 mr-pt-4 mr-relative">
-        <div ref={this.containerRef} id="mapillary-viewer" style={{ width: 335, height: 263 }}></div>
-      </div>
-    )
-  }
+    return () => viewer && viewer.remove();
+  }, [initialImageKey])
+
+  return (
+    <div className="mr-p-2 mr-pt-4 mr-relative">
+      <div ref={containerRef} id="mapillary-viewer" className="mr-w-full mr-h-64"></div>
+    </div>
+  )
 }
 
-const buildImageMarkers = (images, icon, imageClicked, imageAlt, layerId, layerLabel) => {
-  if (!images || images.length === 0) {
+const getLatLng = (imageInfo) => {
+  const lat = imageInfo.position?.lat !== undefined ? imageInfo.position.lat : imageInfo.lat;
+  const lon = imageInfo.position?.lon !== undefined ? imageInfo.position.lon : imageInfo.lon;
+  return (lat !== undefined && lon !== undefined) ? [lat, lon] : null;
+}
+
+const buildImageMarkers = (images, icon, markerColor, imageClicked, layerId, layerLabel) => {
+  try {
+    if (!images || images.length === 0) {
+      return []
+    }
+
+    return _map(images, imageInfo => {
+      const imageLatLon = getLatLng(imageInfo);
+      if (!imageInfo || !imageLatLon) {
+        console.error(`Invalid position for image key: ${imageInfo?.key}`, imageInfo)
+        return null
+      }
+
+      if (imageLatLon.lat === undefined || imageLatLon.lon === undefined) {
+        console.error(`Invalid coordinates for image key: ${imageInfo?.key}`, imageInfo)
+        return null
+      }
+
+      if (!imageInfo.url) {
+        console.error(`Invalid URL for image key: ${imageInfo.key}`, imageInfo)
+        return null
+      }
+
+      const [lat, lon] = imageLatLon;
+
+      return (
+        <Marker
+          key={imageInfo.key}
+          mrLayerId={layerId}
+          mrLayerLabel={layerLabel}
+          position={[lat, lon]}
+          icon={icon}
+        >
+          <Popup pane="mapillaryPopups" maxWidth="351px" offset={[0, 5]}>
+            <div style={{ width: 351, marginTop: 5 }}>
+              <MapillaryViewer initialImageKey={imageInfo.key} />
+            </div>
+            <div
+              style={{
+                width: '100%',
+                textAlign: 'center',
+                color: markerColor,
+                cursor: 'pointer',
+                fontSize: '1.25rem',
+                padding: '8px',
+                transition: 'background-color 0.3s, color 0.3s'
+              }}
+              onClick={() => {
+                imageClicked(imageInfo.key)
+              }}
+              onMouseEnter={e => {
+                e.currentTarget.style.backgroundColor = markerColor
+                e.currentTarget.style.color = 'white'
+              }}
+              onMouseLeave={e => {
+                e.currentTarget.style.backgroundColor = 'transparent'
+                e.currentTarget.style.color = markerColor
+              }}
+            >
+              <FormattedMessage {...messages.openView} />
+            </div>
+          </Popup>
+        </Marker>
+      )
+    }).filter(Boolean)
+  } catch (error) {
+    console.error('Error building image markers:', error)
     return []
   }
-
-  return _map(images, imageInfo => {
-    return (
-      <Marker
-        key={imageInfo.key}
-        mrLayerId={layerId}
-        mrLayerLabel={layerLabel}
-        position={[imageInfo.lat, imageInfo.lon]}
-        icon={icon}
-        onMouseover={({target}) => target.openPopup()}
-        eventHandlers={{
-          click: () => {
-            imageClicked ? imageClicked(imageInfo.key) : null
-          },
-        }}
-      >
-        <Popup maxWidth="351px" offset={ [0.5, -5]}>
-          <div style={{ width: 351, marginTop: 20 }}>
-            <MapillaryViewer
-              key={Date.now()}
-              initialImageKey={imageInfo.key}
-              onClose={() => null}
-            />
-          </div>
-          <div
-            className="mr-w-full mr-text-center mr-text-green mr-cursor-pointer mr-text-lg"
-            onClick={() => imageClicked(imageInfo.key)}
-          >
-            Enlarge
-          </div>
-        </Popup>
-      </Marker>
-    )
-  })
 }
 
 const circleIcon = (color = colors["blue-leaflet"]) => {

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.test.jsx

@@ -0,0 +1,28 @@
+import React from 'react'
+import { render } from '@testing-library/react'
+import { MapContainer } from 'react-leaflet'
+import ImageMarkerLayer from './ImageMarkerLayer'
+
+describe('ImageMarkerLayer Component', () => {
+  beforeAll(() => {
+    vitest.spyOn(console, 'error').mockImplementation(() => {})
+  })
+
+  afterAll(() => {
+    console.error.mockRestore()
+  })
+
+  const mockImages = [
+    { key: 'image1', url: 'http://example.com/image1.jpg', position: { lat: 1, lon: 1 } },
+    { key: 'image2', url: 'http://example.com/image2.jpg', position: { lat: 2, lon: 2 } },
+  ]
+
+  it('renders without crashing', () => {
+    const { container } = render(
+      <MapContainer>
+        <ImageMarkerLayer images={mockImages} />
+      </MapContainer>
+    )
+    expect(container).toBeInTheDocument()
+  })
+})

src/components/EnhancedMap/ImageMarkerLayer/Messages.js

@@ -0,0 +1,12 @@
+import { defineMessages } from 'react-intl'
+
+/**
+ * Internationalized messages for use with FitWorldControl
+ */
+export default defineMessages({
+  openView: {
+    id: "ImafeMarkerLayer.view",
+    defaultMessage: "Open 3D View"
+  },
+})
+

src/components/EnhancedMap/LayerToggle/LayerToggle.jsx

@@ -27,8 +27,10 @@ import messages from './Messages'
  * @author [Neil Rotstan](https://github.com/nrotstan)
  */
 export class LayerToggle extends Component {
-  overlayVisible = layerId => this.props.visibleOverlays.indexOf(layerId) !== -1
-
+  overlayVisible = layerId => {
+   const visibleOverlays = this.props.visibleOverlays || []
+    return visibleOverlays.indexOf(layerId) !== -1
+  }
   toggleOverlay = layerId => {
     this.overlayVisible(layerId) ? this.props.removeVisibleOverlay(layerId) :
                                    this.props.addVisibleOverlay(layerId)
@@ -182,7 +184,7 @@ LayerToggle.propTypes = {
   /** Invoked when the user chooses a new layer source */
   changeLayer: PropTypes.func.isRequired,
   /** Array of overlay layers currently visible */
-  visibleOverlays: PropTypes.array.isRequired,
+  visibleOverlays: PropTypes.array,
   /** Invoked to add an overlay layer to the visible overlays */
   addVisibleOverlay: PropTypes.func.isRequired,
   /** Invoked to remove an overlay layer from the visible overlays */

src/components/MapillaryViewer/MapillaryViewer.jsx

@@ -1,4 +1,4 @@
-import { createRef, Component } from 'react'
+import { useRef, useEffect } from 'react'
 import PropTypes from 'prop-types'
 import { Viewer } from 'mapillary-js';
 import { getAccessToken } from '../../services/Mapillary/Mapillary'
@@ -11,43 +11,46 @@ import Modal from '../Modal/Modal'
  *
  * @author [Neil Rotstan](https://github.com/nrotstan)
  */
-export default class MapillaryViewer extends Component {
-  containerRef = createRef();
-
+const MapillaryViewer = ({ initialImageKey, onClose }) => {
+  const containerRef = useRef(null);
+  const viewerRef = useRef(null);
 
-  componentDidMount() {
-    this.viewer = new Viewer({
-      accessToken: getAccessToken(),
-      container: this.containerRef.current,
-      imageId: this.props.initialImageKey,
-      component: { cover: false },
-    })
-  }
-
-  componentWillUnmount() {
-    if (this.viewer) {
-      this.viewer.remove();
+  useEffect(() => {
+    if (!viewerRef.current) {
+      viewerRef.current = new Viewer({
+        accessToken: getAccessToken(),
+        container: containerRef.current,
+        imageId: initialImageKey,
+        component: { cover: false },
+      });
     }
-  }
 
-  shouldComponentUpdate(nextProps) {
-    return nextProps.initialImageKey !== this.props.initialImageKey
-  }
+    return () => {
+      // Cleanup if necessary
+      viewerRef.current = null;
+    };
+  }, []);
+
+  useEffect(() => {
+    if (viewerRef.current) {
+      viewerRef.current.setImageId(initialImageKey);
+    }
+  }, [initialImageKey]);
 
-  render() {
-    return (
-      <External>
-        <Modal isActive onClose={this.props.onClose}>
-          <div className="mr-p-2 mr-pt-4 mr-relative mr-m-auto" style={{ width: 640 }}>
-            <div ref={this.containerRef} id="mapillary-viewer" style={{ width: 640, height: 480 }}></div>
-          </div>
-        </Modal>
-      </External>
-    )
-  }
+  return (
+    <External>
+      <Modal isActive onClose={onClose}>
+        <div className="mr-p-2 mr-pt-4 mr-relative mr-m-auto" style={{ width: 640 }}>
+          <div ref={containerRef} id="mapillary-viewer" style={{ width: 640, height: 480 }}></div>
+        </div>
+      </Modal>
+    </External>
+  );
 }
 
 MapillaryViewer.propTypes = {
   initialImageKey: PropTypes.string.isRequired,
   onClose: PropTypes.func,
 }
+
+export default MapillaryViewer;