diff --git a/demo/js/index.js b/demo/js/index.js index a63d48cb..b7cdf20a 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -90,7 +90,7 @@ const interactPlugin = createInteractPlugin({ } ], debug: true, - interactionModes: ['selectMarker', 'selectFeature'], // e.g. ['selectMarker'], ['selectFeature'], ['placeMarker'], or combinations + interactionModes: ['selectMarker', 'selectFeature', 'placeMarker'], // e.g. ['selectMarker'], ['selectFeature'], ['placeMarker'], or combinations multiSelect: true, deselectOnClickOutside: true }) @@ -270,7 +270,7 @@ const interactiveMap = new InteractiveMap('map', { }), // maxMobileWidth: 700, // minDesktopWidth: 960, - mapLabel: 'Map showing Carlisle', + mapLabel: 'Map showing field parcels and land use', // zoom: 14, minZoom: 6, maxZoom: 20, @@ -279,7 +279,7 @@ const interactiveMap = new InteractiveMap('map', { bounds: [-2.450804, 54.5599279, -2.403804, 54.6199279], containerHeight: '650px', transformRequest: transformTileRequest, - readMapText: true, + // readMapText: true, // urlPosition: 'none', // enableFullscreen: true, // hasExitButton: true, diff --git a/docs/assets/css/docusaurus.scss b/docs/assets/css/docusaurus.scss index adccfe65..1736d38b 100644 --- a/docs/assets/css/docusaurus.scss +++ b/docs/assets/css/docusaurus.scss @@ -52,7 +52,7 @@ .govuk-template--rebranded .app-masthead .govuk-grid-column-one-third-from-desktop { @media (min-width: 48.125em) { - background-image: url('/images/hero.png'); + background-image: url('https://defra.github.io/interactive-map/images/hero.png'); background-repeat: no-repeat; background-position: center bottom; background-size: 220px; diff --git a/docs/plugins/interact.md b/docs/plugins/interact.md index 2a633c89..4136b808 100644 --- a/docs/plugins/interact.md +++ b/docs/plugins/interact.md @@ -201,7 +201,7 @@ interactPlugin.disable() ### `clear()` -Clear all selected features and markers, and remove the location marker. +Clear all selected features and markers. ```js interactPlugin.clear() diff --git a/plugins/interact/src/InteractInit.jsx b/plugins/interact/src/InteractInit.jsx index a9ef9bbc..8a87ee9c 100755 --- a/plugins/interact/src/InteractInit.jsx +++ b/plugins/interact/src/InteractInit.jsx @@ -1,12 +1,24 @@ -import { useEffect, useRef } from 'react' +import { useEffect } from 'react' import { EVENTS } from '../../../src/config/events.js' import { useInteractionHandlers } from './hooks/useInteractionHandlers.js' import { useMapItemList } from './hooks/useMapItemList.js' import { useHighlightSync } from './hooks/useHighlightSync.js' import { useHoverCursor } from './hooks/useHoverCursor.js' -import { attachEvents } from './events.js' +import { useCrossHairVisibility } from './hooks/useCrossHairVisibility.js' +import { useAttachEvents } from './hooks/useAttachEvents.js' import { isSelectMarkerOnly } from './utils/interactionModes.js' +function useListboxCapable ({ enabled, interactionModes, markers, layers, eventBus }) { + useEffect(() => { + if (!enabled) { return } + const hasLabeledMarkers = interactionModes?.includes('selectMarker') && markers.items.some(m => m.label) + const hasFeatureLayers = interactionModes?.includes('selectFeature') && layers.some(l => l.labelProperty) + if (hasLabeledMarkers || hasFeatureLayers) { + eventBus.emit('interact:listboxcapable') + } + }, [enabled, interactionModes, markers, layers, eventBus]) +} + export const InteractInit = ({ appState, mapState, @@ -15,12 +27,10 @@ export const InteractInit = ({ mapProvider, pluginState }) => { - const { interfaceType } = appState const { dispatch, enabled, selectedFeatures, interactionModes, layers } = pluginState const { eventBus, closeApp } = services - const { crossHair, mapStyle } = mapState + const { crossHair, mapStyle, markers } = mapState - const isTouchOrKeyboard = ['touch', 'keyboard'].includes(interfaceType) const selectMarkerOnly = isSelectMarkerOnly(interactionModes) useMapItemList({ mapState, pluginState, services, mapProvider }) @@ -34,27 +44,6 @@ export const InteractInit = ({ mapProvider }) - // Refs updated synchronously each render — keeps callbacks fresh without re-attaching events - const handleInteractionRef = useRef(handleInteraction) - handleInteractionRef.current = handleInteraction - - const pluginStateRef = useRef(pluginState) - pluginStateRef.current = pluginState - - const appStateRef = useRef(appState) - appStateRef.current = appState - - // Defer click handling by one macrotask so any click that triggered the enable - // (e.g. finishing a draw gesture) fires before this handler is live. - // Managed separately from attachEvents so re-runs of that effect don't reset it — - // only resets when enabled actually changes. - const clickReadyRef = useRef(false) - useEffect(() => { - clickReadyRef.current = false - const timer = setTimeout(() => { clickReadyRef.current = true }, 0) - return () => clearTimeout(timer) - }, [pluginState.enabled]) - // Highlight features and sync state selectedBounds from mapProvider useHighlightSync({ mapProvider, @@ -71,36 +60,13 @@ export const InteractInit = ({ eventBus.emit('interact:active', { active: enabled, interactionModes }) }, [enabled, interactionModes]) - useHoverCursor(mapProvider, enabled, interactionModes, layers) - - // Toggle target marker visibility - useEffect(() => { - if (enabled && isTouchOrKeyboard && !(interfaceType === 'touch' && selectMarkerOnly)) { - crossHair.fixAtCenter() - } else { - crossHair.hide() - } - }, [enabled, interfaceType, interactionModes]) + useListboxCapable({ enabled, interactionModes, markers, layers, eventBus }) - useEffect(() => { - if (!pluginState.enabled) { - return undefined // Explicit return - } + useHoverCursor(mapProvider, enabled, interactionModes, layers) - const cleanupEvents = attachEvents({ - getAppState: () => appStateRef.current, - mapState, - getPluginState: () => pluginStateRef.current, - buttonConfig, - events: EVENTS, - eventBus, - handleInteraction: (event) => handleInteractionRef.current(event), - clickReadyRef, - closeApp - }) + useCrossHairVisibility({ crossHair, enabled, selectMarkerOnly, appState }) - return cleanupEvents - }, [pluginState.enabled, buttonConfig, eventBus, closeApp]) + useAttachEvents({ pluginState, appState, mapState, buttonConfig, eventBus, handleInteraction, closeApp }) return null } diff --git a/plugins/interact/src/InteractInit.test.js b/plugins/interact/src/InteractInit.test.js index 24b3079b..3457b6b1 100644 --- a/plugins/interact/src/InteractInit.test.js +++ b/plugins/interact/src/InteractInit.test.js @@ -1,51 +1,50 @@ -import { act, render } from '@testing-library/react' +import { render } from '@testing-library/react' 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 { useMapItemList } from './hooks/useMapItemList.js' -import { attachEvents } from './events.js' + +const LISTBOX_CAPABLE = 'interact:listboxcapable' jest.mock('./hooks/useInteractionHandlers.js') jest.mock('./hooks/useHighlightSync.js') jest.mock('./hooks/useHoverCursor.js') jest.mock('./hooks/useMapItemList.js') -jest.mock('./events.js') - -describe('InteractInit', () => { - let props - let handleInteractionMock - let cleanupMock - - beforeEach(() => { - handleInteractionMock = jest.fn() - cleanupMock = jest.fn() - - useInteractionHandlers.mockReturnValue({ handleInteraction: handleInteractionMock }) - useHighlightSync.mockReturnValue(undefined) - useHoverCursor.mockReturnValue(undefined) - useMapItemList.mockReturnValue(undefined) - attachEvents.mockReturnValue(cleanupMock) - - props = { - appState: { interfaceType: 'mouse', layoutRefs: { viewportRef: { current: null } } }, - mapState: { crossHair: { fixAtCenter: jest.fn(), hide: jest.fn() }, mapStyle: {} }, - services: { eventBus: { emit: jest.fn() }, closeApp: jest.fn() }, - buttonConfig: {}, - mapProvider: { setHoverCursor: jest.fn() }, - pluginState: { - dispatch: jest.fn(), - enabled: true, - selectedFeatures: [], - selectedMarkers: [], - selectionBounds: {}, - interactionModes: ['selectFeature'], - layers: [] - } +jest.mock('./hooks/useCrossHairVisibility.js') +jest.mock('./hooks/useAttachEvents.js') + +let props +let handleInteractionMock + +beforeEach(() => { + handleInteractionMock = jest.fn() + + useInteractionHandlers.mockReturnValue({ handleInteraction: handleInteractionMock }) + useHighlightSync.mockReturnValue(undefined) + useHoverCursor.mockReturnValue(undefined) + useMapItemList.mockReturnValue(undefined) + + props = { + appState: { interfaceType: 'mouse', layoutRefs: { viewportRef: { current: document.createElement('div') }, appContainerRef: { current: document.createElement('div') } } }, + mapState: { crossHair: { fixAtCenter: jest.fn(), hide: jest.fn() }, mapStyle: {} }, + services: { eventBus: { emit: jest.fn() }, closeApp: jest.fn() }, + buttonConfig: {}, + mapProvider: { setHoverCursor: jest.fn() }, + pluginState: { + dispatch: jest.fn(), + enabled: true, + selectedFeatures: [], + selectedMarkers: [], + selectionBounds: {}, + interactionModes: ['selectFeature'], + layers: [] } - }) + } +}) +describe('InteractInit — hook delegation', () => { it('calls useInteractionHandlers with correct arguments', () => { render() expect(useInteractionHandlers).toHaveBeenCalledWith(expect.objectContaining({ @@ -69,44 +68,9 @@ describe('InteractInit', () => { eventBus: props.services.eventBus })) }) +}) - it('fixes or hides crossHair based on interfaceType and enabled', () => { - // enabled true + non-touch = hide - render() - expect(props.mapState.crossHair.hide).toHaveBeenCalled() - expect(props.mapState.crossHair.fixAtCenter).not.toHaveBeenCalled() - - // touch interface - props.appState.interfaceType = 'touch' - render() - expect(props.mapState.crossHair.fixAtCenter).toHaveBeenCalled() - }) - - it('attaches events and returns cleanup', () => { - const { unmount } = render() - expect(attachEvents).toHaveBeenCalledWith(expect.objectContaining({ - getAppState: expect.any(Function), - getPluginState: expect.any(Function), - handleInteraction: expect.any(Function), - mapState: props.mapState, - buttonConfig: props.buttonConfig, - events: EVENTS, - eventBus: props.services.eventBus, - closeApp: props.services.closeApp - })) - - const { getAppState, getPluginState, handleInteraction } = attachEvents.mock.calls.at(-1)[0] - expect(getAppState()).toMatchObject(props.appState) - expect(getPluginState()).toMatchObject({ enabled: props.pluginState.enabled }) - - const event = { point: {}, coords: [] } - handleInteraction(event) - expect(handleInteractionMock).toHaveBeenCalledWith(event) - - unmount() - expect(cleanupMock).toHaveBeenCalled() - }) - +describe('InteractInit — event bus emissions', () => { it('emits interact:active with active state and interactionModes on enable', () => { render() expect(props.services.eventBus.emit).toHaveBeenCalledWith('interact:active', { @@ -115,20 +79,31 @@ describe('InteractInit', () => { }) }) - it('enables click handling after a macrotask', () => { - jest.useFakeTimers() - render() - act(() => jest.runAllTimers()) - jest.useRealTimers() + it('emits interact:listboxcapable when enabled with a feature layer that has a labelProperty', () => { + const capableProps = { + ...props, + pluginState: { ...props.pluginState, interactionModes: ['selectFeature'], layers: [{ layerId: 'myLayer', labelProperty: 'name' }] } + } + render() + expect(capableProps.services.eventBus.emit).toHaveBeenCalledWith(LISTBOX_CAPABLE) + }) + + it('emits interact:listboxcapable when enabled with a labeled marker', () => { + const capableProps = { + ...props, + mapState: { ...props.mapState, markers: { items: [{ id: 'm1', label: 'My marker' }] } }, + pluginState: { ...props.pluginState, interactionModes: ['selectMarker'] } + } + render() + expect(capableProps.services.eventBus.emit).toHaveBeenCalledWith(LISTBOX_CAPABLE) }) - it('does not attach events if plugin not enabled', () => { + it('does not emit interact:listboxcapable when disabled', () => { const disabledProps = { ...props, - pluginState: { ...props.pluginState, enabled: false } // fresh object + pluginState: { ...props.pluginState, enabled: false, interactionModes: ['selectFeature'], layers: [{ layerId: 'myLayer', labelProperty: 'name' }] } } - attachEvents.mockClear() // ensure previous calls don't interfere render() - expect(attachEvents).not.toHaveBeenCalled() + expect(disabledProps.services.eventBus.emit).not.toHaveBeenCalledWith(LISTBOX_CAPABLE) }) }) diff --git a/plugins/interact/src/hooks/useAttachEvents.js b/plugins/interact/src/hooks/useAttachEvents.js new file mode 100644 index 00000000..be83457e --- /dev/null +++ b/plugins/interact/src/hooks/useAttachEvents.js @@ -0,0 +1,46 @@ +import { useEffect, useRef } from 'react' +import { EVENTS } from '../../../../src/config/events.js' +import { attachEvents } from '../events.js' + +export function useAttachEvents ({ pluginState, appState, mapState, buttonConfig, eventBus, handleInteraction, closeApp }) { + // Refs updated synchronously each render — keeps callbacks fresh without re-attaching events + const handleInteractionRef = useRef(handleInteraction) + handleInteractionRef.current = handleInteraction + + const pluginStateRef = useRef(pluginState) + pluginStateRef.current = pluginState + + const appStateRef = useRef(appState) + appStateRef.current = appState + + // Defer click handling by one macrotask so any click that triggered the enable + // (e.g. finishing a draw gesture) fires before this handler is live. + // Managed separately from attachEvents so re-runs of that effect don't reset it — + // only resets when enabled actually changes. + const clickReadyRef = useRef(false) + useEffect(() => { + clickReadyRef.current = false + const timer = setTimeout(() => { clickReadyRef.current = true }, 0) + return () => clearTimeout(timer) + }, [pluginState.enabled]) + + useEffect(() => { + if (!pluginState.enabled) { + return undefined + } + + const cleanupEvents = attachEvents({ + getAppState: () => appStateRef.current, + mapState, + getPluginState: () => pluginStateRef.current, + buttonConfig, + events: EVENTS, + eventBus, + handleInteraction: (event) => handleInteractionRef.current(event), + clickReadyRef, + closeApp + }) + + return cleanupEvents + }, [pluginState.enabled, buttonConfig, eventBus, closeApp]) +} diff --git a/plugins/interact/src/hooks/useAttachEvents.test.js b/plugins/interact/src/hooks/useAttachEvents.test.js new file mode 100644 index 00000000..07b0b296 --- /dev/null +++ b/plugins/interact/src/hooks/useAttachEvents.test.js @@ -0,0 +1,65 @@ +import { act, renderHook } from '@testing-library/react' +import { useAttachEvents } from './useAttachEvents.js' +import { attachEvents } from '../events.js' +import { EVENTS } from '../../../../src/config/events.js' + +jest.mock('../events.js') + +let props +let cleanupMock +let handleInteractionMock + +beforeEach(() => { + jest.clearAllMocks() + cleanupMock = jest.fn() + handleInteractionMock = jest.fn() + attachEvents.mockReturnValue(cleanupMock) + + props = { + pluginState: { enabled: true }, + appState: { interfaceType: 'mouse' }, + mapState: { mapStyle: {} }, + buttonConfig: {}, + eventBus: { emit: jest.fn() }, + closeApp: jest.fn(), + handleInteraction: handleInteractionMock + } +}) + +describe('useAttachEvents', () => { + it('attaches events and returns cleanup', () => { + const { unmount } = renderHook(() => useAttachEvents(props)) + expect(attachEvents).toHaveBeenCalledWith(expect.objectContaining({ + getAppState: expect.any(Function), + getPluginState: expect.any(Function), + handleInteraction: expect.any(Function), + mapState: props.mapState, + buttonConfig: props.buttonConfig, + events: EVENTS, + eventBus: props.eventBus, + closeApp: props.closeApp + })) + + const { getAppState, getPluginState, handleInteraction } = attachEvents.mock.calls.at(-1)[0] + expect(getAppState()).toBe(props.appState) + expect(getPluginState()).toMatchObject({ enabled: props.pluginState.enabled }) + + handleInteraction({ point: {}, coords: [] }) + expect(handleInteractionMock).toHaveBeenCalled() + + unmount() + expect(cleanupMock).toHaveBeenCalled() + }) + + it('enables click handling after a macrotask', () => { + jest.useFakeTimers() + renderHook(() => useAttachEvents(props)) + act(() => jest.runAllTimers()) + jest.useRealTimers() + }) + + it('does not attach events if plugin not enabled', () => { + renderHook(() => useAttachEvents({ ...props, pluginState: { enabled: false } })) + expect(attachEvents).not.toHaveBeenCalled() + }) +}) diff --git a/plugins/interact/src/hooks/useCrossHairVisibility.js b/plugins/interact/src/hooks/useCrossHairVisibility.js new file mode 100644 index 00000000..2c03c2d6 --- /dev/null +++ b/plugins/interact/src/hooks/useCrossHairVisibility.js @@ -0,0 +1,47 @@ +import { useCallback, useEffect, useRef } from 'react' +import { getInterfaceType, subscribeToInterfaceChangesImmediate } from '../../../../src/utils/detectInterfaceType.js' + +export function useCrossHairVisibility ({ crossHair, enabled, selectMarkerOnly, appState }) { + const enabledRef = useRef(enabled) + enabledRef.current = enabled + const selectMarkerOnlyRef = useRef(selectMarkerOnly) + selectMarkerOnlyRef.current = selectMarkerOnly + const crossHairRef = useRef(crossHair) + crossHairRef.current = crossHair + const listboxFocusRef = useRef(false) + + const updateCrossHair = useCallback(() => { + const type = getInterfaceType() + const isToK = ['touch', 'keyboard'].includes(type) + if (enabledRef.current && !listboxFocusRef.current && isToK && !(type === 'touch' && selectMarkerOnlyRef.current)) { + crossHairRef.current.fixAtCenter() + } else { + crossHairRef.current.hide() + } + }, []) + + // Toggle target marker visibility on enabled/interactionModes changes + useEffect(() => { + updateCrossHair() + }, [enabled, selectMarkerOnly, updateCrossHair]) + + // Toggle target marker visibility immediately on interface type change (no 150ms React delay) + useEffect(() => { + return subscribeToInterfaceChangesImmediate(updateCrossHair) + }, [updateCrossHair]) + + // Hide crosshair when listbox has focus + useEffect(() => { + const container = appState.layoutRefs?.appContainerRef?.current + if (!container) { return undefined } + const handleFocusIn = (e) => { + const inListbox = !!e.target.closest('[role="listbox"], [role="option"]') + if (listboxFocusRef.current !== inListbox) { + listboxFocusRef.current = inListbox + updateCrossHair() + } + } + container.addEventListener('focusin', handleFocusIn) + return () => { container.removeEventListener('focusin', handleFocusIn) } + }, [appState.layoutRefs, updateCrossHair]) +} diff --git a/plugins/interact/src/hooks/useCrossHairVisibility.test.js b/plugins/interact/src/hooks/useCrossHairVisibility.test.js new file mode 100644 index 00000000..9582c85d --- /dev/null +++ b/plugins/interact/src/hooks/useCrossHairVisibility.test.js @@ -0,0 +1,54 @@ +import { act, renderHook } from '@testing-library/react' +import { useCrossHairVisibility } from './useCrossHairVisibility.js' +import { getInterfaceType } from '../../../../src/utils/detectInterfaceType.js' + +jest.mock('../../../../src/utils/detectInterfaceType.js', () => ({ + getInterfaceType: jest.fn(() => 'mouse'), + subscribeToInterfaceChangesImmediate: jest.fn(() => () => {}) +})) + +let crossHair +let appState + +beforeEach(() => { + jest.clearAllMocks() + crossHair = { fixAtCenter: jest.fn(), hide: jest.fn() } + appState = { layoutRefs: { appContainerRef: { current: document.createElement('div') } } } + getInterfaceType.mockReturnValue('mouse') +}) + +describe('useCrossHairVisibility', () => { + it('skips listbox focus listeners when appContainerRef is null', () => { + appState.layoutRefs.appContainerRef = { current: null } + renderHook(() => useCrossHairVisibility({ crossHair, enabled: true, selectMarkerOnly: false, appState })) + expect(crossHair.hide).toHaveBeenCalled() + }) + + it('shows crosshair on touch/keyboard and hides when listbox has focus', () => { + const container = appState.layoutRefs.appContainerRef.current + getInterfaceType.mockReturnValue('touch') + renderHook(() => useCrossHairVisibility({ crossHair, enabled: true, selectMarkerOnly: false, appState })) + + expect(crossHair.fixAtCenter).toHaveBeenCalled() + + // Focus moves into listbox — hide + const listboxEl = document.createElement('div') + listboxEl.setAttribute('role', 'listbox') + container.appendChild(listboxEl) + act(() => listboxEl.dispatchEvent(new FocusEvent('focusin', { bubbles: true }))) + expect(crossHair.hide).toHaveBeenCalled() + + // Second focusin still inside listbox — no-op (state unchanged) + const listboxEl2 = document.createElement('div') + listboxEl2.setAttribute('role', 'listbox') + container.appendChild(listboxEl2) + act(() => listboxEl2.dispatchEvent(new FocusEvent('focusin', { bubbles: true }))) + expect(crossHair.hide).toHaveBeenCalledTimes(1) + + // Focus moves back out of listbox — show again + const otherEl = document.createElement('div') + container.appendChild(otherEl) + act(() => otherEl.dispatchEvent(new FocusEvent('focusin', { bubbles: true }))) + expect(crossHair.fixAtCenter).toHaveBeenCalledTimes(2) + }) +}) diff --git a/plugins/interact/src/hooks/useMapItemList.js b/plugins/interact/src/hooks/useMapItemList.js index 0d11faa9..7f95602a 100644 --- a/plugins/interact/src/hooks/useMapItemList.js +++ b/plugins/interact/src/hooks/useMapItemList.js @@ -79,14 +79,15 @@ function useItemListSync ({ markers, interactionModes, layers, mapProvider, mult useEffect(() => { const handleMoveEnd = () => { const items = [] - if (interactionModes.includes('selectMarker')) { + if (interactionModes?.includes('selectMarker')) { items.push(...collectVisibleMarkers(markers)) } - if (interactionModes.includes('selectFeature') && layers.length > 0) { + if (interactionModes?.includes('selectFeature') && layers.length > 0) { items.push(...collectVisibleFeatures(mapProvider, layers)) } eventBus.emit(EVENTS.MAP_SET_FEATURES, { items, multiselectable: multiSelect }) } + handleMoveEnd() eventBus.on(EVENTS.MAP_MOVE_END, handleMoveEnd) eventBus.on(EVENTS.MAP_DATA_CHANGE, handleMoveEnd) return () => { @@ -116,7 +117,7 @@ function useActiveItemHandler ({ markers, interactionModes, layers, mapProvider, dispatch({ type: 'SET_LISTBOX_ACTIVE', payload: null }) return } - if (interactionModes.includes('selectFeature') && layers.length > 0) { + if (interactionModes?.includes('selectFeature') && layers.length > 0) { const layerIds = layers.map(layer => layer.layerId) const layerConfigMap = buildLayerConfigMap(layers) const features = mapProvider.getVisibleFeatures(layerIds) diff --git a/plugins/interact/src/hooks/useMapItemList.test.js b/plugins/interact/src/hooks/useMapItemList.test.js index 01a3d27d..f3d4996d 100644 --- a/plugins/interact/src/hooks/useMapItemList.test.js +++ b/plugins/interact/src/hooks/useMapItemList.test.js @@ -84,6 +84,30 @@ describe('useMapItemList — lifecycle', () => { }) }) +// ─── useMapItemList — initial population ───────────────────────────────── + +describe('useMapItemList — initial population', () => { + afterEach(() => { document.body.innerHTML = '' }) + + it('emits visible markers immediately on mount without waiting for moveend', () => { + const { el, container } = makeMarkerEl({ inViewport: true }) + const markers = makeMarkers([{ id: 'm1', label: MARKER_LABEL, symbol: 'pin', isVisible: true }]) + markers.markerRefs.set('m1', el) + + const { eb } = setup({ interactionModes: ['selectMarker'], markers }) + + expect(eb.emit).toHaveBeenCalledWith(SET_FEATURES, { + items: [{ id: 'm1', label: MARKER_LABEL }], multiselectable: false + }) + container.remove() + }) + + it('emits empty items immediately when no markers are in viewport', () => { + const { eb } = setup({ interactionModes: ['selectMarker'] }) + expect(eb.emit).toHaveBeenCalledWith(SET_FEATURES, { items: [], multiselectable: false }) + }) +}) + // ─── useMapItemList — datachange trigger ───────────────────────────────── describe('useMapItemList — datachange trigger', () => { diff --git a/plugins/interact/src/index.js b/plugins/interact/src/index.js index 79510197..941ba8cf 100755 --- a/plugins/interact/src/index.js +++ b/plugins/interact/src/index.js @@ -1,6 +1,4 @@ // /plugins/interact/index.js -import './interact.scss' - export default function createPlugin (options = {}) { return { ...options, diff --git a/plugins/interact/src/index.test.js b/plugins/interact/src/index.test.js index b8e2bd27..f63aab6f 100644 --- a/plugins/interact/src/index.test.js +++ b/plugins/interact/src/index.test.js @@ -1,7 +1,5 @@ import createPlugin from './index.js' -jest.mock('./interact.scss', () => ({})) - describe('createPlugin', () => { it('returns plugin with fixed id and load function', () => { const plugin = createPlugin({ foo: 'bar' }) diff --git a/plugins/interact/src/interact.scss b/plugins/interact/src/interact.scss deleted file mode 100755 index 9b532496..00000000 --- a/plugins/interact/src/interact.scss +++ /dev/null @@ -1,24 +0,0 @@ -.im-c-mark-location { - display: flex; - justify-content: center; - gap: var(--divider-gap); -} - -.im-o-app--mobile .im-c-mark-location { - .im-c-button-wrapper, .im-c-map-button { - flex: 1 1 auto; - min-width: fit-content; - } -} - -.im-o-app--mouse.im-o-app--tablet, -.im-o-app--mouse.im-o-app--desktop { - .im-c-mark-location .im-c-map-button { - min-width: 150px; - } -} - -.im-o-app--mobile .im-c-mark-location .im-c-map-button { - flex: 1 1 auto; - min-width: fit-content; -} \ No newline at end of file diff --git a/plugins/interact/src/manifest.js b/plugins/interact/src/manifest.js index c9601756..37d7c0d5 100755 --- a/plugins/interact/src/manifest.js +++ b/plugins/interact/src/manifest.js @@ -10,6 +10,8 @@ import { unselectFeature } from './api/unselectFeature.js' import { selectMarker } from './api/selectMarker.js' import { unselectMarker } from './api/unselectMarker.js' +const SELECT_FEATURES_GROUP = 'Select features' + export const manifest = { InitComponent: InteractInit, @@ -72,12 +74,41 @@ export const manifest = { } }], - keyboardShortcuts: [{ - id: 'selectOrMark', - group: 'Select', - title: 'Select feature', - command: 'Enter' - }], + keyboardShortcuts: [ + { + id: 'clickAtTarget', + title: 'Click at target', + command: 'Enter' + }, + { + id: 'selectFeatures', + group: SELECT_FEATURES_GROUP, + context: 'listbox', + title: 'Select features', + command: 'Tab' + }, + { + id: 'navigateFeatures', + group: SELECT_FEATURES_GROUP, + context: 'listbox', + title: 'Navigate features', + command: '↑ or ↓' + }, + { + id: 'selectFeature', + group: SELECT_FEATURES_GROUP, + context: 'listbox', + title: 'Select a feature', + command: 'Enter or Space' + }, + { + id: 'dismissHintOrReturn', + group: SELECT_FEATURES_GROUP, + context: 'listbox', + title: 'Dismiss hint / return to map', + command: 'Escape' + } + ], icons: [{ id: 'select', diff --git a/src/App/components/Hints/Hints.jsx b/src/App/components/Hints/Hints.jsx new file mode 100644 index 00000000..ffe414f3 --- /dev/null +++ b/src/App/components/Hints/Hints.jsx @@ -0,0 +1,52 @@ +import React, { useState, useEffect } from 'react' +import { createPortal } from 'react-dom' +import { useConfig } from '../../store/configContext.js' +import { useService } from '../../store/serviceContext.js' +import { useApp } from '../../store/appContext.js' + +/** + * Renders the active keyboard hint as a toast portaled into im-o-app__main. + * Positioned above the actions bar using --hint-bottom. All visual + * hints pass through here; screen reader announcements are handled internally + * by the hints service so callers only need hints.show(). + * + * The container element (id="${mapId}-hints") is always in the DOM after mount + * so aria-describedby references remain valid even when no hint is showing. + */ +// eslint-disable-next-line camelcase, react/jsx-pascal-case +// sonarjs/disable-next-line function-name +export const Hints = () => { + const { id, keyboardHintText } = useConfig() + const { hints } = useService() + const { layoutRefs } = useApp() + const [activeHint, setActiveHint] = useState(null) + const [mounted, setMounted] = useState(false) + + useEffect(() => { + setMounted(true) + return hints.subscribe(setActiveHint) + }, [hints]) + + if (!mounted || !layoutRefs.mainRef?.current) { + return null + } + + return createPortal( +
+
+ {activeHint && ( +
+ )} +
+
+
, + layoutRefs.mainRef.current + ) +} diff --git a/src/App/components/Hints/Hints.module.scss b/src/App/components/Hints/Hints.module.scss new file mode 100644 index 00000000..067a13df --- /dev/null +++ b/src/App/components/Hints/Hints.module.scss @@ -0,0 +1,37 @@ +// =================================================== +// Object: KeyboardHints (layout wrapper) +// =================================================== + +.im-o-hints { + position: absolute; + bottom: var(--hint-bottom, var(--primary-gap)); + left: 50%; + transform: translateX(-50%); + z-index: 1001; +} + +// =================================================== +// Component: KeyboardHints +// =================================================== + +.im-c-hints { + display: flex; + flex-direction: column; + align-items: center; + gap: var(--divider-gap); +} + +.im-c-hints__hint { + text-wrap: nowrap; + + color: var(--tooltip-foreground-color); + background-color: var(--tooltip-background-color); + border-radius: var(--tooltip-border-radius); + padding: var(--tooltip-padding); + font-size: var(--tooltip-font-size); + line-height: 1.2; +} + +.im-c-hints__hint kbd { + border: var(--kbd-tooltip-border); +} diff --git a/src/App/components/Hints/Hints.test.jsx b/src/App/components/Hints/Hints.test.jsx new file mode 100644 index 00000000..bb8f4d12 --- /dev/null +++ b/src/App/components/Hints/Hints.test.jsx @@ -0,0 +1,104 @@ +import React from 'react' +import { render, act } from '@testing-library/react' +import { Hints } from './Hints.jsx' +import { useConfig } from '../../store/configContext.js' +import { useService } from '../../store/serviceContext.js' +import { useApp } from '../../store/appContext.js' + +jest.mock('../../store/configContext.js', () => ({ useConfig: jest.fn() })) +jest.mock('../../store/serviceContext.js', () => ({ useService: jest.fn() })) +jest.mock('../../store/appContext.js', () => ({ useApp: jest.fn() })) + +const CONTAINER_ID = '#test-map-hints' +const HINT_CLASS = '.im-c-hints__hint' + +let capturedSubscriber +let mockHints + +const setup = ({ mainEl = document.createElement('div') } = {}) => { + capturedSubscriber = null + mockHints = { + subscribe: jest.fn((fn) => { + capturedSubscriber = fn + return () => {} + }) + } + useConfig.mockReturnValue({ id: 'test-map', keyboardHintText: 'Alt + K' }) + useService.mockReturnValue({ hints: mockHints }) + useApp.mockReturnValue({ layoutRefs: { mainRef: { current: mainEl } } }) + if (mainEl) document.body.appendChild(mainEl) + return mainEl +} + +afterEach(() => { + document.body.innerHTML = '' + jest.clearAllMocks() +}) + +// ─── rendering ─────────────────────────────────────────────────────────────── + +describe('Hints — rendering', () => { + it('renders the container into mainRef after mount', () => { + const mainEl = setup() + render() + expect(mainEl.querySelector(CONTAINER_ID)).toBeTruthy() + }) + + it('renders a persistent keyboard-desc span with the hint text', () => { + const mainEl = setup() + render() + const desc = mainEl.querySelector('#test-map-keyboard-desc') + expect(desc).toBeTruthy() + expect(desc.innerHTML).toBe('Alt + K') + }) + + it('renders no hint content when there is no active hint', () => { + const mainEl = setup() + render() + expect(mainEl.querySelector(HINT_CLASS)).toBeNull() + }) + + it('renders hint content when subscriber fires with a hint', () => { + const mainEl = setup() + render() + act(() => capturedSubscriber({ html: 'Enter to select' })) + const hint = mainEl.querySelector(HINT_CLASS) + expect(hint).toBeTruthy() + expect(hint.innerHTML).toBe('Enter to select') + }) + + it('removes hint content when subscriber fires with null', () => { + const mainEl = setup() + render() + act(() => capturedSubscriber({ html: 'hello' })) + act(() => capturedSubscriber(null)) + expect(mainEl.querySelector(HINT_CLASS)).toBeNull() + }) +}) + +// ─── lifecycle ─────────────────────────────────────────────────────────────── + +describe('Hints — lifecycle', () => { + it('subscribes to hints on mount', () => { + setup() + render() + expect(mockHints.subscribe).toHaveBeenCalled() + }) + + it('unsubscribes on unmount', () => { + const unsub = jest.fn() + setup() + mockHints = { subscribe: jest.fn(() => unsub) } + useService.mockReturnValue({ hints: mockHints }) + const { unmount } = render() + unmount() + expect(unsub).toHaveBeenCalled() + }) + + it('renders nothing when mainRef is null', () => { + setup({ mainEl: null }) + useApp.mockReturnValue({ layoutRefs: { mainRef: { current: null } } }) + const { container } = render() + expect(container.innerHTML).toBe('') + }) +}) diff --git a/src/App/components/KeyboardHelp/KeyboardHelp.jsx b/src/App/components/KeyboardHelp/KeyboardHelp.jsx index 7e159995..e35c927a 100755 --- a/src/App/components/KeyboardHelp/KeyboardHelp.jsx +++ b/src/App/components/KeyboardHelp/KeyboardHelp.jsx @@ -1,33 +1,71 @@ // src/components/KeyboardHelp.jsx import React from 'react' import { useConfig } from '../../store/configContext' +import { useApp } from '../../store/appContext.js' import { getKeyboardShortcuts } from '../../registry/keyboardShortcutRegistry.js' +import { Tabs } from '../Tabs/Tabs.jsx' + +const ShortcutList = ({ items }) => ( +
+ {items.map((item) => ( +
+
{item.title}
+
+
+ ))} +
+) + +const buildGroupMap = (shortcuts) => shortcuts.reduce((acc, shortcut) => { + const group = shortcut.group || 'Navigate' + if (!acc[group]) { + acc[group] = [] + } + acc[group].push(shortcut) + return acc +}, {}) + +const getDefaultTab = (groupEntries, context) => { + const exactMatch = groupEntries.find(([, items]) => + items.some(s => (s.context ?? 'viewport') === context) + ) + if (exactMatch) { + return exactMatch[0] + } + const globalMatch = groupEntries.find(([, items]) => + items.some(s => s.context === 'global') + ) + return globalMatch?.[0] ?? groupEntries[0][0] +} // eslint-disable-next-line camelcase, react/jsx-pascal-case // sonarjs/disable-next-line function-name -export const KeyboardHelp = () => { +export const KeyboardHelp = ({ context = 'viewport' }) => { // NOSONAR: project does not use PropTypes const appConfig = useConfig() - const groups = getKeyboardShortcuts(appConfig).reduce((acc, shortcut) => { - acc[shortcut.group] = acc[shortcut.group] || [] - acc[shortcut.group].push(shortcut) - return acc - }, {}) + const { listboxIsActive } = useApp() + const allShortcuts = getKeyboardShortcuts(appConfig) + const shortcuts = listboxIsActive + ? allShortcuts + : allShortcuts.filter(s => !s.group && s.context !== 'listbox') + const groupMap = buildGroupMap(shortcuts) + const groupEntries = Object.entries(groupMap) + + if (groupEntries.length <= 1) { + return ( +
+ +
+ ) + } + + const tabs = groupEntries.map(([name, items]) => ({ + name, + content: + })) return (
- {Object.entries(groups).map(([groupName, items]) => ( -
- {/*

{groupName}

*/} -
- {items.map((item) => ( -
-
{item.title}
-
-
- ))} -
-
- ))} +
) } diff --git a/src/App/components/KeyboardHelp/KeyboardHelp.module.scss b/src/App/components/KeyboardHelp/KeyboardHelp.module.scss index d775d05a..0dd636e5 100755 --- a/src/App/components/KeyboardHelp/KeyboardHelp.module.scss +++ b/src/App/components/KeyboardHelp/KeyboardHelp.module.scss @@ -2,6 +2,8 @@ // Component: KeyboardHelp // =================================================== +@use '../../../scss/tools/index' as tools; + // 1. Base styles // 2. Elements @@ -15,6 +17,10 @@ padding: 10px 0; width: 100%; border-top: 1px solid var(--content-border-color); + + &:last-child { + padding-bottom: 0; + } } .im-c-keyboard-help__group:last-child .im-c-keyboard-help__item:last-child { @@ -40,3 +46,7 @@ display: block; margin-top: 5px; } + +.im-c-tabs__panel .im-c-keyboard-help__item:first-child { + border-top: 0; +} \ No newline at end of file diff --git a/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx b/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx index 53f808fe..cb1b73cc 100755 --- a/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx +++ b/src/App/components/KeyboardHelp/KeyboardHelp.test.jsx @@ -1,9 +1,10 @@ // src/components/KeyboardHelp.test.jsx import React from 'react' -import { render, screen, within } from '@testing-library/react' +import { render, screen, fireEvent } from '@testing-library/react' import { KeyboardHelp } from './KeyboardHelp' import { getKeyboardShortcuts } from '../../registry/keyboardShortcutRegistry.js' import { useConfig } from '../../store/configContext' +import { useApp } from '../../store/appContext.js' jest.mock('../../registry/keyboardShortcutRegistry.js', () => ({ getKeyboardShortcuts: jest.fn() @@ -13,51 +14,159 @@ jest.mock('../../store/configContext', () => ({ useConfig: jest.fn() })) -describe('KeyboardHelp', () => { +jest.mock('../../store/appContext.js', () => ({ + useApp: jest.fn() +})) + +jest.mock('../../hooks/useResizeObserver.js', () => ({ useResizeObserver: jest.fn() })) + +const SELECT_FEATURES_GROUP = 'Select features' +const DEFAULT_GROUP = 'Navigate' + +// Ungrouped — mirrors real core shortcuts (no group, plain context) +const VIEWPORT_SHORTCUTS = [ + { id: '1', context: 'viewport', title: 'Move', command: '↑' }, + { id: '2', context: 'viewport', title: 'Zoom', command: '+' } +] + +const LISTBOX_SHORTCUTS = [ + { id: '3', group: SELECT_FEATURES_GROUP, context: 'listbox', title: 'Navigate', command: '↓' }, + { id: '4', group: SELECT_FEATURES_GROUP, context: 'listbox', title: 'Select', command: 'Enter' } +] + +const GLOBAL_SHORTCUTS = [ + { id: '5', context: 'global', title: 'Help', command: 'Alt' } +] + +beforeEach(() => { + useConfig.mockReturnValue({}) + useApp.mockReturnValue({ listboxIsActive: false }) +}) + +afterEach(() => { + jest.clearAllMocks() +}) + +// ─── flat list (single group) ───────────────────────────────────────────────── + +describe('KeyboardHelp — flat list', () => { + it('renders a flat list with no tabs when all shortcuts share one group', () => { + getKeyboardShortcuts.mockReturnValue(VIEWPORT_SHORTCUTS) + render() + expect(document.querySelector('.im-c-keyboard-help')).toBeInTheDocument() + expect(screen.queryByRole('tablist')).not.toBeInTheDocument() + expect(screen.getByText('Move')).toBeInTheDocument() + expect(screen.getByText('Zoom')).toBeInTheDocument() + }) + + it('renders a flat list with no tabs when there are no shortcuts', () => { + getKeyboardShortcuts.mockReturnValue([]) + render() + expect(document.querySelector('.im-c-keyboard-help')).toBeInTheDocument() + expect(screen.queryByRole('tablist')).not.toBeInTheDocument() + }) + + it('renders command HTML', () => { + getKeyboardShortcuts.mockReturnValue(VIEWPORT_SHORTCUTS) + render() + expect(screen.getByText('↑')).toBeInTheDocument() + }) +}) + +// ─── tab UI (multiple groups) ───────────────────────────────────────────────── + +describe('KeyboardHelp — tabs', () => { + const allShortcuts = [...GLOBAL_SHORTCUTS, ...VIEWPORT_SHORTCUTS, ...LISTBOX_SHORTCUTS] + beforeEach(() => { - useConfig.mockReturnValue({}) + useApp.mockReturnValue({ listboxIsActive: true }) + }) + + it('renders a tab for each group when listboxIsActive is true', () => { + getKeyboardShortcuts.mockReturnValue(allShortcuts) + render() + expect(screen.getByRole('tablist')).toBeInTheDocument() + expect(screen.getByRole('tab', { name: DEFAULT_GROUP })).toBeInTheDocument() + expect(screen.getByRole('tab', { name: SELECT_FEATURES_GROUP })).toBeInTheDocument() + }) + + it('defaults to first viewport-context tab when context is viewport', () => { + getKeyboardShortcuts.mockReturnValue(allShortcuts) + render() + expect(screen.getByRole('tab', { name: DEFAULT_GROUP })).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText('Move')).toBeInTheDocument() }) - afterEach(() => { - jest.clearAllMocks() + it('defaults to first listbox-context tab when context is listbox', () => { + getKeyboardShortcuts.mockReturnValue(allShortcuts) + render() + expect(screen.getByRole('tab', { name: SELECT_FEATURES_GROUP })).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText('Select')).toBeInTheDocument() }) - it('renders grouped keyboard shortcuts correctly', () => { + it('falls back to first global-context tab when no exact match exists', () => { + getKeyboardShortcuts.mockReturnValue([...GLOBAL_SHORTCUTS, ...LISTBOX_SHORTCUTS]) + render() + expect(screen.getByRole('tab', { name: DEFAULT_GROUP })).toHaveAttribute('aria-selected', 'true') + }) + + it('falls back to first tab when no exact or global-context match exists', () => { getKeyboardShortcuts.mockReturnValue([ - { id: '1', group: 'Navigation', title: 'Go Home', command: 'H' }, - { id: '2', group: 'Navigation', title: 'Search', command: '/' }, - { id: '3', group: 'Editing', title: 'Copy', command: 'Ctrl+C' } + { id: 'a', group: 'Group A', context: 'listbox', title: 'A', command: 'A' }, + { id: 'b', group: 'Group B', context: 'listbox', title: 'B', command: 'B' } ]) + render() + expect(screen.getByRole('tab', { name: 'Group A' })).toHaveAttribute('aria-selected', 'true') + }) - render() + it('shows only the active tab panel content', () => { + getKeyboardShortcuts.mockReturnValue(allShortcuts) + render() + expect(screen.getByText('Move')).toBeInTheDocument() + expect(screen.queryByText('Select')).not.toBeInTheDocument() + }) - // outer container - const container = document.querySelector('.im-c-keyboard-help') - expect(container).toBeInTheDocument() + it('switches tab and content on click', () => { + getKeyboardShortcuts.mockReturnValue(allShortcuts) + render() + fireEvent.click(screen.getByRole('tab', { name: SELECT_FEATURES_GROUP })) + expect(screen.getByRole('tab', { name: SELECT_FEATURES_GROUP })).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText('Select')).toBeInTheDocument() + expect(screen.queryByText('Move')).not.toBeInTheDocument() + }) - // Navigation group contains its shortcuts - const navGroup = screen.getByText('Go Home').closest('.im-c-keyboard-help__group') - expect(navGroup).toBeInTheDocument() - expect(within(navGroup).getByText('Search')).toBeInTheDocument() - expect(within(navGroup).getByText('Go Home')).toBeInTheDocument() + it('ungrouped shortcuts default to Navigate group', () => { + getKeyboardShortcuts.mockReturnValue([ + { id: 'x', title: 'No group', command: 'X' }, + ...LISTBOX_SHORTCUTS + ]) + render() + fireEvent.click(screen.getByRole('tab', { name: DEFAULT_GROUP })) + expect(screen.getByText('No group')).toBeInTheDocument() + }) +}) - // Editing group contains its shortcut - const editGroup = screen.getByText('Copy').closest('.im-c-keyboard-help__group') - expect(editGroup).toBeInTheDocument() +// ─── listboxIsActive filtering ──────────────────────────────────────────────── - // command HTML is injected - expect(screen.getByText('H')).toBeInTheDocument() - expect(screen.getByText('/')).toBeInTheDocument() - expect(screen.getByText('Ctrl+C')).toBeInTheDocument() - }) +describe('KeyboardHelp — listboxIsActive filtering', () => { + const allShortcuts = [...GLOBAL_SHORTCUTS, ...VIEWPORT_SHORTCUTS, ...LISTBOX_SHORTCUTS] - it('renders nothing if there are no shortcuts', () => { - getKeyboardShortcuts.mockReturnValue([]) + it('hides the Select features tab when listboxIsActive is false', () => { + getKeyboardShortcuts.mockReturnValue(allShortcuts) + render() + expect(screen.queryByRole('tab', { name: SELECT_FEATURES_GROUP })).not.toBeInTheDocument() + }) + it('still shows ungrouped shortcuts when listboxIsActive is false', () => { + getKeyboardShortcuts.mockReturnValue(allShortcuts) render() + expect(screen.getByText('Move')).toBeInTheDocument() + }) - const container = document.querySelector('.im-c-keyboard-help') - expect(container).toBeInTheDocument() - expect(container.querySelectorAll('.im-c-keyboard-help__group')).toHaveLength(0) + it('renders as a flat list when only ungrouped shortcuts remain after filtering', () => { + getKeyboardShortcuts.mockReturnValue([...VIEWPORT_SHORTCUTS, ...LISTBOX_SHORTCUTS]) + render() + expect(screen.queryByRole('tablist')).not.toBeInTheDocument() + expect(screen.getByText('Move')).toBeInTheDocument() }) }) diff --git a/src/App/components/Tabs/Tabs.jsx b/src/App/components/Tabs/Tabs.jsx new file mode 100644 index 00000000..61ca293e --- /dev/null +++ b/src/App/components/Tabs/Tabs.jsx @@ -0,0 +1,68 @@ +import React, { useState, useCallback } from 'react' + +const toTabId = (name) => `im-c-tabs-tab-${name.toLowerCase().replaceAll(/\s+/g, '-')}` +const toPanelId = (name) => `im-c-tabs-panel-${name.toLowerCase().replaceAll(/\s+/g, '-')}` + +// eslint-disable-next-line camelcase, react/jsx-pascal-case +// sonarjs/disable-next-line function-name +export const Tabs = ({ tabs, defaultTab }) => { // NOSONAR: project does not use PropTypes + const names = tabs.map(t => t.name) + const [activeTab, setActiveTab] = useState(defaultTab ?? names[0]) + + const activateTab = useCallback((name) => { + setActiveTab(name) + document.getElementById(toTabId(name))?.focus() + }, []) + + const handleKeyDown = useCallback((e) => { + const idx = names.indexOf(activeTab) + if (e.key === 'ArrowRight') { + e.preventDefault() + activateTab(names[(idx + 1) % names.length]) + } else if (e.key === 'ArrowLeft') { + e.preventDefault() + activateTab(names[(idx - 1 + names.length) % names.length]) + } else if (e.key === 'Home') { + e.preventDefault() + activateTab(names[0]) + } else if (e.key === 'End') { + e.preventDefault() + activateTab(names.at(-1)) + } else { + // no action + } + }, [names, activeTab, activateTab]) + + const activeContent = tabs.find(t => t.name === activeTab)?.content + + return ( +
+
+ {tabs.map(({ name }) => ( + + ))} +
+
+ {activeContent} +
+
+ ) +} diff --git a/src/App/components/Tabs/Tabs.module.scss b/src/App/components/Tabs/Tabs.module.scss new file mode 100644 index 00000000..9e61a92b --- /dev/null +++ b/src/App/components/Tabs/Tabs.module.scss @@ -0,0 +1,57 @@ +@use '../../../scss/tools/index' as tools; + +// =================================================== +// Component: Tabs +// =================================================== + +.im-c-tabs__list { + position: relative; + display: flex; + flex-wrap: wrap; + padding: 0; + margin: 0; + border-bottom: 1px solid var(--app-border-color); + z-index: 1; +} + +.im-c-tabs__tab { + position: relative; + appearance: none; + background: var(--button-hover-color); + border: 1px solid transparent; + border-bottom: none; + padding: 7.5px 15px; + margin: 5px 5px 5px 0; + cursor: pointer; + color: var(--foreground-color); + font-size: inherit; + font-weight: normal; + outline: 3px solid transparent; + + &[aria-selected='true'] { + background: var(--background-color); + border-color: var(--app-border-color); + border-bottom-color: var(--background-color); + margin-top: 0; + margin-bottom: -1px; + } + + &:focus .im-c-tabs__label { + @include tools.link-focus; + } +} + +.im-c-tabs__tab:not([aria-selected='true']) { + &:hover { + background-color: var(--content-border-color); + } + + .im-c-tabs__label { + text-decoration: underline; + text-underline-offset: 3px; + } + + &:hover .im-c-tabs__label { + text-decoration-thickness: 3px; + } +} \ No newline at end of file diff --git a/src/App/components/Tabs/Tabs.test.jsx b/src/App/components/Tabs/Tabs.test.jsx new file mode 100644 index 00000000..d14787db --- /dev/null +++ b/src/App/components/Tabs/Tabs.test.jsx @@ -0,0 +1,121 @@ +import React from 'react' +import { render, screen, fireEvent } from '@testing-library/react' +import { Tabs } from './Tabs' + +const TAB_A = { name: 'Alpha', content:

