From 31396ee2252a05c66398156b45ab4c6adc249260 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Thu, 16 Jul 2026 17:37:49 +0100 Subject: [PATCH] Refactor search plugin to use manifest.buttons for its open trigger Replaces the standalone OpenButton control with a standard button+control pair (manifest.buttons entry paired with a controls entry), using the aria-controls support added in button-aria-controls. Also restores focus correctly when suggestions are escaped and disables text selection on suggestion items. --- demo/js/draw-ol.js | 6 +- demo/js/draw.js | 14 ++-- demo/js/esm.js | 14 ++-- demo/js/farming.js | 6 +- demo/js/gep.js | 12 +-- demo/js/index.js | 14 ++-- demo/js/ml-datasets.js | 14 ++-- demo/js/planning-ol.js | 16 ++-- demo/js/planning.js | 20 ++--- plugins/search/src/Search.jsx | 26 +++---- plugins/search/src/Search.test.jsx | 57 ++++++--------- .../src/components/OpenButton/OpenButton.jsx | 31 -------- .../OpenButton/OpenButton.module.scss | 3 - .../components/OpenButton/OpenButton.test.jsx | 51 ------------- .../SubmitButton/SubmitButton.module.scss | 6 ++ .../Suggestions/Suggestions.module.scss | 1 + plugins/search/src/events/formHandlers.js | 38 ++++++++-- .../search/src/events/formHandlers.test.js | 73 ++++++++++++++----- .../search/src/events/suggestionHandlers.js | 3 + .../src/events/suggestionHandlers.test.js | 8 +- plugins/search/src/manifest.js | 34 ++++++++- plugins/search/src/manifest.test.js | 43 +++++++++++ plugins/search/src/reducer.test.js | 13 ++++ plugins/search/src/search.scss | 19 ++++- 24 files changed, 298 insertions(+), 224 deletions(-) delete mode 100755 plugins/search/src/components/OpenButton/OpenButton.jsx delete mode 100644 plugins/search/src/components/OpenButton/OpenButton.module.scss delete mode 100644 plugins/search/src/components/OpenButton/OpenButton.test.jsx create mode 100644 plugins/search/src/manifest.test.js diff --git a/demo/js/draw-ol.js b/demo/js/draw-ol.js index 69c64fdd0..bba20febd 100644 --- a/demo/js/draw-ol.js +++ b/demo/js/draw-ol.js @@ -53,15 +53,15 @@ const interactiveMap = new InteractiveMap('map', { enableZoomControls: true, // readMapText: true, plugins: [ - mapStylesPlugin({ - mapStyles: [...vtsMapStyles27700, apgbAerialStyle] // ngdMapStyles27700 - }), searchPlugin({ transformRequest: transformGeocodeRequest, osNamesURL: process.env.OS_NAMES_URL, width: '300px', showMarker: false, }), + mapStylesPlugin({ + mapStyles: [...vtsMapStyles27700, apgbAerialStyle] // ngdMapStyles27700 + }), interactPlugin, drawPlugin ] diff --git a/demo/js/draw.js b/demo/js/draw.js index 98495edad..b4de26938 100755 --- a/demo/js/draw.js +++ b/demo/js/draw.js @@ -123,13 +123,6 @@ const interactiveMap = new InteractiveMap('map', { backgroundColor: '#f5f5f0' }, plugins: [ - datasetsPlugin, - mapStylesPlugin({ - mapStyles: vtsMapStyles3857 - }), - // scaleBarPlugin({ - // units: 'metric' - // }), searchPlugin({ transformRequest: transformGeocodeRequest, osNamesURL: process.env.OS_NAMES_URL, @@ -138,6 +131,13 @@ const interactiveMap = new InteractiveMap('map', { showMarker: false, // expanded: true }), + datasetsPlugin, + mapStylesPlugin({ + mapStyles: vtsMapStyles3857 + }), + // scaleBarPlugin({ + // units: 'metric' + // }), interactPlugin, drawPlugin ] diff --git a/demo/js/esm.js b/demo/js/esm.js index f2f053f2a..6f6be1d3a 100644 --- a/demo/js/esm.js +++ b/demo/js/esm.js @@ -333,13 +333,6 @@ const interactiveMap = new InteractiveMap('map', { backgroundColor: '#f5f5f0' }, plugins: [ - datasetsPlugin, - mapStylesPlugin({ - mapStyles: vtsMapStyles3857 - }), - scaleBarPlugin({ - units: 'metric' - }), searchPlugin({ transformRequest: transformGeocodeRequest, osNamesURL: process.env.OS_NAMES_URL, @@ -348,6 +341,13 @@ const interactiveMap = new InteractiveMap('map', { showMarker: true // expanded: true }), + datasetsPlugin, + mapStylesPlugin({ + mapStyles: vtsMapStyles3857 + }), + scaleBarPlugin({ + units: 'metric' + }), interactPlugin, framePlugin ] diff --git a/demo/js/farming.js b/demo/js/farming.js index c43a3c7fd..8d0280cca 100755 --- a/demo/js/farming.js +++ b/demo/js/farming.js @@ -105,9 +105,6 @@ var interactiveMap = new InteractiveMap('map', { backgroundColor: '#f5f5f0' }, plugins: [ - scaleBarPlugin({ - units: 'metric' - }), searchPlugin({ transformRequest: transformGeocodeRequest, osNamesURL: process.env.OS_NAMES_URL, @@ -116,6 +113,9 @@ var interactiveMap = new InteractiveMap('map', { showMarker: false, // expanded: true }), + scaleBarPlugin({ + units: 'metric' + }), // useLocationPlugin(), datasetsPlugin, mapStylesPlugin({ diff --git a/demo/js/gep.js b/demo/js/gep.js index 53c2461ba..a445bbe13 100644 --- a/demo/js/gep.js +++ b/demo/js/gep.js @@ -36,18 +36,18 @@ const interactiveMap = new InteractiveMap('map', { maxZoom: 20, containerHeight: '650px', plugins: [ - mapStylesPlugin({ - mapStyles: [...vtsMapStyles27700, apgbAerialStyle] - }), - scaleBarPlugin({ - units: 'metric' - }), searchPlugin({ transformRequest: transformGeocodeRequest, osNamesURL: process.env.OS_NAMES_URL, width: '300px', showMarker: true }), + mapStylesPlugin({ + mapStyles: [...vtsMapStyles27700, apgbAerialStyle] + }), + scaleBarPlugin({ + units: 'metric' + }), interactPlugin ] }) diff --git a/demo/js/index.js b/demo/js/index.js index 177ec9823..4cd19d0ce 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -305,13 +305,6 @@ const interactiveMap = new InteractiveMap('map', { // selectedColor: { outdoor: '#ffdd00', dark: '#ffaa00' } // }, plugins: [ - datasetsPlugin, - mapStylesPlugin({ - mapStyles: vtsMapStyles3857 - }), - scaleBarPlugin({ - units: 'metric' - }), searchPlugin({ transformRequest: transformGeocodeRequest, osNamesURL: process.env.OS_NAMES_URL, @@ -320,6 +313,13 @@ const interactiveMap = new InteractiveMap('map', { showMarker: true, showLabel: true }), + datasetsPlugin, + mapStylesPlugin({ + mapStyles: vtsMapStyles3857 + }), + scaleBarPlugin({ + units: 'metric' + }), // useLocationPlugin(), interactPlugin, framePlugin diff --git a/demo/js/ml-datasets.js b/demo/js/ml-datasets.js index f020cc19b..9a5281b17 100644 --- a/demo/js/ml-datasets.js +++ b/demo/js/ml-datasets.js @@ -333,13 +333,6 @@ const interactiveMap = new InteractiveMap('map', { backgroundColor: '#f5f5f0' }, plugins: [ - datasetsPlugin, - mapStylesPlugin({ - mapStyles: vtsMapStyles3857 - }), - scaleBarPlugin({ - units: 'metric' - }), searchPlugin({ transformRequest: transformGeocodeRequest, osNamesURL: process.env.OS_NAMES_URL, @@ -348,6 +341,13 @@ const interactiveMap = new InteractiveMap('map', { showMarker: true // expanded: true }), + datasetsPlugin, + mapStylesPlugin({ + mapStyles: vtsMapStyles3857 + }), + scaleBarPlugin({ + units: 'metric' + }), interactPlugin, framePlugin ] diff --git a/demo/js/planning-ol.js b/demo/js/planning-ol.js index 87c48dd70..51ad6deb7 100644 --- a/demo/js/planning-ol.js +++ b/demo/js/planning-ol.js @@ -46,14 +46,6 @@ const interactiveMap = new InteractiveMap('map', { enableFullscreen: false, hasExitButton: true, plugins: [ - mapStylesPlugin({ - mapStyles: vtsMapStyles27700, // ngdMapStyles27700, - manifest: { - buttons: [{ id: 'mapStyles', desktop: { slot: 'right-top', showLabel: false } }], - panels: [{ id: 'mapStyles', desktop: { slot: 'map-styles-button', width: '400px', modal: true } }] - } - }), - scaleBarPlugin({ units: 'metric' }), searchPlugin({ transformRequest: transformGeocodeRequest, placeholder: 'Search for a place in England', @@ -66,6 +58,14 @@ const interactiveMap = new InteractiveMap('map', { width: '300px', showMarker: true }), + mapStylesPlugin({ + mapStyles: vtsMapStyles27700, // ngdMapStyles27700, + manifest: { + buttons: [{ id: 'mapStyles', desktop: { slot: 'right-top', showLabel: false } }], + panels: [{ id: 'mapStyles', desktop: { slot: 'map-styles-button', width: '400px', modal: true } }] + } + }), + scaleBarPlugin({ units: 'metric' }), useLocationPlugin(), interactPlugin ] diff --git a/demo/js/planning.js b/demo/js/planning.js index e9722290d..1a94d64c9 100755 --- a/demo/js/planning.js +++ b/demo/js/planning.js @@ -94,16 +94,6 @@ const interactiveMap = new InteractiveMap('map', { enableFullscreen: false, hasExitButton: true, plugins: [ - mapStylesPlugin({ - mapStyles: vtsMapStyles27700, - manifest: { - buttons: [{ id: 'mapStyles', desktop: { slot: 'right-top', showLabel: false }}], - panels: [{ id: 'mapStyles', desktop: { slot: 'map-styles-button', width: '400px', modal: true }}] - } - }), - scaleBarPlugin({ - units: 'metric' - }), searchPlugin({ transformRequest: transformGeocodeRequest, placeholder: 'Search for a place in England', @@ -117,6 +107,16 @@ const interactiveMap = new InteractiveMap('map', { showMarker: true, // expanded: true }), + mapStylesPlugin({ + mapStyles: vtsMapStyles27700, + manifest: { + buttons: [{ id: 'mapStyles', desktop: { slot: 'right-top', showLabel: false }}], + panels: [{ id: 'mapStyles', desktop: { slot: 'map-styles-button', width: '400px', modal: true }}] + } + }), + scaleBarPlugin({ + units: 'metric' + }), useLocationPlugin(), interactPlugin, drawPlugin, diff --git a/plugins/search/src/Search.jsx b/plugins/search/src/Search.jsx index b8f8b48d5..9948d5455 100755 --- a/plugins/search/src/Search.jsx +++ b/plugins/search/src/Search.jsx @@ -1,6 +1,5 @@ // src/plugins/search/Search.jsx import { useRef, useEffect } from 'react' -import { OpenButton } from './components/OpenButton/OpenButton' import { Form } from './components/Form/Form' import { CloseButton } from './components/CloseButton/CloseButton' import { SubmitButton } from './components/SubmitButton/SubmitButton' @@ -12,11 +11,9 @@ export function Search ({ appConfig, iconRegistry, pluginState, pluginConfig, ap const { interfaceType } = appState const { expanded: defaultExpanded, customDatasets, osNamesURL, regions, maxSuggestions } = pluginConfig const { dispatch, isExpanded, areSuggestionsVisible, suggestions } = pluginState - const showLabel = appState?.controlConfig?.search?.[appState?.breakpoint].showLabel || false const closeIcon = iconRegistry.close const searchIcon = iconRegistry.search const searchContainerRef = useRef(null) - const buttonRef = useRef(null) const inputRef = useRef(null) const viewportRef = appState.layoutRefs.viewportRef @@ -79,18 +76,16 @@ export function Search ({ appConfig, iconRegistry, pluginState, pluginConfig, ap } }, [isExpanded, interfaceType, areSuggestionsVisible, suggestions]) + // The form is only visible when expanded (or default-expanded). When collapsed the + // trigger button (a separate slot item) is what shows, so keep this wrapper out of + // layout — otherwise its empty box adds a stray flex gap in the slot column. + const isFormVisible = defaultExpanded || isExpanded + return ( -
- {!defaultExpanded && ( - events.handleOpenClick(appState)} - buttonRef={buttonRef} - searchIcon={searchIcon} - showLabel={showLabel} - /> - )} +
events.handleCloseClick(e, buttonRef, appState)} + onClick={(e) => events.handleCloseClick(e, appState)} closeIcon={closeIcon} /> events.handleCloseClick(e, buttonRef, appState)} submitIcon={searchIcon} /> diff --git a/plugins/search/src/Search.test.jsx b/plugins/search/src/Search.test.jsx index fec06b093..30c9f15e6 100644 --- a/plugins/search/src/Search.test.jsx +++ b/plugins/search/src/Search.test.jsx @@ -5,14 +5,6 @@ import { attachEvents } from './events/index.js' import { createDatasets } from './datasets.js' // Mock sub-components -jest.mock('./components/OpenButton/OpenButton', () => ({ - OpenButton: ({ id, isExpanded, onClick }) => ( - - ) -})) - jest.mock('./components/CloseButton/CloseButton', () => ({ CloseButton: ({ defaultExpanded, onClick }) => ( - ) -} diff --git a/plugins/search/src/components/OpenButton/OpenButton.module.scss b/plugins/search/src/components/OpenButton/OpenButton.module.scss deleted file mode 100644 index 174f313bf..000000000 --- a/plugins/search/src/components/OpenButton/OpenButton.module.scss +++ /dev/null @@ -1,3 +0,0 @@ -.im-c-search-open-button { - position: relative; -} \ No newline at end of file diff --git a/plugins/search/src/components/OpenButton/OpenButton.test.jsx b/plugins/search/src/components/OpenButton/OpenButton.test.jsx deleted file mode 100644 index 84357bec6..000000000 --- a/plugins/search/src/components/OpenButton/OpenButton.test.jsx +++ /dev/null @@ -1,51 +0,0 @@ -// src/plugins/search/OpenButton.test.jsx - -import { render, screen, fireEvent } from '@testing-library/react' -import { OpenButton } from './OpenButton' - -describe('OpenButton', () => { - const baseProps = { - id: 'test', - isExpanded: false, - onClick: jest.fn(), - buttonRef: { current: null }, - searchIcon: null - } - - beforeEach(() => { - jest.clearAllMocks() - }) - - it('renders the button with correct ARIA attributes and calls onClick', () => { - render() - const button = screen.getByRole('button', { name: /open search/i }) - expect(button).toHaveAttribute('aria-controls', 'test-search-form') - fireEvent.click(button) - expect(baseProps.onClick).toHaveBeenCalledTimes(1) - }) - - it('applies display:none when isExpanded is true', () => { - render() - const button = screen.getByLabelText('Open search') - expect(button).toHaveStyle({ display: 'none' }) - }) - - it('renders the search icon SVG when searchIcon is provided', () => { - const svgContent = '' - const { container } = render( - - ) - const svg = container.querySelector('svg') - expect(svg).toBeTruthy() - expect(svg.innerHTML).toContain('M1 1L23 23') - }) - - it('does not render an SVG when searchIcon is not provided', () => { - const { container } = render() - expect(container.querySelector('svg')).toBeNull() - }) - it('should show a label if showLabel is true', () => { - const { container } = render() - expect(container.querySelector('span').innerHTML).toContain('Search') - }) -}) diff --git a/plugins/search/src/components/SubmitButton/SubmitButton.module.scss b/plugins/search/src/components/SubmitButton/SubmitButton.module.scss index 931a8b3a9..30fb256a1 100755 --- a/plugins/search/src/components/SubmitButton/SubmitButton.module.scss +++ b/plugins/search/src/components/SubmitButton/SubmitButton.module.scss @@ -1,7 +1,13 @@ .im-c-search-submit-button { + // Needed so the z-index below is effective (z-index only applies to positioned + // elements). Without it the keyboard focus ring renders behind the input + // container's border and is hidden — matching the close button's own treatment. + position: relative; + &:before { box-shadow: none; } + &[data-focus-visible="true"] { z-index: 1; } diff --git a/plugins/search/src/components/Suggestions/Suggestions.module.scss b/plugins/search/src/components/Suggestions/Suggestions.module.scss index 66340744f..219d8a82a 100755 --- a/plugins/search/src/components/Suggestions/Suggestions.module.scss +++ b/plugins/search/src/components/Suggestions/Suggestions.module.scss @@ -18,6 +18,7 @@ color: var(--secondary-text-color); border-top: 1px solid var(--button-hover-color); cursor: pointer; + user-select: none; @media (-ms-high-contrast:active), screen and (forced-colors: active) { color: Highlight; diff --git a/plugins/search/src/events/formHandlers.js b/plugins/search/src/events/formHandlers.js index 939718356..a47be5cda 100755 --- a/plugins/search/src/events/formHandlers.js +++ b/plugins/search/src/events/formHandlers.js @@ -2,6 +2,35 @@ import { fetchSuggestions } from './fetchSuggestions.js' import { updateMap } from '../utils/updateMap.js' import { DEFAULTS } from '../defaults.js' +// Resolve the open trigger — a core-rendered MapButton. Prefer the shared buttonRefs +// map (keyed by the manifest button id), falling back to a DOM query on the button's +// class so focus restoration works even if the ref is transiently null / not yet set. +const getTriggerButton = (buttonRefs) => + buttonRefs?.current?.search || document.querySelector('.im-c-map-button--search') + +// Restore focus to the open trigger once the closing DOM has settled. The trigger only +// becomes focusable again a frame or two after the form closes (its hidden/exclusive +// state clears across re-renders), so keep trying until focus actually lands. +const MAX_FOCUS_FRAMES = 30 + +const restoreTriggerFocus = (buttonRefs) => { + let frames = 0 + const focusWhenReady = () => { + const button = getTriggerButton(buttonRefs) + if (button) { + button.focus() + if (document.activeElement === button) { + return + } + } + if (frames < MAX_FOCUS_FRAMES) { + frames += 1 + requestAnimationFrame(focusWhenReady) + } + } + requestAnimationFrame(focusWhenReady) +} + export const createFormHandlers = ({ dispatch, services, @@ -16,16 +45,11 @@ export const createFormHandlers = ({ let lastFetchedValue = '' return { - handleOpenClick () { - dispatch({ type: 'TOGGLE_EXPANDED', payload: true }) - services.eventBus.emit('search:open') - }, - - handleCloseClick (_e, buttonRef) { + handleCloseClick (_e, appState) { dispatch({ type: 'TOGGLE_EXPANDED', payload: false }) dispatch({ type: 'UPDATE_SUGGESTIONS', payload: { results: [], hasError: false } }) dispatch({ type: 'SET_VALUE', payload: '' }) - setTimeout(() => buttonRef.current.focus(), 0) + restoreTriggerFocus(appState?.buttonRefs) markers.remove('search') services.eventBus.emit('search:clear') services.eventBus.emit('search:close') diff --git a/plugins/search/src/events/formHandlers.test.js b/plugins/search/src/events/formHandlers.test.js index 4cbbf2ea5..63ef68743 100644 --- a/plugins/search/src/events/formHandlers.test.js +++ b/plugins/search/src/events/formHandlers.test.js @@ -46,34 +46,67 @@ describe('createFormHandlers', () => { jest.clearAllMocks() }) - test('handleOpenClick dispatches and emits', () => { - handlers.handleOpenClick() + describe('handleCloseClick focus restoration', () => { + let raf + let button + + beforeEach(() => { + // Run rAF callbacks synchronously so focus assertions are deterministic + raf = jest.spyOn(global, 'requestAnimationFrame').mockImplementation((cb) => { + cb() + return 0 + }) + // A real, focusable element so element.focus() updates document.activeElement + button = document.createElement('button') + button.className = 'im-c-map-button im-c-map-button--search' + document.body.appendChild(button) + }) - expect(dispatch).toHaveBeenCalledWith({ - type: 'TOGGLE_EXPANDED', - payload: true + afterEach(() => { + raf.mockRestore() + button.remove() }) - expect(services.eventBus.emit).toHaveBeenCalledWith('search:open') - }) - test('handleCloseClick resets state and focuses button', () => { - jest.useFakeTimers() - const buttonRef = { current: { focus: jest.fn() } } + test('resets state, emits events and returns focus to the trigger via buttonRefs', () => { + handlers.handleCloseClick(null, { buttonRefs: { current: { search: button } } }) - handlers.handleCloseClick(null, buttonRef) + expect(dispatch).toHaveBeenCalledWith({ type: 'TOGGLE_EXPANDED', payload: false }) + expect(dispatch).toHaveBeenCalledWith({ type: 'UPDATE_SUGGESTIONS', payload: { results: [], hasError: false } }) + expect(dispatch).toHaveBeenCalledWith({ type: 'SET_VALUE', payload: '' }) + expect(markers.remove).toHaveBeenCalledWith('search') + expect(services.eventBus.emit).toHaveBeenCalledWith('search:clear') + expect(services.eventBus.emit).toHaveBeenCalledWith('search:close') - expect(dispatch).toHaveBeenCalledWith({ type: 'TOGGLE_EXPANDED', payload: false }) - expect(dispatch).toHaveBeenCalledWith({ type: 'UPDATE_SUGGESTIONS', payload: { results: [], hasError: false } }) - expect(dispatch).toHaveBeenCalledWith({ type: 'SET_VALUE', payload: '' }) - expect(markers.remove).toHaveBeenCalledWith('search') + expect(document.activeElement).toBe(button) + }) - jest.runAllTimers() - expect(buttonRef.current.focus).toHaveBeenCalled() + test('falls back to a DOM query when the ref is not set', () => { + handlers.handleCloseClick(null, { buttonRefs: { current: {} } }) - expect(services.eventBus.emit).toHaveBeenCalledWith('search:clear') - expect(services.eventBus.emit).toHaveBeenCalledWith('search:close') + expect(document.activeElement).toBe(button) + }) - jest.useRealTimers() + test('does not throw when no trigger exists (default-expanded)', () => { + button.remove() // no trigger in the DOM and none in the ref map + + expect(() => + handlers.handleCloseClick(null, { buttonRefs: { current: {} } }) + ).not.toThrow() + }) + + test('retries across frames and gives up when the trigger never becomes active', () => { + // Disabled buttons can't receive focus in jsdom, so activeElement never + // matches — forces the retry loop through to its MAX_FOCUS_FRAMES exit + button.disabled = true + const focusSpy = jest.spyOn(button, 'focus') + + handlers.handleCloseClick(null, { buttonRefs: { current: { search: button } } }) + + // rAF is mocked synchronous, so the whole retry loop unwinds within this call + expect(focusSpy.mock.calls.length).toBeGreaterThan(1) + expect(raf.mock.calls.length).toBeGreaterThan(1) + expect(document.activeElement).not.toBe(button) + }) }) test('handleSubmit uses selected suggestion when selectedIndex >= 0', async () => { diff --git a/plugins/search/src/events/suggestionHandlers.js b/plugins/search/src/events/suggestionHandlers.js index 35c3a3832..7b19aa068 100755 --- a/plugins/search/src/events/suggestionHandlers.js +++ b/plugins/search/src/events/suggestionHandlers.js @@ -54,6 +54,9 @@ export const createSuggestionHandlers = ({ dispatch, services, mapProvider, mark case 'Escape': { e.preventDefault() + // Restore the focus ring to the input container first — SET_KEYBOARD_FOCUS_WITHIN + // also flips areSuggestionsVisible on, so HIDE_SUGGESTIONS must run after to win + dispatch({ type: 'SET_KEYBOARD_FOCUS_WITHIN', payload: true }) dispatch({ type: 'HIDE_SUGGESTIONS' }) dispatch({ type: 'SET_SELECTED', payload: -1 }) break diff --git a/plugins/search/src/events/suggestionHandlers.test.js b/plugins/search/src/events/suggestionHandlers.test.js index 8065078bd..7b9e67d02 100644 --- a/plugins/search/src/events/suggestionHandlers.test.js +++ b/plugins/search/src/events/suggestionHandlers.test.js @@ -144,7 +144,7 @@ describe('createSuggestionHandlers', () => { // ---------- Escape & default ---------- - test('Escape hides suggestions and clears selection', () => { + test('Escape hides suggestions, clears selection and restores the input focus ring', () => { const e = { key: 'Escape', preventDefault: jest.fn() } handlers.handleInputKeyDown(e, { @@ -153,8 +153,14 @@ describe('createSuggestionHandlers', () => { }) expect(e.preventDefault).toHaveBeenCalled() + expect(dispatch).toHaveBeenCalledWith({ type: 'SET_KEYBOARD_FOCUS_WITHIN', payload: true }) expect(dispatch).toHaveBeenCalledWith({ type: 'HIDE_SUGGESTIONS' }) expect(dispatch).toHaveBeenCalledWith({ type: 'SET_SELECTED', payload: -1 }) + + // SET_KEYBOARD_FOCUS_WITHIN must be dispatched before HIDE_SUGGESTIONS — it has a + // side effect of turning areSuggestionsVisible back on, which HIDE_SUGGESTIONS must win over + const order = dispatch.mock.calls.map(([action]) => action.type) + expect(order.indexOf('SET_KEYBOARD_FOCUS_WITHIN')).toBeLessThan(order.indexOf('HIDE_SUGGESTIONS')) }) test('Other keys do nothing', () => { diff --git a/plugins/search/src/manifest.js b/plugins/search/src/manifest.js index 3a517c9b2..30f141df7 100755 --- a/plugins/search/src/manifest.js +++ b/plugins/search/src/manifest.js @@ -8,8 +8,27 @@ export const manifest = { actions }, - controls: [{ + // The open trigger is a standard MapButton so it inherits the shared tooltip + // (shown whenever its label is hidden). It toggles the plugin's own `isExpanded` + // state, which the `search` control below reads to reveal the form. + buttons: [{ id: 'search', + label: 'Search', + iconId: 'search', + // aria-controls the form rendered by the control (matches the form's id in Form.jsx) + ariaControls: ({ appConfig }) => `${appConfig.id}-search-form`, + // No trigger in default-expanded mode — the form is always visible there + excludeWhen: ({ pluginConfig }) => Boolean(pluginConfig.expanded), + // Keep focus in the search (the control focuses the input on open) rather than + // letting the button return focus to the map viewport after the click. + // NB: the trigger is hidden while the form is open via the `im-o-app--exclusive-control` + // CSS (opacity + out-of-flow) rather than `hiddenWhen`, so it stays focusable and + // focus can be returned to it on close without a display:none race. + keepFocus: true, + onClick: (_e, { pluginState, services }) => { + pluginState.dispatch({ type: 'TOGGLE_EXPANDED', payload: true }) + services.eventBus.emit('search:open') + }, mobile: { slot: 'top-right', showLabel: false @@ -21,6 +40,19 @@ export const manifest = { desktop: { slot: 'top-left', showLabel: false + } + }], + + controls: [{ + id: 'search', + mobile: { + slot: 'top-right' + }, + tablet: { + slot: 'top-left' + }, + desktop: { + slot: 'top-left' }, render: Search }], diff --git a/plugins/search/src/manifest.test.js b/plugins/search/src/manifest.test.js new file mode 100644 index 000000000..ef2e5289e --- /dev/null +++ b/plugins/search/src/manifest.test.js @@ -0,0 +1,43 @@ +// /plugins/search/manifest.test.js +import { manifest } from './manifest.js' + +describe('search manifest', () => { + const getButton = () => manifest.buttons.find(b => b.id === 'search') + + it('declares a single search button and control', () => { + expect(manifest.buttons).toHaveLength(1) + expect(getButton()).toBeDefined() + expect(manifest.controls.find(c => c.id === 'search')).toBeDefined() + }) + + it('hides the button label at every breakpoint so the shared tooltip is shown', () => { + const btn = getButton() + expect(btn.mobile.showLabel).toBe(false) + expect(btn.tablet.showLabel).toBe(false) + expect(btn.desktop.showLabel).toBe(false) + // The accessible name / tooltip text comes from the label + expect(btn.label).toBe('Search') + expect(btn.iconId).toBe('search') + }) + + it('points aria-controls at the form rendered by the control', () => { + expect(getButton().ariaControls({ appConfig: { id: 'map' } })).toBe('map-search-form') + }) + + it('excludes the trigger in default-expanded mode', () => { + expect(getButton().excludeWhen({ pluginConfig: { expanded: true } })).toBe(true) + expect(getButton().excludeWhen({ pluginConfig: { expanded: false } })).toBe(false) + }) + + it('keeps focus in the search rather than returning it to the map on open', () => { + expect(getButton().keepFocus).toBe(true) + }) + + it('opens the form and emits search:open on click', () => { + const dispatch = jest.fn() + const emit = jest.fn() + getButton().onClick(null, { pluginState: { dispatch }, services: { eventBus: { emit } } }) + expect(dispatch).toHaveBeenCalledWith({ type: 'TOGGLE_EXPANDED', payload: true }) + expect(emit).toHaveBeenCalledWith('search:open') + }) +}) diff --git a/plugins/search/src/reducer.test.js b/plugins/search/src/reducer.test.js index 733206ac1..d4bca26e7 100644 --- a/plugins/search/src/reducer.test.js +++ b/plugins/search/src/reducer.test.js @@ -72,6 +72,19 @@ describe('search state actions', () => { expect(newState.hasFetchedSuggestions).toBe(false) }) + it('Escape sequence (SET_KEYBOARD_FOCUS_WITHIN then HIDE_SUGGESTIONS then SET_SELECTED) restores the ring without reopening the list', () => { + // Reproduces suggestionHandlers.js's Escape dispatch order: the ring must come back + // while HIDE_SUGGESTIONS/SET_SELECTED still win on areSuggestionsVisible + let state = { ...initialState, areSuggestionsVisible: true, hasKeyboardFocusWithin: false, selectedIndex: 0 } + state = actions.SET_KEYBOARD_FOCUS_WITHIN(state, true) + state = actions.HIDE_SUGGESTIONS(state) + state = actions.SET_SELECTED(state, -1) + + expect(state.hasKeyboardFocusWithin).toBe(true) + expect(state.areSuggestionsVisible).toBe(false) + expect(state.selectedIndex).toBe(-1) + }) + it('SET_SELECTED updates selectedIndex and visibility', () => { const state = { ...initialState, areSuggestionsVisible: false } diff --git a/plugins/search/src/search.scss b/plugins/search/src/search.scss index 8c000c423..26a5c02f1 100755 --- a/plugins/search/src/search.scss +++ b/plugins/search/src/search.scss @@ -2,7 +2,6 @@ // Components, using 'CSS in Component' pattern @use './components/Form/Form.module'; -@use './components/OpenButton/OpenButton.module'; @use './components/CloseButton/CloseButton.module'; @use './components/SubmitButton/SubmitButton.module'; @use './components/Suggestions/Suggestions.module'; @@ -20,6 +19,14 @@ // To follow... } +// When the form is hidden (collapsed), the trigger button — a separate slot item — +// is what shows. Take this wrapper out of layout entirely so its empty box doesn't +// add a stray flex gap in the slot column. The form remains in the DOM (display:none +// ancestor) so the trigger's aria-controls target still resolves. +.im-c-search--collapsed { + display: none; +} + // 2. Elements .im-c-search__input-container { display: flex; @@ -109,6 +116,16 @@ opacity: 0; z-index: -1; } + + // The search open trigger shares its slot with the search form. While the form is + // open we take the trigger out of layout flow (it is already opacity:0 above) so the + // form occupies its place with no gap — but crucially it stays display:visible and + // therefore focusable, so focus can be returned to it when the form closes. + .im-c-button-wrapper--search { + position: absolute; + pointer-events: none; + } + .im-o-app__left-top, .im-o-app__left-bottom, .im-o-app__right-top,