From 84840b90e37716df78df54faf1d3b9fd81610a07 Mon Sep 17 00:00:00 2001 From: Viktor Andersson <30777521+VIKTORVAV99@users.noreply.github.com> Date: Tue, 23 Jul 2024 19:57:23 +0200 Subject: [PATCH 1/2] wrap map handlers in useCallbacks --- web/src/features/map/Map.tsx | 166 +++++++++++++++++++---------------- 1 file changed, 88 insertions(+), 78 deletions(-) diff --git a/web/src/features/map/Map.tsx b/web/src/features/map/Map.tsx index 9dbf587c08..6f2922db7f 100644 --- a/web/src/features/map/Map.tsx +++ b/web/src/features/map/Map.tsx @@ -230,86 +230,93 @@ export default function MapPage({ onMapLoad }: MapPageProps): ReactElement { setLeftPanelOpen, ]); - const onClick = (event: maplibregl.MapLayerMouseEvent) => { - if (!map || !event.features) { - return; - } - const feature = event.features[0]; - - // Remove state from old feature if we are no longer hovering anything, - // or if we are hovering a different feature than the previous one - if (selectedZoneId && (!feature || selectedZoneId !== feature.id)) { - map.setFeatureState( - { source: ZONE_SOURCE, id: selectedZoneId }, - { selected: false } - ); - } - - if (hoveredZone && (!feature || hoveredZone.featureId !== selectedZoneId)) { - map.setFeatureState( - { source: ZONE_SOURCE, id: hoveredZone.featureId }, - { hover: false } - ); - } - setHoveredZone(null); - if (feature?.properties) { - const zoneId = feature.properties.zoneId; - navigate(createToWithState(`/zone/${zoneId}`)); - } else { - navigate(createToWithState('/map')); - } - }; + const onClick = useCallback( + (event: maplibregl.MapLayerMouseEvent) => { + if (!map || !event.features) { + return; + } + const feature = event.features[0]; - // TODO: Consider if we need to ignore zone hovering if the map is dragging - const onMouseMove = (event: maplibregl.MapLayerMouseEvent) => { - if (!map || !event.features) { - return; - } - const feature = event.features[0]; - const isHoveringAZone = feature?.id !== undefined; - const isHoveringANewZone = isHoveringAZone && hoveredZone?.featureId !== feature?.id; + // Remove state from old feature if we are no longer hovering anything, + // or if we are hovering a different feature than the previous one + if (selectedZoneId && (!feature || selectedZoneId !== feature.id)) { + map.setFeatureState( + { source: ZONE_SOURCE, id: selectedZoneId }, + { selected: false } + ); + } - // Reset currently hovered zone if we are no longer hovering anything - if (!isHoveringAZone && hoveredZone) { + if (hoveredZone && (!feature || hoveredZone.featureId !== selectedZoneId)) { + map.setFeatureState( + { source: ZONE_SOURCE, id: hoveredZone.featureId }, + { hover: false } + ); + } setHoveredZone(null); - map.setFeatureState( - { source: ZONE_SOURCE, id: hoveredZone?.featureId }, - { hover: false } - ); - } - - // Do no more if we are not hovering a zone - if (!isHoveringAZone) { - return; - } + if (feature?.properties) { + const zoneId = feature.properties.zoneId; + navigate(createToWithState(`/zone/${zoneId}`)); + } else { + navigate(createToWithState('/map')); + } + }, + [map, selectedZoneId, hoveredZone, setHoveredZone, navigate] + ); - // Update mouse position to help position the tooltip - setMousePosition({ - x: event.point.x, - y: event.point.y, - }); + // TODO: Consider if we need to ignore zone hovering if the map is dragging + const onMouseMove = useCallback( + (event: maplibregl.MapLayerMouseEvent) => { + if (!map || !event.features) { + return; + } + const feature = event.features[0]; + const isHoveringAZone = feature?.id !== undefined; + const isHoveringANewZone = + isHoveringAZone && hoveredZone?.featureId !== feature?.id; - // Update hovered zone if we are hovering a new zone - if (isHoveringANewZone) { - // Reset the old one first - if (hoveredZone) { + // Reset currently hovered zone if we are no longer hovering anything + if (!isHoveringAZone && hoveredZone) { + setHoveredZone(null); map.setFeatureState( { source: ZONE_SOURCE, id: hoveredZone?.featureId }, { hover: false } ); } - setHoveredZone({ featureId: feature.id, zoneId: feature.properties?.zoneId }); - map.setFeatureState({ source: ZONE_SOURCE, id: feature.id }, { hover: true }); - } - }; + // Do no more if we are not hovering a zone + if (!isHoveringAZone) { + return; + } + + // Update mouse position to help position the tooltip + setMousePosition({ + x: event.point.x, + y: event.point.y, + }); + + // Update hovered zone if we are hovering a new zone + if (isHoveringANewZone) { + // Reset the old one first + if (hoveredZone) { + map.setFeatureState( + { source: ZONE_SOURCE, id: hoveredZone?.featureId }, + { hover: false } + ); + } - const onMouseOut = () => { + setHoveredZone({ featureId: feature.id, zoneId: feature.properties?.zoneId }); + map.setFeatureState({ source: ZONE_SOURCE, id: feature.id }, { hover: true }); + } + }, + [map, hoveredZone, setHoveredZone, setMousePosition] + ); + + const onMouseOut = useCallback(() => { if (!map) { return; } - // Reset hovered state when mouse leaves map (e.g. cursor moving into panel) + // Reset hovered state when mouse leaves map (e.g., cursor moving into panel) if (hoveredZone?.featureId !== undefined) { map.setFeatureState( { source: ZONE_SOURCE, id: hoveredZone?.featureId }, @@ -317,30 +324,33 @@ export default function MapPage({ onMapLoad }: MapPageProps): ReactElement { ); setHoveredZone(null); } - }; + }, [map, hoveredZone, setHoveredZone]); - const onError = (event: ErrorEvent) => { - console.error(event.error); - setIsLoadingMap(false); - // TODO: Show error message to user - // TODO: Send to Sentry - // TODO: Handle the "no webgl" error gracefully - }; + const onError = useCallback( + (event: ErrorEvent) => { + console.error(event.error); + setIsLoadingMap(false); + // TODO: Show error message to user + // TODO: Send to Sentry + // TODO: Handle the "no webgl" error gracefully + }, + [setIsLoadingMap] + ); - const onLoad = () => { + const onLoad = useCallback(() => { setIsLoadingMap(false); if (onMapLoad && mapReference) { onMapLoad(mapReference.getMap()); } - }; + }, [setIsLoadingMap, onMapLoad, mapReference]); - const onMoveStart = () => { + const onMoveStart = useCallback(() => { setIsMoving(true); - }; + }, [setIsMoving]); - const onMoveEnd = () => { + const onMoveEnd = useCallback(() => { setIsMoving(false); - }; + }, [setIsMoving]); return ( Date: Tue, 23 Jul 2024 21:32:01 +0200 Subject: [PATCH 2/2] deconstruct events --- web/src/features/map/Map.tsx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/web/src/features/map/Map.tsx b/web/src/features/map/Map.tsx index 6f2922db7f..d292896eb2 100644 --- a/web/src/features/map/Map.tsx +++ b/web/src/features/map/Map.tsx @@ -231,11 +231,11 @@ export default function MapPage({ onMapLoad }: MapPageProps): ReactElement { ]); const onClick = useCallback( - (event: maplibregl.MapLayerMouseEvent) => { - if (!map || !event.features) { + ({ features }: maplibregl.MapLayerMouseEvent) => { + if (!map || !features) { return; } - const feature = event.features[0]; + const feature = features[0]; // Remove state from old feature if we are no longer hovering anything, // or if we are hovering a different feature than the previous one @@ -265,11 +265,11 @@ export default function MapPage({ onMapLoad }: MapPageProps): ReactElement { // TODO: Consider if we need to ignore zone hovering if the map is dragging const onMouseMove = useCallback( - (event: maplibregl.MapLayerMouseEvent) => { - if (!map || !event.features) { + ({ features, point }: maplibregl.MapLayerMouseEvent) => { + if (!map || !features) { return; } - const feature = event.features[0]; + const feature = features[0]; const isHoveringAZone = feature?.id !== undefined; const isHoveringANewZone = isHoveringAZone && hoveredZone?.featureId !== feature?.id; @@ -290,8 +290,8 @@ export default function MapPage({ onMapLoad }: MapPageProps): ReactElement { // Update mouse position to help position the tooltip setMousePosition({ - x: event.point.x, - y: event.point.y, + x: point.x, + y: point.y, }); // Update hovered zone if we are hovering a new zone @@ -327,8 +327,8 @@ export default function MapPage({ onMapLoad }: MapPageProps): ReactElement { }, [map, hoveredZone, setHoveredZone]); const onError = useCallback( - (event: ErrorEvent) => { - console.error(event.error); + ({ error }: ErrorEvent) => { + console.error(error); setIsLoadingMap(false); // TODO: Show error message to user // TODO: Send to Sentry