diff --git a/demo/js/draw.js b/demo/js/draw.js index 83717cf4..0dccb165 100755 --- a/demo/js/draw.js +++ b/demo/js/draw.js @@ -18,7 +18,7 @@ import createInteractPlugin from '/plugins/interact/src/index.js' const pointData = {type: 'FeatureCollection','features': [{'type': 'Feature','properties': {},'geometry': {'coordinates': [-2.882445487962059,54.70938250564518],'type': 'Point'}},{'type': 'Feature','properties': {},'geometry': {'coordinates': [-2.8775970686837695,54.70966586215056],'type': 'Point'}},{'type': 'Feature','properties': {},'geometry': {'coordinates': [-2.8732152153681056,54.70892223300439],'type': 'Point'}}]} const interactPlugin = createInteractPlugin({ - dataLayers: [{ + layers: [{ layerId: 'field-parcels', // idProperty: 'gid' },{ @@ -35,7 +35,7 @@ const interactPlugin = createInteractPlugin({ idProperty: 'id' }], debug: true, - interactionMode: 'select', // 'auto', 'select', 'marker' // defaults to 'marker' + interactionModes: ['selectMarker', 'selectFeature'], // e.g. ['selectMarker'], ['selectFeature'], ['placeMarker'], or combinations multiSelect: true, contiguous: true, deselectOnClickOutside: true diff --git a/demo/js/farming.js b/demo/js/farming.js index f61faf0b..9d95a31a 100755 --- a/demo/js/farming.js +++ b/demo/js/farming.js @@ -19,14 +19,14 @@ import createFramePlugin from '/plugins/beta/frame/src/index.js' var feature = { id: 'test1234', type: 'Feature', geometry: { coordinates: [[[-2.9406643378873127,54.918060570259456],[-2.9092219779267054,54.91564249172612],[-2.904350626383433,54.90329530000005],[-2.909664828067463,54.89540129642464],[-2.9225074821353587,54.88979816151294],[-2.937121536764323,54.88826989853317],[-2.95682836800691,54.88916139231736],[-2.965463945742613,54.898966521920045],[-2.966349646023133,54.910805898763385],[-2.9406643378873127,54.918060570259456]]], type: 'Polygon' }} var interactPlugin = createInteractPlugin({ - dataLayers: [{ + layers: [{ layerId: 'field-parcels', // idProperty: 'gid' },{ layerId: 'linked-parcels', // idProperty: 'gid' }], - interactionMode: 'auto', // 'auto', 'select', 'marker' // defaults to 'marker' + interactionModes: ['selectMarker', 'selectFeature', 'placeMarker'], // e.g. ['selectMarker'], ['selectFeature'], ['placeMarker'], or combinations multiSelect: true, // excludeModes: ['draw'] }) diff --git a/demo/js/index.js b/demo/js/index.js index d9c2d312..d4a483b0 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -29,7 +29,7 @@ import createFramePlugin from '/plugins/beta/frame/src/index.js' const pointData = {type: 'FeatureCollection',features: [{type: 'Feature',properties: {category:'prehistoric'},geometry: {coordinates: [-2.4558622,54.5617135],type: 'Point'}},{type: 'Feature',properties: {category:'roman'},geometry: {coordinates: [-2.439823,54.5525437],type: 'Point'}},{type: 'Feature',properties: {category:'medieval'},geometry: {coordinates: [-2.4481939,54.5575261],type: 'Point'}}]} const interactPlugin = createInteractPlugin({ - dataLayers: [{ + layers: [{ layerId: 'historic-monuments-prehistoric-symbol', // idProperty: 'gid' },{ @@ -50,6 +50,9 @@ const interactPlugin = createInteractPlugin({ },{ layerId: 'OS/TopographicArea_1/Agricultural Land', idProperty: 'TOID' + },{ + layerId: 'hedge-control', + idProperty: 'id' },{ layerId: 'fill-inactive.cold', idProperty: 'id' @@ -58,8 +61,8 @@ const interactPlugin = createInteractPlugin({ idProperty: 'id' }], debug: true, - interactionMode: 'select', // 'auto', 'select', 'marker' // defaults to 'marker' - multiSelect: true, + interactionModes: ['selectMarker', 'selectFeature'], // e.g. ['selectMarker'], ['selectFeature'], ['placeMarker'], or combinations + // multiSelect: true, contiguous: true, deselectOnClickOutside: true }) @@ -302,6 +305,10 @@ interactiveMap.on('map:ready', function (e) { // aspectRatio: 1 // }) interactPlugin.enable() + interactiveMap.addMarker('my-marker-1', [-2.4555608,54.5655407]) + interactiveMap.addMarker('my-marker-2', [-2.4511636,54.5638338], { + symbol: 'square' + }) }) interactiveMap.on('datasets:ready', function () { @@ -323,6 +330,7 @@ interactiveMap.on('interact:cancel', function (e) { }) interactiveMap.on('interact:selectionchange', function (e) { + console.log(e) const drawLayers = ['stroke-inactive.cold', 'fill-inactive.cold'] const singleFeature = e.selectedFeatures.length === 1 const anyFeature = e.selectedFeatures.length > 0 diff --git a/demo/js/planning.js b/demo/js/planning.js index dde8ddd0..e5cb0bee 100755 --- a/demo/js/planning.js +++ b/demo/js/planning.js @@ -36,7 +36,7 @@ const interactPlugin = createInteractPlugin({ backgroundColor: { outdoor: '#0b0c0c', dark: '#ffffff' }, foregroundColor: { outdoor: '#ffff', dark: '#0b0c0c' } }, - // interactionMode: 'marker', // 'auto', 'select', 'marker' // defaults to 'marker' + // interactionModes: ['selectMarker'], // e.g. ['selectMarker'], ['selectFeature'], ['placeMarker'], or combinations // multiSelect: true }) diff --git a/docs/api/symbol-config.md b/docs/api/symbol-config.md index 9d0d5964..03d4482e 100644 --- a/docs/api/symbol-config.md +++ b/docs/api/symbol-config.md @@ -6,7 +6,7 @@ Symbol properties control the appearance of markers and point dataset features. Each property is optional. A value set directly on a marker or dataset layer takes priority over everything else. If a property is not set there, the value registered with the symbol is used. If the symbol has no value for that property, the app-wide `symbolDefaults` from the constructor applies. If none of those are set, the built-in fallback listed under each property below is used. -`haloColor`, `selectedColor`, `haloWidth`, and `selectedWidth` are required tokens in the SVG structure (see [SVG structure](#svg-structure)). Include them in any custom `symbolSvgContent` — the app resolves their values automatically. +`haloColor`, `selectedColor`, `haloWidth`, and `selectedWidth` are required tokens in the SVG structure (see [SVG structure](#svg-structure)). Include them in any custom `symbolSvgContent` — the app resolves their values automatically. Note that `haloColor` and `selectedColor` are always derived from the active map style and cannot be configured. ## Style-keyed colours @@ -87,6 +87,22 @@ Foreground fill colour — the inner graphic element (e.g. the dot inside a pin) --- +### `haloWidth` +**Type:** `number` +**Default:** `1` + +Stroke width of the halo around the symbol background shape. Can be set in constructor `symbolDefaults`, at symbol registration, or per marker/dataset layer. + +--- + +### `selectedWidth` +**Type:** `number` +**Default:** `6` + +Stroke width of the selection ring shown when a marker is selected. Can be set in constructor `symbolDefaults` or per marker/dataset layer. + +--- + ### `graphic` **Type:** `string` @@ -140,3 +156,5 @@ svg: ` - **Layer 1** — selection ring (stroke only, fill none) — hidden in normal rendering, visible when selected - **Layer 2** — background shape with halo stroke - **Layer 3** — foreground graphic (e.g. inner dot) + +> `{{haloColor}}` and `{{selectedColor}}` are always injected from the active map style. They must be present in the SVG but cannot be configured. diff --git a/docs/plugins/interact.md b/docs/plugins/interact.md index 0f17228a..f8eae24f 100644 --- a/docs/plugins/interact.md +++ b/docs/plugins/interact.md @@ -8,9 +8,9 @@ The interact plugin provides a unified way to handle user interactions for selec import createInteractPlugin from '@defra/interactive-map/plugins/interact' const interactPlugin = createInteractPlugin({ - interactionMode: 'auto', + interactionModes: ['selectMarker', 'selectFeature'], multiSelect: true, - dataLayers: [ + layers: [ { layerId: 'my-layer', idProperty: 'id' } ] }) @@ -40,32 +40,43 @@ Array of mode identifiers. When set, the plugin does not render when the app is --- -### `interactionMode` -**Type:** `'marker' | 'select' | 'auto'` -**Default:** `'marker'` +### `interactionModes` +**Type:** `Array<'selectMarker' | 'selectFeature' | 'placeMarker'>` +**Default:** `['selectMarker']` -Controls how user clicks are interpreted. +Controls which interactions are active when the user clicks the map. Values can be combined freely — the plugin always processes them in a fixed priority order: marker selection → feature selection → place marker. -- `'marker'` — clicking always places a location marker at the clicked coordinates -- `'select'` — clicking attempts to match a feature from `dataLayers`; click outside clears selection (unless `deselectOnClickOutside` is `false`) -- `'auto'` — attempts feature matching first, falls back to placing a marker if no feature is found +- `'selectMarker'` — clicking a placed marker toggles its selection state +- `'selectFeature'` — clicking the map attempts to match a feature from `layers` +- `'placeMarker'` — if no feature is matched (or `selectFeature` is not active), places a location marker at the clicked coordinates + +**Common combinations:** + +```js +interactionModes: ['selectMarker'] // marker selection only (default) +interactionModes: ['selectFeature'] // feature selection only +interactionModes: ['placeMarker'] // always place a marker on click +interactionModes: ['selectMarker', 'selectFeature'] // select markers or features +interactionModes: ['selectFeature', 'placeMarker'] // select features, fall back to placing a marker +interactionModes: ['selectMarker', 'selectFeature', 'placeMarker'] // all interactions active +``` --- -### `dataLayers` -**Type:** `Array` +### `layers` +**Type:** `Array` **Default:** `[]` Array of map layer configurations that are selectable. Each entry specifies which layer to watch and how to identify features. ```js -dataLayers: [ +layers: [ { layerId: 'my-polygons', idProperty: 'id' }, { layerId: 'my-lines' } ] ``` -#### `DataLayer` properties +#### `LayerConfig` properties | Property | Type | Description | |----------|------|-------------| @@ -75,6 +86,16 @@ dataLayers: [ | `selectedFill` | `string` | Overrides the selection fill colour for this layer. Defaults to `transparent` | | `selectedStrokeWidth` | `number` | Overrides the selection stroke width for this layer. Defaults to `3` | +#### Finding layer IDs + +What to use as `layerId` depends on how your data is added to the map — these are the layers the plugin will enable for feature selection: + +- **MapLibre directly** — use the layer IDs defined in your style or added via `map.addLayer()` +- **Datasets plugin** — use the dataset ID, or the sublayer ID for datasets with sublayers +- **Draw plugin** — uses generated layer IDs such as `fill-inactive.cold` and `stroke-inactive.cold` + +If you're unsure of the layer IDs available at runtime, set `debug: true` in the map config — this lets you query the map and inspect layer names in the browser console. + --- ### `multiSelect` @@ -105,7 +126,7 @@ When `true`, clicking outside any selectable layer clears the current selection. **Type:** `number` **Default:** `10` -Click detection radius in pixels. Increases the hit area around the cursor when matching features, which is useful for lines and points. +Click detection radius in pixels applied to line features. Lines have a 1px rendered width so a buffer is required for reliable selection. Polygon and symbol/icon features use exact hit detection and are unaffected by this value. --- @@ -140,9 +161,9 @@ When not set, the marker inherits from the constructor `symbolDefaults` cascade. **Type:** `number` **Default:** `3` -Stroke width used to highlight selected features. Can be overridden per layer via `dataLayers[].selectedStrokeWidth`. +Stroke width used to highlight selected features. Can be overridden per layer via `layers[].selectedStrokeWidth`. -> **Selection colours** — stroke and fill colours for selected features are not configured here. Stroke colour comes from `MapStyleConfig.selectedColor` (falling back to the `mapColorScheme` scheme default), ensuring the selection colour stays consistent with the rest of the map theme. Fill defaults to `transparent`. Both can be overridden per layer via `dataLayers[].selectedStroke` and `dataLayers[].selectedFill`. +> **Selection colours** — stroke and fill colours for selected features are not configured here. Stroke colour comes from `MapStyleConfig.selectedColor` (falling back to the `mapColorScheme` scheme default), ensuring the selection colour stays consistent with the rest of the map theme. Fill defaults to `transparent`. Both can be overridden per layer via `layers[].selectedStroke` and `layers[].selectedFill`. --- @@ -166,7 +187,7 @@ interactiveMap.on('map:ready', () => { }) // Override options at runtime -interactPlugin.enable({ multiSelect: true, interactionMode: 'select' }) +interactPlugin.enable({ multiSelect: true, interactionModes: ['selectFeature'] }) ``` --- diff --git a/plugins/beta/datasets/src/adapters/maplibre/layerIds.js b/plugins/beta/datasets/src/adapters/maplibre/layerIds.js index 0284f00e..c0e7f995 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/layerIds.js +++ b/plugins/beta/datasets/src/adapters/maplibre/layerIds.js @@ -47,7 +47,7 @@ export const getLayerIds = (dataset) => { if (hasSymbol(dataset)) { return { fillLayerId: null, strokeLayerId: null, symbolLayerId: dataset.id } } - const hasFill = !!dataset.fill || hasPattern(dataset) + const hasFill = (!!dataset.fill && dataset.fill !== 'transparent') || hasPattern(dataset) const hasStroke = !!dataset.stroke const fillLayerId = hasFill ? dataset.id : null let strokeLayerId = null diff --git a/plugins/interact/src/InteractInit.jsx b/plugins/interact/src/InteractInit.jsx index 55612eb0..e000f42c 100755 --- a/plugins/interact/src/InteractInit.jsx +++ b/plugins/interact/src/InteractInit.jsx @@ -2,6 +2,7 @@ import { useEffect, useRef } from 'react' import { EVENTS } from '../../../src/config/events.js' import { useInteractionHandlers } from './hooks/useInteractionHandlers.js' import { useHighlightSync } from './hooks/useHighlightSync.js' +import { useHoverCursor } from './hooks/useHoverCursor.js' import { attachEvents } from './events.js' export const InteractInit = ({ @@ -61,6 +62,13 @@ export const InteractInit = ({ eventBus }) + // Notify other components (e.g. Markers) whether interact is active + useEffect(() => { + eventBus.emit('interact:active', { active: enabled }) + }, [enabled]) + + useHoverCursor(mapProvider, enabled, pluginState.interactionModes, pluginState.layers) + // Toggle target marker visibility useEffect(() => { if (enabled && isTouchOrKeyboard) { diff --git a/plugins/interact/src/InteractInit.test.js b/plugins/interact/src/InteractInit.test.js index 9c5e03ea..280ab6c3 100644 --- a/plugins/interact/src/InteractInit.test.js +++ b/plugins/interact/src/InteractInit.test.js @@ -3,10 +3,12 @@ import { EVENTS } from '../../../src/config/events.js' import { InteractInit } from './InteractInit.jsx' import { useInteractionHandlers } from './hooks/useInteractionHandlers.js' import { useHighlightSync } from './hooks/useHighlightSync.js' +import { useHoverCursor } from './hooks/useHoverCursor.js' import { attachEvents } from './events.js' jest.mock('./hooks/useInteractionHandlers.js') jest.mock('./hooks/useHighlightSync.js') +jest.mock('./hooks/useHoverCursor.js') jest.mock('./events.js') describe('InteractInit', () => { @@ -20,15 +22,24 @@ describe('InteractInit', () => { useInteractionHandlers.mockReturnValue({ handleInteraction: handleInteractionMock }) useHighlightSync.mockReturnValue(undefined) + useHoverCursor.mockReturnValue(undefined) attachEvents.mockReturnValue(cleanupMock) props = { - appState: { interfaceType: 'mouse' }, + appState: { interfaceType: 'mouse', layoutRefs: { viewportRef: { current: null } } }, mapState: { crossHair: { fixAtCenter: jest.fn(), hide: jest.fn() }, mapStyle: {} }, - services: { eventBus: {}, closeApp: jest.fn() }, + services: { eventBus: { emit: jest.fn() }, closeApp: jest.fn() }, buttonConfig: {}, - mapProvider: {}, - pluginState: { dispatch: jest.fn(), enabled: true, selectedFeatures: [], selectionBounds: {} } + mapProvider: { setHoverCursor: jest.fn() }, + pluginState: { + dispatch: jest.fn(), + enabled: true, + selectedFeatures: [], + selectedMarkers: [], + selectionBounds: {}, + interactionModes: ['selectFeature'], + layers: [] + } } }) diff --git a/plugins/interact/src/api/enable.test.js b/plugins/interact/src/api/enable.test.js index b54d1be4..00b24dd0 100644 --- a/plugins/interact/src/api/enable.test.js +++ b/plugins/interact/src/api/enable.test.js @@ -10,7 +10,7 @@ describe('enable', () => { it('dispatches ENABLE with merged payload correctly', () => { const pluginConfig = { marker: { symbol: 'pin', backgroundColor: 'blue' } } - const options = { interactionMode: 'select', marker: { symbol: 'circle', backgroundColor: 'green' }, dataLayers: [{ layerId: 'test' }] } + const options = { interactionModes: ['selectFeature'], marker: { symbol: 'circle', backgroundColor: 'green' }, layers: [{ layerId: 'test' }] } enable({ pluginState: { dispatch }, pluginConfig }, options) @@ -18,10 +18,10 @@ describe('enable', () => { expect(dispatch).toHaveBeenCalledWith({ type: 'ENABLE', payload: expect.objectContaining({ - interactionMode: 'select', + interactionModes: ['selectFeature'], multiSelect: DEFAULTS.multiSelect, marker: { symbol: 'circle', backgroundColor: 'green' }, - dataLayers: [{ layerId: 'test' }] + layers: [{ layerId: 'test' }] }) }) }) diff --git a/plugins/interact/src/defaults.js b/plugins/interact/src/defaults.js index 66816f77..f50e3172 100755 --- a/plugins/interact/src/defaults.js +++ b/plugins/interact/src/defaults.js @@ -1,6 +1,6 @@ export const DEFAULTS = { tolerance: 10, - interactionMode: 'marker', + interactionModes: ['selectMarker'], multiSelect: false, contiguous: false, deselectOnClickOutside: false, diff --git a/plugins/interact/src/events.js b/plugins/interact/src/events.js index eb14edab..b50570d4 100755 --- a/plugins/interact/src/events.js +++ b/plugins/interact/src/events.js @@ -1,3 +1,10 @@ +const buildDonePayload = (coords, selectedFeatures, selectedMarkers, selectionBounds) => ({ + ...(coords && { coords }), + ...(!coords && selectedFeatures && { selectedFeatures }), + ...(!coords && selectedMarkers?.length && { selectedMarkers }), + ...(!coords && selectionBounds && { selectionBounds }) +}) + // Helper for feature toggling logic const createFeatureHandler = (mapState, getPluginState) => (args, addToExisting) => { const pluginState = getPluginState() @@ -48,17 +55,13 @@ export function attachEvents ({ const pluginState = getPluginState() const marker = mapState.markers.getMarker('location') const { coords } = marker || {} - const { selectionBounds, selectedFeatures } = pluginState + const { selectionBounds, selectedFeatures, selectedMarkers } = pluginState if (getAppState().disabledButtons.has('selectDone')) { return } - eventBus.emit('interact:done', { - ...(coords && { coords }), - ...(!coords && selectedFeatures && { selectedFeatures }), - ...(!coords && selectionBounds && { selectionBounds }) - }) + eventBus.emit('interact:done', buildDonePayload(coords, selectedFeatures, selectedMarkers, selectionBounds)) if (pluginState.closeOnAction ?? true) { closeApp() diff --git a/plugins/interact/src/events.test.js b/plugins/interact/src/events.test.js index 910aedb3..4db4313d 100644 --- a/plugins/interact/src/events.test.js +++ b/plugins/interact/src/events.test.js @@ -172,14 +172,11 @@ describe('attachEvents', () => { Object.values(params.buttonConfig).forEach(btn => expect(btn.onClick).toBeNull()) }) - it('selectDone handles emission when no marker/coords exist', () => { + it('selectDone emits selectedFeatures and selectionBounds when no marker/coords', () => { const params = createParams() cleanup = attachEvents(params) - // Ensure marker returns null (no coords) params.mapState.markers.getMarker.mockReturnValue(null) - - // Set up features and bounds params.pluginState.selectedFeatures = [{ id: 'f1' }] params.pluginState.selectionBounds = { sw: [0, 0], ne: [1, 1] } @@ -191,6 +188,33 @@ describe('attachEvents', () => { }) }) + it('selectDone includes selectedMarkers in payload when present', () => { + const params = createParams() + cleanup = attachEvents(params) + + params.mapState.markers.getMarker.mockReturnValue(null) + params.pluginState.selectedMarkers = ['m1', 'm2'] + + params.buttonConfig.selectDone.onClick() + + expect(params.eventBus.emit).toHaveBeenCalledWith('interact:done', + expect.objectContaining({ selectedMarkers: ['m1', 'm2'] }) + ) + }) + + it('selectDone omits selectedMarkers from payload when empty', () => { + const params = createParams() + cleanup = attachEvents(params) + + params.mapState.markers.getMarker.mockReturnValue(null) + params.pluginState.selectedMarkers = [] + + params.buttonConfig.selectDone.onClick() + + const payload = params.eventBus.emit.mock.calls.find(c => c[0] === 'interact:done')[1] + expect(payload).not.toHaveProperty('selectedMarkers') + }) + it('respects default closeOnAction when value is undefined (fallback to true)', () => { const params = createParams() // Explicitly set to undefined to trigger the ?? fallback diff --git a/plugins/interact/src/hooks/useHighlightSync.js b/plugins/interact/src/hooks/useHighlightSync.js index 0dfca13f..800769e3 100755 --- a/plugins/interact/src/hooks/useHighlightSync.js +++ b/plugins/interact/src/hooks/useHighlightSync.js @@ -10,15 +10,15 @@ export const useHighlightSync = ({ events, eventBus }) => { - const { dataLayers } = pluginState + const { layers } = pluginState // Memoize stylesMap so it only recalculates when style or layers change const stylesMap = useMemo(() => { if (!mapStyle) { return null } - return buildStylesMap(dataLayers, mapStyle) - }, [dataLayers, mapStyle]) + return buildStylesMap(layers, mapStyle) + }, [layers, mapStyle]) // Force re-application of all selected features const updateHighlightedFeatures = () => { diff --git a/plugins/interact/src/hooks/useHighlightSync.test.js b/plugins/interact/src/hooks/useHighlightSync.test.js index 560f1a42..3189902d 100644 --- a/plugins/interact/src/hooks/useHighlightSync.test.js +++ b/plugins/interact/src/hooks/useHighlightSync.test.js @@ -23,7 +23,7 @@ describe('useHighlightSync', () => { }, mapStyle: { id: 'default-style' }, pluginState: { - dataLayers: [{ layerId: 'layer1' }] + layers: [{ layerId: 'layer1' }] }, selectedFeatures: [], dispatch: jest.fn(), @@ -93,21 +93,21 @@ describe('useHighlightSync', () => { ) }) - it('rebuilds styles when dataLayers change', () => { + it('rebuilds styles when layers change', () => { mockDeps.selectedFeatures = [{ featureId: 'F1' }] const { rerender } = renderHook( - ({ dataLayers }) => + ({ layers }) => useHighlightSync({ ...mockDeps, - pluginState: { dataLayers } + pluginState: { layers } }), - { initialProps: { dataLayers: [{ layerId: 'layer1' }] } } + { initialProps: { layers: [{ layerId: 'layer1' }] } } ) buildStylesMap.mockClear() - rerender({ dataLayers: [{ layerId: 'layer1' }, { layerId: 'layer2' }] }) + rerender({ layers: [{ layerId: 'layer1' }, { layerId: 'layer2' }] }) expect(buildStylesMap).toHaveBeenCalled() }) diff --git a/plugins/interact/src/hooks/useHoverCursor.js b/plugins/interact/src/hooks/useHoverCursor.js new file mode 100644 index 00000000..71614eba --- /dev/null +++ b/plugins/interact/src/hooks/useHoverCursor.js @@ -0,0 +1,10 @@ +import { useEffect } from 'react' + +export const useHoverCursor = (mapProvider, enabled, interactionModes, layers) => { + useEffect(() => { + const canSelect = enabled && interactionModes?.includes('selectFeature') + const layerIds = canSelect ? layers.map(l => l.layerId) : [] + mapProvider.setHoverCursor?.(layerIds) + return () => mapProvider.setHoverCursor?.([]) + }, [enabled, interactionModes, layers]) +} diff --git a/plugins/interact/src/hooks/useHoverCursor.test.js b/plugins/interact/src/hooks/useHoverCursor.test.js new file mode 100644 index 00000000..981cdd5f --- /dev/null +++ b/plugins/interact/src/hooks/useHoverCursor.test.js @@ -0,0 +1,44 @@ +import { renderHook } from '@testing-library/react' +import { useHoverCursor } from './useHoverCursor.js' + +const makeProvider = () => ({ setHoverCursor: jest.fn() }) + +describe('useHoverCursor', () => { + const dataLayers = [{ layerId: 'layer-a' }, { layerId: 'layer-b' }] + + it('calls setHoverCursor with layer IDs when enabled with selectFeature mode', () => { + const mapProvider = makeProvider() + renderHook(() => useHoverCursor(mapProvider, true, ['selectFeature'], dataLayers)) + expect(mapProvider.setHoverCursor).toHaveBeenCalledWith(['layer-a', 'layer-b']) + }) + + it('calls setHoverCursor with layer IDs when selectFeature is combined with other interactionModes', () => { + const mapProvider = makeProvider() + renderHook(() => useHoverCursor(mapProvider, true, ['selectMarker', 'selectFeature'], dataLayers)) + expect(mapProvider.setHoverCursor).toHaveBeenCalledWith(['layer-a', 'layer-b']) + }) + + it('calls setHoverCursor with empty array when disabled', () => { + const mapProvider = makeProvider() + renderHook(() => useHoverCursor(mapProvider, false, ['selectFeature'], dataLayers)) + expect(mapProvider.setHoverCursor).toHaveBeenCalledWith([]) + }) + + it('calls setHoverCursor with empty array when selectFeature is not in interactionModes', () => { + const mapProvider = makeProvider() + renderHook(() => useHoverCursor(mapProvider, true, ['selectMarker', 'placeMarker'], dataLayers)) + expect(mapProvider.setHoverCursor).toHaveBeenCalledWith([]) + }) + + it('clears cursor on unmount', () => { + const mapProvider = makeProvider() + const { unmount } = renderHook(() => useHoverCursor(mapProvider, true, ['selectFeature'], dataLayers)) + mapProvider.setHoverCursor.mockClear() + unmount() + expect(mapProvider.setHoverCursor).toHaveBeenCalledWith([]) + }) + + it('does not throw when setHoverCursor is absent', () => { + expect(() => renderHook(() => useHoverCursor({}, true, ['selectFeature'], dataLayers))).not.toThrow() + }) +}) diff --git a/plugins/interact/src/hooks/useInteractionHandlers.js b/plugins/interact/src/hooks/useInteractionHandlers.js index 81d5fdd7..7c21677b 100755 --- a/plugins/interact/src/hooks/useInteractionHandlers.js +++ b/plugins/interact/src/hooks/useInteractionHandlers.js @@ -1,8 +1,42 @@ import { useCallback, useEffect, useRef } from 'react' import { isContiguousWithAny, canSplitFeatures, areAllContiguous } from '../utils/spatial.js' import { getFeaturesAtPoint, findMatchingFeature, buildLayerConfigMap } from '../utils/featureQueries.js' +import { scaleFactor } from '../../../../src/config/appConfig.js' + +/** + * Returns the id of the first DOM marker whose visual bounds contain the given point. + * + * MAP_CLICK point is container-relative; getBoundingClientRect is viewport-relative. + * We convert by subtracting the parent element's top-left (markers share a parent with + * the map container, so parentElement.getBoundingClientRect() gives the offset). + * + * @param {Object} markers - markers object from mapState (has .items and .markerRefs) + * @param {{ x: number, y: number }} point - container-relative pixel coordinates + * @param {number} scale - scaleFactor for the current mapSize (e.g. 1.5 for medium) + * @returns {string|null} + */ +const findMarkerAtPoint = (markers, point, scale) => { + for (const marker of markers.items) { + const el = markers.markerRefs?.get(marker.id) + if (!el) { + continue + } + const parent = el.parentElement + const parentRect = parent ? parent.getBoundingClientRect() : { left: 0, top: 0 } + const { left, top, right, bottom } = el.getBoundingClientRect() + const scaledX = point.x * scale + const scaledY = point.y * scale + if ( + scaledX >= left - parentRect.left && scaledX <= right - parentRect.left && + scaledY >= top - parentRect.top && scaledY <= bottom - parentRect.top + ) { + return marker.id + } + } + return null +} -const useSelectionChangeEmitter = (eventBus, selectedFeatures, selectionBounds) => { +const useSelectionChangeEmitter = (eventBus, selectedFeatures, selectedMarkers, selectionBounds) => { const lastEmittedSelectionChange = useRef(null) useEffect(() => { @@ -14,105 +48,113 @@ const useSelectionChangeEmitter = (eventBus, selectedFeatures, selectionBounds) // Skip if selection was already empty and remains empty const prev = lastEmittedSelectionChange.current - const wasEmpty = prev === null || prev.length === 0 - if (wasEmpty && selectedFeatures.length === 0) { + const wasEmpty = prev === null || (prev.features.length === 0 && prev.markers.length === 0) + if (wasEmpty && selectedFeatures.length === 0 && selectedMarkers.length === 0) { return } eventBus.emit('interact:selectionchange', { selectedFeatures, + selectedMarkers, selectionBounds, canMerge: areAllContiguous(selectedFeatures), canSplit: canSplitFeatures(selectedFeatures) }) - lastEmittedSelectionChange.current = selectedFeatures - }, [selectedFeatures, selectionBounds]) + lastEmittedSelectionChange.current = { features: selectedFeatures, markers: selectedMarkers } + }, [selectedFeatures, selectedMarkers, selectionBounds]) } -export const useInteractionHandlers = ({ - mapState, - pluginState, - services, - mapProvider -}) => { - const { markers } = mapState - const { dispatch, dataLayers, interactionMode, multiSelect, contiguous, marker: markerOptions, tolerance, selectedFeatures, selectionBounds, deselectOnClickOutside } = pluginState +/** + * Core interaction hook. Processes map clicks in fixed priority order: + * selectMarker → selectFeature → placeMarker (fallback). + * + * Which steps are active is controlled by `pluginState.interactionModes`. Steps not + * present in the array are skipped entirely — e.g. omitting `'selectMarker'` means + * marker hit-testing is never performed. + * + * @param {Object} deps + * @param {Object} deps.mapState - Map state including markers and mapSize + * @param {Object} deps.pluginState - Plugin state including interactionModes, layers, etc. + * @param {Object} deps.services - Services including eventBus + * @param {Object} deps.mapProvider - Map provider instance for feature queries + * @returns {{ handleInteraction: Function }} + */ +export const useInteractionHandlers = ({ mapState, pluginState, services, mapProvider }) => { + const { markers, mapSize } = mapState + const { dispatch, layers, interactionModes, multiSelect, contiguous, marker: markerOptions, tolerance, selectedFeatures, selectedMarkers, selectionBounds, deselectOnClickOutside } = pluginState const { eventBus } = services - const layerConfigMap = buildLayerConfigMap(dataLayers) - - const handleInteraction = useCallback(({ point, coords }) => { - const allFeatures = getFeaturesAtPoint(mapProvider, point, { radius: tolerance }) - const hasDataLayers = dataLayers.length > 0 - - if (pluginState?.debug) { - console.log(`--- Features at ${coords} ---`, allFeatures) + const layerConfigMap = buildLayerConfigMap(layers) + const scale = scaleFactor[mapSize] ?? 1 + const processFeatureMatch = useCallback(({ feature, config }) => { + markers.remove('location') + const isNewContiguous = contiguous && isContiguousWithAny(feature, selectedFeatures) + const featureId = feature.properties?.[config.idProperty] ?? feature.id + if (featureId == null) { + return } + dispatch({ + type: 'TOGGLE_SELECTED_FEATURES', + payload: { + featureId, + multiSelect, + layerId: config.layerId, + idProperty: config.idProperty, + properties: feature.properties, + geometry: feature.geometry, + replaceAll: contiguous && !isNewContiguous + } + }) + }, [markers, contiguous, selectedFeatures, dispatch, multiSelect]) - const canMatch = hasDataLayers && (interactionMode === 'select' || interactionMode === 'auto') - const match = canMatch ? findMatchingFeature(allFeatures, layerConfigMap) : null - - // 1. Handle Feature Match - if (match) { - processFeatureMatch(match) + const processFallback = useCallback(({ coords }) => { + const canPlace = interactionModes.includes('placeMarker') + if (!canPlace && !deselectOnClickOutside) { return } - - // 2. Handle Marker Mode (Fallback) - const isMarkerMode = interactionMode === 'marker' || (interactionMode === 'auto' && hasDataLayers) - if (isMarkerMode) { - dispatch({ type: 'CLEAR_SELECTED_FEATURES' }) + dispatch({ type: 'CLEAR_SELECTED_FEATURES' }) + if (canPlace) { markers.add('location', coords, markerOptions) eventBus.emit('interact:markerchange', { coords }) - } else if (deselectOnClickOutside) { - dispatch({ type: 'CLEAR_SELECTED_FEATURES' }) - } else { - // No action } + }, [interactionModes, dispatch, markers, markerOptions, eventBus, deselectOnClickOutside]) - // Internal helper to keep complexity low - function processFeatureMatch ({ feature, config }) { - markers.remove('location') - const isNewContiguous = contiguous && isContiguousWithAny(feature, selectedFeatures) - const featureId = feature.properties?.[config.idProperty] ?? feature.id - - if (featureId == null) { + const handleInteraction = useCallback(({ point, coords }) => { + if (interactionModes.includes('selectMarker')) { + const markerHit = findMarkerAtPoint(markers, point, scale) + if (markerHit) { + dispatch({ type: 'TOGGLE_SELECTED_MARKERS', payload: { markerId: markerHit, multiSelect } }) return } + } - dispatch({ - type: 'TOGGLE_SELECTED_FEATURES', - payload: { - featureId, - multiSelect, - layerId: config.layerId, - idProperty: config.idProperty, - properties: feature.properties, - geometry: feature.geometry, - replaceAll: contiguous && !isNewContiguous - } - }) + if (interactionModes.includes('selectFeature') && layers.length > 0) { + const allFeatures = getFeaturesAtPoint(mapProvider, point, { radius: tolerance }) + if (pluginState?.debug) { + console.log(`--- Features at ${coords} ---`, allFeatures) + } + const match = findMatchingFeature(allFeatures, layerConfigMap) + if (match) { + processFeatureMatch(match) + return + } } + + processFallback({ coords }) }, [ mapProvider, - dataLayers, - interactionMode, + layers, + interactionModes, multiSelect, - eventBus, dispatch, markers, - contiguous, - selectedFeatures, layerConfigMap, pluginState?.debug, tolerance, - markerOptions, - deselectOnClickOutside + processFeatureMatch, + processFallback, + scale ]) - - useSelectionChangeEmitter(eventBus, selectedFeatures, selectionBounds) - - return { - handleInteraction - } + useSelectionChangeEmitter(eventBus, selectedFeatures, selectedMarkers, selectionBounds) + return { handleInteraction } } diff --git a/plugins/interact/src/hooks/useInteractionHandlers.test.js b/plugins/interact/src/hooks/useInteractionHandlers.test.js index 5a3ae40f..ee443370 100644 --- a/plugins/interact/src/hooks/useInteractionHandlers.test.js +++ b/plugins/interact/src/hooks/useInteractionHandlers.test.js @@ -30,19 +30,27 @@ const click = result => }) ) -const setup = (pluginOverrides = {}) => { +const makeMarkerEl = (rect) => ({ + getBoundingClientRect: () => rect, + parentElement: { + getBoundingClientRect: () => ({ left: 0, top: 0 }) + } +}) + +const setup = (pluginOverrides = {}, markerItems = [], markerRefs = new Map()) => { const deps = { mapState: { - markers: { add: jest.fn(), remove: jest.fn() } + markers: { add: jest.fn(), remove: jest.fn(), items: markerItems, markerRefs } }, pluginState: { dispatch: jest.fn(), - dataLayers: [{ layerId: 'parcels', idProperty: 'parcelId' }], - interactionMode: 'select', + layers: [{ layerId: 'parcels', idProperty: 'parcelId' }], + interactionModes: ['selectMarker', 'selectFeature'], multiSelect: false, contiguous: false, marker: { symbol: 'pin', backgroundColor: 'red' }, selectedFeatures: [], + selectedMarkers: [], selectionBounds: null, ...pluginOverrides }, @@ -73,12 +81,94 @@ beforeEach(() => { }) /* ------------------------------------------------------------------ */ -/* Marker mode */ +/* DOM marker hit detection */ +/* ------------------------------------------------------------------ */ + +describe('DOM marker hit detection', () => { + it('dispatches TOGGLE_SELECTED_MARKERS when click is within a marker bounds', () => { + const markerEl = makeMarkerEl({ left: 5, top: 15, right: 15, bottom: 25 }) + const markerRefs = new Map([['marker-1', markerEl]]) + const markerItems = [{ id: 'marker-1', coords: [1, 2] }] + + const { result, deps } = setup({}, markerItems, markerRefs) + click(result) + + expect(deps.pluginState.dispatch).toHaveBeenCalledWith({ + type: 'TOGGLE_SELECTED_MARKERS', + payload: { markerId: 'marker-1', multiSelect: false } + }) + }) + + it('markers take precedence over features when both hit', () => { + const markerEl = makeMarkerEl({ left: 5, top: 15, right: 15, bottom: 25 }) + const markerRefs = new Map([['marker-1', markerEl]]) + const markerItems = [{ id: 'marker-1', coords: [1, 2] }] + + const { result, deps } = setup({}, markerItems, markerRefs) + click(result) + + expect(deps.pluginState.dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'TOGGLE_SELECTED_FEATURES' }) + ) + expect(featureQueries.getFeaturesAtPoint).not.toHaveBeenCalled() + }) + + it('skips markers with no ref and continues to next', () => { + // marker in items but not in markerRefs — should not throw, should fall through to features + const markerItems = [{ id: 'marker-no-ref', coords: [1, 2] }] + const markerRefs = new Map() // no entry for marker-no-ref + + const { result, deps } = setup({}, markerItems, markerRefs) + click(result) + + expect(featureQueries.getFeaturesAtPoint).toHaveBeenCalled() + expect(deps.pluginState.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: 'TOGGLE_SELECTED_FEATURES' }) + ) + }) + + it('uses zero offset when marker element has no parentElement', () => { + // parentElement is null — fallback parentRect { left: 0, top: 0 } should be used + const markerEl = { + getBoundingClientRect: () => ({ left: 5, top: 15, right: 15, bottom: 25 }), + parentElement: null + } + const markerRefs = new Map([['marker-1', markerEl]]) + const markerItems = [{ id: 'marker-1', coords: [1, 2] }] + + const { result, deps } = setup({}, markerItems, markerRefs) + click(result) + + // click point { x: 10, y: 20 } is within [5,15,15,25] with zero parent offset + expect(deps.pluginState.dispatch).toHaveBeenCalledWith({ + type: 'TOGGLE_SELECTED_MARKERS', + payload: { markerId: 'marker-1', multiSelect: false } + }) + }) + + it('falls through to feature selection when click misses all markers', () => { + // marker bounds don't include the click point { x: 10, y: 20 } + const markerEl = makeMarkerEl({ left: 50, top: 50, right: 80, bottom: 80 }) + const markerRefs = new Map([['marker-1', markerEl]]) + const markerItems = [{ id: 'marker-1', coords: [1, 2] }] + + const { result, deps } = setup({}, markerItems, markerRefs) + click(result) + + expect(featureQueries.getFeaturesAtPoint).toHaveBeenCalled() + expect(deps.pluginState.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: 'TOGGLE_SELECTED_FEATURES' }) + ) + }) +}) + +/* ------------------------------------------------------------------ */ +/* placeMarker mode */ /* ------------------------------------------------------------------ */ -describe('marker mode', () => { +describe('placeMarker mode', () => { it('places marker, clears selection, and emits event', () => { - const { result, deps } = setup({ interactionMode: 'marker' }) + const { result, deps } = setup({ interactionModes: ['placeMarker'] }) click(result) @@ -98,32 +188,30 @@ describe('marker mode', () => { }) /* ------------------------------------------------------------------ */ -/* Select & Auto modes */ +/* selectFeature mode */ /* ------------------------------------------------------------------ */ -describe.each(['select', 'auto'])('%s mode feature selection', mode => { - it('dispatches selection for matching feature', () => { - const { result, deps } = setup({ interactionMode: mode }) +it('selectFeature mode dispatches selection for matching feature', () => { + const { result, deps } = setup({ interactionModes: ['selectFeature'] }) - click(result) + click(result) - expect(deps.pluginState.dispatch).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'TOGGLE_SELECTED_FEATURES', - payload: expect.objectContaining({ - featureId: 'P1', - layerId: 'parcels' - }) + expect(deps.pluginState.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'TOGGLE_SELECTED_FEATURES', + payload: expect.objectContaining({ + featureId: 'P1', + layerId: 'parcels' }) - ) - }) + }) + ) }) -it('falls back to marker in auto mode when no feature found', () => { +it('falls back to placeMarker when selectFeature finds no match', () => { featureQueries.getFeaturesAtPoint.mockReturnValue([]) featureQueries.findMatchingFeature.mockReturnValue(null) - const { result, deps } = setup({ interactionMode: 'auto' }) + const { result, deps } = setup({ interactionModes: ['selectFeature', 'placeMarker'] }) click(result) @@ -248,32 +336,58 @@ describe('deselectOnClickOutside', () => { }) /* ------------------------------------------------------------------ */ -/* Marker condition guard (FULL COVERAGE) */ +/* placeMarker / selectFeature guards */ /* ------------------------------------------------------------------ */ -it('does NOT place marker in auto mode when no dataLayers exist', () => { +it('places marker with placeMarker mode even when no layers exist', () => { featureQueries.getFeaturesAtPoint.mockReturnValue([]) featureQueries.findMatchingFeature.mockReturnValue(null) const { result, deps } = setup({ - interactionMode: 'auto', - dataLayers: [] + interactionModes: ['selectFeature', 'placeMarker'], + layers: [] }) click(result) + expect(deps.mapState.markers.add).toHaveBeenCalled() +}) + +it('does not place marker when placeMarker is not in interactionModes', () => { + featureQueries.getFeaturesAtPoint.mockReturnValue([]) + featureQueries.findMatchingFeature.mockReturnValue(null) + + const { result, deps } = setup({ interactionModes: ['selectFeature'] }) + + click(result) + expect(deps.mapState.markers.add).not.toHaveBeenCalled() }) +it('does not check markers when selectMarker is not in interactionModes', () => { + const markerEl = makeMarkerEl({ left: 5, top: 15, right: 15, bottom: 25 }) + const markerRefs = new Map([['marker-1', markerEl]]) + const markerItems = [{ id: 'marker-1', coords: [1, 2] }] + + const { result, deps } = setup({ interactionModes: ['selectFeature'] }, markerItems, markerRefs) + click(result) + + expect(deps.pluginState.dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'TOGGLE_SELECTED_MARKERS' }) + ) + expect(featureQueries.getFeaturesAtPoint).toHaveBeenCalled() +}) + /* ------------------------------------------------------------------ */ /* Selection change event */ /* ------------------------------------------------------------------ */ it('emits selectionchange once when bounds exist', () => { const deps = { - mapState: { markers: { add: jest.fn(), remove: jest.fn() } }, + mapState: { markers: { add: jest.fn(), remove: jest.fn(), items: [], markerRefs: new Map() } }, pluginState: { selectedFeatures: [{ featureId: 'F1' }], + selectedMarkers: [], selectionBounds: { sw: [0, 0], ne: [1, 1] } }, services: { eventBus: { emit: jest.fn() } }, @@ -286,6 +400,7 @@ it('emits selectionchange once when bounds exist', () => { 'interact:selectionchange', expect.objectContaining({ selectedFeatures: deps.pluginState.selectedFeatures, + selectedMarkers: [], selectionBounds: deps.pluginState.selectionBounds, canMerge: false, canSplit: false @@ -299,8 +414,8 @@ it('skips emission when selection remains empty after being cleared', () => { // 1. First render with a feature (prev is null, emission happens) const { rerender } = renderHook( ({ features }) => useInteractionHandlers({ - mapState: { markers: {} }, - pluginState: { selectedFeatures: features, selectionBounds: { b: 1 } }, + mapState: { markers: { items: [], markerRefs: new Map() } }, + pluginState: { selectedFeatures: features, selectedMarkers: [], selectionBounds: { b: 1 } }, services: { eventBus }, mapProvider: {} }), diff --git a/plugins/interact/src/reducer.js b/plugins/interact/src/reducer.js index 3e4f469f..e8b53e72 100755 --- a/plugins/interact/src/reducer.js +++ b/plugins/interact/src/reducer.js @@ -1,12 +1,13 @@ const initialState = { enabled: false, - dataLayers: [], + layers: [], marker: null, - interactionMode: null, + interactionModes: null, multiSelect: false, contiguous: false, deselectOnClickOutside: false, selectedFeatures: [], + selectedMarkers: [], selectionBounds: null, closeOnAction: true // Done or Cancel } @@ -24,6 +25,7 @@ const disable = (state) => { ...state, enabled: false, selectedFeatures: [], + selectedMarkers: [], selectionBounds: null } } @@ -67,7 +69,7 @@ const toggleSelectedFeatures = (state, payload) => { nextSelected = isSameSingle ? [] : [featureObj] } - return { ...state, selectedFeatures: nextSelected, selectionBounds: null } + return { ...state, selectedFeatures: nextSelected, selectedMarkers: multiSelect && !replaceAll ? state.selectedMarkers : [], selectionBounds: null } } // Update bounds (called from useEffect after map provider calculates them) @@ -81,10 +83,26 @@ const updateSelectedBounds = (state, payload) => { } } +const toggleSelectedMarkers = (state, { markerId, multiSelect }) => { + const current = state.selectedMarkers + const exists = current.includes(markerId) + if (multiSelect) { + const next = exists ? current.filter(id => id !== markerId) : [...current, markerId] + return { ...state, selectedMarkers: next } + } + return { + ...state, + selectedFeatures: [], + selectionBounds: null, + selectedMarkers: exists && current.length === 1 ? [] : [markerId] + } +} + const clearSelectedFeatures = (state) => { return { ...state, selectedFeatures: [], + selectedMarkers: [], selectionBounds: null } } @@ -93,6 +111,7 @@ const actions = { ENABLE: enable, DISABLE: disable, TOGGLE_SELECTED_FEATURES: toggleSelectedFeatures, + TOGGLE_SELECTED_MARKERS: toggleSelectedMarkers, UPDATE_SELECTED_BOUNDS: updateSelectedBounds, CLEAR_SELECTED_FEATURES: clearSelectedFeatures } diff --git a/plugins/interact/src/reducer.test.js b/plugins/interact/src/reducer.test.js index 67c66667..108686d4 100644 --- a/plugins/interact/src/reducer.test.js +++ b/plugins/interact/src/reducer.test.js @@ -4,13 +4,14 @@ describe('initialState', () => { it('has correct defaults', () => { expect(initialState).toEqual({ enabled: false, - dataLayers: [], + layers: [], marker: null, - interactionMode: null, + interactionModes: null, multiSelect: false, contiguous: false, deselectOnClickOutside: false, selectedFeatures: [], + selectedMarkers: [], selectionBounds: null, closeOnAction: true }) @@ -21,24 +22,25 @@ describe('ENABLE/DISABLE actions', () => { it('ENABLE sets enabled and merges payload', () => { const state = { ...initialState, enabled: false } const marker = { symbol: 'pin', backgroundColor: 'red' } - const payload = { dataLayers: [1], marker } + const payload = { layers: [1], marker } const result = actions.ENABLE(state, payload) expect(result.enabled).toBe(true) - expect(result.dataLayers).toEqual([1]) + expect(result.layers).toEqual([1]) expect(result.marker).toEqual(marker) expect(result).not.toBe(state) }) - it('DISABLE sets enabled to false, clears selection, and preserves other state', () => { + it('DISABLE sets enabled to false, clears selection and markers, and preserves other state', () => { const marker = { symbol: 'pin', backgroundColor: 'red' } - const state = { ...initialState, enabled: true, dataLayers: [1], marker, selectedFeatures: [{ featureId: 'f1' }], selectionBounds: [0, 0, 1, 1] } + const state = { ...initialState, enabled: true, layers: [1], marker, selectedFeatures: [{ featureId: 'f1' }], selectedMarkers: ['m1'], selectionBounds: [0, 0, 1, 1] } const result = actions.DISABLE(state) expect(result.enabled).toBe(false) - expect(result.dataLayers).toEqual([1]) + expect(result.layers).toEqual([1]) expect(result.marker).toEqual(marker) expect(result.selectedFeatures).toEqual([]) + expect(result.selectedMarkers).toEqual([]) expect(result.selectionBounds).toBeNull() expect(result).not.toBe(state) }) @@ -98,6 +100,19 @@ describe('TOGGLE_SELECTED_FEATURES action', () => { expect(state.selectedFeatures[0].featureId).toBe('f3') }) + it('clears selectedMarkers in single-select; preserves them in multi-select', () => { + const state = { ...initialState, selectedMarkers: ['m1'] } + const feature = createFeature('f1') + + // single-select clears markers + const single = actions.TOGGLE_SELECTED_FEATURES(state, feature) + expect(single.selectedMarkers).toEqual([]) + + // multi-select preserves markers + const multi = actions.TOGGLE_SELECTED_FEATURES(state, { ...feature, multiSelect: true }) + expect(multi.selectedMarkers).toEqual(['m1']) + }) + it('handles null or empty selectedFeatures gracefully', () => { let state = { ...initialState, selectedFeatures: null } state = actions.TOGGLE_SELECTED_FEATURES(state, createFeature('f1')) @@ -129,15 +144,46 @@ describe('UPDATE_SELECTED_BOUNDS action', () => { }) }) +describe('TOGGLE_SELECTED_MARKERS action', () => { + it('selects a marker in single-select mode and clears features', () => { + const state = { ...initialState, selectedFeatures: [{ featureId: 'f1' }], selectionBounds: { sw: [0, 0], ne: [1, 1] } } + const result = actions.TOGGLE_SELECTED_MARKERS(state, { markerId: 'm1', multiSelect: false }) + expect(result.selectedMarkers).toEqual(['m1']) + expect(result.selectedFeatures).toEqual([]) + expect(result.selectionBounds).toBeNull() + }) + + it('toggles off the only selected marker in single-select mode', () => { + const state = { ...initialState, selectedMarkers: ['m1'] } + const result = actions.TOGGLE_SELECTED_MARKERS(state, { markerId: 'm1', multiSelect: false }) + expect(result.selectedMarkers).toEqual([]) + }) + + it('adds a marker in multi-select mode without clearing features', () => { + const state = { ...initialState, selectedFeatures: [{ featureId: 'f1' }], selectedMarkers: ['m1'] } + const result = actions.TOGGLE_SELECTED_MARKERS(state, { markerId: 'm2', multiSelect: true }) + expect(result.selectedMarkers).toEqual(['m1', 'm2']) + expect(result.selectedFeatures).toEqual([{ featureId: 'f1' }]) + }) + + it('removes a marker in multi-select mode', () => { + const state = { ...initialState, selectedMarkers: ['m1', 'm2'] } + const result = actions.TOGGLE_SELECTED_MARKERS(state, { markerId: 'm1', multiSelect: true }) + expect(result.selectedMarkers).toEqual(['m2']) + }) +}) + describe('CLEAR_SELECTED_FEATURES action', () => { - it('resets selection and bounds', () => { + it('resets features, markers and bounds', () => { const state = { ...initialState, selectedFeatures: [1], + selectedMarkers: ['m1'], selectionBounds: { sw: [0, 0], ne: [1, 1] } } const result = actions.CLEAR_SELECTED_FEATURES(state) expect(result.selectedFeatures).toEqual([]) + expect(result.selectedMarkers).toEqual([]) expect(result.selectionBounds).toBeNull() expect(result).not.toBe(state) }) @@ -149,6 +195,7 @@ describe('actions object', () => { 'ENABLE', 'DISABLE', 'TOGGLE_SELECTED_FEATURES', + 'TOGGLE_SELECTED_MARKERS', 'UPDATE_SELECTED_BOUNDS', 'CLEAR_SELECTED_FEATURES' ]) diff --git a/providers/maplibre/src/maplibreProvider.js b/providers/maplibre/src/maplibreProvider.js index cd8f167f..cdb13bb0 100755 --- a/providers/maplibre/src/maplibreProvider.js +++ b/providers/maplibre/src/maplibreProvider.js @@ -12,6 +12,7 @@ import { getAreaDimensions, getCardinalMove, getBboxFromGeoJSON, isGeometryObscu import { createMapLabelNavigator } from './utils/labels.js' import { updateHighlightedFeatures } from './utils/highlightFeatures.js' import { queryFeatures } from './utils/queryFeatures.js' +import { setupHoverCursor } from './utils/hoverCursor.js' import { registerSymbols } from './utils/symbolImages.js' import { registerPatterns } from './utils/patternImages.js' @@ -116,6 +117,7 @@ export default class MapLibreProvider { /** Destroy the map and clean up resources. */ destroyMap () { + this.setHoverCursor([]) this.mapEvents?.remove() this.appEvents?.remove() @@ -125,6 +127,19 @@ export default class MapLibreProvider { this.map.remove() } + /** + * Set pointer cursor on the map canvas when hovering over any of the given layer IDs. + * Call with an empty array to remove all hover cursor listeners. + * + * @param {string[]} layerIds + */ + setHoverCursor (layerIds) { + if (!this.map) { + return + } + this._onHoverMove = setupHoverCursor(this.map, layerIds, this._onHoverMove) + } + // ========================== // Side-effects // ========================== diff --git a/providers/maplibre/src/maplibreProvider.test.js b/providers/maplibre/src/maplibreProvider.test.js index 85a6a956..317856c3 100644 --- a/providers/maplibre/src/maplibreProvider.test.js +++ b/providers/maplibre/src/maplibreProvider.test.js @@ -58,7 +58,10 @@ describe('MapLibreProvider', () => { getCenter: jest.fn(() => ({ lng: 1.2345678, lat: 2.3456789 })), getZoom: jest.fn(() => 10), getBounds: jest.fn(() => ({ toArray: jest.fn(() => [[0, 0], [1, 1]]) })), - getPixelRatio: jest.fn(() => 1) + getPixelRatio: jest.fn(() => 1), + getCanvas: jest.fn(() => ({ style: {} })), + getLayer: jest.fn(() => true), + queryRenderedFeatures: jest.fn(() => []) } eventBus = { emit: jest.fn() } maplibreModule = { Map: jest.fn(() => map), LngLatBounds: jest.fn() } @@ -264,6 +267,63 @@ describe('MapLibreProvider', () => { expect(registerPatterns).toHaveBeenCalledWith(map, configs, 'test', registry) }) + describe('setHoverCursor', () => { + test('registers mousemove handler on the map when layerIds provided', async () => { + const p = makeProvider() + await doInitMap(p) + p.setHoverCursor(['layer-a']) + expect(map.on).toHaveBeenCalledWith('mousemove', expect.any(Function)) + }) + + test('sets cursor to pointer when queryRenderedFeatures returns a hit', async () => { + const canvas = { style: {} } + map.getCanvas.mockReturnValue(canvas) + map.queryRenderedFeatures.mockReturnValue([{ id: 'f1' }]) + + const p = makeProvider() + await doInitMap(p) + p.setHoverCursor(['layer-a']) + + const moveHandler = map.on.mock.calls.find(([e]) => e === 'mousemove')?.[1] + moveHandler({ point: { x: 10, y: 20 } }) + + expect(canvas.style.cursor).toBe('pointer') + }) + + test('clears cursor when queryRenderedFeatures returns no hit', async () => { + const canvas = { style: { cursor: 'pointer' } } + map.getCanvas.mockReturnValue(canvas) + map.queryRenderedFeatures.mockReturnValue([]) + + const p = makeProvider() + await doInitMap(p) + p.setHoverCursor(['layer-a']) + + const moveHandler = map.on.mock.calls.find(([e]) => e === 'mousemove')?.[1] + moveHandler({ point: { x: 10, y: 20 } }) + + expect(canvas.style.cursor).toBe('') + }) + + test('removes mousemove handler and clears cursor when called with empty array', async () => { + const canvas = { style: { cursor: 'pointer' } } + map.getCanvas.mockReturnValue(canvas) + + const p = makeProvider() + await doInitMap(p) + p.setHoverCursor(['layer-a']) + p.setHoverCursor([]) + + expect(map.off).toHaveBeenCalledWith('mousemove', expect.any(Function)) + expect(canvas.style.cursor).toBe('') + }) + + test('does nothing when map is not initialised', () => { + const p = makeProvider() + expect(() => p.setHoverCursor(['layer-a'])).not.toThrow() + }) + }) + test('label methods return null without labelNavigator; delegate when set', async () => { const p = makeProvider() await doInitMap(p) diff --git a/providers/maplibre/src/utils/hoverCursor.js b/providers/maplibre/src/utils/hoverCursor.js new file mode 100644 index 00000000..a7c2ff3c --- /dev/null +++ b/providers/maplibre/src/utils/hoverCursor.js @@ -0,0 +1,61 @@ +/** + * Attaches a mousemove listener that changes the map cursor to a pointer when + * hovering over any of the specified layers. + * + * Line layers use a 10px tolerance bbox. Stroke layers that have a companion + * fill layer are skipped — the fill handles hover. Fill and symbol layers use + * exact point hit-testing. + * + * @param {Object} map - MapLibre map instance + * @param {string[]} layerIds - Layer IDs to watch + * @param {Function|null} prevHandler - Previous mousemove handler to remove + * @returns {Function|null} The new handler, or null if layerIds is empty + */ +const splitLayers = (map, layerIds) => { + const lineLayers = [] + const otherLayers = [] + for (const id of layerIds) { + const type = map.getLayer(id).type + if (type === 'line') { + const fillId = id.endsWith('-stroke') ? id.slice(0, -7) : null // NOSONAR + const hasFillCompanion = fillId !== null && layerIds.includes(fillId) + if (!hasFillCompanion) { + lineLayers.push(id) + } + } else { + otherLayers.push(id) + } + } + return { lineLayers, otherLayers } +} + +export const setupHoverCursor = (map, layerIds, prevHandler) => { + const canvas = map.getCanvas() + + if (prevHandler) { + map.off('mousemove', prevHandler) + } + + if (!layerIds?.length) { + canvas.style.cursor = '' + return null + } + + const handler = (e) => { + const existingLayers = layerIds.filter(id => map.getLayer(id)) + if (existingLayers.length === 0) { + canvas.style.cursor = '' + return + } + + const { lineLayers, otherLayers } = splitLayers(map, existingLayers) + const { x, y } = e.point + const bbox = [[x - 10, y - 10], [x + 10, y + 10]] + const lineHit = lineLayers.length > 0 && map.queryRenderedFeatures(bbox, { layers: lineLayers }).length > 0 + const otherHit = otherLayers.length > 0 && map.queryRenderedFeatures(e.point, { layers: otherLayers }).length > 0 + canvas.style.cursor = (lineHit || otherHit) ? 'pointer' : '' + } + + map.on('mousemove', handler) + return handler +} diff --git a/providers/maplibre/src/utils/hoverCursor.test.js b/providers/maplibre/src/utils/hoverCursor.test.js new file mode 100644 index 00000000..c6801bd3 --- /dev/null +++ b/providers/maplibre/src/utils/hoverCursor.test.js @@ -0,0 +1,130 @@ +import { setupHoverCursor } from './hoverCursor.js' + +const makeMap = (layerTypes = {}, queryResults = []) => ({ + getCanvas: () => ({ style: { cursor: '' } }), + getLayer: (id) => layerTypes[id] ? { type: layerTypes[id] } : undefined, + queryRenderedFeatures: jest.fn(() => queryResults), + on: jest.fn(), + off: jest.fn() +}) + +const move = (handler, x = 10, y = 10) => + handler({ point: { x, y } }) + +describe('setupHoverCursor', () => { + /* ------------------------------------------------------------------ */ + /* Setup / teardown */ + /* ------------------------------------------------------------------ */ + + it('returns null and clears cursor when layerIds is empty', () => { + const map = makeMap() + const result = setupHoverCursor(map, [], null) + expect(result).toBeNull() + expect(map.getCanvas().style.cursor).toBe('') + expect(map.on).not.toHaveBeenCalled() + }) + + it('returns null and clears cursor when layerIds is null', () => { + const map = makeMap() + const result = setupHoverCursor(map, null, null) + expect(result).toBeNull() + expect(map.getCanvas().style.cursor).toBe('') + }) + + it('removes previous handler before attaching a new one', () => { + const map = makeMap({ 'layer-a': 'fill' }) + const prev = jest.fn() + setupHoverCursor(map, ['layer-a'], prev) + expect(map.off).toHaveBeenCalledWith('mousemove', prev) + }) + + it('removes previous handler when clearing layers', () => { + const map = makeMap() + const prev = jest.fn() + setupHoverCursor(map, [], prev) + expect(map.off).toHaveBeenCalledWith('mousemove', prev) + }) + + it('attaches a mousemove listener and returns the handler', () => { + const map = makeMap({ 'layer-a': 'fill' }) + const handler = setupHoverCursor(map, ['layer-a'], null) + expect(typeof handler).toBe('function') + expect(map.on).toHaveBeenCalledWith('mousemove', handler) + }) + + /* ------------------------------------------------------------------ */ + /* Mousemove — cursor state */ + /* ------------------------------------------------------------------ */ + + it('sets pointer cursor when a fill layer is hit', () => { + const canvas = { style: { cursor: '' } } + const map = { ...makeMap({ 'fill-layer': 'fill' }, [{ id: 'f1' }]), getCanvas: () => canvas } + const handler = setupHoverCursor(map, ['fill-layer'], null) + move(handler) + expect(canvas.style.cursor).toBe('pointer') + }) + + it('clears cursor when no layers are hit', () => { + const canvas = { style: { cursor: 'pointer' } } + const map = { ...makeMap({ 'fill-layer': 'fill' }, []), getCanvas: () => canvas } + const handler = setupHoverCursor(map, ['fill-layer'], null) + move(handler) + expect(canvas.style.cursor).toBe('') + }) + + it('clears cursor when no registered layers exist on the map', () => { + const canvas = { style: { cursor: 'pointer' } } + const map = { ...makeMap({}, []), getCanvas: () => canvas } + const handler = setupHoverCursor(map, ['missing-layer'], null) + move(handler) + expect(canvas.style.cursor).toBe('') + }) + + /* ------------------------------------------------------------------ */ + /* Line layer tolerance */ + /* ------------------------------------------------------------------ */ + + it('uses bbox query for a pure line layer', () => { + const map = makeMap({ hedge: 'line' }, [{ id: 'f1' }]) + const handler = setupHoverCursor(map, ['hedge'], null) + move(handler, 50, 50) + expect(map.queryRenderedFeatures).toHaveBeenCalledWith( + [[40, 40], [60, 60]], + { layers: ['hedge'] } + ) + }) + + it('sets pointer when line layer is hit via bbox', () => { + const canvas = { style: { cursor: '' } } + const map = { ...makeMap({ hedge: 'line' }, [{ id: 'f1' }]), getCanvas: () => canvas } + const handler = setupHoverCursor(map, ['hedge'], null) + move(handler) + expect(canvas.style.cursor).toBe('pointer') + }) + + /* ------------------------------------------------------------------ */ + /* Stroke + fill companion */ + /* ------------------------------------------------------------------ */ + + it('skips stroke layer when a companion fill layer exists', () => { + const map = makeMap({ poly: 'fill', 'poly-stroke': 'line' }, []) + const handler = setupHoverCursor(map, ['poly', 'poly-stroke'], null) + move(handler) + // Only the fill layer should be queried (exact point), not the stroke + expect(map.queryRenderedFeatures).toHaveBeenCalledTimes(1) + expect(map.queryRenderedFeatures).toHaveBeenCalledWith( + expect.objectContaining({ x: 10, y: 10 }), + { layers: ['poly'] } + ) + }) + + it('does not skip a stroke layer that has no companion fill', () => { + const map = makeMap({ 'hedge-stroke': 'line' }, []) + const handler = setupHoverCursor(map, ['hedge-stroke'], null) + move(handler) + expect(map.queryRenderedFeatures).toHaveBeenCalledWith( + expect.any(Array), + { layers: ['hedge-stroke'] } + ) + }) +}) diff --git a/providers/maplibre/src/utils/queryFeatures.js b/providers/maplibre/src/utils/queryFeatures.js index e9b2a3b0..1e3ab325 100644 --- a/providers/maplibre/src/utils/queryFeatures.js +++ b/providers/maplibre/src/utils/queryFeatures.js @@ -83,12 +83,23 @@ const getMinDistToGeometry = (map, point, geometry) => { */ export const queryFeatures = (map, point, options = {}) => { const { radius = 10 } = options - const queryArea = [[point.x - radius, point.y - radius], [point.x + radius, point.y + radius]] - const rawFeatures = map.queryRenderedFeatures(queryArea) + + const bbox = [[point.x - radius, point.y - radius], [point.x + radius, point.y + radius]] + const rawFeatures = map.queryRenderedFeatures(bbox) if (rawFeatures.length === 0) { return [] } + // For symbol/point features, tolerance must not apply — selection should only + // fire when the click lands within the rendered icon bounds. An exact point + // query uses MapLibre's own icon hit-testing, mirroring the hover cursor behaviour. + const exactFeatureKeys = new Set( + map.queryRenderedFeatures([point.x, point.y]).map(f => { + const rawId = f.id === undefined ? JSON.stringify(f.properties) : f.id + return `${f.layer?.source}:${rawId}` + }) + ) + // Identify layer visual hierarchy const layerStack = [] rawFeatures.forEach(f => { @@ -115,7 +126,24 @@ export const queryFeatures = (map, point, options = {}) => { const clickLngLat = map.unproject(point) const clickPt = [clickLngLat.lng, clickLngLat.lat] - return uniqueFeatures + // Discard features where tolerance should not apply: + // - Polygons: only include if click is geometrically inside + // - Points/symbols: only include if under the exact click point (respects icon bounds) + // - Lines: allowed through — tolerance bbox is intentional for them + const candidates = uniqueFeatures.filter((f) => { + const type = f.geometry.type + if (type.includes('Polygon')) { + const polys = type === 'Polygon' ? [f.geometry.coordinates] : f.geometry.coordinates + return polys.some((ring) => isPointInPolygon(clickPt, ring[0])) + } + if (type === 'Point' || type === 'MultiPoint') { + const rawId = f.id === undefined ? JSON.stringify(f.properties) : f.id + return exactFeatureKeys.has(`${f.layer?.source}:${rawId}`) + } + return true + }) + + return candidates .map((f) => { let score = 0 const type = f.geometry.type @@ -125,18 +153,9 @@ export const queryFeatures = (map, point, options = {}) => { const layerRank = layerStack.indexOf(f.layer.id) score += (layerRank * 1000000) - // PRIORITY 2: CONTAINMENT (Polygon Special Treatment) + // PRIORITY 2: POLYGON BOOST (already filtered to inside-only) if (type.includes('Polygon')) { - const polys = type === 'Polygon' ? [f.geometry.coordinates] : f.geometry.coordinates - const isInside = polys.some((ring) => isPointInPolygon(clickPt, ring[0])) - - if (isInside === true) { - // Massive boost for polygons if we are actually inside them - score -= 500000 // NOSONAR - tolerance used only here - } else { - // If we are outside a polygon, it loses significantly to anything we ARE inside - score += 100000 // NOSONAR - tolerance used only here - } + score -= 500000 // NOSONAR } // PRIORITY 3: DISTANCE (Final Tie-breaker) diff --git a/providers/maplibre/src/utils/queryFeatures.test.js b/providers/maplibre/src/utils/queryFeatures.test.js index b8e3dc67..e2caf410 100644 --- a/providers/maplibre/src/utils/queryFeatures.test.js +++ b/providers/maplibre/src/utils/queryFeatures.test.js @@ -20,7 +20,7 @@ describe('queryFeatures coverage', () => { { type: 'MultiPoint', coords: [[0, 0], [10, 10]], p: { x: 1, y: 0 } }, { type: 'MultiLineString', coords: [[[0, 0], [10, 0]]], p: { x: 5, y: 1 } }, { type: 'Polygon', coords: [[[0, 0], [10, 0], [10, 10], [0, 10], [0, 0]]], p: { x: 5, y: 5 } }, // Inside - { type: 'MultiPolygon', coords: [[[[0, 0], [10, 0], [10, 10], [0, 0]]]], p: { x: 20, y: 20 } }, // Outside + { type: 'MultiPolygon', coords: [[[[0, 0], [10, 0], [10, 10], [0, 0]]]], p: { x: 5, y: 3 } }, // Inside { type: 'Unknown', coords: [], p: { x: 0, y: 0 } } ] @@ -49,12 +49,29 @@ describe('queryFeatures coverage', () => { expect(result.length).toBe(2) expect(result[0].layer.id).toBe('layer-A') // Sorted by layerStack index - // 4. Hit ray-casting intersect logic (Line 42 branch) + // 4. Hit ray-casting intersect logic — point inside the polygon const polyFeat = { layer: { id: 'L' }, geometry: { type: 'Polygon', coordinates: [[[0, 0], [10, 10], [0, 10], [0, 0]]] } } const rayMap = { ...mockMap, queryRenderedFeatures: () => [polyFeat] } - expect(queryFeatures(rayMap, { x: -1, y: 5 }).length).toBe(1) + expect(queryFeatures(rayMap, { x: 2, y: 8 }).length).toBe(1) + + // 5. Outside polygon is filtered out (tolerance only applies to lines) + const outsideMap = { ...mockMap, queryRenderedFeatures: () => [polyFeat] } + expect(queryFeatures(outsideMap, { x: -1, y: 5 }).length).toBe(0) + + // 6. Symbol under exact click point is included + const symbolFeat = { id: 'sym', layer: { id: 'S', source: 'src' }, geometry: { type: 'Point', coordinates: [0, 0] } } + const symbolMap = { ...mockMap, queryRenderedFeatures: () => [symbolFeat] } // both calls return it + expect(queryFeatures(symbolMap, { x: 5, y: 5 }).length).toBe(1) + + // 7. Symbol NOT under exact click point is filtered out + let call = 0 + const symbolMissMap = { + ...mockMap, + queryRenderedFeatures: () => call++ === 0 ? [symbolFeat] : [] // bbox returns it, exact does not + } + expect(queryFeatures(symbolMissMap, { x: 5, y: 5 }).length).toBe(0) }) }) diff --git a/src/App/components/Markers/Markers.jsx b/src/App/components/Markers/Markers.jsx index c165143f..de57a90c 100755 --- a/src/App/components/Markers/Markers.jsx +++ b/src/App/components/Markers/Markers.jsx @@ -1,8 +1,10 @@ +import { useEffect, useRef, useState } from 'react' import { useMarkers } from '../../hooks/useMarkersAPI.js' import { useConfig } from '../../store/configContext.js' import { useMap } from '../../store/mapContext.js' import { useService } from '../../store/serviceContext.js' import { stringToKebab } from '../../../utils/stringToKebab.js' +import { scaleFactor } from '../../../config/appConfig.js' // Marker properties handled internally — excluded from style value resolution const INTERNAL_KEYS = new Set(['id', 'coords', 'x', 'y', 'isVisible', 'symbol', 'symbolSvgContent', 'viewBox', 'anchor', 'selectedColor', 'selectedWidth']) @@ -22,13 +24,67 @@ const resolveViewBox = (marker, defaults, symbolDef) => const resolveAnchor = (marker, defaults, symbolDef) => marker.anchor ?? defaults.anchor ?? symbolDef?.anchor ?? [0.5, 0.5] +/** + * When the interact plugin is active, watch mousemove to set cursor:pointer whenever + * the mouse is over one of the marker SVG elements (which are pointer-events:none). + */ +const useMarkerCursor = (markers, interactActive, viewportRef) => { + useEffect(() => { + if (!interactActive) { + return undefined + } + const viewport = viewportRef.current + if (!viewport) { + return undefined + } + const onMove = (e) => { + const hit = markers.items.some(marker => { + const el = markers.markerRefs?.get(marker.id) + if (!el) { + return false + } + const { left, top, right, bottom } = el.getBoundingClientRect() + return e.clientX >= left && e.clientX <= right && e.clientY >= top && e.clientY <= bottom + }) + viewport.style.cursor = hit ? 'pointer' : '' + } + viewport.addEventListener('mousemove', onMove) + return () => { + viewport.removeEventListener('mousemove', onMove) + viewport.style.cursor = '' + } + }, [markers, interactActive, viewportRef]) +} + // eslint-disable-next-line camelcase, react/jsx-pascal-case // sonarjs/disable-next-line function-name export const Markers = () => { const { id } = useConfig() - const { mapStyle } = useMap() + const { mapStyle, mapSize } = useMap() const { markers, markerRef } = useMarkers() - const { symbolRegistry } = useService() + const { symbolRegistry, eventBus } = useService() + + const [interactActive, setInteractActive] = useState(false) + const [selectedMarkers, setSelectedMarkers] = useState([]) + const viewportRef = useRef(null) + + useEffect(() => { + const handleActive = ({ active }) => setInteractActive(active) + const handleSelectionChange = ({ selectedMarkers: next = [] }) => setSelectedMarkers(next) + eventBus.on('interact:active', handleActive) + eventBus.on('interact:selectionchange', handleSelectionChange) + return () => { + eventBus.off('interact:active', handleActive) + eventBus.off('interact:selectionchange', handleSelectionChange) + } + }, [eventBus]) + + // Resolve viewport element once on mount for cursor tracking + useEffect(() => { + viewportRef.current = document.querySelector('.im-c-viewport') + }, []) + + useMarkerCursor(markers, interactActive, viewportRef) if (!mapStyle) { return undefined @@ -44,27 +100,37 @@ export const Markers = () => { const styleValues = Object.fromEntries( Object.entries(marker).filter(([k]) => !INTERNAL_KEYS.has(k)) ) - const resolvedSvg = symbolRegistry.resolve(symbolDef, styleValues, mapStyle) + const isSelected = selectedMarkers.includes(marker.id) + const resolvedSvg = isSelected + ? symbolRegistry.resolveSelected(symbolDef, styleValues, mapStyle) + : symbolRegistry.resolve(symbolDef, styleValues, mapStyle) const viewBox = resolveViewBox(marker, defaults, symbolDef) const [,, svgWidth, svgHeight] = viewBox.split(' ').map(Number) const anchor = resolveAnchor(marker, defaults, symbolDef) const shapeId = marker.symbol || defaults.symbol + const scale = scaleFactor[mapSize] ?? 1 + const scaledWidth = svgWidth * scale + const scaledHeight = svgHeight * scale return ( diff --git a/src/App/components/Markers/Markers.module.scss b/src/App/components/Markers/Markers.module.scss index 34b50891..7de96c06 100755 --- a/src/App/components/Markers/Markers.module.scss +++ b/src/App/components/Markers/Markers.module.scss @@ -7,6 +7,6 @@ position: absolute; left: 0; top: 0; - margin: -17px 0 0 -19px; pointer-events: none; } + diff --git a/src/App/components/Markers/Markers.test.jsx b/src/App/components/Markers/Markers.test.jsx new file mode 100644 index 00000000..991cc212 --- /dev/null +++ b/src/App/components/Markers/Markers.test.jsx @@ -0,0 +1,232 @@ +import { render, act } from '@testing-library/react' +import { Markers } from './Markers.jsx' +import { useMarkers } from '../../hooks/useMarkersAPI.js' +import { useConfig } from '../../store/configContext.js' +import { useMap } from '../../store/mapContext.js' +import { useService } from '../../store/serviceContext.js' + +jest.mock('../../hooks/useMarkersAPI.js', () => ({ useMarkers: jest.fn() })) +jest.mock('../../store/configContext.js', () => ({ useConfig: jest.fn() })) +jest.mock('../../store/mapContext.js', () => ({ useMap: jest.fn() })) +jest.mock('../../store/serviceContext.js', () => ({ useService: jest.fn() })) +jest.mock('../../../config/appConfig.js', () => ({ scaleFactor: { small: 1, medium: 1.5, large: 2 } })) + +const makeEventBus = () => { + const listeners = {} + return { + on: jest.fn((e, fn) => { listeners[e] = fn }), + off: jest.fn(), + emit: (e, payload) => listeners[e]?.(payload) + } +} + +const makeSymbolRegistry = (overrides = {}) => ({ + get: jest.fn(() => ({ svg: '', viewBox: '0 0 38 38', anchor: [0.5, 1] })), + getDefaults: jest.fn(() => ({ symbol: 'pin', viewBox: '0 0 38 38', anchor: [0.5, 1] })), + resolve: jest.fn(() => ''), + resolveSelected: jest.fn(() => ''), + ...overrides +}) + +const makeMarker = (overrides = {}) => ({ + id: 'marker-1', isVisible: true, symbol: 'pin', ...overrides +}) + +const setup = ({ markers = [], mapSize = 'small', eventBus, symbolRegistry, mapStyle = 'outdoor' } = {}) => { + const eb = eventBus ?? makeEventBus() + const sr = symbolRegistry ?? makeSymbolRegistry() + const markerRefs = new Map() + useConfig.mockReturnValue({ id: 'test-app' }) + useMap.mockReturnValue({ mapStyle, mapSize }) + useService.mockReturnValue({ symbolRegistry: sr, eventBus: eb }) + useMarkers.mockReturnValue({ + markers: { items: markers, markerRefs }, + markerRef: (id) => (el) => { if (el) markerRefs.set(id, el) } + }) + return { eb, sr, result: render() } +} + +describe('Markers', () => { + it('renders nothing when mapStyle is not set', () => { + expect(setup({ mapStyle: null }).result.container.firstChild).toBeNull() + }) + + it('renders nothing when there are no markers', () => { + expect(setup().result.container.querySelectorAll('svg')).toHaveLength(0) + }) + + it('renders one svg per marker with correct id and classes', () => { + const { result } = setup({ markers: [makeMarker(), makeMarker({ id: 'b', symbol: undefined })] }) + const [svg1, svg2] = result.container.querySelectorAll('svg') + expect(svg1.getAttribute('id')).toBe('test-app-marker-marker-1') + expect(svg1).toHaveClass('im-c-marker', 'im-c-marker--pin') + expect(svg2).toHaveClass('im-c-marker--pin') + }) + + it.each([ + [true, 'block'], + [false, 'none'] + ])('display is %s when isVisible=%s', (isVisible, display) => { + const svg = setup({ markers: [makeMarker({ isVisible })] }).result.container.querySelector('svg') + expect(svg).toHaveStyle({ display }) + }) + + it('uses inline symbolSvgContent over the symbol registry', () => { + const sr = makeSymbolRegistry() + setup({ markers: [makeMarker({ symbolSvgContent: '' })], symbolRegistry: sr }) + expect(sr.get).not.toHaveBeenCalled() + }) + + it('falls back to defaults.symbolSvgContent', () => { + const sr = makeSymbolRegistry({ + getDefaults: jest.fn(() => ({ symbolSvgContent: '', viewBox: '0 0 38 38', anchor: [0.5, 1] })) + }) + setup({ markers: [makeMarker({ symbol: undefined })], symbolRegistry: sr }) + expect(sr.get).not.toHaveBeenCalled() + }) + + it('uses marker.viewBox when provided', () => { + const svg = setup({ markers: [makeMarker({ viewBox: '0 0 50 60' })] }).result.container.querySelector('svg') + expect(svg.getAttribute('viewBox')).toBe('0 0 50 60') + expect(svg.getAttribute('width')).toBe('50') + expect(svg.getAttribute('height')).toBe('60') + }) + + it("falls back to '0 0 38 38' viewBox when none is provided", () => { + const sr = makeSymbolRegistry({ + get: jest.fn(() => ({ svg: '' })), + getDefaults: jest.fn(() => ({ symbol: 'pin' })) + }) + expect(setup({ markers: [makeMarker()], symbolRegistry: sr }).result.container.querySelector('svg').getAttribute('viewBox')).toBe('0 0 38 38') + }) + + it.each([ + ['marker.anchor', makeMarker({ anchor: [0, 0] }), null, '0px', '0px'], + ['symbolDef.anchor', makeMarker(), { get: jest.fn(() => ({ svg: '', viewBox: '0 0 38 38', anchor: [0, 0.5] })), getDefaults: jest.fn(() => ({ symbol: 'pin', viewBox: '0 0 38 38' })) }, '0px', '-19px'], + ['[0.5, 0.5] fallback', makeMarker(), { get: jest.fn(() => ({ svg: '', viewBox: '0 0 38 38' })), getDefaults: jest.fn(() => ({ symbol: 'pin', viewBox: '0 0 38 38' })) }, '-19px', '-19px'] + ])('resolveAnchor uses %s', (_, marker, srOverrides, left, top) => { + const sr = srOverrides ? makeSymbolRegistry(srOverrides) : undefined + expect(setup({ markers: [marker], symbolRegistry: sr }).result.container.querySelector('svg')).toHaveStyle({ marginLeft: left, marginTop: top }) + }) + + it.each([ + ['small', '38', '38'], + ['medium', '57', '57'], + ['large', '76', '76'], + ['huge', '38', '38'] + ])('scales svg dimensions for mapSize=%s', (mapSize, width, height) => { + const svg = setup({ markers: [makeMarker()], mapSize }).result.container.querySelector('svg') + expect(svg.getAttribute('width')).toBe(width) + expect(svg.getAttribute('height')).toBe(height) + }) + + it('scales anchor offsets for medium mapSize', () => { + expect(setup({ markers: [makeMarker()], mapSize: 'medium' }).result.container.querySelector('svg')) + .toHaveStyle({ marginLeft: '-28.5px', marginTop: '-57px' }) + }) + + it('adds selected class and calls resolveSelected when marker is selected', () => { + const { eb, sr, result } = setup({ markers: [makeMarker()] }) + act(() => eb.emit('interact:selectionchange', { selectedMarkers: ['marker-1'] })) + expect(result.container.querySelector('svg')).toHaveClass('im-c-marker--selected') + expect(sr.resolveSelected).toHaveBeenCalled() + expect(sr.resolve).not.toHaveBeenCalledAfter?.(sr.resolveSelected) + }) + + it('uses resolve (not resolveSelected) for unselected markers', () => { + const { sr } = setup({ markers: [makeMarker()] }) + expect(sr.resolve).toHaveBeenCalled() + expect(sr.resolveSelected).not.toHaveBeenCalled() + }) + + it.each([ + ['explicit empty array', { selectedMarkers: [] }], + ['missing selectedMarkers key', {}] + ])('deselects when selectionchange has %s', (_, payload) => { + const { eb, result } = setup({ markers: [makeMarker()] }) + act(() => eb.emit('interact:selectionchange', { selectedMarkers: ['marker-1'] })) + act(() => eb.emit('interact:selectionchange', payload)) + expect(result.container.querySelector('svg')).not.toHaveClass('im-c-marker--selected') + }) + + it('wires interact:active and interact:selectionchange on mount and removes them on unmount', () => { + const { eb, result } = setup() + expect(eb.on).toHaveBeenCalledWith('interact:active', expect.any(Function)) + expect(eb.on).toHaveBeenCalledWith('interact:selectionchange', expect.any(Function)) + result.unmount() + expect(eb.off).toHaveBeenCalledWith('interact:active', expect.any(Function)) + expect(eb.off).toHaveBeenCalledWith('interact:selectionchange', expect.any(Function)) + }) + + describe('useMarkerCursor', () => { + let viewport + + beforeEach(() => { + viewport = document.createElement('div') + viewport.className = 'im-c-viewport' + document.body.appendChild(viewport) + }) + + afterEach(() => { + if (viewport.parentNode) document.body.removeChild(viewport) + }) + + const activate = (eb) => act(() => eb.emit('interact:active', { active: true })) + const deactivate = (eb) => act(() => eb.emit('interact:active', { active: false })) + const fireMove = (clientX, clientY) => act(() => { + viewport.dispatchEvent(new MouseEvent('mousemove', { bubbles: true, clientX, clientY })) + }) + + const setupCursor = (markerBounds) => { + const eb = makeEventBus() + const markerRefs = new Map() + if (markerBounds) markerRefs.set('marker-1', { getBoundingClientRect: () => markerBounds }) + useConfig.mockReturnValue({ id: 'test-app' }) + useMap.mockReturnValue({ mapStyle: 'outdoor', mapSize: 'small' }) + useService.mockReturnValue({ symbolRegistry: makeSymbolRegistry(), eventBus: eb }) + useMarkers.mockReturnValue({ markers: { items: [makeMarker()], markerRefs }, markerRef: () => () => {} }) + render() + return eb + } + + it('does not track mousemove when interactActive is false', () => { + setupCursor({ left: 0, top: 0, right: 50, bottom: 50 }) + fireMove(20, 20) + expect(viewport.style.cursor).toBe('') + }) + + it('does not track mousemove when viewport element is absent', () => { + document.body.removeChild(viewport) + const eb = setupCursor({ left: 0, top: 0, right: 50, bottom: 50 }) + activate(eb) + expect(viewport.style.cursor).toBe('') + }) + + it('sets cursor to pointer when mousemove lands inside a marker', () => { + const eb = setupCursor({ left: 10, top: 10, right: 50, bottom: 50 }) + activate(eb) + fireMove(20, 20) + expect(viewport.style.cursor).toBe('pointer') + }) + + it.each([ + ['outside all markers', 100, 100], + ['marker has no ref element', 20, 20] + ])('cursor stays empty when %s', (label, x, y) => { + const bounds = label.includes('no ref') ? null : { left: 10, top: 10, right: 50, bottom: 50 } + const eb = setupCursor(bounds) + activate(eb) + fireMove(x, y) + expect(viewport.style.cursor).toBe('') + }) + + it('clears cursor and stops tracking when interact becomes inactive', () => { + const eb = setupCursor({ left: 10, top: 10, right: 50, bottom: 50 }) + activate(eb) + fireMove(20, 20) + expect(viewport.style.cursor).toBe('pointer') + deactivate(eb) + expect(viewport.style.cursor).toBe('') + }) + }) +})