Alpha content

} +const TAB_B = { name: 'Beta', content:

Beta content

} +const TAB_C = { name: 'Gamma', content:

Gamma content

} + +// ─── rendering ─────────────────────────────────────────────────────────────── + +describe('Tabs — rendering', () => { + it('renders a tab button for each entry', () => { + render() + expect(screen.getByRole('tab', { name: 'Alpha' })).toBeInTheDocument() + expect(screen.getByRole('tab', { name: 'Beta' })).toBeInTheDocument() + }) + + it('shows content for the active tab only', () => { + render() + expect(screen.getByText('Alpha content')).toBeInTheDocument() + expect(screen.queryByText('Beta content')).not.toBeInTheDocument() + }) + + it('uses defaultTab to set the initial active tab', () => { + render() + expect(screen.getByRole('tab', { name: 'Beta' })).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText('Beta content')).toBeInTheDocument() + expect(screen.queryByText('Alpha content')).not.toBeInTheDocument() + }) + + it('falls back to first tab when defaultTab is not provided', () => { + render() + expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('aria-selected', 'true') + }) +}) + +// ─── click behaviour ────────────────────────────────────────────────────────── + +describe('Tabs — click behaviour', () => { + it('switches to the clicked tab', () => { + render() + fireEvent.click(screen.getByRole('tab', { name: 'Beta' })) + expect(screen.getByRole('tab', { name: 'Beta' })).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText('Beta content')).toBeInTheDocument() + expect(screen.queryByText('Alpha content')).not.toBeInTheDocument() + }) +}) + +// ─── WCAG attributes ───────────────────────────────────────────────────────── + +describe('Tabs — WCAG attributes', () => { + it('active tab has tabIndex 0, inactive tabs have tabIndex -1', () => { + render() + expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('tabindex', '0') + expect(screen.getByRole('tab', { name: 'Beta' })).toHaveAttribute('tabindex', '-1') + }) + + it('each tab has aria-controls pointing to an element in the DOM', () => { + render() + const tab = screen.getByRole('tab', { name: 'Alpha' }) + expect(document.getElementById(tab.getAttribute('aria-controls'))).toBeInTheDocument() + }) + + it('panel has aria-labelledby pointing to the active tab', () => { + render() + const panel = screen.getByRole('tabpanel') + const tabId = panel.getAttribute('aria-labelledby') + expect(document.getElementById(tabId)).toHaveAttribute('aria-selected', 'true') + }) + + it('panel has tabIndex -1 per ARIA tabs pattern', () => { + render() + expect(screen.getByRole('tabpanel')).toHaveAttribute('tabindex', '-1') + }) +}) + +// ─── WCAG keyboard navigation ───────────────────────────────────────────────── + +describe('Tabs — WCAG keyboard navigation', () => { + it('ArrowRight moves to next tab', () => { + render() + fireEvent.keyDown(screen.getByRole('tab', { name: 'Alpha' }), { key: 'ArrowRight' }) + expect(screen.getByRole('tab', { name: 'Beta' })).toHaveAttribute('aria-selected', 'true') + }) + + it('ArrowLeft moves to previous tab', () => { + render() + fireEvent.keyDown(screen.getByRole('tab', { name: 'Beta' }), { key: 'ArrowLeft' }) + expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('aria-selected', 'true') + }) + + it('ArrowRight wraps from last tab to first', () => { + render() + fireEvent.keyDown(screen.getByRole('tab', { name: 'Gamma' }), { key: 'ArrowRight' }) + expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('aria-selected', 'true') + }) + + it('ArrowLeft wraps from first tab to last', () => { + render() + fireEvent.keyDown(screen.getByRole('tab', { name: 'Alpha' }), { key: 'ArrowLeft' }) + expect(screen.getByRole('tab', { name: 'Gamma' })).toHaveAttribute('aria-selected', 'true') + }) + + it('Home moves to first tab', () => { + render() + fireEvent.keyDown(screen.getByRole('tab', { name: 'Gamma' }), { key: 'Home' }) + expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('aria-selected', 'true') + }) + + it('End moves to last tab', () => { + render() + fireEvent.keyDown(screen.getByRole('tab', { name: 'Alpha' }), { key: 'End' }) + expect(screen.getByRole('tab', { name: 'Gamma' })).toHaveAttribute('aria-selected', 'true') + }) + + it('unhandled keys do not change the active tab', () => { + render() + fireEvent.keyDown(screen.getByRole('tab', { name: 'Alpha' }), { key: 'Enter' }) + expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('aria-selected', 'true') + }) +}) diff --git a/src/App/components/Viewport/Features.jsx b/src/App/components/Viewport/Features.jsx index 1766e1b2..d240dee9 100644 --- a/src/App/components/Viewport/Features.jsx +++ b/src/App/components/Viewport/Features.jsx @@ -12,6 +12,7 @@ export const Features = forwardRef(({ activeFeatureId, selectedIds = [], multise tabIndex={hasItems ? '0' : '-1'} aria-hidden={hasItems ? undefined : true} aria-label='Map features' + aria-describedby={`${id}-keyboard-desc`} aria-multiselectable={multiselectable || undefined} aria-activedescendant={activeFeatureId ? `${id}-feature-${activeFeatureId}` : undefined} className='im-c-features' diff --git a/src/App/components/Viewport/Features.test.jsx b/src/App/components/Viewport/Features.test.jsx index e752125b..b2fc568f 100644 --- a/src/App/components/Viewport/Features.test.jsx +++ b/src/App/components/Viewport/Features.test.jsx @@ -8,6 +8,7 @@ jest.mock('../../store/configContext.js', () => ({ useConfig: jest.fn() })) const APP_ID = 'test-app' const LISTBOX = '[role="listbox"]' // NOSONAR const OPTION = '[role="option"]' // NOSONAR +const ARIA_SELECTED = 'aria-selected' const ITEMS = [ { id: 'f1', label: 'Feature One' }, { id: 'f2', label: 'Feature Two' } @@ -46,22 +47,22 @@ describe('Features — rendering', () => { it('sets aria-selected on items present in selectedIds', () => { const { container } = render() const options = container.querySelectorAll(OPTION) - expect(options[0]).toHaveAttribute('aria-selected', 'true') - expect(options[1]).toHaveAttribute('aria-selected', 'false') + expect(options[0]).toHaveAttribute(ARIA_SELECTED, 'true') + expect(options[1]).toHaveAttribute(ARIA_SELECTED, 'false') }) it('sets aria-selected on multiple items when selectedIds has multiple entries', () => { const { container } = render() const options = container.querySelectorAll(OPTION) - expect(options[0]).toHaveAttribute('aria-selected', 'true') - expect(options[1]).toHaveAttribute('aria-selected', 'true') + expect(options[0]).toHaveAttribute(ARIA_SELECTED, 'true') + expect(options[1]).toHaveAttribute(ARIA_SELECTED, 'true') }) it('does not set aria-selected from activeFeatureId alone', () => { const { container } = render() const options = container.querySelectorAll(OPTION) - expect(options[0]).toHaveAttribute('aria-selected', 'false') - expect(options[1]).toHaveAttribute('aria-selected', 'false') + expect(options[0]).toHaveAttribute(ARIA_SELECTED, 'false') + expect(options[1]).toHaveAttribute(ARIA_SELECTED, 'false') }) it('sets aria-activedescendant to the option element id when activeFeatureId is provided', () => { @@ -97,6 +98,11 @@ describe('Features — rendering', () => { expect(ul.getAttribute('tabIndex')).toBe('-1') expect(ul.getAttribute('aria-hidden')).toBe('true') }) + + it('sets aria-describedby to the shared hints container id', () => { + const { container } = render() + expect(container.querySelector(LISTBOX).getAttribute('aria-describedby')).toBe(`${APP_ID}-keyboard-desc`) // NOSONAR + }) }) // ─── Features — interactions ────────────────────────────────────────────────── diff --git a/src/App/components/Viewport/Viewport.jsx b/src/App/components/Viewport/Viewport.jsx index 17b72da5..97c88409 100755 --- a/src/App/components/Viewport/Viewport.jsx +++ b/src/App/components/Viewport/Viewport.jsx @@ -1,8 +1,7 @@ -import React, { useRef, useEffect, useState } from 'react' +import React, { useRef, useEffect } from 'react' import { useFeatureFocus } from '../../hooks/useFeatureFocus.js' import { useFeatureItems } from '../../hooks/useFeatureItems.js' import { EVENTS as events } from '../../../config/events.js' -import { createPortal } from 'react-dom' import { useConfig } from '../../store/configContext.js' import { useApp } from '../../store/appContext.js' import { useMap } from '../../store/mapContext.js' @@ -20,36 +19,40 @@ import { Markers } from '../Markers/Markers' // sonarjs/disable-next-line function-name export const Viewport = () => { const { id, mapProvider, mapLabel, keyboardHintText } = useConfig() - const { interfaceType, mode, previousMode, layoutRefs, safeZoneInset } = useApp() - const { mainRef } = layoutRefs + const { mode, previousMode, layoutRefs, safeZoneInset, dispatch } = useApp() const { mapSize } = useMap() - const { eventBus } = useService() + const { eventBus, hints } = useService() const mapContainerRef = useRef(null) - const keyboardHintRef = useRef(null) const featuresRef = useRef(null) - // Local state for keyboard hint visibility - const [keyboardHintVisible, setKeyboardHintVisible] = useState(false) - const { items: featureItems, multiselectable } = useFeatureItems(eventBus) - const { activeFeatureId, selectedIds, onFocus: onFeaturesFocus, onBlur: onFeaturesBlur } = useFeatureFocus({ viewportRef: layoutRefs.viewportRef, featuresRef, items: featureItems, eventBus }) + const { activeFeatureId, selectedIds, onFocus: handleFeaturesFocus, onBlur: handleFeaturesBlur } = useFeatureFocus({ viewportRef: layoutRefs.viewportRef, featuresRef, items: featureItems, eventBus, hints }) + + useEffect(() => { + const handler = () => dispatch({ type: 'SET_LISTBOX_ACTIVE' }) + eventBus.on('interact:listboxcapable', handler) + return () => eventBus.off('interact:listboxcapable', handler) + }, [eventBus]) + + const onFeaturesFocus = () => { handleFeaturesFocus(); hints.show(keyboardHintText, { duration: 0 }) } + const onFeaturesBlur = () => { handleFeaturesBlur(); hints.dismiss() } - // Attach map keyboard controls useKeyboardShortcuts(layoutRefs.viewportRef) - // Attach map events useMapEvents({ [events.MAP_CLICK]: () => mapProvider?.clearHighlightedLabel?.() }) - // Manage keyboard hint visibility using local state - const { showHint, handleFocus, handleBlur } = useKeyboardHint({ - interfaceType, + const { handleFocus, handleBlur } = useKeyboardHint({ containerRef: layoutRefs.viewportRef, - keyboardHintRef, - keyboardHintVisible, - onViewportFocusChange: setKeyboardHintVisible // update local state only + onViewportFocusChange: (visible) => { + if (visible) { + hints.show(keyboardHintText, { duration: 0 }) + } else { + hints.dismiss() + } + } }) // Set focus on viewport on mode change @@ -71,18 +74,9 @@ export const Viewport = () => { onFocus={handleFocus} onBlur={handleBlur} ref={layoutRefs.viewportRef} - aria-describedby={`${id}-keyboard-hint`} + aria-describedby={`${id}-keyboard-desc`} aria-controls={`${id}-features`} > - {mainRef?.current && createPortal( -
, - mainRef.current - )} ) diff --git a/src/App/layout/Layout.test.jsx b/src/App/layout/Layout.test.jsx index 782f1ebe..012a6327 100755 --- a/src/App/layout/Layout.test.jsx +++ b/src/App/layout/Layout.test.jsx @@ -21,6 +21,9 @@ jest.mock('../components/Attributions/Attributions', () => ({ jest.mock('../renderer/SlotRenderer', () => ({ SlotRenderer: jest.fn(({ slot }) =>
) })) +jest.mock('../components/Hints/Hints', () => ({ + Hints: jest.fn(() => null) +})) // Mock hooks jest.mock('../store/configContext', () => ({ useConfig: jest.fn() })) diff --git a/src/App/registry/keyboardShortcutRegistry.js b/src/App/registry/keyboardShortcutRegistry.js index b00c5ec3..14207882 100755 --- a/src/App/registry/keyboardShortcutRegistry.js +++ b/src/App/registry/keyboardShortcutRegistry.js @@ -10,8 +10,9 @@ const pluginShortcutIds = new Set() let providerSupportedIds = new Set() export const registerKeyboardShortcut = ({ shortcut }) => { - // Only add if we haven't seen this ID before - if (!pluginShortcutIds.has(shortcut.id)) { + if (pluginShortcutIds.has(shortcut.id)) { + pluginShortcutHelp[pluginShortcutHelp.findIndex(s => s.id === shortcut.id)] = shortcut + } else { pluginShortcutIds.add(shortcut.id) pluginShortcutHelp.push(shortcut) } diff --git a/src/App/registry/keyboardShortcutRegistry.test.js b/src/App/registry/keyboardShortcutRegistry.test.js index 0aea5531..79079466 100755 --- a/src/App/registry/keyboardShortcutRegistry.test.js +++ b/src/App/registry/keyboardShortcutRegistry.test.js @@ -29,17 +29,15 @@ describe('keyboardShortcutRegistry', () => { expect(shortcuts).toContain(shortcut) }) - test('registerKeyboardShortcut should ignore duplicate plugin shortcuts', () => { + test('registerKeyboardShortcut updates an existing shortcut when re-registered with the same id', () => { const shortcut = { id: 'duplicate', description: 'First' } - const duplicateShortcut = { id: 'duplicate', description: 'Second' } - // Register the first one + const updatedShortcut = { id: 'duplicate', description: 'Second' } registerKeyboardShortcut({ shortcut }) - // Try to register a second shortcut with the same ID - registerKeyboardShortcut({ shortcut: duplicateShortcut }) + registerKeyboardShortcut({ shortcut: updatedShortcut }) const shortcuts = getKeyboardShortcuts() - // Only the first shortcut should exist - expect(shortcuts).toContain(shortcut) - expect(shortcuts).not.toContain(duplicateShortcut) + expect(shortcuts).toContain(updatedShortcut) + expect(shortcuts).not.toContain(shortcut) + expect(shortcuts.filter(s => s.id === 'duplicate')).toHaveLength(1) }) test('setProviderSupportedShortcuts should filter core shortcuts', () => { diff --git a/src/App/store/ServiceProvider.jsx b/src/App/store/ServiceProvider.jsx index 35c2ddc7..948064b3 100755 --- a/src/App/store/ServiceProvider.jsx +++ b/src/App/store/ServiceProvider.jsx @@ -1,6 +1,7 @@ // src/App/store/ServiceProvider.jsx import React, { createContext, useMemo, useRef } from 'react' import { createAnnouncer } from '../../services/announcer.js' +import { createHints } from '../../services/hints.js' import { reverseGeocode } from '../../services/reverseGeocode.js' import { useConfig } from '../store/configContext.js' import { closeApp } from '../../services/closeApp.js' @@ -13,18 +14,20 @@ export const ServiceProvider = ({ eventBus, children }) => { const { id, handleExitClick, symbolDefaults: constructorSymbolDefaults } = useConfig() const mapStatusRef = useRef(null) const announce = useMemo(() => createAnnouncer(mapStatusRef), []) + const hints = useMemo(() => createHints(announce), [announce]) symbolRegistry.setDefaults(constructorSymbolDefaults || {}) const services = useMemo(() => ({ announce, + hints, reverseGeocode: (zoom, center) => reverseGeocode(zoom, center), eventBus, mapStatusRef, closeApp: () => closeApp(id, handleExitClick, eventBus), symbolRegistry, patternRegistry - }), [announce]) + }), [announce, hints]) return ( diff --git a/src/App/store/ServiceProvider.test.jsx b/src/App/store/ServiceProvider.test.jsx index d1ce769a..2f8b370f 100755 --- a/src/App/store/ServiceProvider.test.jsx +++ b/src/App/store/ServiceProvider.test.jsx @@ -11,6 +11,11 @@ jest.mock('../../services/announcer.js', () => ({ createAnnouncer: jest.fn(() => jest.fn()) })) +const mockShow = jest.fn() +jest.mock('../../services/hints.js', () => ({ + createHints: jest.fn(() => ({ show: mockShow, dismiss: jest.fn(), subscribe: jest.fn() })) +})) + jest.mock('../../services/reverseGeocode.js', () => ({ reverseGeocode: jest.fn(() => 'mockedReverseGeocode') })) @@ -57,6 +62,12 @@ describe('ServiceProvider', () => { expect(result.current).toBeTruthy() }) + test('hints.show() delegates to createHints show', () => { + const { result } = renderHook(() => React.useContext(ServiceContext), { wrapper }) + result.current.hints.show('test', { duration: 2000 }) + expect(mockShow).toHaveBeenCalledWith('test', { duration: 2000 }) + }) + test('closeApp calls closeApp service with id and handleExitClick', () => { const mockHandleExitClick = jest.fn() const { useConfig } = require('../store/configContext.js') diff --git a/src/App/store/appActionsMap.js b/src/App/store/appActionsMap.js index 64c506ca..9256fefa 100755 --- a/src/App/store/appActionsMap.js +++ b/src/App/store/appActionsMap.js @@ -158,6 +158,9 @@ const setPluginsEvaluated = (state) => const clearPluginsEvaluated = (state) => state.arePluginsEvaluated ? { ...state, arePluginsEvaluated: false } : state +const setListboxActive = (state) => + state.listboxIsActive ? state : { ...state, listboxIsActive: true } + const setSafeZoneInset = (state, { safeZoneInset, syncMapPadding = true }) => { return shallowEqual(state.safeZoneInset, safeZoneInset) ? state @@ -373,6 +376,7 @@ export const actionsMap = { SET_MODE: setMode, PLUGINS_EVALUATED: setPluginsEvaluated, CLEAR_PLUGINS_EVALUATED: clearPluginsEvaluated, + SET_LISTBOX_ACTIVE: setListboxActive, SET_SAFE_ZONE_INSET: setSafeZoneInset, REVERT_MODE: revertMode, OPEN_PANEL: openPanel, diff --git a/src/App/store/appActionsMap.test.js b/src/App/store/appActionsMap.test.js index 7e38bd53..07384007 100755 --- a/src/App/store/appActionsMap.test.js +++ b/src/App/store/appActionsMap.test.js @@ -425,4 +425,17 @@ describe('actionsMap full coverage', () => { expect(res.openPanels).toHaveProperty('p1') }) }) + + describe('SET_LISTBOX_ACTIVE', () => { + test('sets listboxIsActive to true when currently false', () => { + const res = actionsMap.SET_LISTBOX_ACTIVE({ ...state, listboxIsActive: false }) + expect(res.listboxIsActive).toBe(true) + }) + + test('returns same state reference when listboxIsActive is already true', () => { + const activeState = { ...state, listboxIsActive: true } + const res = actionsMap.SET_LISTBOX_ACTIVE(activeState) + expect(res).toBe(activeState) + }) + }) }) diff --git a/src/App/store/appReducer.js b/src/App/store/appReducer.js index 39739274..c47c3527 100755 --- a/src/App/store/appReducer.js +++ b/src/App/store/appReducer.js @@ -47,6 +47,7 @@ export const initialState = (config) => { hasExclusiveControl: false, openPanels, previousOpenPanels: {}, + listboxIsActive: false, syncMapPadding: true, pluginRegistry, buttonRegistry, diff --git a/src/config/appConfig.js b/src/config/appConfig.js index fa36e52e..faf20490 100755 --- a/src/config/appConfig.js +++ b/src/config/appConfig.js @@ -80,7 +80,7 @@ export const defaultAppConfig = { ...keyboardBasePanelSlots, width: '500px' }, - render: () => + render: (props) => }], icons: [{ diff --git a/src/config/appConfig.test.js b/src/config/appConfig.test.js index 59503be7..2e1044dc 100755 --- a/src/config/appConfig.test.js +++ b/src/config/appConfig.test.js @@ -1,6 +1,10 @@ import { render } from '@testing-library/react' import { defaultAppConfig, defaultButtonConfig, scaleFactor } from './appConfig' +jest.mock('../App/store/appContext.js', () => ({ + useApp: () => ({ listboxIsActive: false }) +})) + describe('defaultAppConfig', () => { const appState = { layoutRefs: { appContainerRef: { current: document.createElement('div') } }, diff --git a/src/scss/main.scss b/src/scss/main.scss index 4ce6b390..fe7abf8e 100755 --- a/src/scss/main.scss +++ b/src/scss/main.scss @@ -17,6 +17,7 @@ // Components: specific UI components. Using 'CSS in Component' pattern @use '../App/components/KeyboardHelp/KeyboardHelp.module'; +@use '../App/components/Tabs/Tabs.module'; @use '../App/components/MapButton/MapButton.module'; @use '../App/components/Panel/Panel.module'; @use '../App/components/Icon/Icon.module'; @@ -25,6 +26,7 @@ @use '../App/components/PopupMenu/PopupMenu.module'; @use '../App/components/Attributions/Attributions.module'; @use '../App/components/Logo/Logo.module'; +@use '../App/components/Hints/Hints.module'; @use '../App/components/CrossHair/CrossHair.module'; @use '../App/components/Markers/Markers.module'; @use '../App/components/Viewport/Features.module'; diff --git a/src/services/hints.js b/src/services/hints.js new file mode 100644 index 00000000..194dad7d --- /dev/null +++ b/src/services/hints.js @@ -0,0 +1,47 @@ +const stripHtml = (html) => html.replace(/<[^<>]*>/g, '') + +/** + * Creates the hints service. Manages a single active toast hint shown in the + * Hints container. Calling show() replaces any current hint and + * restarts the dismiss timer. Internally calls announce() so screen readers + * receive the message through the live region without callers needing to pair + * the two calls manually. + */ +export function createHints (announce) { + const subscribers = new Set() + let current = null + let timer = null + + const notify = () => subscribers.forEach(fn => fn(current)) + + const clearTimer = () => { + if (timer) { + clearTimeout(timer) + timer = null + } + } + + const dismiss = () => { + clearTimer() + current = null + notify() + } + + const show = (html, options = {}) => { + const { duration = 4000, announce: announceText } = options + clearTimer() + current = { html } + notify() + announce(announceText ?? stripHtml(html), 'plugin') + if (duration > 0) { + timer = setTimeout(dismiss, duration) + } + } + + const subscribe = (fn) => { + subscribers.add(fn) + return () => subscribers.delete(fn) + } + + return { show, dismiss, subscribe } +} diff --git a/src/services/hints.test.js b/src/services/hints.test.js new file mode 100644 index 00000000..cf530341 --- /dev/null +++ b/src/services/hints.test.js @@ -0,0 +1,110 @@ +import { createHints } from './hints.js' + +let announce +let hints + +beforeEach(() => { + jest.useFakeTimers() + announce = jest.fn() + hints = createHints(announce) +}) + +afterEach(() => { + jest.useRealTimers() +}) + +// ─── show() ────────────────────────────────────────────────────────────────── + +describe('show', () => { + it('notifies subscribers with the html payload', () => { + const sub = jest.fn() + hints.subscribe(sub) + hints.show('Enter to select') + expect(sub).toHaveBeenCalledWith({ html: 'Enter to select' }) + }) + + it('calls announce with stripped html', () => { + hints.show('Enter to select') + expect(announce).toHaveBeenCalledWith('Enter to select', 'plugin') + }) + + it('calls announce with custom text when announce option is provided', () => { + hints.show('Alt+K help', { announce: 'Press Alt+K for keyboard controls' }) + expect(announce).toHaveBeenCalledWith('Press Alt+K for keyboard controls', 'plugin') + }) + + it('replaces an existing hint and resets the timer', () => { + const sub = jest.fn() + hints.subscribe(sub) + hints.show('first', { duration: 2000 }) + hints.show('second', { duration: 2000 }) + jest.advanceTimersByTime(2000) + expect(sub).toHaveBeenLastCalledWith(null) + // Only one dismiss fired (the second hint's timer) + expect(sub).toHaveBeenCalledTimes(3) // show1, show2, dismiss + }) + + it('auto-dismisses after duration', () => { + const sub = jest.fn() + hints.subscribe(sub) + hints.show('hello', { duration: 3000 }) + jest.advanceTimersByTime(3000) + expect(sub).toHaveBeenLastCalledWith(null) + }) + + it('does not auto-dismiss when duration is 0', () => { + const sub = jest.fn() + hints.subscribe(sub) + hints.show('persistent', { duration: 0 }) + jest.advanceTimersByTime(60000) + expect(sub).not.toHaveBeenCalledWith(null) + }) +}) + +// ─── dismiss() ─────────────────────────────────────────────────────────────── + +describe('dismiss', () => { + it('clears the active hint and notifies', () => { + const sub = jest.fn() + hints.subscribe(sub) + hints.show('hello') + hints.dismiss() + expect(sub).toHaveBeenLastCalledWith(null) + }) + + it('cancels the auto-dismiss timer', () => { + const sub = jest.fn() + hints.subscribe(sub) + hints.show('hello', { duration: 3000 }) + hints.dismiss() + sub.mockClear() + jest.advanceTimersByTime(3000) + expect(sub).not.toHaveBeenCalled() + }) + + it('is safe to call when no hint is active', () => { + expect(() => hints.dismiss()).not.toThrow() + }) +}) + +// ─── subscribe / unsubscribe ────────────────────────────────────────────────── + +describe('subscribe', () => { + it('returns an unsubscribe function', () => { + const sub = jest.fn() + const unsub = hints.subscribe(sub) + unsub() + hints.show('hello') + expect(sub).not.toHaveBeenCalled() + }) + + it('notifies multiple subscribers', () => { + const sub1 = jest.fn() + const sub2 = jest.fn() + hints.subscribe(sub1) + hints.subscribe(sub2) + hints.show('hello') + expect(sub1).toHaveBeenCalled() + expect(sub2).toHaveBeenCalled() + }) +}) diff --git a/src/utils/constrainKeyboardFocus.js b/src/utils/constrainKeyboardFocus.js index 531cda39..91dcde6b 100755 --- a/src/utils/constrainKeyboardFocus.js +++ b/src/utils/constrainKeyboardFocus.js @@ -4,11 +4,11 @@ export function constrainKeyboardFocus (containerEl, e) { } const selectors = [ - 'a[href]:not([disabled])', - 'button:not([disabled])', - 'textarea:not([disabled])', - 'input:not([disabled])', - 'select:not([disabled])', + 'a[href]:not([disabled]):not([tabindex="-1"])', + 'button:not([disabled]):not([tabindex="-1"])', + 'textarea:not([disabled]):not([tabindex="-1"])', + 'input:not([disabled]):not([tabindex="-1"])', + 'select:not([disabled]):not([tabindex="-1"])', '*[tabindex="0"]:not([disabled])' ] let focusableEls = Array.from(containerEl.querySelectorAll(selectors.join(','))) diff --git a/src/utils/detectInterfaceType.js b/src/utils/detectInterfaceType.js index 66d60a58..900497c9 100755 --- a/src/utils/detectInterfaceType.js +++ b/src/utils/detectInterfaceType.js @@ -1,5 +1,6 @@ let lastInterfaceType = window.matchMedia('(pointer: coarse)').matches ? 'touch' : 'unknown' const interfaceTypeListeners = new Set() +const interfaceTypeImmediateListeners = new Set() // ----------------------------------------------------------------------------- // Internal (not exported) @@ -20,9 +21,8 @@ function normalizePointerType (pointerType) { function notifyListeners (newType) { if (lastInterfaceType !== newType) { lastInterfaceType = newType - interfaceTypeListeners.forEach((listener) => { - listener(newType) - }) + interfaceTypeImmediateListeners.forEach(listener => listener(newType)) + interfaceTypeListeners.forEach(listener => listener(newType)) } } @@ -44,7 +44,18 @@ function createInterfaceDetector () { const handlePointer = event => { const type = normalizePointerType(event.pointerType) - setTimeout(() => notifyListeners(type), REACT_CLICK_DELAY) + if (type === lastInterfaceType) { + return + } + // Update synchronously so getInterfaceType() returns the new value immediately — + // this prevents focusin handlers from seeing the stale 'keyboard' type during a + // pointer-triggered focus move. Listeners (React state) are still notified async + // to avoid layout thrashing during the click event. + lastInterfaceType = type + interfaceTypeImmediateListeners.forEach(listener => listener(type)) + setTimeout(() => { + interfaceTypeListeners.forEach(listener => listener(type)) + }, REACT_CLICK_DELAY) } const handleKeyDown = e => { @@ -81,8 +92,19 @@ function subscribeToInterfaceChanges (onInterfaceTypeChange) { } } +// Fires synchronously within the same event cycle — use for direct DOM updates +// that need to be in sync with the pointer event (no 150ms React delay). +function subscribeToInterfaceChangesImmediate (onInterfaceTypeChange) { + interfaceTypeImmediateListeners.add(onInterfaceTypeChange) + + return () => { + interfaceTypeImmediateListeners.delete(onInterfaceTypeChange) + } +} + export { createInterfaceDetector, getInterfaceType, - subscribeToInterfaceChanges + subscribeToInterfaceChanges, + subscribeToInterfaceChangesImmediate } diff --git a/src/utils/detectInterfaceType.test.js b/src/utils/detectInterfaceType.test.js index dd5c16d9..938e17ea 100644 --- a/src/utils/detectInterfaceType.test.js +++ b/src/utils/detectInterfaceType.test.js @@ -54,7 +54,7 @@ const triggerDomEvent = (event, payload) => { // --- TEST SUITE --- describe('Interface Detector Utility Module', () => { - let createInterfaceDetector, getInterfaceType, subscribeToInterfaceChanges + let createInterfaceDetector, getInterfaceType, subscribeToInterfaceChanges, subscribeToInterfaceChangesImmediate let cleanup beforeEach(async () => { @@ -72,6 +72,7 @@ describe('Interface Detector Utility Module', () => { createInterfaceDetector = importedModule.createInterfaceDetector getInterfaceType = importedModule.getInterfaceType subscribeToInterfaceChanges = importedModule.subscribeToInterfaceChanges + subscribeToInterfaceChangesImmediate = importedModule.subscribeToInterfaceChangesImmediate Object.keys(eventListeners).forEach(key => delete eventListeners[key]) Object.keys(mediaListeners).forEach(key => delete mediaListeners[key]) @@ -99,18 +100,21 @@ describe('Interface Detector Utility Module', () => { // --- Path 1a: pointerdown 'touch' --- triggerDomEvent('pointerdown', { pointerType: 'touch' }) + expect(getInterfaceType()).toBe('touch') // synchronous update + expect(handler).not.toHaveBeenCalled() // listener still pending jest.advanceTimersByTime(150) - expect(getInterfaceType()).toBe('touch') expect(handler).toHaveBeenCalledWith('touch') handler.mockClear() // --- Path 1b: pointerdown 'pen' --- triggerDomEvent('pointerdown', { pointerType: 'pen' }) jest.advanceTimersByTime(150) - expect(handler).not.toHaveBeenCalled() + expect(handler).not.toHaveBeenCalled() // 'pen' normalises to 'touch' — no change // --- Path 1c: pointerdown 'mouse' --- triggerDomEvent('pointerdown', { pointerType: 'mouse' }) + expect(getInterfaceType()).toBe('mouse') // synchronous update + expect(handler).not.toHaveBeenCalled() // listener still pending jest.advanceTimersByTime(150) expect(getInterfaceType()).toBe('mouse') expect(handler).toHaveBeenCalledWith('mouse') @@ -129,6 +133,10 @@ describe('Interface Detector Utility Module', () => { expect(handler).toHaveBeenCalledWith('keyboard') handler.mockClear() + // Tab again when already keyboard — notifyListeners no-op branch + triggerDomEvent('keydown', { key: 'Tab' }) + expect(handler).not.toHaveBeenCalled() + // keydown with non-Tab triggerDomEvent('keydown', { key: 'a' }) expect(handler).not.toHaveBeenCalled() @@ -152,6 +160,24 @@ describe('Interface Detector Utility Module', () => { expect(handler).not.toHaveBeenCalled() }) + it('subscribeToInterfaceChangesImmediate fires synchronously and unsubscribes', () => { + const handler = jest.fn() + const unsubscribe = subscribeToInterfaceChangesImmediate(handler) + cleanup = createInterfaceDetector() + + triggerDomEvent('pointerdown', { pointerType: 'touch' }) + expect(handler).toHaveBeenCalledWith('touch') // fires before timer advance + + // Tab keydown goes through notifyListeners — exercises immediate forEach callback + triggerDomEvent('keydown', { key: 'Tab' }) + expect(handler).toHaveBeenCalledWith('keyboard') + expect(handler).toHaveBeenCalledTimes(2) + + unsubscribe() + triggerDomEvent('pointerdown', { pointerType: 'mouse' }) + expect(handler).toHaveBeenCalledTimes(2) // no longer called after unsubscribe + }) + it('should return "touch" when matchMedia initially matches coarse pointer', async () => { jest.resetModules()