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,31 @@ const colors = resolveConfig(tailwindConfig).theme.colors
  */
 const ImageMarkerLayer = props => {
   const [imageMarkers, setImageMarkers] = useState([])
+  const map = useMap()
+
+  const { images, markerColor = colors["blue-leaflet"], imageAlt, imageClicked, icon, mrLayerId, mrLayerLabel, style } = props
 
-  const {images, markerColor, imageAlt, imageClicked, icon, mrLayerId, mrLayerLabel, style} = props
   useEffect(() => {
-    setImageMarkers(
-      buildImageMarkers(
-        images,
-        icon ? icon : circleIcon(markerColor),
-        imageClicked,
-        imageAlt,
-        mrLayerId,
-        mrLayerLabel
+    try {
+      if (!map.getPane('mapillaryPopups')) {
+        map.createPane('mapillaryPopups')
+        map.getPane('mapillaryPopups').style.zIndex = 700
+      }
+
+      setImageMarkers(
+        buildImageMarkers(
+          images,
+          icon ? icon : circleIcon(markerColor),
+          markerColor,
+          imageClicked,
+          mrLayerId,
+          mrLayerLabel
+        )
       )
-    )
-  }, [images, markerColor, imageAlt, imageClicked, icon, mrLayerId, mrLayerLabel])
+    } catch (error) {
+      console.error('Error creating map markers:', error)
+    }
+  }, [images, markerColor, imageClicked, icon, mrLayerId, mrLayerLabel, map])
 
   return (
     <LayerGroup style={style}>
@@ -50,85 +64,129 @@ const ImageMarkerLayer = props => {
 ImageMarkerLayer.propTypes = {
   layerKey: PropTypes.string,
   images: PropTypes.arrayOf(PropTypes.shape({
-    key: PropTypes.string,
-    url: PropTypes.string,
-    position: PropTypes.object,
-  })),
-  imageClicked: PropTypes.func,
+    key: PropTypes.string.isRequired,
+    url: PropTypes.string.isRequired,
+    position: PropTypes.shape({
+      lat: PropTypes.number.isRequired,
+      lon: PropTypes.number.isRequired,
+    }).isRequired,
+  })).isRequired,
+  imageClicked: PropTypes.func.isRequired,
   imageAlt: PropTypes.string,
   buildIcon: PropTypes.func,
   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(() => {
+    if (!initialImageKey) {
+      console.error('Initial image key is null or undefined')
+      return
+    }
+
+    let viewer
+    try {
+      const accessToken = getAccessToken();
+      if (!accessToken) {
+        console.error('Access token is not available');
+        return;
+      }
 
-  componentWillUnmount() {
-    if (this.viewer) {
-      this.viewer.remove();
+      if (imageCache.has(initialImageKey)) {
+        viewer = imageCache.get(initialImageKey);
+      } else {
+        viewer = new Viewer({
+          accessToken: accessToken,
+          container: containerRef.current,
+          imageId: initialImageKey,
+        });
+        imageCache.set(initialImageKey, viewer);
+      }
+    } catch (error) {
+      console.error('Error initializing Mapillary viewer:', error)
     }
-  }
 
-  shouldComponentUpdate(nextProps) {
-    return nextProps.initialImageKey !== this.props.initialImageKey
-  }
+    return () => {
+      try {
+        if (viewer) {
+          viewer.remove()
+        }
+      } catch (error) {
+        console.error('Error removing Mapillary viewer:', error)
+      }
+    }
+  }, [initialImageKey])
 
-  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 (
+    <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 buildImageMarkers = (images, icon, markerColor, imageClicked, layerId, layerLabel) => {
+  try {
+    if (!images || images.length === 0) {
+      return []
+    }
+
+    return _map(images, imageInfo => {
+      if (!imageInfo || (!imageInfo.position?.lat || !imageInfo.position?.lon) && (!imageInfo.lat || !imageInfo.lon)) {
+        console.error(`Invalid position for image key: ${imageInfo?.key}`, imageInfo)
+        return null
+      }
+
+      if (!imageInfo.url) {
+        console.error(`Invalid URL for image key: ${imageInfo.key}`, imageInfo)
+        return null
+      }
+
+      return (
+        <Marker
+          key={imageInfo.key}
+          mrLayerId={layerId}
+          mrLayerLabel={layerLabel}
+          position={[imageInfo.position?.lat || imageInfo.lat, imageInfo.position?.lon || imageInfo.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.js

@@ -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(() => {
+    jest.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

@@ -34,6 +34,12 @@ export default class MapillaryViewer extends Component {
     return nextProps.initialImageKey !== this.props.initialImageKey
   }
 
+  componentDidUpdate(prevProps) {
+    if (prevProps.initialImageKey !== this.props.initialImageKey) {
+      this.viewer.setImageId(this.props.initialImageKey)
+    }
+  }
+
   render() {
     return (
       <External>

src/components/MapillaryViewer/MapillaryViewer.test.js

@@ -0,0 +1,27 @@
+import React from 'react'
+import { render } from '@testing-library/react'
+import MapillaryViewer from './MapillaryViewer'
+
+jest.mock('mapillary-js', () => { 
+  return {
+    Viewer: jest.fn().mockImplementation(() => ({
+      setImageId: jest.fn(),
+      remove: jest.fn(),
+    })),
+  }
+})
+
+describe('MapillaryViewer Component', () => {
+  beforeAll(() => {
+    jest.spyOn(console, 'error').mockImplementation(() => {})
+  })
+
+  afterAll(() => {
+    console.error.mockRestore()
+  })
+
+  it('renders without crashing', () => {
+    const { container } = render(<MapillaryViewer initialImageKey="abc123" />)
+    expect(container).toBeInTheDocument()
+  })
+})