From b694857e00091e88cfd797e8adcc5d6bd9f063e6 Mon Sep 17 00:00:00 2001 From: Drew Slobodnjak <60050885+drew08t@users.noreply.github.com> Date: Wed, 16 Apr 2025 08:07:57 -0700 Subject: [PATCH] Geomap: Tooltip for multiple features same coord (#103163) * Geomap: Tooltip for multiple features same coord * Add basic test coverage * Be explicit with sorting to avoid truthiness error * Add sorting to tests and clean up * Check again for duplicates --- .betterer.results | 3 +- .../panel/geomap/utils/tooltip.test.ts | 220 ++++++++++++++++++ .../app/plugins/panel/geomap/utils/tooltip.ts | 48 +++- 3 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 public/app/plugins/panel/geomap/utils/tooltip.test.ts diff --git a/.betterer.results b/.betterer.results index 0cb370261b0..e1485ba6b40 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3468,7 +3468,8 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], "public/app/plugins/panel/geomap/utils/tooltip.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] + [0, 0, 0, "Do not use any type assertions.", "0"], + [0, 0, 0, "Do not use any type assertions.", "1"] ], "public/app/plugins/panel/heatmap/HeatmapPanel.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], diff --git a/public/app/plugins/panel/geomap/utils/tooltip.test.ts b/public/app/plugins/panel/geomap/utils/tooltip.test.ts new file mode 100644 index 00000000000..a5b721da7d4 --- /dev/null +++ b/public/app/plugins/panel/geomap/utils/tooltip.test.ts @@ -0,0 +1,220 @@ +import { Feature, MapBrowserEvent } from 'ol'; +import { Point } from 'ol/geom'; +import WebGLPointsLayer from 'ol/layer/WebGLPoints'; +import VectorSource from 'ol/source/Vector'; + +import { DataFrame, PanelProps } from '@grafana/data'; + +import { GeomapPanel } from '../GeomapPanel'; +import { GeomapHoverPayload, GeomapLayerHover } from '../event'; +import { Options } from '../types'; + +import { pointerMoveListener } from './tooltip'; + +// Mock the GeomapPanel class +jest.mock('../GeomapPanel', () => { + return { + GeomapPanel: jest.fn().mockImplementation(() => { + return { + map: { + getEventPixel: jest.fn().mockReturnValue([100, 100]), + getCoordinateFromPixel: jest.fn().mockReturnValue([0, 0]), + forEachFeatureAtPixel: jest.fn(), + }, + mapDiv: { + style: { cursor: 'auto' }, + }, + state: { + measureMenuActive: false, + ttipOpen: false, + ttip: undefined, + }, + hoverPayload: {} as GeomapHoverPayload, + hoverEvent: {}, + props: { + eventBus: { + publish: jest.fn(), + }, + }, + setState: jest.fn(), + layers: [], + }; + }), + }; +}); + +// Mock the getMapLayerState function +jest.mock('./layers', () => { + return { + getMapLayerState: jest.fn().mockReturnValue({ + options: { tooltip: true }, + mouseEvents: { next: jest.fn() }, + }), + }; +}); + +describe('tooltip utils', () => { + let panel: GeomapPanel; + let mockEvent: MapBrowserEvent; + let mockWebGLLayer: WebGLPointsLayer>; + let mockVectorSource: VectorSource; + + // Consolidated feature constants + let feature1: Feature; + let feature2: Feature; + let feature3: Feature; + let feature4: Feature; + let differentFeature: Feature; + + beforeEach(() => { + // Reset mocks + jest.clearAllMocks(); + + // Create mock objects + panel = new GeomapPanel({} as PanelProps); + mockEvent = { + originalEvent: { + pageX: 100, + pageY: 100, + }, + } as MapBrowserEvent; + + // Create features for testing + feature1 = new Feature({ + geometry: new Point([0, 0]), + rowIndex: 1, + frame: {} as DataFrame, + }); + + feature2 = new Feature({ + geometry: new Point([0, 0]), + rowIndex: 2, + frame: {} as DataFrame, + }); + + feature3 = new Feature({ + geometry: new Point([0, 0]), + rowIndex: 3, + frame: {} as DataFrame, + }); + + feature4 = new Feature({ + geometry: new Point([0, 0]), + // No rowIndex + frame: {} as DataFrame, + }); + + differentFeature = new Feature({ + geometry: new Point([1, 1]), + rowIndex: 4, + frame: {} as DataFrame, + }); + + // Create mock vector source + mockVectorSource = new VectorSource(); + mockVectorSource.forEachFeature = jest.fn(); + + // Create mock WebGL layer + mockWebGLLayer = new WebGLPointsLayer({ + source: mockVectorSource, + style: { + symbol: { + symbolType: 'circle', + size: 8, + color: '#000000', + opacity: 1, + }, + }, + }); + mockWebGLLayer.getSource = jest.fn().mockReturnValue(mockVectorSource); + + // Setup the forEachFeatureAtPixel mock + if (panel.map) { + (panel.map.forEachFeatureAtPixel as jest.Mock).mockImplementation((pixel, callback) => { + callback(feature1, mockWebGLLayer, null); + }); + } + }); + + describe('WebGLPointsLayer condition', () => { + it('should add additional features at the same coordinates for WebGLPointsLayer', () => { + // Setup the mock vector source to return multiple features at the same coordinates + (mockVectorSource.forEachFeature as jest.Mock).mockImplementation((callback) => { + callback(feature2); + }); + + // Call the function + pointerMoveListener(mockEvent, panel); + + // Verify that forEachFeatureAtPixel was called + if (panel.map) { + expect(panel.map.forEachFeatureAtPixel).toHaveBeenCalled(); + } + + // Verify that the layer was added to hoverPayload + expect(panel.hoverPayload.layers).toBeDefined(); + expect(panel.hoverPayload.layers?.length).toBe(1); + + // Verify that both features were added to the layer + const layerHover = panel.hoverPayload.layers?.[0] as GeomapLayerHover; + expect(layerHover.features.length).toBe(2); + expect(layerHover.features).toContain(feature1); + expect(layerHover.features).toContain(feature2); + }); + + it('should not add features with different coordinates', () => { + // Setup the mock vector source to return a feature with different coordinates + (mockVectorSource.forEachFeature as jest.Mock).mockImplementation((callback) => { + callback(differentFeature); + }); + + // Call the function + pointerMoveListener(mockEvent, panel); + + // Verify that only the original feature was added + const layerHover = panel.hoverPayload.layers?.[0] as GeomapLayerHover; + expect(layerHover.features.length).toBe(1); + expect(layerHover.features).toContain(feature1); + expect(layerHover.features).not.toContain(differentFeature); + }); + + it('should not add the same feature twice', () => { + // Setup the mock vector source to return the same feature + (mockVectorSource.forEachFeature as jest.Mock).mockImplementation((callback) => { + callback(feature1); + }); + + // Call the function + pointerMoveListener(mockEvent, panel); + + // Verify that the feature was only added once + const layerHover = panel.hoverPayload.layers?.[0] as GeomapLayerHover; + expect(layerHover.features.length).toBe(1); + expect(layerHover.features).toContain(feature1); + }); + + it('should sort features by rowIndex when multiple features are at the same coordinates', () => { + // Setup the mock vector source to return multiple features + (mockVectorSource.forEachFeature as jest.Mock).mockImplementation((callback) => { + callback(feature2); + callback(feature3); + callback(feature4); + }); + + // Call the function + pointerMoveListener(mockEvent, panel); + + // Verify that the features were added and sorted by rowIndex + const layerHover = panel.hoverPayload.layers?.[0] as GeomapLayerHover; + expect(layerHover.features.length).toBe(4); // feature1 + 3 new features + + // Check that features are sorted by rowIndex (1, 2, 3, MAX_SAFE_INTEGER) + // Since rowIndex is unique, we can check the exact order + expect(layerHover.features[0].getProperties()['rowIndex']).toBe(1); // feature1 + expect(layerHover.features[1].getProperties()['rowIndex']).toBe(2); // feature2 + expect(layerHover.features[2].getProperties()['rowIndex']).toBe(3); // feature3 + // The last feature (feature4) has no rowIndex, so it should be at the end + expect(layerHover.features[3].getProperties()['rowIndex']).toBeUndefined(); + }); + }); +}); diff --git a/public/app/plugins/panel/geomap/utils/tooltip.ts b/public/app/plugins/panel/geomap/utils/tooltip.ts index 547ef6e498b..81ed086afac 100644 --- a/public/app/plugins/panel/geomap/utils/tooltip.ts +++ b/public/app/plugins/panel/geomap/utils/tooltip.ts @@ -1,6 +1,10 @@ import { debounce } from 'lodash'; import { MapBrowserEvent } from 'ol'; +import { FeatureLike } from 'ol/Feature'; +import { Point } from 'ol/geom'; +import WebGLPointsLayer from 'ol/layer/WebGLPoints'; import { toLonLat } from 'ol/proj'; +import VectorSource from 'ol/source/Vector'; import { DataFrame, DataHoverClearEvent } from '@grafana/data'; @@ -87,7 +91,49 @@ export const pointerMoveListener = (evt: MapBrowserEvent, panel: Geo layerLookup.set(s, h); layers.push(h); } - h.features.push(feature); + + // Only add if not already present + if (!h.features.some((f) => f === feature)) { + h.features.push(feature); + } + + // For WebGLPointsLayer, check for additional features at the same coordinates + if (layer instanceof WebGLPointsLayer) { + const featureGeom = feature.getGeometry(); + if (featureGeom instanceof Point) { + const featureCoords = featureGeom.getCoordinates(); + const source = layer.getSource() as VectorSource; + let addedFeatures = false; + source.forEachFeature((otherFeature: FeatureLike) => { + // Ignore duplicates + if (otherFeature !== feature && !h.features.some((f) => f === otherFeature)) { + const otherGeom = otherFeature.getGeometry(); + if (otherGeom instanceof Point) { + const otherCoords = otherGeom.getCoordinates(); + // Check for matching coordinates + if (otherCoords[0] === featureCoords[0] && otherCoords[1] === featureCoords[1]) { + h.features.push(otherFeature); + addedFeatures = true; + } + } + } + }); + // If we found multiple features at the same coordinates, sort them by rowIndex + if (addedFeatures) { + h.features.sort((a, b) => { + const aIndex = + a.getProperties()['rowIndex'] !== undefined + ? Number(a.getProperties()['rowIndex']) + : Number.MAX_SAFE_INTEGER; + const bIndex = + b.getProperties()['rowIndex'] !== undefined + ? Number(b.getProperties()['rowIndex']) + : Number.MAX_SAFE_INTEGER; + return aIndex - bIndex; + }); + } + } + } } }, {