Skip to content

Commit 16becc7

Browse files
goldflaggoldflag
committed
Refactor map event handlers and improve tooltip management across multiple components
This commit enhances the handling of map events by introducing named functions for event listeners, improving readability and maintainability. Additionally, it updates tooltip management in various components to ensure consistent styling and behavior. The changes include the use of `dispose` for cleanup of event listeners, enhancing performance and preventing memory leaks. Furthermore, the sorting methods in several components have been updated to utilize `toSorted`, improving code clarity and performance.
1 parent f9776f9 commit 16becc7

21 files changed

Lines changed: 431 additions & 365 deletions

‎client/src/app/[site]/components/shared/Map/hooks/useMapInstance.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { FilterParameter } from "@rybbit/shared";
22
import Map from "ol/Map";
3+
import { unByKey as dispose } from "ol/Observable";
34
import View from "ol/View";
45
import { fromLonLat } from "ol/proj";
56
import { useEffect, useRef } from "react";
@@ -62,17 +63,18 @@ export function useMapInstance({
6263
mapInstanceRef.current = map;
6364

6465
// Handle zoom changes
65-
map.getView().on("change:resolution", () => {
66+
const handleResolutionChange = () => {
6667
const currentZoom = map.getView().getZoom() || 1;
6768
const newMapView = currentZoom >= 5 ? "subdivisions" : "countries";
6869
if (newMapView !== mapViewRef.current) {
6970
setInternalMapView(newMapView);
7071
setTooltipContent(null);
7172
}
72-
});
73+
};
74+
const resolutionChangeKey = map.getView().on("change:resolution", handleResolutionChange);
7375

7476
// Handle pointer move for hover effects
75-
map.on("pointermove", evt => {
77+
const handlePointerMove = (evt: any) => {
7678
if (evt.dragging) {
7779
return;
7880
}
@@ -112,10 +114,11 @@ export function useMapInstance({
112114
setHoveredId(null);
113115
setTooltipContent(null);
114116
}
115-
});
117+
};
118+
const pointerMoveKey = map.on("pointermove", handlePointerMove);
116119

117120
// Handle click for filtering
118-
map.on("click", evt => {
121+
const handleClick = (evt: any) => {
119122
const pixel = map.getEventPixel(evt.originalEvent);
120123
const feature = map.forEachFeatureAtPixel(pixel, feature => feature);
121124

@@ -131,9 +134,11 @@ export function useMapInstance({
131134
type: "equals",
132135
});
133136
}
134-
});
137+
};
138+
const clickKey = map.on("click", handleClick);
135139

136140
return () => {
141+
dispose([resolutionChangeKey, pointerMoveKey, clickKey]);
137142
map.setTarget(undefined);
138143
};
139144
}, []);

