diff --git a/src/components/EnhancedMap/OSMDataLayer/OSMDataLayer.jsx b/src/components/EnhancedMap/OSMDataLayer/OSMDataLayer.jsx index 70ae5bc7c..196fc3ccf 100644 --- a/src/components/EnhancedMap/OSMDataLayer/OSMDataLayer.jsx +++ b/src/components/EnhancedMap/OSMDataLayer/OSMDataLayer.jsx @@ -83,6 +83,19 @@ const generateLayer = (props, map, _leaflet) => { pane: props.leaflet?.pane, }); + // Clear existing layers to ensure proper ordering + layerGroup.clearLayers(); + + // Get all features and sort them by type to ensure consistent ordering + const features = layerGroup.buildFeatures(props.xmlData); + const sortedFeatures = features.sort((a, b) => { + const typeOrder = { way: 1, area: 2, node: 3, changeset: 4 }; + return typeOrder[a.type] - typeOrder[b.type]; + }); + + // Add features in the sorted order + layerGroup.addData(sortedFeatures); + layerGroup.eachLayer((layer) => { layer.options.mrLayerLabel = props.intl.formatMessage(layerMessages.showOSMDataLabel); layer.options.fill = false; @@ -166,7 +179,8 @@ const OSMDataLayerComponent = createPathComponent( if (!_isEqual(prevProps.showOSMElements, props.showOSMElements)) { instance.clearLayers(); const newLayers = generateLayer(props, instance); - newLayers.eachLayer((layer) => instance.addLayer(layer)); + // Transfer all layers at once to maintain ordering + instance.addLayer(newLayers); } }, ); diff --git a/src/components/HOCs/WithCurrentTask/WithCurrentTask.jsx b/src/components/HOCs/WithCurrentTask/WithCurrentTask.jsx index 19f5479fa..ea0b0ad0d 100644 --- a/src/components/HOCs/WithCurrentTask/WithCurrentTask.jsx +++ b/src/components/HOCs/WithCurrentTask/WithCurrentTask.jsx @@ -324,6 +324,7 @@ export const mapDispatchToProps = (dispatch, ownProps) => { fetchOSMData: (bbox) => { return fetchOSMData(bbox).catch((error) => { dispatch(addError(error)); + throw error; }); }, diff --git a/src/components/TaskPane/TaskMap/Messages.js b/src/components/TaskPane/TaskMap/Messages.js index a59cf0298..f0839c355 100644 --- a/src/components/TaskPane/TaskMap/Messages.js +++ b/src/components/TaskPane/TaskMap/Messages.js @@ -8,4 +8,21 @@ export default defineMessages({ id: "Map.layerSelectionList.header", defaultMessage: "Select Desired Feature", }, + osmDataAreaTooLarge: { + id: "Task.osmData.areaTooLarge", + defaultMessage: + "The selected area is too large to load OSM data. Please zoom in further to view OSM features.", + }, + osmDataTooLarge: { + id: "Task.map.osmData.tooLarge", + defaultMessage: "OSM Data Area Too Large", + }, + zoomInForOSMData: { + id: "Task.map.osmData.zoomInRequired", + defaultMessage: "Please zoom in closer to view OSM data for this area", + }, + osmDataError: { + id: "Task.map.osmData.error", + defaultMessage: "Error Loading OSM Data", + }, }); diff --git a/src/components/TaskPane/TaskMap/TaskMap.jsx b/src/components/TaskPane/TaskMap/TaskMap.jsx index 28ed19941..4f1cee9c4 100644 --- a/src/components/TaskPane/TaskMap/TaskMap.jsx +++ b/src/components/TaskPane/TaskMap/TaskMap.jsx @@ -79,7 +79,11 @@ export const TaskMapContent = (props) => { const [showTaskFeatures, setShowTaskFeatures] = useState(true); const [osmData, setOsmData] = useState(null); const [showOSMData, setShowOSMData] = useState(false); - const [showOSMElements, setShowOSMElements] = useState({ nodes: true, ways: true, areas: true }); + const [showOSMElements, setShowOSMElements] = useState({ + nodes: true, + ways: true, + areas: true, + }); const [osmDataLoading, setOsmDataLoading] = useState(false); const [mapillaryViewerImage, setMapillaryViewerImage] = useState(null); const [openStreetCamViewerImage, setOpenStreetCamViewerImage] = useState(null); @@ -264,43 +268,62 @@ export const TaskMapContent = (props) => { setShowTaskFeatures((prevState) => !prevState); }; - const fetchOSMData = () => { - setOsmDataLoading(true); - props.fetchOSMData(map.getBounds().toBBoxString()).then((xmlData) => { - // Indicate the map should skip fitting to bounds as the OSM data could - // extend beyond the current view and we don't want the map to zoom out - setOsmData(xmlData); + const fetchOSMData = async () => { + try { + if (showOSMData) { + const bounds = map.getBounds()?.toBBoxString(); + if (!bounds) { + throw new Error("Invalid map bounds"); + } + + const xmlData = await props.fetchOSMData(bounds); + setOsmData(xmlData); + setShowOSMData(true); + } else { + setOsmData(null); + setShowOSMData(false); + } + } catch (error) { + console.error("Error handling OSM data:", error); + setOsmData(null); + setShowOSMData(false); + } finally { setOsmDataLoading(false); - }); + } }; /** * Invoked by LayerToggle when the user wishes to toggle visibility of * OSM data on or off. */ - const toggleOSMDataVisibility = () => { + const toggleOSMDataVisibility = async () => { + // Prevent multiple requests while loading if (osmDataLoading) { return; } const loadOSMData = !showOSMData; setOsmDataLoading(true); - setShowOSMData(loadOSMData); - - if (loadOSMData) { - props - .fetchOSMData(map.getBounds().toBBoxString()) - .then((xmlData) => { - setOsmData(xmlData); - }) - .catch((error) => { - console.error("Error fetching OSM data:", error); - }) - .finally(() => { - setOsmDataLoading(false); - }); - } else { + + try { + if (loadOSMData) { + const bounds = map.getBounds()?.toBBoxString(); + if (!bounds) { + throw new Error("Invalid map bounds"); + } + + const xmlData = await props.fetchOSMData(bounds); + setOsmData(xmlData); + setShowOSMData(true); + } else { + setOsmData(null); + setShowOSMData(false); + } + } catch (error) { + console.error("Error handling OSM data:", error); setOsmData(null); + setShowOSMData(false); + } finally { setOsmDataLoading(false); } }; @@ -426,7 +449,7 @@ export const TaskMapContent = (props) => { if (showOSMData && !osmData) { fetchOSMData(); } - }, [props, osmData]); + }, [showOSMData, osmData]); useEffect(() => { if (features.length !== 0) { @@ -625,7 +648,11 @@ export const TaskMapContent = (props) => { } return ( -