From c3bae907e0e1ff63330b47ffec90114fbcb0d502 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Fri, 6 Feb 2026 10:08:33 +0000 Subject: [PATCH 1/5] Interact event and resize bug fix --- plugins/beta/draw-ml/src/mapboxSnap.js | 6 ++++-- plugins/interact/src/hooks/useInteractionHandlers.js | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/plugins/beta/draw-ml/src/mapboxSnap.js b/plugins/beta/draw-ml/src/mapboxSnap.js index ddbd98a7..d5ba14b9 100644 --- a/plugins/beta/draw-ml/src/mapboxSnap.js +++ b/plugins/beta/draw-ml/src/mapboxSnap.js @@ -140,6 +140,8 @@ function applyMapboxSnapPatches(colors) { function pollUntil(checkFn, onSuccess) { (function poll() { const result = checkFn() + // null signals to stop polling, falsy continues polling + if (result === null) return result ? onSuccess(result) : requestAnimationFrame(poll) })() } @@ -267,7 +269,7 @@ export function initMapLibreSnap(map, draw, snapOptions = {}) { // Handle style changes - re-patch source and ensure snap layer exists map.on('style.load', () => { pollUntil( - () => map.getSource('mapbox-gl-draw-hot'), + () => map._removed ? null : map.getSource('mapbox-gl-draw-hot'), (source) => { patchSourceData(source) @@ -312,7 +314,7 @@ export function initMapLibreSnap(map, draw, snapOptions = {}) { // Initial setup - poll until draw source exists pollUntil( - () => map.getSource('mapbox-gl-draw-hot'), + () => map._removed ? null : map.getSource('mapbox-gl-draw-hot'), createSnap ) } diff --git a/plugins/interact/src/hooks/useInteractionHandlers.js b/plugins/interact/src/hooks/useInteractionHandlers.js index 855d32a5..bd9c7041 100755 --- a/plugins/interact/src/hooks/useInteractionHandlers.js +++ b/plugins/interact/src/hooks/useInteractionHandlers.js @@ -62,9 +62,12 @@ export const useInteractionHandlers = ({ useEffect(() => { // Skip if features exist but bounds not yet calculated const awaitingBounds = selectedFeatures.length > 0 && !selectionBounds - if (awaitingBounds || selectedFeatures === lastEmittedSelectionChange.current) { - return - } + if (awaitingBounds) return + + // 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) return eventBus.emit('interact:selectionchange', { selectedFeatures, From 47fbd4955483a89935e299c8d7dcdf06e0c0e739 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Fri, 6 Feb 2026 12:25:17 +0000 Subject: [PATCH 2/5] Behaviour fix and docs update --- demo/js/index.js | 1 + docs/api.md | 8 + providers/maplibre/src/utils/labels.js | 294 +++++++++++------- sonar-project.properties | 19 +- .../components/KeyboardHelp/KeyboardHelp.jsx | 4 +- .../KeyboardHelp/KeyboardHelp.test.jsx | 9 + src/App/controls/keyboardShortcuts.js | 6 +- src/App/initialiseApp.js | 4 +- src/App/registry/keyboardShortcutRegistry.js | 14 +- .../registry/keyboardShortcutRegistry.test.js | 28 ++ src/InteractiveMap/InteractiveMap.js | 60 +++- src/InteractiveMap/InteractiveMap.test.js | 77 ++++- src/InteractiveMap/behaviourController.js | 14 +- .../behaviourController.test.js | 67 +++- src/InteractiveMap/historyManager.js | 17 +- src/InteractiveMap/historyManager.test.js | 43 ++- src/config/appConfig.js | 10 +- src/config/appConfig.test.js | 20 +- src/config/defaults.js | 1 + src/types.js | 5 + src/utils/detectBreakpoint.js | 153 +++++---- 21 files changed, 616 insertions(+), 238 deletions(-) diff --git a/demo/js/index.js b/demo/js/index.js index 7cf280c3..0792254a 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -106,6 +106,7 @@ var interactiveMap = new InteractiveMap('map', { containerHeight: '650px', transformRequest: transformTileRequest, enableZoomControls: true, + readMapText: true, // enableFullscreen: true, // hasExitButton: true, // markers: [{ diff --git a/docs/api.md b/docs/api.md index 104a7ea4..389dc0e2 100644 --- a/docs/api.md +++ b/docs/api.md @@ -388,6 +388,14 @@ See [PluginDescriptor](./plugins/plugin-descriptor.md) for full details. --- +### `preserveStateOnClose` +**Type:** `boolean` +**Default:** `false` + +Controls whether closing the map (via the browser back button or the exit map button when `hasExitButton` is `true` and the map is fullscreen) destroys the map instance or hides it while preserving its current state. Set to `true` to keep the map state intact, which is useful for implementations like a toggle map view list view pattern. + +--- + ### `readMapText` **Type:** `boolean` **Default:** `false` diff --git a/providers/maplibre/src/utils/labels.js b/providers/maplibre/src/utils/labels.js index 001cdba7..5d1e1442 100755 --- a/providers/maplibre/src/utils/labels.js +++ b/providers/maplibre/src/utils/labels.js @@ -2,8 +2,9 @@ import { spatialNavigate } from './spatial.js' import { calculateLinearTextSize } from './calculateLinearTextSize.js' const HIGHLIGHT_SCALE_FACTOR = 1.5 +const HIGHLIGHT_LABEL_SOURCE = 'highlighted-label' -function getGeometryCenter(geometry) { +export function getGeometryCenter(geometry) { const { type, coordinates } = geometry if (type === 'Point') { return coordinates @@ -23,13 +24,12 @@ function getGeometryCenter(geometry) { return null } -function evalInterpolate(expr, zoom) { +export function evalInterpolate(expr, zoom) { if (typeof expr === 'number') { return expr } if (!Array.isArray(expr) || expr[0] !== 'interpolate') { return calculateLinearTextSize(expr, zoom) - // throw new Error('Only interpolate expressions supported') } const [, , input, ...stops] = expr if (input[0] !== 'zoom') { @@ -39,7 +39,7 @@ function evalInterpolate(expr, zoom) { const z0 = stops[i] const v0 = stops[i + 1] const z1 = stops[i + 2] - const v1 = stops[i + 3] + const v1 = stops[i + 3] // NOSONAR: array index offset for interpolation pairs if (zoom <= z0) { return v0 } @@ -50,166 +50,220 @@ function evalInterpolate(expr, zoom) { return stops[stops.length - 1] } -export function createMapLabelNavigator(map, mapColorScheme, events, eventBus) { - let isDarkStyle = mapColorScheme === 'dark' - let labels = [] - let currentPixel = null - let highlightLayerId = null - let highlightedExpr = null - let highlightedFeature = null - - const colors = { - get current() { - if (isDarkStyle) { - return { text: '#ffffff', halo: '#000000' } - } - return { text: '#000000', halo: '#ffffff' } - } +export function getHighlightColors(isDarkStyle) { + if (isDarkStyle) { + return { text: '#ffffff', halo: '#000000' } } + return { text: '#000000', halo: '#ffffff' } +} - const initLabelSource = () => { - if (!map.getSource('highlighted-label')) { - map.addSource('highlighted-label', { type: 'geojson', data: { type: 'FeatureCollection', features: [] } }) - } +export function extractTextPropertyName(textField) { + if (typeof textField === 'string') { + return /^{(.+)}$/.exec(textField)?.[1] } + if (Array.isArray(textField)) { + return textField.find(e => Array.isArray(e) && e[0] === 'get')?.[1] + } + return null +} - map.getStyle().layers.filter(l => l.layout?.['symbol-placement'] === 'line').forEach(l => { - map.setLayoutProperty(l.id, 'symbol-placement', 'line-center') - }) - initLabelSource() +export function buildLabelFromFeature(feature, layer, propName, map) { + const center = getGeometryCenter(feature.geometry) + if (!center) { + return null + } + const projected = map.project({ lng: center[0], lat: center[1] }) + return { text: feature.properties[propName], x: projected.x, y: projected.y, feature, layer } +} - eventBus?.on(events.MAP_SET_STYLE, style => { - map.once('styledata', () => map.once('idle', () => { - map.getStyle().layers.filter(l => l.layout?.['symbol-placement'] === 'line').forEach(l => { - map.setLayoutProperty(l.id, 'symbol-placement', 'line-center') - }) - initLabelSource() - isDarkStyle = style?.mapColorScheme === 'dark' - })) +export function buildLabelsFromLayers(map, symbolLayers, features) { + return symbolLayers.flatMap(layer => { + const textField = layer.layout?.['text-field'] + const propName = extractTextPropertyName(textField) + if (!propName) { + return [] + } + return features + .filter(f => f.layer.id === layer.id && f.properties?.[propName]) + .map(f => buildLabelFromFeature(f, layer, propName, map)) + .filter(Boolean) }) +} - function refreshLabels() { - const symbolLayers = map.getStyle().layers.filter(l => l.type === 'symbol') - const features = map.queryRenderedFeatures({ layers: symbolLayers.map(l => l.id) }) - labels = symbolLayers.flatMap(layer => { - const textField = layer.layout?.['text-field'] - const propName = typeof textField === 'string' - ? textField.match(/^{(.+)}$/)?.[1] - : Array.isArray(textField) - ? textField.find(e => Array.isArray(e) && e[0] === 'get')?.[1] - : null - if (!propName) { - return [] - } - return features.filter(f => f.layer.id === layer.id && f.properties?.[propName]).map(f => { - const center = getGeometryCenter(f.geometry) - if (!center) { - return null - } - const projected = map.project({ lng: center[0], lat: center[1] }) - return { text: f.properties[propName], x: projected.x, y: projected.y, feature: f, layer } - }).filter(Boolean) - }) - } +export function findClosestLabel(labels, centerPoint) { + return labels.reduce((best, label) => { + const dist = (label.x - centerPoint.x) ** 2 + (label.y - centerPoint.y) ** 2 + if (!best || dist < best.dist) { + return { label, dist } + } + return best + }, null)?.label +} - function removeHighlight() { - if (highlightLayerId && map.getLayer(highlightLayerId)) { - try { - map.removeLayer(highlightLayerId) - } - catch {} - highlightLayerId = null - highlightedExpr = null - highlightedFeature = null +export function createHighlightLayerConfig(sourceLayer, highlightSize, colors) { + return { + id: `highlight-${sourceLayer.id}`, + type: sourceLayer.type, + source: HIGHLIGHT_LABEL_SOURCE, + layout: { + ...sourceLayer.layout, + 'text-size': highlightSize, + 'text-allow-overlap': true, + 'text-ignore-placement': true, + 'text-max-angle': 90 + }, + paint: { + ...sourceLayer.paint, + 'text-color': colors.text, + 'text-halo-color': colors.halo, + 'text-halo-width': 3, + 'text-halo-blur': 1, + 'text-opacity': 1 } } +} - function highlight(labelData) { - if (!labelData?.feature?.layer) { - return +export function removeHighlightLayer(map, state) { + if (state.highlightLayerId && map.getLayer(state.highlightLayerId)) { + try { + map.removeLayer(state.highlightLayerId) } - removeHighlight() - const { feature, layer } = labelData - highlightLayerId = `highlight-${layer.id}` - - const { id, type, properties, geometry } = feature - highlightedFeature = { id, type, properties, geometry } - map.getSource('highlighted-label').setData(highlightedFeature) - highlightedExpr = layer.layout['text-size'] - const zoom = map.getZoom() - const baseSize = evalInterpolate(highlightedExpr, zoom) - const highlightSize = baseSize * HIGHLIGHT_SCALE_FACTOR - map.addLayer({ - id: highlightLayerId, - type: layer.type, - source: 'highlighted-label', - layout: { ...layer.layout, 'text-size': highlightSize, 'text-allow-overlap': true, 'text-ignore-placement': true, 'text-max-angle': 90 }, - paint: { ...layer.paint, 'text-color': colors.current.text, 'text-halo-color': colors.current.halo, 'text-halo-width': 3, 'text-halo-blur': 1, 'text-opacity': 1 } + catch {} + state.highlightLayerId = null + state.highlightedExpr = null + } +} + +export function applyHighlight(map, labelData, state) { + if (!labelData?.feature?.layer) { + return + } + removeHighlightLayer(map, state) + const { feature, layer } = labelData + state.highlightLayerId = `highlight-${layer.id}` + + const { id, type, properties, geometry } = feature + map.getSource(HIGHLIGHT_LABEL_SOURCE).setData({ id, type, properties, geometry }) + state.highlightedExpr = layer.layout['text-size'] + + const zoom = map.getZoom() + const baseSize = evalInterpolate(state.highlightedExpr, zoom) + const highlightSize = baseSize * HIGHLIGHT_SCALE_FACTOR + const colors = getHighlightColors(state.isDarkStyle) + const layerConfig = createHighlightLayerConfig(layer, highlightSize, colors) + + map.addLayer(layerConfig) + map.moveLayer(state.highlightLayerId) +} + +export function navigateToNextLabel(direction, state) { + if (!state.currentPixel) { + return null + } + const filtered = state.labels + .map((l, i) => ({ pixel: [l.x, l.y], index: i })) + .filter(l => l.pixel[0] !== state.currentPixel.x || l.pixel[1] !== state.currentPixel.y) + if (!filtered.length) { + return null + } + const pixelArray = filtered.map(l => l.pixel) + let nextFilteredIndex = spatialNavigate(direction, [state.currentPixel.x, state.currentPixel.y], pixelArray) + if (nextFilteredIndex == null || nextFilteredIndex < 0 || nextFilteredIndex >= filtered.length) { + nextFilteredIndex = 0 + } + return state.labels[filtered[nextFilteredIndex].index] +} + +function initLabelSource(map) { + if (!map.getSource(HIGHLIGHT_LABEL_SOURCE)) { + map.addSource(HIGHLIGHT_LABEL_SOURCE, { type: 'geojson', data: { type: 'FeatureCollection', features: [] } }) + } +} + +function setLineCenterPlacement(map) { + map.getStyle().layers + .filter(l => l.layout?.['symbol-placement'] === 'line') + .forEach(l => map.setLayoutProperty(l.id, 'symbol-placement', 'line-center')) +} + +function setSymbolTextOpacity(map) { + map.getStyle().layers + .filter(l => l.type === 'symbol') + .forEach(layer => { + map.setPaintProperty(layer.id, 'text-opacity', ['case', ['boolean', ['feature-state', 'highlighted'], false], 0, 1]) }) - map.moveLayer(highlightLayerId) +} + +export function createMapLabelNavigator(map, mapColorScheme, events, eventBus) { + const state = { + isDarkStyle: mapColorScheme === 'dark', + labels: [], + currentPixel: null, + highlightLayerId: null, + highlightedExpr: null } + setLineCenterPlacement(map) + initLabelSource(map) + + eventBus?.on(events.MAP_SET_STYLE, style => { + map.once('styledata', () => map.once('idle', () => { + setLineCenterPlacement(map) + initLabelSource(map) + state.isDarkStyle = style?.mapColorScheme === 'dark' + })) + }) + map.on('zoom', () => { - if (highlightLayerId && highlightedExpr) { - const zoom = map.getZoom() - const baseSize = evalInterpolate(highlightedExpr, zoom) - map.setLayoutProperty(highlightLayerId, 'text-size', baseSize * HIGHLIGHT_SCALE_FACTOR) + if (state.highlightLayerId && state.highlightedExpr) { + const baseSize = evalInterpolate(state.highlightedExpr, map.getZoom()) + map.setLayoutProperty(state.highlightLayerId, 'text-size', baseSize * HIGHLIGHT_SCALE_FACTOR) } }) + function refreshLabels() { + const symbolLayers = map.getStyle().layers.filter(l => l.type === 'symbol') + const features = map.queryRenderedFeatures({ layers: symbolLayers.map(l => l.id) }) + state.labels = buildLabelsFromLayers(map, symbolLayers, features) + } + function highlightCenter() { refreshLabels() - if (!labels.length) { + if (!state.labels.length) { return null } const centerPoint = map.project(map.getCenter()) - const closest = labels.reduce((best, label) => { - const dist = (label.x - centerPoint.x) ** 2 + (label.y - centerPoint.y) ** 2 - if (!best || dist < best.dist) { - return { label, dist } - } - return best - }, null)?.label + const closest = findClosestLabel(state.labels, centerPoint) if (closest) { - currentPixel = { x: closest.x, y: closest.y } + state.currentPixel = { x: closest.x, y: closest.y } } - highlight(closest) + applyHighlight(map, closest, state) return `${closest.text} (${closest.layer.id})` } function highlightNext(direction) { refreshLabels() - if (!labels.length) { + if (!state.labels.length) { return null } - if (!currentPixel) { + if (!state.currentPixel) { return highlightCenter() } - const filtered = labels - .map((l, i) => ({ pixel: [l.x, l.y], index: i })) - .filter(l => l.pixel[0] !== currentPixel.x || l.pixel[1] !== currentPixel.y) - if (!filtered.length) { + const labelData = navigateToNextLabel(direction, state) + if (!labelData) { return null } - const pixelArray = filtered.map(l => l.pixel) - let nextFilteredIndex = spatialNavigate(direction, [currentPixel.x, currentPixel.y], pixelArray) - if (nextFilteredIndex == null || nextFilteredIndex < 0 || nextFilteredIndex >= filtered.length) { - nextFilteredIndex = 0 - } - const labelData = labels[filtered[nextFilteredIndex].index] - currentPixel = { x: labelData.x, y: labelData.y } - highlight(labelData) + state.currentPixel = { x: labelData.x, y: labelData.y } + applyHighlight(map, labelData, state) return `${labelData.text} (${labelData.layer.id})` } - map.getStyle().layers.filter(l => l.type === 'symbol').forEach(layer => { - map.setPaintProperty(layer.id, 'text-opacity', ['case', ['boolean', ['feature-state', 'highlighted'], false], 0, 1]) - }) + setSymbolTextOpacity(map) return { refreshLabels, highlightNextLabel: highlightNext, highlightLabelAtCenter: highlightCenter, - clearHighlightedLabel: removeHighlight + clearHighlightedLabel: () => removeHighlightLayer(map, state) } } diff --git a/sonar-project.properties b/sonar-project.properties index c6e42061..f250107f 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -16,8 +16,23 @@ sonar.tests=src,plugins,providers sonar.test.inclusions=**/*.test.*,**/__mocks__/**,**/__stubs__/** sonar.cpd.exclusions=**/*.test.*,**/__mocks__/**,**/__stubs__/** -sonar.issue.ignore.multicriteria=reactPropsJs,reactPropsJsx +# Ignored rules + +# S6774: React props validation - using TypeScript/JSDoc for prop types instead +sonar.issue.ignore.multicriteria=reactPropsJs,reactPropsJsx,preferGlobalThisJs,preferGlobalThisJsx,preferAtJs,preferAtJsx sonar.issue.ignore.multicriteria.reactPropsJs.ruleKey=javascript:S6774 sonar.issue.ignore.multicriteria.reactPropsJs.resourceKey=**/*.js sonar.issue.ignore.multicriteria.reactPropsJsx.ruleKey=javascript:S6774 -sonar.issue.ignore.multicriteria.reactPropsJsx.resourceKey=**/*.jsx \ No newline at end of file +sonar.issue.ignore.multicriteria.reactPropsJsx.resourceKey=**/*.jsx + +# S7764: Prefer globalThis over window - globalThis cannot be polyfilled for older browsers +sonar.issue.ignore.multicriteria.preferGlobalThisJs.ruleKey=javascript:S7764 +sonar.issue.ignore.multicriteria.preferGlobalThisJs.resourceKey=**/*.js +sonar.issue.ignore.multicriteria.preferGlobalThisJsx.ruleKey=javascript:S7764 +sonar.issue.ignore.multicriteria.preferGlobalThisJsx.resourceKey=**/*.jsx + +# S7755: Prefer .at() over [array.length - index] - .at() is ES2022 and requires polyfilling +sonar.issue.ignore.multicriteria.preferAtJs.ruleKey=javascript:S7755 +sonar.issue.ignore.multicriteria.preferAtJs.resourceKey=**/*.js +sonar.issue.ignore.multicriteria.preferAtJsx.ruleKey=javascript:S7755 +sonar.issue.ignore.multicriteria.preferAtJsx.resourceKey=**/*.jsx \ No newline at end of file diff --git a/src/App/components/KeyboardHelp/KeyboardHelp.jsx b/src/App/components/KeyboardHelp/KeyboardHelp.jsx index bb5f7ddf..7e159995 100755 --- a/src/App/components/KeyboardHelp/KeyboardHelp.jsx +++ b/src/App/components/KeyboardHelp/KeyboardHelp.jsx @@ -1,11 +1,13 @@ // src/components/KeyboardHelp.jsx import React from 'react' +import { useConfig } from '../../store/configContext' import { getKeyboardShortcuts } from '../../registry/keyboardShortcutRegistry.js' // eslint-disable-next-line camelcase, react/jsx-pascal-case // sonarjs/disable-next-line function-name export const KeyboardHelp = () => { - const groups = getKeyboardShortcuts().reduce((acc, shortcut) => { + const appConfig = useConfig() + const groups = getKeyboardShortcuts(appConfig).reduce((acc, shortcut) => { acc[shortcut.group] = acc[shortcut.group] || [] acc[shortcut.group].push(shortcut) return acc diff --git a/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx b/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx index 1e74e8a8..53f808fe 100755 --- a/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx +++ b/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx @@ -3,12 +3,21 @@ import React from 'react' import { render, screen, within } from '@testing-library/react' import { KeyboardHelp } from './KeyboardHelp' import { getKeyboardShortcuts } from '../../registry/keyboardShortcutRegistry.js' +import { useConfig } from '../../store/configContext' jest.mock('../../registry/keyboardShortcutRegistry.js', () => ({ getKeyboardShortcuts: jest.fn() })) +jest.mock('../../store/configContext', () => ({ + useConfig: jest.fn() +})) + describe('KeyboardHelp', () => { + beforeEach(() => { + useConfig.mockReturnValue({}) + }) + afterEach(() => { jest.clearAllMocks() }) diff --git a/src/App/controls/keyboardShortcuts.js b/src/App/controls/keyboardShortcuts.js index 71f602a3..c9f52018 100755 --- a/src/App/controls/keyboardShortcuts.js +++ b/src/App/controls/keyboardShortcuts.js @@ -48,13 +48,15 @@ export const coreShortcuts = [ group: 'Labels', title: 'Highlight label at centre', command: 'Alt + Enter', - enabled: false + enabled: false, + requiredConfig: ['readMapText'] }, { id: 'highlightNextLabel', group: 'Labels', title: 'Highlight nearby label', command: 'Alt + →, ←, ↑ or ↓', - enabled: false + enabled: false, + requiredConfig: ['readMapText'] } ] diff --git a/src/App/initialiseApp.js b/src/App/initialiseApp.js index 657daef1..a905b63e 100755 --- a/src/App/initialiseApp.js +++ b/src/App/initialiseApp.js @@ -1,6 +1,6 @@ import { createRoot } from 'react-dom/client' import { EVENTS as events } from '../config/events.js' -import { appConfig } from '../config/appConfig.js' +import { defaultAppConfig } from '../config/appConfig.js' import { createButtonRegistry } from './registry/buttonRegistry.js' import { createPanelRegistry } from './registry/panelRegistry.js' import { createControlRegistry } from './registry/controlRegistry.js' @@ -59,7 +59,7 @@ export async function initialiseApp (rootElement, { // Register default appConfig as a plugin registerPlugin({ id: 'appConfig', - manifest: appConfig + manifest: defaultAppConfig }) // Create root if not already present diff --git a/src/App/registry/keyboardShortcutRegistry.js b/src/App/registry/keyboardShortcutRegistry.js index c553eef7..b00c5ec3 100755 --- a/src/App/registry/keyboardShortcutRegistry.js +++ b/src/App/registry/keyboardShortcutRegistry.js @@ -21,8 +21,18 @@ export const setProviderSupportedShortcuts = (ids = []) => { providerSupportedIds = new Set(ids) } -export const getKeyboardShortcuts = () => { - const filteredCore = coreShortcuts.filter(s => providerSupportedIds.has(s.id)) +export const getKeyboardShortcuts = (appConfig = {}) => { + const filteredCore = coreShortcuts.filter(s => { + // Must be supported by provider + if (!providerSupportedIds.has(s.id)) { + return false + } + // Check requiredConfig - all specified config values must be truthy + if (s.requiredConfig) { + return s.requiredConfig.every(key => appConfig[key]) + } + return true + }) return [ ...filteredCore, // supported core shortcuts diff --git a/src/App/registry/keyboardShortcutRegistry.test.js b/src/App/registry/keyboardShortcutRegistry.test.js index 6f593f5e..0aea5531 100755 --- a/src/App/registry/keyboardShortcutRegistry.test.js +++ b/src/App/registry/keyboardShortcutRegistry.test.js @@ -70,4 +70,32 @@ describe('keyboardShortcutRegistry', () => { const shortcuts = getKeyboardShortcuts() expect(shortcuts).toEqual([]) }) + + test('getKeyboardShortcuts filters by requiredConfig when appConfig provided', () => { + jest.resetModules() + jest.doMock('../controls/keyboardShortcuts.js', () => ({ + coreShortcuts: [ + { id: 'always', description: 'Always shown' }, + { id: 'conditional', description: 'Conditional', requiredConfig: ['featureEnabled'] } + ] + })) + const module = require('./keyboardShortcutRegistry.js') + module.setProviderSupportedShortcuts(['always', 'conditional']) + + // Without config, conditional shortcut is excluded + expect(module.getKeyboardShortcuts()).toEqual([ + { id: 'always', description: 'Always shown' } + ]) + + // With config false, conditional shortcut is excluded + expect(module.getKeyboardShortcuts({ featureEnabled: false })).toEqual([ + { id: 'always', description: 'Always shown' } + ]) + + // With config true, conditional shortcut is included + expect(module.getKeyboardShortcuts({ featureEnabled: true })).toEqual([ + { id: 'always', description: 'Always shown' }, + { id: 'conditional', description: 'Conditional', requiredConfig: ['featureEnabled'] } + ]) + }) }) diff --git a/src/InteractiveMap/InteractiveMap.js b/src/InteractiveMap/InteractiveMap.js index 93119a80..e9ae284a 100755 --- a/src/InteractiveMap/InteractiveMap.js +++ b/src/InteractiveMap/InteractiveMap.js @@ -22,6 +22,7 @@ import { createInterfaceDetector, getInterfaceType } from '../utils/detectInterf import { createReverseGeocode } from '../services/reverseGeocode.js' import { EVENTS as events } from '../config/events.js' import { createEventBus } from '../services/eventBus.js' +import { toggleInertElements } from '../utils/toggleInertElements.js' /** * Main entry point for the Interactive Map component. @@ -33,6 +34,7 @@ export default class InteractiveMap { _breakpointDetector = null _interfaceDetectorCleanup = null _hybridBehaviourCleanup = null + _isHidden = false // tracks if map is hidden but preserved (hybrid mode) /** * Create a new InteractiveMap instance. @@ -98,12 +100,20 @@ export default class InteractiveMap { } _handleButtonClick (e) { - this.loadApp() history.pushState({ isBack: true }, '', e.currentTarget.getAttribute('href')) + if (this._isHidden) { + this.showApp() + } else { + this.loadApp() + } } _handleExitClick () { - this.removeApp() + if (this.config.preserveStateOnClose) { + this.hideApp() + } else { + this.removeApp() + } // Remove the map param from the URL using regex to prevent encoding const key = this.config.mapViewParamKey const href = location.href @@ -190,6 +200,52 @@ export default class InteractiveMap { this.eventBus.emit(events.MAP_DESTROY, { mapId: this.id }) } + /** + * Hide the map application without destroying it (preserves state). + * Used in hybrid mode when resizing below breakpoint. + * + * @internal Not intended for end-user use. + */ + hideApp () { + this._isHidden = true + this.rootEl.style.display = 'none' + + // Restore inert elements before focusing button + toggleInertElements({ containerEl: this.rootEl, isFullscreen: false }) + + if (this._openButton) { + this._openButton.removeAttribute('style') + this._openButton.focus() + } + + // Remove fullscreen classes + document.documentElement.classList.remove('im-is-fullscreen') + this.rootEl.classList.remove('im-is-fullscreen') + + // Reset page title (remove prepended map title) + const parts = document.title.split(': ') + if (parts.length > 1) { + document.title = parts[parts.length - 1] + } + } + + /** + * Show a previously hidden map application. + * Used in hybrid mode when resizing above breakpoint or clicking button. + * + * @internal Not intended for end-user use. + */ + showApp () { + this._isHidden = false + this.rootEl.style.display = '' + + if (this._openButton) { + this._openButton.style.display = 'none' + } + + updateDOMState(this) + } + /** * Destroy the map instance and clean up all resources. * diff --git a/src/InteractiveMap/InteractiveMap.test.js b/src/InteractiveMap/InteractiveMap.test.js index d08882ef..5411dcf4 100755 --- a/src/InteractiveMap/InteractiveMap.test.js +++ b/src/InteractiveMap/InteractiveMap.test.js @@ -268,20 +268,23 @@ describe('InteractiveMap Core Functionality', () => { expect(() => map.destroy()).not.toThrow() }) - it('_handleExitClick removes app and calls replaceState', () => { + it('_handleExitClick removes app when preserveStateOnClose is false', () => { const replaceStateSpy = jest.spyOn(history, 'replaceState').mockImplementation(() => {}) const map = new InteractiveMap('map', { behaviour: 'buttonFirst', mapProvider: mapProviderMock, - mapViewParamKey: 'mv' + mapViewParamKey: 'mv', + preserveStateOnClose: false }) const removeAppSpy = jest.spyOn(map, 'removeApp').mockImplementation(() => {}) + const hideAppSpy = jest.spyOn(map, 'hideApp').mockImplementation(() => {}) map._handleExitClick() expect(removeAppSpy).toHaveBeenCalled() + expect(hideAppSpy).not.toHaveBeenCalled() expect(replaceStateSpy).toHaveBeenCalledWith( history.state, '', @@ -289,8 +292,78 @@ describe('InteractiveMap Core Functionality', () => { ) removeAppSpy.mockRestore() + hideAppSpy.mockRestore() replaceStateSpy.mockRestore() }) + + it('_handleExitClick hides app when preserveStateOnClose is true', () => { + const replaceStateSpy = jest.spyOn(history, 'replaceState').mockImplementation(() => {}) + + const map = new InteractiveMap('map', { + behaviour: 'buttonFirst', + mapProvider: mapProviderMock, + mapViewParamKey: 'mv', + preserveStateOnClose: true + }) + + const removeAppSpy = jest.spyOn(map, 'removeApp').mockImplementation(() => {}) + const hideAppSpy = jest.spyOn(map, 'hideApp').mockImplementation(() => {}) + + map._handleExitClick() + + expect(hideAppSpy).toHaveBeenCalled() + expect(removeAppSpy).not.toHaveBeenCalled() + expect(replaceStateSpy).toHaveBeenCalled() + + removeAppSpy.mockRestore() + hideAppSpy.mockRestore() + replaceStateSpy.mockRestore() + }) + + it('_handleButtonClick calls showApp when map is hidden', async () => { + const map = new InteractiveMap('map', { behaviour: 'buttonFirst', mapProvider: mapProviderMock }) + map._isHidden = true + const showAppSpy = jest.spyOn(map, 'showApp').mockImplementation(() => {}) + const loadAppSpy = jest.spyOn(map, 'loadApp').mockResolvedValue() + const pushStateSpy = jest.spyOn(history, 'pushState').mockImplementation(() => {}) + const fakeEvent = { currentTarget: { getAttribute: jest.fn().mockReturnValue('/?mv=map') } } + + await openButtonCallback(fakeEvent) + + expect(showAppSpy).toHaveBeenCalled() + expect(loadAppSpy).not.toHaveBeenCalled() + expect(pushStateSpy).toHaveBeenCalled() + + showAppSpy.mockRestore() + loadAppSpy.mockRestore() + pushStateSpy.mockRestore() + }) + + it('hideApp sets _isHidden and hides element', () => { + const map = new InteractiveMap('map', { behaviour: 'buttonFirst', mapProvider: mapProviderMock }) + map._openButton = mockButtonInstance + + map.hideApp() + + expect(map._isHidden).toBe(true) + expect(map.rootEl.style.display).toBe('none') + expect(mockButtonInstance.removeAttribute).toHaveBeenCalledWith('style') + expect(mockButtonInstance.focus).toHaveBeenCalled() + }) + + it('showApp sets _isHidden false and shows element', () => { + const map = new InteractiveMap('map', { behaviour: 'buttonFirst', mapProvider: mapProviderMock }) + map._isHidden = true + map._openButton = mockButtonInstance + map.rootEl.style.display = 'none' + + map.showApp() + + expect(map._isHidden).toBe(false) + expect(map.rootEl.style.display).toBe('') + expect(mockButtonInstance.style.display).toBe('none') + expect(updateDOMState).toHaveBeenCalledWith(map) + }) }) describe('InteractiveMap Public API Methods', () => { diff --git a/src/InteractiveMap/behaviourController.js b/src/InteractiveMap/behaviourController.js index dd45758d..03cc3018 100755 --- a/src/InteractiveMap/behaviourController.js +++ b/src/InteractiveMap/behaviourController.js @@ -1,5 +1,6 @@ import { getQueryParam } from '../utils/queryString.js' import { isHybridFullscreen } from '../utils/getIsFullscreen.js' +import { updateDOMState } from './domStateManager.js' import defaults from '../config/defaults.js' // ----------------------------------------------------------------------------- @@ -56,9 +57,18 @@ function setupBehavior (mapInstance) { const handleChange = () => { if (shouldLoadComponent(mapInstance.config)) { - mapInstance.loadApp() + if (mapInstance._isHidden) { + mapInstance.showApp() + } else if (!mapInstance._root) { + mapInstance.loadApp() + } else { + // Map is showing - update DOM state for fullscreen/inline transition + updateDOMState(mapInstance) + } } else { - mapInstance.removeApp() + if (mapInstance._root) { + mapInstance.hideApp() + } } } diff --git a/src/InteractiveMap/behaviourController.test.js b/src/InteractiveMap/behaviourController.test.js index 1d9b0314..033af75c 100755 --- a/src/InteractiveMap/behaviourController.test.js +++ b/src/InteractiveMap/behaviourController.test.js @@ -4,8 +4,10 @@ import { setupBehavior, shouldLoadComponent } from './behaviourController.js' import * as queryString from '../utils/queryString.js' +import { updateDOMState } from './domStateManager.js' jest.mock('../utils/queryString.js') +jest.mock('./domStateManager.js', () => ({ updateDOMState: jest.fn() })) describe('shouldLoadComponent', () => { beforeEach(() => { @@ -60,8 +62,12 @@ describe('setupBehavior', () => { mockMapInstance = { config: { hybridWidth: null, maxMobileWidth: 640 }, _breakpointDetector: mockBreakpointDetector, + _root: null, + _isHidden: false, loadApp: jest.fn(), - removeApp: jest.fn() + removeApp: jest.fn(), + hideApp: jest.fn(), + showApp: jest.fn() } // Default: viewport is wide window.matchMedia = jest.fn().mockImplementation(() => ({ @@ -145,4 +151,63 @@ describe('setupBehavior', () => { const cleanup = setupBehavior(mockMapInstance) expect(cleanup).toBeNull() }) + + describe('hybrid behaviour handleChange', () => { + let handleChange + + beforeEach(() => { + const mockAddEventListener = jest.fn((event, cb) => { handleChange = cb }) + window.matchMedia = jest.fn().mockImplementation(() => ({ + matches: false, + addEventListener: mockAddEventListener, + removeEventListener: jest.fn() + })) + mockMapInstance.config = { id: 'test', behaviour: 'hybrid', hybridWidth: null, maxMobileWidth: 640 } + setupBehavior(mockMapInstance) + }) + + it('calls showApp when map is hidden and should load', () => { + mockMapInstance._isHidden = true + queryString.getQueryParam.mockReturnValue(null) // wide viewport, should load + + handleChange() + + expect(mockMapInstance.showApp).toHaveBeenCalled() + expect(mockMapInstance.loadApp).not.toHaveBeenCalled() + }) + + it('calls loadApp when map has no root and should load', () => { + mockMapInstance._isHidden = false + mockMapInstance._root = null + queryString.getQueryParam.mockReturnValue(null) + + handleChange() + + expect(mockMapInstance.loadApp).toHaveBeenCalled() + expect(mockMapInstance.showApp).not.toHaveBeenCalled() + }) + + it('calls updateDOMState when map is showing and should load', () => { + mockMapInstance._isHidden = false + mockMapInstance._root = {} // has root + queryString.getQueryParam.mockReturnValue(null) + + handleChange() + + expect(updateDOMState).toHaveBeenCalledWith(mockMapInstance) + expect(mockMapInstance.loadApp).not.toHaveBeenCalled() + expect(mockMapInstance.showApp).not.toHaveBeenCalled() + }) + + it('calls hideApp when should not load and has root', () => { + mockMapInstance._root = {} + // Simulate narrow viewport where shouldLoadComponent returns false + window.matchMedia = jest.fn().mockImplementation(() => ({ matches: true })) + queryString.getQueryParam.mockReturnValue(null) + + handleChange() + + expect(mockMapInstance.hideApp).toHaveBeenCalled() + }) + }) }) diff --git a/src/InteractiveMap/historyManager.js b/src/InteractiveMap/historyManager.js index 0330dae9..f13e05d2 100755 --- a/src/InteractiveMap/historyManager.js +++ b/src/InteractiveMap/historyManager.js @@ -28,11 +28,20 @@ function handlePopstate () { const isHybridVisible = mapInstance.config.behaviour === 'hybrid' && !isHybridFullscreen(mapInstance.config) const isOpen = mapInstance.rootEl?.children.length - if (shouldBeOpen && !isOpen) { - mapInstance.loadApp?.() + if (shouldBeOpen && (!isOpen || mapInstance._isHidden)) { + if (mapInstance._isHidden) { + mapInstance.showApp?.() + } else { + mapInstance.loadApp?.() + } } else if (!shouldBeOpen && isOpen && !isHybridVisible) { - mapInstance.removeApp?.() - mapInstance.openButton?.focus?.() + if (mapInstance.config.preserveStateOnClose) { + mapInstance.hideApp?.() + } else { + mapInstance.removeApp?.() + } + } else { + // No action } } } diff --git a/src/InteractiveMap/historyManager.test.js b/src/InteractiveMap/historyManager.test.js index f3c94e6c..2a340009 100755 --- a/src/InteractiveMap/historyManager.test.js +++ b/src/InteractiveMap/historyManager.test.js @@ -13,19 +13,25 @@ describe('historyManager', () => { beforeEach(() => { component1 = { id: 'map', - config: { behaviour: 'buttonFirst', hybridWidth: null, maxMobileWidth: 640 }, + config: { behaviour: 'buttonFirst', hybridWidth: null, maxMobileWidth: 640, preserveStateOnClose: false }, rootEl: document.createElement('div'), loadApp: jest.fn(), removeApp: jest.fn(), - openButton: { focus: jest.fn() } + hideApp: jest.fn(), + showApp: jest.fn(), + openButton: { focus: jest.fn() }, + _isHidden: false } component2 = { id: 'list', - config: { behaviour: 'hybrid', hybridWidth: null, maxMobileWidth: 640 }, + config: { behaviour: 'hybrid', hybridWidth: null, maxMobileWidth: 640, preserveStateOnClose: false }, rootEl: document.createElement('div'), loadApp: jest.fn(), removeApp: jest.fn(), - openButton: { focus: jest.fn() } + hideApp: jest.fn(), + showApp: jest.fn(), + openButton: { focus: jest.fn() }, + _isHidden: false } popstateEvent = new PopStateEvent('popstate') jest.clearAllMocks() @@ -63,7 +69,8 @@ describe('historyManager', () => { expect(component1.loadApp).not.toHaveBeenCalled() }) - it('removes component and focuses button when view param does not match', () => { + it('removes component when view param does not match and preserveStateOnClose is false', () => { + component1.config.preserveStateOnClose = false component1.rootEl.appendChild(document.createElement('div')) historyManager.register(component1) queryString.getQueryParam.mockReturnValue(null) @@ -71,7 +78,19 @@ describe('historyManager', () => { window.dispatchEvent(popstateEvent) expect(component1.removeApp).toHaveBeenCalled() - expect(component1.openButton.focus).toHaveBeenCalled() + expect(component1.hideApp).not.toHaveBeenCalled() + }) + + it('hides component when view param does not match and preserveStateOnClose is true', () => { + component1.config.preserveStateOnClose = true + component1.rootEl.appendChild(document.createElement('div')) + historyManager.register(component1) + queryString.getQueryParam.mockReturnValue(null) + + window.dispatchEvent(popstateEvent) + + expect(component1.hideApp).toHaveBeenCalled() + expect(component1.removeApp).not.toHaveBeenCalled() }) it('does not remove hybrid component when viewport is wide (inline mode)', () => { @@ -87,6 +106,7 @@ describe('historyManager', () => { }) it('removes hybrid component when viewport is narrow and view does not match', () => { + component2.config.preserveStateOnClose = false component2.rootEl.appendChild(document.createElement('div')) historyManager.register(component2) queryString.getQueryParam.mockReturnValue(null) @@ -98,6 +118,17 @@ describe('historyManager', () => { expect(component2.removeApp).toHaveBeenCalled() }) + it('calls showApp when view param matches and component is hidden', () => { + component1._isHidden = true + historyManager.register(component1) + queryString.getQueryParam.mockReturnValue('map') + + window.dispatchEvent(popstateEvent) + + expect(component1.showApp).toHaveBeenCalled() + expect(component1.loadApp).not.toHaveBeenCalled() + }) + it('uses hybridWidth for media query when provided', () => { component2.config.hybridWidth = 768 component2.rootEl.appendChild(document.createElement('div')) diff --git a/src/config/appConfig.js b/src/config/appConfig.js index aadf4bdc..a1ae97e9 100755 --- a/src/config/appConfig.js +++ b/src/config/appConfig.js @@ -18,12 +18,12 @@ const exitButtonSlots = { } // Default app buttons, panels and icons -export const appConfig = { +export const defaultAppConfig = { buttons: [{ id: 'exit', label: 'Exit', iconId: 'close', - onClick: (e, { services }) => services.closeApp(), + onClick: (_e, { services }) => services.closeApp(), excludeWhen: ({ appConfig, appState }) => !appConfig.hasExitButton || !(appState.isFullscreen && (new URL(window.location.href)).searchParams.has(appConfig.mapViewParamKey)), mobile: exitButtonSlots, tablet: exitButtonSlots, @@ -32,7 +32,7 @@ export const appConfig = { id: 'fullscreen', label: () => `${document.fullscreenElement ? 'Exit' : 'Enter'} fullscreen`, iconId: () => document.fullscreenElement ? 'minimise' : 'maximise', - onClick: (e, { appState }) => { + onClick: (_e, { appState }) => { const container = appState.layoutRefs.appContainerRef.current document.fullscreenElement ? document.exitFullscreen() : container.requestFullscreen() }, @@ -45,7 +45,7 @@ export const appConfig = { group: 'zoom', label: 'Zoom in', iconId: 'plus', - onClick: (e, { mapProvider, appConfig }) => mapProvider.zoomIn(appConfig.zoomDelta), + onClick: (_e, { mapProvider, appConfig }) => mapProvider.zoomIn(appConfig.zoomDelta), excludeWhen: ({ appState, appConfig }) => !appConfig.enableZoomControls || appState.interfaceType === 'touch', enableWhen: ({ mapState }) => !mapState.isAtMaxZoom, mobile: buttonSlots, @@ -56,7 +56,7 @@ export const appConfig = { group: 'zoom', label: 'Zoom out', iconId: 'minus', - onClick: (e, { mapProvider, appConfig }) => mapProvider.zoomOut(appConfig.zoomDelta), + onClick: (_e, { mapProvider, appConfig }) => mapProvider.zoomOut(appConfig.zoomDelta), excludeWhen: ({ appState, appConfig }) => !appConfig.enableZoomControls || appState.interfaceType === 'touch', enableWhen: ({ mapState }) => !mapState.isAtMinZoom, mobile: buttonSlots, diff --git a/src/config/appConfig.test.js b/src/config/appConfig.test.js index e137ef9a..0c821fb5 100755 --- a/src/config/appConfig.test.js +++ b/src/config/appConfig.test.js @@ -1,14 +1,14 @@ import { render } from '@testing-library/react' -import { appConfig } from './appConfig' +import { defaultAppConfig } from './appConfig' -describe('appConfig', () => { +describe('defaultAppConfig', () => { const appState = { layoutRefs: { appContainerRef: { current: document.createElement('div') } }, isFullscreen: false } - const buttons = appConfig.buttons + const buttons = defaultAppConfig.buttons const fullscreenBtn = buttons.find(b => b.id === 'fullscreen') const exitBtn = buttons.find(b => b.id === 'exit') it('renders KeyboardHelp panel', () => { - const panel = appConfig.panels.find(p => p.id === 'keyboardHelp') + const panel = defaultAppConfig.panels.find(p => p.id === 'keyboardHelp') const { container } = render(panel.render()) expect(container.querySelector('.im-c-keyboard-help')).toBeInTheDocument() }) @@ -16,11 +16,11 @@ describe('appConfig', () => { it('evaluates dynamic button properties', () => { // label expect(typeof fullscreenBtn.label).toBe('function') - expect(fullscreenBtn.label({ appState, appConfig })).toMatch(/fullscreen/) + expect(fullscreenBtn.label({ appState, appConfig: defaultAppConfig })).toMatch(/fullscreen/) // iconId expect(typeof fullscreenBtn.iconId).toBe('function') - expect(fullscreenBtn.iconId({ appState, appConfig })).toBe('maximise') + expect(fullscreenBtn.iconId({ appState, appConfig: defaultAppConfig })).toBe('maximise') // excludeWhen expect(exitBtn.excludeWhen({ appConfig: { hasExitButton: false } })).toBe(true) @@ -50,13 +50,13 @@ describe('appConfig', () => { // Not in fullscreen Object.defineProperty(document, 'fullscreenElement', { value: null, writable: true }) - expect(fullscreenBtn.label({ appState, appConfig })).toBe('Enter fullscreen') - expect(fullscreenBtn.iconId({ appState, appConfig })).toBe('maximise') + expect(fullscreenBtn.label({ appState, appConfig: defaultAppConfig })).toBe('Enter fullscreen') + expect(fullscreenBtn.iconId({ appState, appConfig: defaultAppConfig })).toBe('maximise') // In fullscreen Object.defineProperty(document, 'fullscreenElement', { value: containerMock, writable: true }) - expect(fullscreenBtn.label({ appState, appConfig })).toBe('Exit fullscreen') - expect(fullscreenBtn.iconId({ appState, appConfig })).toBe('minimise') + expect(fullscreenBtn.label({ appState, appConfig: defaultAppConfig })).toBe('Exit fullscreen') + expect(fullscreenBtn.iconId({ appState, appConfig: defaultAppConfig })).toBe('minimise') }) it('calls exit button onClick correctly', () => { diff --git a/src/config/defaults.js b/src/config/defaults.js index 5978516e..c956661d 100755 --- a/src/config/defaults.js +++ b/src/config/defaults.js @@ -33,6 +33,7 @@ const defaults = { nudgeZoomDelta: 0.1, panDelta: 100, pageTitle: 'Map view', + preserveStateOnClose: false, readMapText: false, reverseGeocodeProvider: null, zoomDelta: 1 diff --git a/src/types.js b/src/types.js index 148cb14b..4dae6abd 100644 --- a/src/types.js +++ b/src/types.js @@ -568,6 +568,11 @@ * @property {PluginDescriptor[]} [plugins] * Plugins to load. * + * @property {boolean} [preserveStateOnClose=false] + * Whether to preserve the map state when closed via back button or exit button. + * When true, the map is hidden but not destroyed, preserving markers, zoom, etc. + * Useful for list/map toggle scenarios. Only applies to 'hybrid' and 'buttonFirst' behaviours. + * * @property {boolean} [readMapText=false] * Whether map text labels can be selected and read aloud by assistive technologies. * diff --git a/src/utils/detectBreakpoint.js b/src/utils/detectBreakpoint.js index 8341ff5a..baf85da1 100755 --- a/src/utils/detectBreakpoint.js +++ b/src/utils/detectBreakpoint.js @@ -1,106 +1,105 @@ -function createBreakpointDetector ({ maxMobileWidth, minDesktopWidth, containerEl }) { - let lastBreakpoint = 'unknown' - const listeners = new Set() - let cleanup = null - - const getBreakpointType = (width) => { - if (width <= maxMobileWidth) { - return 'mobile' - } - if (width >= minDesktopWidth) { - return 'desktop' - } - return 'tablet' +function getBreakpointType(width, maxMobileWidth, minDesktopWidth) { + if (width <= maxMobileWidth) { + return 'mobile' } - - const notifyListeners = (type) => { - if (type !== lastBreakpoint) { - lastBreakpoint = type // Set synchronously BEFORE RAF - requestAnimationFrame(() => { - // Double-check it hasn't changed again - if (lastBreakpoint === type) { - listeners.forEach(fn => fn(type)) - } - }) - } + if (width >= minDesktopWidth) { + return 'desktop' } + return 'tablet' +} - // Container-based detection - if (containerEl) { - containerEl.style.containerType = 'inline-size' - - // Set initial detection BEFORE observing to prevent double notification - const initialWidth = containerEl.getBoundingClientRect().width - const initialType = getBreakpointType(initialWidth) - containerEl.setAttribute('data-breakpoint', initialType) - lastBreakpoint = initialType // Set this directly, don't notify yet +function createContainerDetector(containerEl, getType, notifyListeners) { + containerEl.style.containerType = 'inline-size' - const observer = new ResizeObserver((entries) => { - const width = entries[0]?.borderBoxSize?.[0]?.inlineSize || entries[0]?.contentRect.width - const type = getBreakpointType(width) - containerEl.setAttribute('data-breakpoint', type) - notifyListeners(type) - }) + const initialWidth = containerEl.getBoundingClientRect().width + const initialType = getType(initialWidth) + containerEl.dataset.breakpoint = initialType - observer.observe(containerEl) + const observer = new ResizeObserver((entries) => { + const width = entries[0]?.borderBoxSize?.[0]?.inlineSize || entries[0]?.contentRect.width + const type = getType(width) + containerEl.dataset.breakpoint = type + notifyListeners(type) + }) - // Now notify listeners after observer is set up - notifyListeners(initialType) + observer.observe(containerEl) - cleanup = () => { + return { + initialType, + cleanup: () => { observer.disconnect() containerEl.style.containerType = '' - containerEl.removeAttribute('data-breakpoint') + delete containerEl.dataset.breakpoint } - } else { - // Viewport-based fallback - const mq = { - mobile: window.matchMedia(`(max-width: ${maxMobileWidth}px)`), - desktop: window.matchMedia(`(min-width: ${minDesktopWidth}px)`) - } - - const detect = () => { - let type + } +} - if (mq.mobile.matches) { - type = 'mobile' - } else if (mq.desktop.matches) { - type = 'desktop' - } else { - type = 'tablet' - } +function createViewportDetector(maxMobileWidth, minDesktopWidth, notifyListeners) { + const mq = { + mobile: window.matchMedia(`(max-width: ${maxMobileWidth}px)`), + desktop: window.matchMedia(`(min-width: ${minDesktopWidth}px)`) + } - notifyListeners(type) + const detect = () => { + let type = 'tablet' + if (mq.mobile.matches) { + type = 'mobile' + } else if (mq.desktop.matches) { + type = 'desktop' } + notifyListeners(type) + } - mq.mobile.addEventListener('change', detect) - mq.desktop.addEventListener('change', detect) - detect() + mq.mobile.addEventListener('change', detect) + mq.desktop.addEventListener('change', detect) + detect() - cleanup = () => { + return { + cleanup: () => { mq.mobile.removeEventListener('change', detect) mq.desktop.removeEventListener('change', detect) } } +} - const subscribe = (fn) => { - listeners.add(fn) - return () => listeners.delete(fn) - } +function createBreakpointDetector({ maxMobileWidth, minDesktopWidth, containerEl }) { + let lastBreakpoint = 'unknown' + const listeners = new Set() - const getBreakpoint = () => { - return lastBreakpoint === 'unknown' ? 'desktop' : lastBreakpoint + const notifyListeners = (type) => { + if (type !== lastBreakpoint) { + lastBreakpoint = type + requestAnimationFrame(() => { + if (lastBreakpoint === type) { + listeners.forEach(fn => fn(type)) + } + }) + } } - const destroy = () => { - cleanup?.() - listeners.clear() + const getType = (width) => getBreakpointType(width, maxMobileWidth, minDesktopWidth) + + let cleanup + if (containerEl) { + const detector = createContainerDetector(containerEl, getType, notifyListeners) + lastBreakpoint = detector.initialType + notifyListeners(detector.initialType) + cleanup = detector.cleanup + } else { + const detector = createViewportDetector(maxMobileWidth, minDesktopWidth, notifyListeners) + cleanup = detector.cleanup } return { - subscribe, - getBreakpoint, - destroy + subscribe: (fn) => { + listeners.add(fn) + return () => listeners.delete(fn) + }, + getBreakpoint: () => lastBreakpoint, + destroy: () => { + cleanup?.() + listeners.clear() + } } } From 4a475cbafcd83e527be8521421724d375e74d31c Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Fri, 6 Feb 2026 12:28:55 +0000 Subject: [PATCH 3/5] Lint fix --- src/utils/detectBreakpoint.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utils/detectBreakpoint.js b/src/utils/detectBreakpoint.js index baf85da1..d4463798 100755 --- a/src/utils/detectBreakpoint.js +++ b/src/utils/detectBreakpoint.js @@ -1,4 +1,4 @@ -function getBreakpointType(width, maxMobileWidth, minDesktopWidth) { +function getBreakpointType (width, maxMobileWidth, minDesktopWidth) { if (width <= maxMobileWidth) { return 'mobile' } @@ -8,7 +8,7 @@ function getBreakpointType(width, maxMobileWidth, minDesktopWidth) { return 'tablet' } -function createContainerDetector(containerEl, getType, notifyListeners) { +function createContainerDetector (containerEl, getType, notifyListeners) { containerEl.style.containerType = 'inline-size' const initialWidth = containerEl.getBoundingClientRect().width @@ -34,7 +34,7 @@ function createContainerDetector(containerEl, getType, notifyListeners) { } } -function createViewportDetector(maxMobileWidth, minDesktopWidth, notifyListeners) { +function createViewportDetector (maxMobileWidth, minDesktopWidth, notifyListeners) { const mq = { mobile: window.matchMedia(`(max-width: ${maxMobileWidth}px)`), desktop: window.matchMedia(`(min-width: ${minDesktopWidth}px)`) @@ -62,7 +62,7 @@ function createViewportDetector(maxMobileWidth, minDesktopWidth, notifyListeners } } -function createBreakpointDetector({ maxMobileWidth, minDesktopWidth, containerEl }) { +function createBreakpointDetector ({ maxMobileWidth, minDesktopWidth, containerEl }) { let lastBreakpoint = 'unknown' const listeners = new Set() From 05d2ea7afd56a19faec7316743491aab42fe704e Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Fri, 6 Feb 2026 15:42:04 +0000 Subject: [PATCH 4/5] Sonar fixes --- demo/js/index.js | 7 +-- demo/js/planning.js | 2 +- .../src/hooks/useInteractionHandlers.js | 8 ++- src/InteractiveMap/behaviourController.js | 8 +-- src/InteractiveMap/historyManager.js | 53 ++++++++++++++----- src/utils/detectBreakpoint.js | 2 + 6 files changed, 57 insertions(+), 23 deletions(-) diff --git a/demo/js/index.js b/demo/js/index.js index 0792254a..a09b73db 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -22,8 +22,10 @@ var interactPlugin = createInteractPlugin({ },{ layerId: 'linked-parcels', // idProperty: 'id' + },{ + layerId: 'OS/TopographicArea_1/Agricultural Land' }], - interactionMode: 'auto', // 'auto', 'select', 'marker' // defaults to 'marker' + interactionMode: 'select', // 'auto', 'select', 'marker' // defaults to 'marker' multiSelect: true, contiguous: true, // excludeModes: ['draw'] @@ -150,7 +152,6 @@ interactiveMap.on('app:ready', function (e) { }) interactiveMap.on('map:ready', function (e) { - // console.log('map:ready') // framePlugin.addFrame('test', { // aspectRatio: 1 // }) @@ -179,7 +180,7 @@ interactiveMap.on('draw:ready', function () { // drawPlugin.split('test1234', { // snapLayers: ['OS/TopographicArea_1/Agricultural Land'] // }) - // drawPlugin.newLine('test', { + // drawPlugin.newPolygon('test', { // snapLayers: ['OS/TopographicArea_1/Agricultural Land'] // }) // drawPlugin.editFeature('test1234') diff --git a/demo/js/planning.js b/demo/js/planning.js index 75db6fe6..2fdb62e1 100755 --- a/demo/js/planning.js +++ b/demo/js/planning.js @@ -131,7 +131,7 @@ const interactiveMap = new InteractiveMap('map', { // search }) -interactiveMap.on('map:ready', function (e) { +interactiveMap.on('app:ready', function (e) { interactiveMap.addButton('menu', { label: 'Menu', panelId: 'menu', diff --git a/plugins/interact/src/hooks/useInteractionHandlers.js b/plugins/interact/src/hooks/useInteractionHandlers.js index bd9c7041..af216b21 100755 --- a/plugins/interact/src/hooks/useInteractionHandlers.js +++ b/plugins/interact/src/hooks/useInteractionHandlers.js @@ -62,12 +62,16 @@ export const useInteractionHandlers = ({ useEffect(() => { // Skip if features exist but bounds not yet calculated const awaitingBounds = selectedFeatures.length > 0 && !selectionBounds - if (awaitingBounds) return + if (awaitingBounds) { + return + } // 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) return + if (wasEmpty && selectedFeatures.length === 0) { + return + } eventBus.emit('interact:selectionchange', { selectedFeatures, diff --git a/src/InteractiveMap/behaviourController.js b/src/InteractiveMap/behaviourController.js index 03cc3018..0ca44b07 100755 --- a/src/InteractiveMap/behaviourController.js +++ b/src/InteractiveMap/behaviourController.js @@ -59,16 +59,16 @@ function setupBehavior (mapInstance) { if (shouldLoadComponent(mapInstance.config)) { if (mapInstance._isHidden) { mapInstance.showApp() - } else if (!mapInstance._root) { + } else if (mapInstance._root == null) { mapInstance.loadApp() } else { // Map is showing - update DOM state for fullscreen/inline transition updateDOMState(mapInstance) } + } else if (mapInstance._root) { + mapInstance.hideApp() } else { - if (mapInstance._root) { - mapInstance.hideApp() - } + // No action } } diff --git a/src/InteractiveMap/historyManager.js b/src/InteractiveMap/historyManager.js index f13e05d2..ef9ae58c 100755 --- a/src/InteractiveMap/historyManager.js +++ b/src/InteractiveMap/historyManager.js @@ -6,6 +6,40 @@ import defaults from '../config/defaults.js' // Internal helpers // ----------------------------------------------------------------------------- +/** + * Opens the map application for a given map instance. + * + * If the map instance was previously hidden, it restores the existing app. + * Otherwise, it loads the app for the first time. + * + * @param {MapInstance} mapInstance + * The map instance whose application should be opened. + */ +function openMap(mapInstance) { + if (mapInstance._isHidden) { + mapInstance.showApp?.() + } else { + mapInstance.loadApp?.() + } +} + +/** + * Closes the map application for a given map instance. + * + * Depending on the configuration, the map state is either preserved + * by hiding the app or fully removed from the DOM. + * + * @param {MapInstance} mapInstance + * The map instance whose application should be closed. + */ +function closeMap(mapInstance) { + if (mapInstance.config.preserveStateOnClose) { + mapInstance.hideApp?.() + } else { + mapInstance.removeApp?.() + } +} + /** * Handles the `popstate` event triggered by browser back/forward navigation. * @@ -29,19 +63,12 @@ function handlePopstate () { const isOpen = mapInstance.rootEl?.children.length if (shouldBeOpen && (!isOpen || mapInstance._isHidden)) { - if (mapInstance._isHidden) { - mapInstance.showApp?.() - } else { - mapInstance.loadApp?.() - } - } else if (!shouldBeOpen && isOpen && !isHybridVisible) { - if (mapInstance.config.preserveStateOnClose) { - mapInstance.hideApp?.() - } else { - mapInstance.removeApp?.() - } - } else { - // No action + openMap(mapInstance) + continue + } + + if (!shouldBeOpen && isOpen && !isHybridVisible) { + closeMap(mapInstance) } } } diff --git a/src/utils/detectBreakpoint.js b/src/utils/detectBreakpoint.js index d4463798..9106b02f 100755 --- a/src/utils/detectBreakpoint.js +++ b/src/utils/detectBreakpoint.js @@ -46,6 +46,8 @@ function createViewportDetector (maxMobileWidth, minDesktopWidth, notifyListener type = 'mobile' } else if (mq.desktop.matches) { type = 'desktop' + } else { + // No action } notifyListeners(type) } From 4782da7aec24b987155382cb912c919f5fb5768e Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Fri, 6 Feb 2026 15:43:32 +0000 Subject: [PATCH 5/5] Lint fixes --- src/InteractiveMap/historyManager.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/InteractiveMap/historyManager.js b/src/InteractiveMap/historyManager.js index ef9ae58c..baca10d8 100755 --- a/src/InteractiveMap/historyManager.js +++ b/src/InteractiveMap/historyManager.js @@ -15,7 +15,7 @@ import defaults from '../config/defaults.js' * @param {MapInstance} mapInstance * The map instance whose application should be opened. */ -function openMap(mapInstance) { +function openMap (mapInstance) { if (mapInstance._isHidden) { mapInstance.showApp?.() } else { @@ -32,7 +32,7 @@ function openMap(mapInstance) { * @param {MapInstance} mapInstance * The map instance whose application should be closed. */ -function closeMap(mapInstance) { +function closeMap (mapInstance) { if (mapInstance.config.preserveStateOnClose) { mapInstance.hideApp?.() } else {