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 mapillary infinite loop and improve ui #2455

Merged
merged 11 commits into from
Jan 8, 2025
97 changes: 38 additions & 59 deletions src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,23 @@
const [imageMarkers, setImageMarkers] = useState([])
const map = useMap()

const { images, markerColor = colors["blue-leaflet"], imageAlt, imageClicked, icon, mrLayerId, mrLayerLabel, style } = props
const { images, markerColor = colors["blue-leaflet"], imageClicked, icon, mrLayerId, mrLayerLabel, style } = props

useEffect(() => {
try {
if (!map.getPane('mapillaryPopups')) {
map.createPane('mapillaryPopups')
map.getPane('mapillaryPopups').style.zIndex = 700
}
if (!map.getPane('mapillaryPopups')) {
map.createPane('mapillaryPopups').style.zIndex = 700
}

setImageMarkers(
buildImageMarkers(
images,
icon ? icon : circleIcon(markerColor),
markerColor,
imageClicked,
mrLayerId,
mrLayerLabel
)
setImageMarkers(
buildImageMarkers(
images,
icon || circleIcon(markerColor),
markerColor,
imageClicked,
mrLayerId,
mrLayerLabel
)
} catch (error) {
console.error('Error creating map markers:', error)
}
)
}, [images, markerColor, imageClicked, icon, mrLayerId, mrLayerLabel, map])

return (
Expand All @@ -66,125 +61,109 @@
images: PropTypes.arrayOf(PropTypes.shape({
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,
}

const MapillaryViewer = ({ initialImageKey }) => {
const containerRef = useRef()

Check warning on line 70 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L70

Added line #L70 was not covered by tests

useEffect(() => {
if (!initialImageKey) {
console.error('Initial image key is null or undefined')
return
}
const accessToken = getAccessToken();

Check warning on line 73 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L72-L73

Added lines #L72 - L73 were not covered by tests

let viewer
try {
const accessToken = getAccessToken();
if (!accessToken) {
console.error('Access token is not available');
return;
}
if (!initialImageKey || !accessToken) return

Check warning on line 75 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L75

Added line #L75 was not covered by tests

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)
}
const viewer = imageCache.has(initialImageKey) ? imageCache.get(initialImageKey) : new Viewer({
accessToken,
container: containerRef.current,
imageId: initialImageKey,
});
imageCache.set(initialImageKey, viewer);

Check warning on line 82 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L77-L82

Added lines #L77 - L82 were not covered by tests

return () => {
try {
if (viewer) {
viewer.remove()
}
} catch (error) {
console.error('Error removing Mapillary viewer:', error)
}
}
return () => viewer && viewer.remove();
}, [initialImageKey])

Check warning on line 85 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L84-L85

Added lines #L84 - L85 were not covered by tests

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>

Check warning on line 89 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L87-L89

Added lines #L87 - L89 were not covered by tests
</div>
)
}

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 []
}

Check warning on line 104 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L103-L104

Added lines #L103 - L104 were not covered by tests

return _map(images, imageInfo => {
if (!imageInfo || (!imageInfo.position?.lat || !imageInfo.position?.lon) && (!imageInfo.lat || !imageInfo.lon)) {
const imageLatLon = getLatLng(imageInfo);
if (!imageInfo || !imageLatLon) {
console.error(`Invalid position for image key: ${imageInfo?.key}`, imageInfo)
return null
}

Check warning on line 111 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L109-L111

Added lines #L109 - L111 were not covered by tests

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
}

Check warning on line 121 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L118-L121

Added lines #L118 - L121 were not covered by tests

const [lat, lon] = imageLatLon;

Check warning on line 123 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L123

Added line #L123 was not covered by tests

return (
<Marker
key={imageInfo.key}
mrLayerId={layerId}
mrLayerLabel={layerLabel}
position={[imageInfo.position?.lat || imageInfo.lat, imageInfo.position?.lon || imageInfo.lon]}
position={[lat, lon]}
icon={icon}

Check warning on line 131 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L125-L131

Added lines #L125 - L131 were not covered by tests
>
<Popup pane="mapillaryPopups" maxWidth="351px" offset={[0, 5]}>
<div style={{ width: 351, marginTop: 5 }}>
<MapillaryViewer initialImageKey={imageInfo.key} />

Check warning on line 135 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L133-L135

Added lines #L133 - L135 were not covered by tests
</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
}}

Check warning on line 157 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L137-L157

Added lines #L137 - L157 were not covered by tests
>
<FormattedMessage {...messages.openView} />

Check warning on line 159 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L159

Added line #L159 was not covered by tests
</div>
</Popup>
</Marker>
)
}).filter(Boolean)
} catch (error) {
console.error('Error building image markers:', error)

Check warning on line 166 in src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/EnhancedMap/ImageMarkerLayer/ImageMarkerLayer.jsx#L166

Added line #L166 was not covered by tests
return []
}
}
Expand Down
67 changes: 32 additions & 35 deletions src/components/MapillaryViewer/MapillaryViewer.jsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -11,49 +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;
};
}, []);

componentDidUpdate(prevProps) {
if (prevProps.initialImageKey !== this.props.initialImageKey) {
this.viewer.setImageId(this.props.initialImageKey)
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;
Loading