‎client/src/app/[site]/events/components/CustomEventsChart.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ export function CustomEventsChart({ eventLimit }: { eventLimit: number }) {
198198
lineWidth={2}
199199
sliceTooltip={({ slice }: SliceTooltipProps<EventSeries>) => {
200200
const currentTime = slice.points[0]?.data.currentTime as DateTime | undefined;
201-
const sortedPoints = [...slice.points].sort(
201+
const sortedPoints = slice.points.toSorted(
202202
(a, b) => Number(b.data.yFormatted) - Number(a.data.yFormatted)
203203
);
204204

‎client/src/app/[site]/events/components/EventTypesChart.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export function EventTypesChart() {
7979
return { series: [] as Series[], legendItems: [], maxValue: 1, totalPoints: 0 };
8080
}
8181

82-
const sortedData = [...data].sort((a, b) => {
82+
const sortedData = data.toSorted((a, b) => {
8383
const ta = DateTime.fromSQL(a.time, { zone: timezone }).toMillis();
8484
const tb = DateTime.fromSQL(b.time, { zone: timezone }).toMillis();
8585
return ta - tb;
@@ -202,7 +202,7 @@ export function EventTypesChart() {
202202
lineWidth={2}
203203
sliceTooltip={({ slice }: SliceTooltipProps<Series>) => {
204204
const currentTime = slice.points[0]?.data.currentTime as DateTime | undefined;
205-
const sortedPoints = [...slice.points].sort(
205+
const sortedPoints = slice.points.toSorted(
206206
(a, b) => Number(b.data.yFormatted) - Number(a.data.yFormatted)
207207
);
208208
const total = sortedPoints.reduce((acc, p) => acc + Number(p.data.yFormatted), 0);

‎client/src/app/[site]/globe/2d/hooks/useOpenLayersCoordinatesLayer.ts‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { FilterParameter } from "@rybbit/shared/dist/filters";
22
import round from "lodash/round";
33
import { useEffect, useRef } from "react";
44
import Map from "ol/Map";
5+
import { unByKey as dispose } from "ol/Observable";
56
import VectorLayer from "ol/layer/Vector";
67
import VectorSource from "ol/source/Vector";
78
import Feature, { FeatureLike } from "ol/Feature";
@@ -149,11 +150,11 @@ export function useOpenLayersCoordinatesLayer({ mapInstanceRef, mapViewRef, mapV
149150
}
150151
};
151152

152-
map.on("moveend", handleZoomChange);
153+
const moveEndKey = map.on("moveend", handleZoomChange);
153154

154155
// Cleanup
155156
return () => {
156-
map.un("moveend", handleZoomChange);
157+
dispose(moveEndKey);
157158

158159
// Remove layer on cleanup
159160
if (vectorLayerRef.current) {
@@ -192,9 +193,6 @@ export function useOpenLayersCoordinatesLayer({ mapInstanceRef, mapViewRef, mapV
192193
if (!tooltip) {
193194
tooltip = document.createElement("div");
194195
tooltip.id = "ol-coordinates-tooltip";
195-
tooltip.style.position = "absolute";
196-
tooltip.style.pointerEvents = "none";
197-
tooltip.style.zIndex = "10000";
198196
document.body.appendChild(tooltip);
199197
}
200198

@@ -211,9 +209,9 @@ export function useOpenLayersCoordinatesLayer({ mapInstanceRef, mapViewRef, mapV
211209
</div>
212210
`;
213211

214-
tooltip.style.left = event.originalEvent.pageX + 10 + "px";
215-
tooltip.style.top = event.originalEvent.pageY + 10 + "px";
216-
tooltip.style.display = "block";
212+
tooltip.style.cssText = `position: absolute; pointer-events: none; z-index: 10000; left: ${
213+
event.originalEvent.pageX + 10
214+
}px; top: ${event.originalEvent.pageY + 10}px; display: block;`;
217215
} else {
218216
map.getTargetElement().style.cursor = "";
219217
if (tooltip) {
@@ -264,12 +262,11 @@ export function useOpenLayersCoordinatesLayer({ mapInstanceRef, mapViewRef, mapV
264262
}
265263
};
266264

267-
map.on("pointermove", handlePointerMove);
268-
map.on("click", handleClick);
265+
const pointerMoveKey = map.on("pointermove", handlePointerMove);
266+
const clickKey = map.on("click", handleClick);
269267

270268
return () => {
271-
map.un("pointermove", handlePointerMove);
272-
map.un("click", handleClick);
269+
dispose([pointerMoveKey, clickKey]);
273270

274271
// Remove tooltip from DOM
275272
if (tooltip && tooltip.parentNode) {

‎client/src/app/[site]/globe/2d/hooks/useOpenLayersCountriesLayer.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { FilterParameter } from "@rybbit/shared";
22
import Map from "ol/Map";
3+
import { unByKey as dispose } from "ol/Observable";
34
import GeoJSON from "ol/format/GeoJSON";
45
import VectorLayer from "ol/layer/Vector";
56
import VectorSource from "ol/source/Vector";
@@ -153,13 +154,12 @@ export function useOpenLayersCountriesLayer({ mapInstanceRef, mapViewRef, mapVie
153154
}
154155
};
155156

156-
mapInstanceRef.current.on("pointermove", handlePointerMove);
157-
mapInstanceRef.current.on("click", handleClick);
157+
const pointerMoveKey = mapInstanceRef.current.on("pointermove", handlePointerMove);
158+
const clickKey = mapInstanceRef.current.on("click", handleClick);
158159

159160
return () => {
160161
if (mapInstanceRef.current) {
161-
mapInstanceRef.current.un("pointermove", handlePointerMove);
162-
mapInstanceRef.current.un("click", handleClick);
162+
dispose([pointerMoveKey, clickKey]);
163163

164164
// Remove layer on cleanup
165165
if (vectorLayerRef.current) {
@@ -198,9 +198,6 @@ export function useOpenLayersCountriesLayer({ mapInstanceRef, mapViewRef, mapVie
198198
if (!tooltip) {
199199
tooltip = document.createElement("div");
200200
tooltip.id = "ol-countries-tooltip";
201-
tooltip.style.position = "fixed";
202-
tooltip.style.pointerEvents = "none";
203-
tooltip.style.zIndex = "9999";
204201
document.body.appendChild(tooltip);
205202
}
206203

@@ -219,9 +216,9 @@ export function useOpenLayersCountriesLayer({ mapInstanceRef, mapViewRef, mapVie
219216
</div>
220217
`;
221218

222-
tooltip.style.left = `${tooltipData.x}px`;
223-
tooltip.style.top = `${tooltipData.y - 10}px`;
224-
tooltip.style.transform = "translate(-50%, -100%)";
219+
tooltip.style.cssText = `position: fixed; pointer-events: none; z-index: 9999; left: ${tooltipData.x}px; top: ${
220+
tooltipData.y - 10
221+
}px; transform: translate(-50%, -100%);`;
225222

226223
return () => {
227224
const existingTooltip = document.getElementById("ol-countries-tooltip");

‎client/src/app/[site]/globe/2d/hooks/useOpenLayersSubdivisionsLayer.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { FilterParameter } from "@rybbit/shared";
22
import Map from "ol/Map";
3+
import { unByKey as dispose } from "ol/Observable";
34
import GeoJSON from "ol/format/GeoJSON";
45
import VectorLayer from "ol/layer/Vector";
56
import VectorSource from "ol/source/Vector";
@@ -157,13 +158,12 @@ export function useOpenLayersSubdivisionsLayer({
157158
}
158159
};
159160

160-
mapInstanceRef.current.on("pointermove", handlePointerMove);
161-
mapInstanceRef.current.on("click", handleClick);
161+
const pointerMoveKey = mapInstanceRef.current.on("pointermove", handlePointerMove);
162+
const clickKey = mapInstanceRef.current.on("click", handleClick);
162163

163164
return () => {
164165
if (mapInstanceRef.current) {
165-
mapInstanceRef.current.un("pointermove", handlePointerMove);
166-
mapInstanceRef.current.un("click", handleClick);
166+
dispose([pointerMoveKey, clickKey]);
167167

168168
// Remove layer on cleanup
169169
if (vectorLayerRef.current) {
@@ -202,9 +202,6 @@ export function useOpenLayersSubdivisionsLayer({
202202
if (!tooltip) {
203203
tooltip = document.createElement("div");
204204
tooltip.id = "ol-subdivisions-tooltip";
205-
tooltip.style.position = "fixed";
206-
tooltip.style.pointerEvents = "none";
207-
tooltip.style.zIndex = "9999";
208205
document.body.appendChild(tooltip);
209206
}
210207

@@ -225,9 +222,9 @@ export function useOpenLayersSubdivisionsLayer({
225222
</div>
226223
`;
227224

228-
tooltip.style.left = `${tooltipData.x}px`;
229-
tooltip.style.top = `${tooltipData.y - 10}px`;
230-
tooltip.style.transform = "translate(-50%, -100%)";
225+
tooltip.style.cssText = `position: fixed; pointer-events: none; z-index: 9999; left: ${tooltipData.x}px; top: ${
226+
tooltipData.y - 10
227+
}px; transform: translate(-50%, -100%);`;
231228

232229
return () => {
233230
const existingTooltip = document.getElementById("ol-subdivisions-tooltip");

‎client/src/app/[site]/globe/2d/hooks/useOpenLayersTimelineLayer.ts‎

Lines changed: 16 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import Feature, { FeatureLike } from "ol/Feature";
22
import OLMap from "ol/Map";
3+
import { unByKey as dispose } from "ol/Observable";
34
import Overlay from "ol/Overlay";
45
import Point from "ol/geom/Point";
6+
import type { EventsKey } from "ol/events";
57
import VectorLayer from "ol/layer/Vector";
68
import { fromLonLat } from "ol/proj";
79
import Cluster from "ol/source/Cluster";
@@ -16,6 +18,8 @@ import { buildTooltipHTML } from "../../utils/timelineTooltipBuilder";
1618

1719
// OpenLayers-specific clustering constants
1820
const CLUSTER_RADIUS = 50; // pixels (OpenLayers specific)
21+
const AVATAR_MARKER_STYLE =
22+
"cursor: pointer; border-radius: 50%; overflow: hidden; width: 32px; height: 32px; box-shadow: 0 2px 8px rgba(0,0,0,0.3); transform: translate(-50%, -50%);";
1923

2024
interface TimelineLayerProps {
2125
mapInstanceRef: React.RefObject<OLMap | null>;
@@ -61,10 +65,10 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
6165
// Set initial zoom
6266
handleZoomChange();
6367

64-
map.on("moveend", handleZoomChange);
68+
const moveEndKey = map.on("moveend", handleZoomChange);
6569

6670
return () => {
67-
map.un("moveend", handleZoomChange);
71+
dispose(moveEndKey);
6872
};
6973
}, [mapInstanceRef]);
7074

@@ -76,8 +80,7 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
7680
if (!tooltipOverlayRef.current) {
7781
const tooltipElement = document.createElement("div");
7882
tooltipElement.className = "ol-timeline-tooltip";
79-
tooltipElement.style.position = "absolute";
80-
tooltipElement.style.zIndex = "1000";
83+
tooltipElement.style.cssText = "position: absolute; z-index: 1000;";
8184

8285
const tooltip = new Overlay({
8386
element: tooltipElement,
@@ -122,6 +125,8 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
122125
return;
123126
}
124127

128+
let clusterMoveEndKey: EventsKey | EventsKey[] | null = null;
129+
125130
// Determine if we should use clustering
126131
const shouldCluster = activeSessions.length > CLUSTERING_THRESHOLD && currentZoomRef.current < CLUSTER_MAX_ZOOM;
127132

@@ -251,13 +256,7 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
251256
// Create new overlay (same code as non-clustering case)
252257
const avatarContainer = document.createElement("div");
253258
avatarContainer.className = "timeline-avatar-marker";
254-
avatarContainer.style.cursor = "pointer";
255-
avatarContainer.style.borderRadius = "50%";
256-
avatarContainer.style.overflow = "hidden";
257-
avatarContainer.style.width = "32px";
258-
avatarContainer.style.height = "32px";
259-
avatarContainer.style.boxShadow = "0 2px 8px rgba(0,0,0,0.3)";
260-
avatarContainer.style.transform = "translate(-50%, -50%)";
259+
avatarContainer.style.cssText = AVATAR_MARKER_STYLE;
261260

262261
const avatarSVG = generateAvatarSVG(session.user_id, 32);
263262
avatarContainer.innerHTML = avatarSVG;
@@ -351,10 +350,7 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
351350
const handleMoveEndForClusters = () => {
352351
updateUnclusteredOverlays();
353352
};
354-
map.on("moveend", handleMoveEndForClusters);
355-
356-
// Store handler for cleanup
357-
(map as any).__clusterMoveEndHandler = handleMoveEndForClusters;
353+
clusterMoveEndKey = map.on("moveend", handleMoveEndForClusters);
358354
} else {
359355
// Not clustering - use individual overlays
360356
// Remove cluster layer
@@ -363,12 +359,6 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
363359
clusterLayerRef.current = null;
364360
}
365361

366-
// Remove cluster moveend handler if it exists
367-
if ((map as any).__clusterMoveEndHandler) {
368-
map.un("moveend", (map as any).__clusterMoveEndHandler);
369-
delete (map as any).__clusterMoveEndHandler;
370-
}
371-
372362
// Build set of current session IDs
373363
const currentSessionIds = new Set(activeSessions.map(s => s.session_id));
374364

@@ -400,13 +390,7 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
400390
// Create new overlay
401391
const avatarContainer = document.createElement("div");
402392
avatarContainer.className = "timeline-avatar-marker";
403-
avatarContainer.style.cursor = "pointer";
404-
avatarContainer.style.borderRadius = "50%";
405-
avatarContainer.style.overflow = "hidden";
406-
avatarContainer.style.width = "32px";
407-
avatarContainer.style.height = "32px";
408-
avatarContainer.style.boxShadow = "0 2px 8px rgba(0,0,0,0.3)";
409-
avatarContainer.style.transform = "translate(-50%, -50%)"; // Center the avatar
393+
avatarContainer.style.cssText = AVATAR_MARKER_STYLE;
410394

411395
const avatarSVG = generateAvatarSVG(session.user_id, 32);
412396
avatarContainer.innerHTML = avatarSVG;
@@ -525,16 +509,14 @@ export function useOpenLayersTimelineLayer({ mapInstanceRef, mapViewRef, mapView
525509
}
526510
};
527511

528-
map.on("click", handleMapClick);
512+
const mapClickKey = map.on("click", handleMapClick);
529513

530514
// Cleanup function
531515
return () => {
532-
map.un("click", handleMapClick);
516+
dispose(mapClickKey);
533517

534-
// Clean up cluster moveend handler if it exists
535-
if ((map as any).__clusterMoveEndHandler) {
536-
map.un("moveend", (map as any).__clusterMoveEndHandler);
537-
delete (map as any).__clusterMoveEndHandler;
518+
if (clusterMoveEndKey) {
519+
dispose(clusterMoveEndKey);
538520
}
539521

540522
overlaysMap.forEach(({ overlay, cleanup }) => {

‎client/src/app/[site]/globe/3d/hooks/timelineLayer/timelineLayerSetup.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
import mapboxgl from "mapbox-gl";
2-
import {
3-
SOURCE_ID,
4-
CLUSTER_LAYER_ID,
5-
CLUSTER_COUNT_LAYER_ID,
6-
UNCLUSTERED_LAYER_ID,
7-
} from "./timelineLayerConstants";
2+
import { SOURCE_ID, CLUSTER_LAYER_ID, CLUSTER_COUNT_LAYER_ID, UNCLUSTERED_LAYER_ID } from "./timelineLayerConstants";
83
import { MIN_CLUSTER_SIZE } from "../../../utils/clusteringConstants";
94

105
/**
@@ -81,12 +76,19 @@ export function disableClusterTransitions(mapInstance: mapboxgl.Map): void {
8176
* Setup cursor change on cluster hover
8277
*/
8378
export function setupClusterHoverHandlers(mapInstance: mapboxgl.Map, layerId: string): () => void {
79+
const setCursor = (cursor: string) => {
80+
const canvas = mapInstance.getCanvas() as HTMLCanvasElement | undefined;
81+
if (canvas) {
82+
canvas.style.cursor = cursor;
83+
}
84+
};
85+
8486
const handleClusterMouseEnter = () => {
85-
mapInstance.getCanvas().style.cursor = "pointer";
87+
setCursor("pointer");
8688
};
8789

8890
const handleClusterMouseLeave = () => {
89-
mapInstance.getCanvas().style.cursor = "";
91+
setCursor("");
9092
};
9193

9294
mapInstance.on("mouseenter", layerId, handleClusterMouseEnter);

0 commit comments

Comments
 (0)