From 3ca65b8d91546220b258d0380c49d627bc8d80a8 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Mon, 2 Mar 2026 11:53:47 +0000 Subject: [PATCH 1/8] Button group wrapper added --- demo/js/index.js | 8 +- docs/api/button-definition.md | 23 ++- plugins/beta/use-location/src/manifest.js | 2 +- src/App/components/MapButton/MapButton.jsx | 20 +-- .../MapButton/MapButton.module.scss | 24 +-- .../components/MapButton/MapButton.test.jsx | 9 -- src/App/layout/Layout.jsx | 8 +- src/App/layout/layout.module.scss | 1 + src/App/renderer/mapButtons.js | 140 ++++++++++++++---- src/App/renderer/mapButtons.test.js | 54 ++++--- src/config/appConfig.js | 4 +- 11 files changed, 199 insertions(+), 94 deletions(-) diff --git a/demo/js/index.js b/demo/js/index.js index ab3d7d0b..07dfcb8f 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -247,7 +247,7 @@ let selectedFeatureIds = [] interactiveMap.on('draw:ready', function () { // interactiveMap.addButton('drawPolygon', { // label: 'Draw polygon', - // group: 'Drawing tools', + // group: { name: 'drawing-tools', label: 'Drawing tools', order: 2 }, // iconSvgContent: '', // isPressed: false, // mobile: { slot: 'right-top' }, @@ -263,7 +263,7 @@ interactiveMap.on('draw:ready', function () { // }) // interactiveMap.addButton('drawLine', { // label: 'Draw line', - // group: 'Drawing tools', + // group: { name: 'drawing-tools', label: 'Drawing tools', order: 2 }, // iconSvgContent: '', // isPressed: false, // mobile: { slot: 'right-top' }, @@ -278,7 +278,7 @@ interactiveMap.on('draw:ready', function () { // }) // interactiveMap.addButton('editFeature', { // label: 'Edit feature', - // group: 'Drawing tools', + // group: { name: 'drawing-tools', label: 'Drawing tools', order: 2 }, // iconSvgContent: '', // isDisabled: true, // mobile: { slot: 'right-top' }, @@ -294,7 +294,7 @@ interactiveMap.on('draw:ready', function () { // }) // interactiveMap.addButton('deleteFeature', { // label: 'Delete feature', - // group: 'Drawing tools', + // group: { name: 'drawing-tools', label: 'Drawing tools', order: 2 }, // iconSvgContent: '', // isDisabled: true, // mobile: { slot: 'right-top' }, diff --git a/docs/api/button-definition.md b/docs/api/button-definition.md index c83597ce..a09ac028 100644 --- a/docs/api/button-definition.md +++ b/docs/api/button-definition.md @@ -64,9 +64,24 @@ Raw SVG content for the button icon. The outer `` tag should be excluded. --- ### `group` -**Type:** `string` +**Type:** `{ name: string, label?: string, order?: number }` + +Groups this button with other buttons that share the same `name`. Two or more buttons sharing a group are rendered inside a semantic `
` wrapper, collapsing their visual borders into a single grouped unit. + +- **`name`** — Internal identifier used to collect group members. Buttons with the same `name` are grouped together. +- **`label`** — Accessible label for the group, announced by screen readers. Defaults to `name` if not provided. Should be a short human-readable description (e.g. `'Zoom controls'`). +- **`order`** — The group's position within its slot, equivalent to the `order` property on singleton buttons. All buttons in the same group must share the same value. Defaults to `0`. -Button group label for grouping related buttons. +```js +// Two buttons rendered as a labelled group at slot position 2 +{ group: { name: 'zoom', label: 'Zoom controls', order: 2 }, ... } +``` + +The `order` property on each button's breakpoint config (e.g. `desktop.order`) controls the button's position *within* the group when an explicit sequence is needed. If omitted, buttons appear in their declaration order. + +> If only one button declares a given group name, it is rendered as a standalone button and the group wrapper is not created. + +> **Note:** Passing a plain string (e.g. `group: 'zoom'`) is deprecated and will log a warning in development. --- @@ -216,7 +231,9 @@ The [slot](./slots.md) where the button should appear at this breakpoint. Slots ### `order` **Type:** `number` -The order the button appears within its slot. +For **ungrouped buttons**, this is the button's position within its slot. + +For **grouped buttons**, this is the button's position *within its group*. The group's position within the slot is controlled by `group.order` instead. If omitted, buttons appear in their declaration order within the group. ### `showLabel` **Type:** `boolean` diff --git a/plugins/beta/use-location/src/manifest.js b/plugins/beta/use-location/src/manifest.js index 85bb8540..b229ba3f 100755 --- a/plugins/beta/use-location/src/manifest.js +++ b/plugins/beta/use-location/src/manifest.js @@ -18,7 +18,7 @@ export const manifest = { buttons: [{ id: 'useLocation', - group: 'location', + group: { name: 'location', label: 'Location', order: 0 }, label: 'Use your location', iconId: 'locateFixed', hiddenWhen: () => !navigator.geolocation, diff --git a/src/App/components/MapButton/MapButton.jsx b/src/App/components/MapButton/MapButton.jsx index 9bc7fba2..bd33fc6d 100755 --- a/src/App/components/MapButton/MapButton.jsx +++ b/src/App/components/MapButton/MapButton.jsx @@ -26,18 +26,12 @@ const buildButtonClassNames = (buttonId, variant, showLabel) => [ * Builds CSS class names for the wrapper div that contains the button. * @param {string} buttonId - Unique identifier for the button * @param {boolean} showLabel - Whether the button label is displayed - * @param {boolean} groupStart - Whether this button is at the start of a button group - * @param {boolean} groupMiddle - Whether this button is in the middle of a button group - * @param {boolean} groupEnd - Whether this button is at the end of a button group * @returns {string} Space-separated CSS class names for the wrapper */ -const buildWrapperClassNames = (buttonId, showLabel, groupStart, groupMiddle, groupEnd) => [ +const buildWrapperClassNames = (buttonId, showLabel) => [ 'im-c-button-wrapper', buttonId && `im-c-button-wrapper--${stringToKebab(buttonId)}`, - showLabel && 'im-c-button-wrapper--wide', - groupStart && 'im-c-button-wrapper--group-start', - groupMiddle && 'im-c-button-wrapper--group-middle', - groupEnd && 'im-c-button-wrapper--group-end' + showLabel && 'im-c-button-wrapper--wide' ].filter(Boolean).join(' ') /** @@ -159,9 +153,6 @@ const buildButtonProps = ({ * @param {Array} [props.menuItems] - Array of items for popup menu * @param {string} [props.idPrefix=''] - Prefix for generated panel/popup IDs * @param {string} [props.href] - URL for anchor element; if provided, renders as instead of + ), +})) + jest.mock('./components/Form/Form', () => ({ Form: ({ children }) =>
{children}
, })) @@ -114,6 +122,18 @@ describe('Search component', () => { expect(events.handleOpenClick).toHaveBeenCalledTimes(1) }) + it('renders SubmitButton when expanded is false', () => { + render() + expect(screen.getByTestId('submit-button')).toBeInTheDocument() + }) + + it('SubmitButton click triggers handleCloseClick', () => { + render() + const events = attachEvents.mock.results[0].value + fireEvent.click(screen.getByTestId('submit-button')) + expect(events.handleCloseClick).toHaveBeenCalledTimes(1) + }) + it('CloseButton click triggers handleCloseClick', () => { render() const events = attachEvents.mock.results[0].value diff --git a/plugins/search/src/components/Form/Form.module.scss b/plugins/search/src/components/Form/Form.module.scss index fcf1b17d..e05a3c6a 100755 --- a/plugins/search/src/components/Form/Form.module.scss +++ b/plugins/search/src/components/Form/Form.module.scss @@ -6,6 +6,7 @@ &:before { border-radius: 0; + box-shadow: none; } } diff --git a/plugins/search/src/components/SubmitButton/SubmitButton.jsx b/plugins/search/src/components/SubmitButton/SubmitButton.jsx new file mode 100755 index 00000000..4c4cb1cc --- /dev/null +++ b/plugins/search/src/components/SubmitButton/SubmitButton.jsx @@ -0,0 +1,28 @@ +// src/plugins/search/SubmitButton.jsx +export const SubmitButton = ({ defaultExpanded, submitIcon }) => { + return ( + + ) +} diff --git a/plugins/search/src/components/SubmitButton/SubmitButton.module.scss b/plugins/search/src/components/SubmitButton/SubmitButton.module.scss new file mode 100755 index 00000000..931a8b3a --- /dev/null +++ b/plugins/search/src/components/SubmitButton/SubmitButton.module.scss @@ -0,0 +1,8 @@ +.im-c-search-submit-button { + &:before { + box-shadow: none; + } + &[data-focus-visible="true"] { + z-index: 1; + } +} \ No newline at end of file diff --git a/plugins/search/src/components/SubmitButton/SubmitButton.test.jsx b/plugins/search/src/components/SubmitButton/SubmitButton.test.jsx new file mode 100644 index 00000000..262ab152 --- /dev/null +++ b/plugins/search/src/components/SubmitButton/SubmitButton.test.jsx @@ -0,0 +1,33 @@ +// src/plugins/search/components/SubmitButton/SubmitButton.test.jsx + +import { render, screen } from '@testing-library/react' +import { SubmitButton } from './SubmitButton' + +describe('SubmitButton', () => { + it('renders a submit button with the correct aria-label', () => { + render() + const button = screen.getByRole('button', { name: 'Search' }) + expect(button).toBeInTheDocument() + expect(button).toHaveAttribute('type', 'submit') + }) + + it('hides the button when defaultExpanded is false', () => { + const { container } = render() + expect(container.querySelector('button')).toHaveStyle({ display: 'none' }) + }) + + it('shows the button when defaultExpanded is true', () => { + render() + expect(screen.getByRole('button', { name: 'Search' })).not.toHaveStyle({ display: 'none' }) + }) + + it('renders an svg when submitIcon is provided', () => { + const { container } = render() + expect(container.querySelector('svg')).toBeTruthy() + }) + + it('does not render an svg when submitIcon is not provided', () => { + const { container } = render() + expect(container.querySelector('svg')).toBeNull() + }) +}) diff --git a/plugins/search/src/datasets.js b/plugins/search/src/datasets.js index 5096ce9c..963a4dc1 100755 --- a/plugins/search/src/datasets.js +++ b/plugins/search/src/datasets.js @@ -1,15 +1,19 @@ // src/plugins/search/datasets.js import { parseOsNamesResults } from './utils/parseOsNamesResults.js' -export function createDatasets({ customDatasets = [], osNamesURL, crs }) { - - const defaultDatasets = [{ - name: 'osNames', - urlTemplate: osNamesURL, - parseResults: (json, query) => parseOsNamesResults(json, query, crs), - includeRegex: /[a-zA-Z0-9]/, - excludeRegex: /^(?:[a-z]{2}\s*(?:\d{3}\s*\d{3}|\d{4}\s*\d{4}|\d{5}\s*\d{5})|\d+\s*,?\s*\d+)$/i // NOSONAR - complexity unavoidable for gridref/coordinate matching - }] - - return [...defaultDatasets, ...customDatasets] +export function createDatasets({ customDatasets = [], osNamesURL, crs, regions = ['england', 'scotland', 'wales'] }) { + + if (!osNamesURL) { + return customDatasets + } + + const defaultDatasets = [{ + name: 'osNames', + urlTemplate: osNamesURL, + parseResults: (json, query) => parseOsNamesResults(json, query, regions, crs), + includeRegex: /[a-zA-Z0-9]/, + excludeRegex: /^(?:[a-z]{2}\s*(?:\d{3}\s*\d{3}|\d{4}\s*\d{4}|\d{5}\s*\d{5})|\d+\s*,?\s*\d+)$/i // NOSONAR - complexity unavoidable for gridref/coordinate matching + }] + + return [...defaultDatasets, ...customDatasets] } diff --git a/plugins/search/src/datasets.test.js b/plugins/search/src/datasets.test.js index 0a9598d3..f16dde41 100644 --- a/plugins/search/src/datasets.test.js +++ b/plugins/search/src/datasets.test.js @@ -11,6 +11,21 @@ describe('createDatasets', () => { jest.clearAllMocks() }) + it('returns only customDatasets if osNamesURL is not provided', () => { + const custom = [ + { name: 'custom1', urlTemplate: 'https://custom.com' }, + { name: 'custom2', urlTemplate: 'https://custom2.com' } + ] + const datasets = createDatasets({ customDatasets: custom, crs }) + expect(datasets).toHaveLength(2) + expect(datasets).toEqual(custom) + }) + + it('returns empty array if neither osNamesURL nor customDatasets are provided', () => { + const datasets = createDatasets({ crs }) + expect(datasets).toEqual([]) + }) + it('returns default dataset with correct properties', () => { const datasets = createDatasets({ osNamesURL, crs }) expect(datasets).toHaveLength(1) @@ -39,7 +54,7 @@ describe('createDatasets', () => { const query = 'test query' const result = datasets[0].parseResults(json, query) - expect(parseMock).toHaveBeenCalledWith(json, query, crs) + expect(parseMock).toHaveBeenCalledWith(json, query, ['england', 'scotland', 'wales'], crs) expect(result).toBe('parsed') parseMock.mockRestore() }) diff --git a/plugins/search/src/defaults.js b/plugins/search/src/defaults.js index 10c81890..f4c4ac6d 100644 --- a/plugins/search/src/defaults.js +++ b/plugins/search/src/defaults.js @@ -1,3 +1,4 @@ export const DEFAULTS = { - minSearchLength: 3 + minSearchLength: 3, + noResultsText: 'No results are available' } \ No newline at end of file diff --git a/plugins/search/src/search.scss b/plugins/search/src/search.scss index 3526b695..8332d093 100755 --- a/plugins/search/src/search.scss +++ b/plugins/search/src/search.scss @@ -3,6 +3,7 @@ // Components, using 'CSS in Component' pattern @use './components/Form/Form.module'; @use './components/CloseButton/CloseButton.module'; +@use './components/SubmitButton/SubmitButton.module'; @use './components/Suggestions/Suggestions.module'; :root { diff --git a/plugins/search/src/utils/parseOsNamesResults.js b/plugins/search/src/utils/parseOsNamesResults.js index 6be41cab..c73857e5 100755 --- a/plugins/search/src/utils/parseOsNamesResults.js +++ b/plugins/search/src/utils/parseOsNamesResults.js @@ -10,12 +10,27 @@ const isPostcode = (value) => { return regex.test(value) } +const removeRegions = (results, regions) => results.filter(r => { + const country = r?.GAZETTEER_ENTRY?.COUNTRY?.toLowerCase() + return country && regions.includes(country) +}) + const removeDuplicates = (results) => Array.from(new Map(results.map(r => [r.GAZETTEER_ENTRY.ID, r])).values()) +const transformGazetteerResult = (result) => { + const gazetterEntry = result.GAZETTEER_ENTRY + // Sometimes the NAME1 value can be welsh, so we should use NAME2 instead + const name = gazetterEntry.NAME2_LANG === 'eng' ? gazetterEntry.NAME2 : gazetterEntry.NAME1 + // Force NAME1 to be the english value + result.GAZETTEER_ENTRY.NAME1 = name + result.GAZETTEER_ENTRY.NAME1_LANG = 'eng' + return result +} + const removeTenuousResults = (results, query) => { const words = query.toLowerCase().replace(/,/g, '').split(' ') - return results.filter(l => words.some(w => l.GAZETTEER_ENTRY.NAME1.toLowerCase().includes(w) || isPostcode(query))) + return results.map(transformGazetteerResult).filter(l => words.some(w => l.GAZETTEER_ENTRY.NAME1.toLowerCase().includes(w) || isPostcode(query))) } const markString = (string, find) => { @@ -86,13 +101,14 @@ const label = (query, { NAME1, COUNTY_UNITARY, DISTRICT_BOROUGH, POSTCODE_DISTRI } } -const parseOsNamesResults = (json, query, crs) => { +const parseOsNamesResults = (json, query, regions, crs) => { if (!json || json.error || json.header?.totalresults === 0) { return [] } let results = json.results results = removeTenuousResults(results, query) results = removeDuplicates(results) + results = removeRegions(results, regions) results = results.slice(0, MAX_RESULTS) return results.map(l => ({ diff --git a/plugins/search/src/utils/parseOsNamesResults.test.js b/plugins/search/src/utils/parseOsNamesResults.test.js index a58a7b21..56b7d9e0 100644 --- a/plugins/search/src/utils/parseOsNamesResults.test.js +++ b/plugins/search/src/utils/parseOsNamesResults.test.js @@ -2,7 +2,6 @@ * @jest-environment jsdom */ import { parseOsNamesResults, point } from './parseOsNamesResults.js' -import OsGridRef from 'geodesy/osgridref.js' // Mock OsGridRef so we can control toLatLon outputs jest.mock('geodesy/osgridref.js', () => { @@ -14,6 +13,8 @@ jest.mock('geodesy/osgridref.js', () => { }) describe('osNamesUtils', () => { + const regions = ['england', 'scotland', 'wales'] + const sampleEntry = { GAZETTEER_ENTRY: { ID: 1, @@ -22,6 +23,7 @@ describe('osNamesUtils', () => { DISTRICT_BOROUGH: 'Camden', POSTCODE_DISTRICT: 'WC1', LOCAL_TYPE: 'Town', + COUNTRY: 'England', MBR_XMIN: 1000, MBR_YMIN: 2000, MBR_XMAX: 3000, @@ -32,9 +34,9 @@ describe('osNamesUtils', () => { } test('returns empty array for null/invalid/error results', () => { - expect(parseOsNamesResults(null, 'x', 'EPSG:27700')).toEqual([]) - expect(parseOsNamesResults({ error: true }, 'x', 'EPSG:27700')).toEqual([]) - expect(parseOsNamesResults({ header: { totalresults: 0 } }, 'x', 'EPSG:27700')).toEqual([]) + expect(parseOsNamesResults(null, 'x', regions, 'EPSG:27700')).toEqual([]) + expect(parseOsNamesResults({ error: true }, 'x', regions, 'EPSG:27700')).toEqual([]) + expect(parseOsNamesResults({ header: { totalresults: 0 } }, 'x', regions, 'EPSG:27700')).toEqual([]) }) test('removes tenuous results when query does not match', () => { @@ -42,14 +44,14 @@ describe('osNamesUtils', () => { { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY, NAME1: 'Bristol', ID: 2 } } ] const json = { results } - const output = parseOsNamesResults(json, 'London', 'EPSG:27700') + const output = parseOsNamesResults(json, 'London', regions, 'EPSG:27700') expect(output).toHaveLength(0) }) test('removes duplicate IDs', () => { const dup = { ...sampleEntry, GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY } } const json = { results: [sampleEntry, dup] } - const output = parseOsNamesResults(json, 'London', 'EPSG:27700') + const output = parseOsNamesResults(json, 'London', regions, 'EPSG:27700') expect(output).toHaveLength(1) }) @@ -58,13 +60,13 @@ describe('osNamesUtils', () => { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY, ID: i } })) const json = { results: manyResults } - const output = parseOsNamesResults(json, 'London', 'EPSG:27700') + const output = parseOsNamesResults(json, 'London', regions, 'EPSG:27700') expect(output).toHaveLength(8) }) test('bounds returns raw OSGB values for EPSG:27700', () => { const json = { results: [sampleEntry] } - const output = parseOsNamesResults(json, 'London', 'EPSG:27700') + const output = parseOsNamesResults(json, 'London', regions, 'EPSG:27700') expect(output[0].bounds).toEqual([ sampleEntry.GAZETTEER_ENTRY.MBR_XMIN, sampleEntry.GAZETTEER_ENTRY.MBR_YMIN, @@ -79,7 +81,7 @@ describe('osNamesUtils', () => { test('bounds converts to WGS84 for EPSG:4326', () => { const json = { results: [sampleEntry] } - const output = parseOsNamesResults(json, 'London', 'EPSG:4326') + const output = parseOsNamesResults(json, 'London', regions, 'EPSG:4326') const expectedBounds = [ Math.round(1000 / 1e5 * 1e6) / 1e6, Math.round(2000 / 1e5 * 1e6) / 1e6, @@ -96,7 +98,7 @@ describe('osNamesUtils', () => { test('label generates marked text', () => { const json = { results: [sampleEntry] } - const output = parseOsNamesResults(json, 'London', 'EPSG:27700') + const output = parseOsNamesResults(json, 'London', regions, 'EPSG:27700') expect(output[0].text).toContain('London') expect(output[0].marked).toContain('') }) @@ -106,7 +108,7 @@ describe('osNamesUtils', () => { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY, MBR_XMIN: null, MBR_YMIN: null } } const json = { results: [entry] } - const output = parseOsNamesResults(json, 'London', 'EPSG:27700') + const output = parseOsNamesResults(json, 'London', regions, 'EPSG:27700') expect(output[0].bounds).toEqual([ entry.GAZETTEER_ENTRY.GEOMETRY_X - 500, entry.GAZETTEER_ENTRY.GEOMETRY_Y - 500, @@ -117,24 +119,40 @@ describe('osNamesUtils', () => { test('label falls back to DISTRICT_BOROUGH when COUNTY_UNITARY is absent', () => { const entry = { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY, COUNTY_UNITARY: null } } - const output = parseOsNamesResults({ results: [entry] }, 'London', 'EPSG:27700') + const output = parseOsNamesResults({ results: [entry] }, 'London', regions, 'EPSG:27700') expect(output[0].text).toContain(sampleEntry.GAZETTEER_ENTRY.DISTRICT_BOROUGH) }) test('label omits qualifier for City type', () => { const entry = { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY, LOCAL_TYPE: 'City' } } - const output = parseOsNamesResults({ results: [entry] }, 'London', 'EPSG:27700') + const output = parseOsNamesResults({ results: [entry] }, 'London', regions, 'EPSG:27700') expect(output[0].text).toBe('London') }) + /** + * Cover transformGazetteerResult branches + */ + + test('uses NAME2 when NAME2_LANG is eng', () => { + const entry = { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY, NAME1: 'Caerdydd', NAME2: 'Cardiff', NAME2_LANG: 'eng' }} + const output = parseOsNamesResults({ results: [entry] }, 'Cardiff', regions, 'EPSG:27700') + expect(output[0].text).toContain('Cardiff') + }) + + test('falls back to NAME1 when NAME2_LANG is not eng', () => { + const entry = { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY, NAME1: 'London', NAME2: 'Londres', NAME2_LANG: 'fra' }} + const output = parseOsNamesResults({ results: [entry] }, 'London', regions, 'EPSG:27700') + expect(output[0].text).toContain('London') + }) + test('throws error for unsupported CRS', () => { const entry = { GAZETTEER_ENTRY: { ...sampleEntry.GAZETTEER_ENTRY } } const json = { results: [entry] } - expect(() => parseOsNamesResults(json, 'London', 'EPSG:9999')).toThrow('Unsupported CRS') + expect(() => parseOsNamesResults(json, 'London', regions, 'EPSG:9999')).toThrow('Unsupported CRS') }) test('point function throws error for unsupported CRS', () => { const coords = { GEOMETRY_X: 1500, GEOMETRY_Y: 2500 } expect(() => point('EPSG:9999', coords)).toThrow('Unsupported CRS: EPSG:9999') }) -}) \ No newline at end of file +}) diff --git a/src/App/renderer/mapButtons.js b/src/App/renderer/mapButtons.js index 1bd294d0..92f5d1b9 100755 --- a/src/App/renderer/mapButtons.js +++ b/src/App/renderer/mapButtons.js @@ -170,15 +170,15 @@ function mapButtons ({ slot, appState, appConfig, evaluateProp }) { const label = resolveGroupLabel(group) const order = resolveGroupOrder(group) - if (!groupMap.has(name)) { - groupMap.set(name, { label, order, members: [] }) - } else { + if (groupMap.has(name)) { const existing = groupMap.get(name) /* istanbul ignore next */ if (process.env.NODE_ENV !== 'production' && existing.order !== order) { console.warn(`[interactive-map] Group "${name}" has inconsistent order values (${existing.order} vs ${order}). Using the lower value.`) existing.order = Math.min(existing.order, order) } + } else { + groupMap.set(name, { label, order, members: [] }) } groupMap.get(name).members.push([buttonId, config]) @@ -238,5 +238,8 @@ function mapButtons ({ slot, appState, appConfig, evaluateProp }) { export { mapButtons, getMatchingButtons, - renderButton + renderButton, + resolveGroupName, + resolveGroupLabel, + resolveGroupOrder } diff --git a/src/App/renderer/mapButtons.test.js b/src/App/renderer/mapButtons.test.js index 78f0b482..d022b919 100755 --- a/src/App/renderer/mapButtons.test.js +++ b/src/App/renderer/mapButtons.test.js @@ -1,5 +1,5 @@ import React from 'react' -import { mapButtons, getMatchingButtons, renderButton } from './mapButtons.js' +import { mapButtons, getMatchingButtons, renderButton, resolveGroupName, resolveGroupLabel, resolveGroupOrder } from './mapButtons.js' import { getPanelConfig } from '../registry/panelRegistry.js' jest.mock('../registry/buttonRegistry.js') @@ -44,6 +44,52 @@ describe('mapButtons module', () => { getPanelConfig.mockReturnValue({}) }) + // ------------------------- + // resolveGroup* helper tests + // ------------------------- + describe('resolveGroupName', () => { + it('returns null when group is null or undefined', () => { + expect(resolveGroupName(null)).toBeNull() + expect(resolveGroupName(undefined)).toBeNull() + }) + it('returns the string when group is a string', () => { + expect(resolveGroupName('g1')).toBe('g1') + }) + it('returns group.name when group is an object', () => { + expect(resolveGroupName({ name: 'g1' })).toBe('g1') + expect(resolveGroupName({ name: undefined })).toBeNull() + }) + }) + + describe('resolveGroupLabel', () => { + it('returns empty string when group is falsy', () => { + expect(resolveGroupLabel(null)).toBe('') + expect(resolveGroupLabel(undefined)).toBe('') + }) + it('returns the string itself when group is a string', () => { + expect(resolveGroupLabel('My Group')).toBe('My Group') + }) + it('returns group.label when provided, else group.name, else empty string', () => { + expect(resolveGroupLabel({ name: 'g1', label: 'Group One' })).toBe('Group One') + expect(resolveGroupLabel({ name: 'g1' })).toBe('g1') + expect(resolveGroupLabel({ order: 5 })).toBe('') + }) + }) + + describe('resolveGroupOrder', () => { + it('returns 0 when group is falsy', () => { + expect(resolveGroupOrder(null)).toBe(0) + expect(resolveGroupOrder(undefined)).toBe(0) + }) + it('returns 0 when group is a string', () => { + expect(resolveGroupOrder('g1')).toBe(0) + }) + it('returns group.order when provided, else 0', () => { + expect(resolveGroupOrder({ name: 'g1', order: 5 })).toBe(5) + expect(resolveGroupOrder({ name: 'g1' })).toBe(0) + }) + }) + // ------------------------- // getMatchingButtons tests // ------------------------- @@ -241,6 +287,28 @@ describe('mapButtons module', () => { expect(result[0]).toMatchObject({ id: 'b1', type: 'button', order: 3 }) }) + it('falls back to breakpoint order for singleton group when group order is 0', () => { + appState.buttonConfig = ({ b1: { ...baseBtn, desktop: { slot: 'header', order: 4 }, group: { name: 'g1', order: 0 } } }) + expect(map()[0].order).toBe(4) + }) + + it('falls back to 0 for singleton group when both group order and breakpoint order are absent', () => { + appState.buttonConfig = ({ b1: { ...baseBtn, desktop: { slot: 'header' }, group: { name: 'g1', order: 0 } } }) + expect(map()[0].order).toBe(0) + }) + + it('sorts group members treating missing breakpoint order as 0', () => { + appState.buttonConfig = ({ + b1: { ...baseBtn, group: { name: 'g1', order: 0 }, desktop: { slot: 'header', order: 2 } }, + b2: { ...baseBtn, group: { name: 'g1', order: 0 }, desktop: { slot: 'header' } }, + b3: { ...baseBtn, group: { name: 'g1', order: 0 }, desktop: { slot: 'header', order: 1 } } + }) + const children = map()[0].element.props.children + expect(children[0].props.buttonId).toBe('b2') + expect(children[1].props.buttonId).toBe('b3') + expect(children[2].props.buttonId).toBe('b1') + }) + it('falls back to order 0 when order is not specified in breakpoint config', () => { appState.buttonConfig = ({ b1: { ...baseBtn, desktop: { slot: 'header' } } }) expect(map()[0].order).toBe(0) From 5b42e3420c5e31abe672862e257c25e170d15613 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Mon, 2 Mar 2026 15:43:01 +0000 Subject: [PATCH 3/8] Search live region updates --- plugins/search/src/Search.jsx | 1 + plugins/search/src/components/Form/Form.jsx | 25 ++++- .../src/components/Form/Form.module.scss | 14 +++ .../search/src/components/Form/Form.test.jsx | 97 +++++++++++++++++++ plugins/search/src/defaults.js | 3 +- plugins/search/src/reducer.js | 13 ++- plugins/search/src/reducer.test.js | 19 ++-- src/App/hooks/useLayoutMeasurements.js | 2 +- src/App/hooks/useLayoutMeasurements.test.js | 2 +- 9 files changed, 159 insertions(+), 17 deletions(-) diff --git a/plugins/search/src/Search.jsx b/plugins/search/src/Search.jsx index 443830fc..a928ad90 100755 --- a/plugins/search/src/Search.jsx +++ b/plugins/search/src/Search.jsx @@ -94,6 +94,7 @@ export function Search({ appConfig, iconRegistry, pluginState, pluginConfig, app appState={appState} inputRef={inputRef} events={events} + services={services} > { + const { areSuggestionsVisible, hasFetchedSuggestions, suggestions = [] } = pluginState + + // Announce when a fetch has completed (hasFetchedSuggestions flips to true), + // not when the input is merely focused/clicked (SHOW_SUGGESTIONS resets it to false). + useEffect(() => { + if (!areSuggestionsVisible || !hasFetchedSuggestions) { + return + } + const count = suggestions.length + const plural = count === 1 ? '' : 's' + const message = count === 0 ? 'No results available' : `${count} result${plural} available` + services.announce(message) + }, [suggestions, hasFetchedSuggestions]) const classNames = [ 'im-c-search-form', @@ -17,6 +32,8 @@ export const Form = ({ 'im-c-panel' ].filter(Boolean).join(' ') + const showNoResults = areSuggestionsVisible && hasFetchedSuggestions && !suggestions.length + return (
- + {showNoResults && ( + + )} ) -} \ No newline at end of file +} diff --git a/plugins/search/src/components/Form/Form.module.scss b/plugins/search/src/components/Form/Form.module.scss index e05a3c6a..157fb422 100755 --- a/plugins/search/src/components/Form/Form.module.scss +++ b/plugins/search/src/components/Form/Form.module.scss @@ -16,6 +16,8 @@ top: 0; right: 0; height: auto; + overflow: visible; + filter: var(--search-drop-shadow); } .im-o-app--tablet .im-c-search-form, @@ -39,6 +41,18 @@ } } +// Status +.im-c-search__status { + position: absolute; + top: calc(100% + 1px); + left: 0; + right: 0; + padding: var(--divider-gap) calc(var(--divider-gap) + 2px); + background-color: var(--background-color); + border-bottom-left-radius: var(--button-border-radius); + border-bottom-right-radius: var(--button-border-radius); +} + // Default expanded .im-o-app--mobile .im-c-search-form--default-expanded { position: relative; diff --git a/plugins/search/src/components/Form/Form.test.jsx b/plugins/search/src/components/Form/Form.test.jsx index c9f8381f..0d13e91b 100644 --- a/plugins/search/src/components/Form/Form.test.jsx +++ b/plugins/search/src/components/Form/Form.test.jsx @@ -24,6 +24,9 @@ describe('Form', () => { isExpanded: false, value: '', suggestionsVisible: false, + areSuggestionsVisible: false, + hasFetchedSuggestions: false, + suggestions: [], selectedIndex: -1, hasKeyboardFocusWithin: false, }, @@ -44,12 +47,21 @@ describe('Form', () => { handleInputKeyDown: jest.fn(), handleSuggestionClick: jest.fn(), }, + services: { + announce: jest.fn(), + }, } beforeEach(() => { jest.clearAllMocks() }) + it('renders without error when suggestions is undefined (uses default empty array)', () => { + const { suggestions: _omit, ...pluginStateWithoutSuggestions } = baseProps.pluginState + render(
) + expect(screen.getByRole('search')).toBeInTheDocument() + }) + it('renders the form element with correct role, ID, and base classes', () => { render() const form = screen.getByRole('search') @@ -155,4 +167,89 @@ describe('Form', () => { baseProps.appState ) }) + + describe('status element and announce', () => { + // Helper: pluginState representing a completed fetch (hasFetchedSuggestions: true) + const searchedState = { areSuggestionsVisible: true, hasFetchedSuggestions: true } + + it('hides the status element when suggestions are not visible', () => { + const { container } = render() + expect(container.querySelector('.im-c-search__status')).toBeNull() + }) + + it('shows the status element when a search returned no results', () => { + render( + + ) + expect(screen.getByText('No results available')).toBeInTheDocument() + }) + + it('hides the status element when there are results', () => { + const { container } = render( + + ) + expect(container.querySelector('.im-c-search__status')).toBeNull() + }) + + it('hides the status element when the fetch has not yet completed', () => { + const { container } = render( + + ) + expect(container.querySelector('.im-c-search__status')).toBeNull() + }) + + it('announces "No results available" when a search returned no results', () => { + render( + + ) + expect(baseProps.services.announce).toHaveBeenCalledWith('No results available') + }) + + it('announces result count when suggestions are visible and populated', () => { + render( + + ) + expect(baseProps.services.announce).toHaveBeenCalledWith('2 results available') + }) + + it('uses singular "result" for a single result', () => { + render( + + ) + expect(baseProps.services.announce).toHaveBeenCalledWith('1 result available') + }) + + it('does not announce when suggestions are not visible', () => { + render() + expect(baseProps.services.announce).not.toHaveBeenCalled() + }) + + it('does not announce when the fetch has not yet completed', () => { + render( + + ) + expect(baseProps.services.announce).not.toHaveBeenCalled() + }) + }) }) \ No newline at end of file diff --git a/plugins/search/src/defaults.js b/plugins/search/src/defaults.js index f4c4ac6d..10c81890 100644 --- a/plugins/search/src/defaults.js +++ b/plugins/search/src/defaults.js @@ -1,4 +1,3 @@ export const DEFAULTS = { - minSearchLength: 3, - noResultsText: 'No results are available' + minSearchLength: 3 } \ No newline at end of file diff --git a/plugins/search/src/reducer.js b/plugins/search/src/reducer.js index 3d23ed84..636302ec 100755 --- a/plugins/search/src/reducer.js +++ b/plugins/search/src/reducer.js @@ -4,6 +4,7 @@ const initialState = { value: '', suggestions: [], areSuggestionsVisible: false, + hasFetchedSuggestions: false, selectedIndex: -1 } @@ -12,7 +13,8 @@ const toggleExpanded = (state, payload) => { return { ...state, isExpanded: payload, - areSuggestionsVisible: payload + areSuggestionsVisible: payload, + hasFetchedSuggestions: false } } @@ -43,21 +45,24 @@ const setValue = (state, payload) => { const updateSuggestions = (state, payload) => { return { ...state, - suggestions: payload + suggestions: payload, + hasFetchedSuggestions: true } } const showSuggestions = (state) => { return { ...state, - areSuggestionsVisible: true + areSuggestionsVisible: true, + hasFetchedSuggestions: false } } const hideSuggestions = (state) => { return { ...state, - areSuggestionsVisible: false + areSuggestionsVisible: false, + hasFetchedSuggestions: false } } diff --git a/plugins/search/src/reducer.test.js b/plugins/search/src/reducer.test.js index 9b362b7c..faf18541 100644 --- a/plugins/search/src/reducer.test.js +++ b/plugins/search/src/reducer.test.js @@ -3,15 +3,17 @@ import { initialState, actions } from './reducer' describe('search state actions', () => { - it('TOGGLE_EXPANDED sets isExpanded and areSuggestionsVisible', () => { - const state = { ...initialState } + it('TOGGLE_EXPANDED sets isExpanded and areSuggestionsVisible, and resets hasFetchedSuggestions', () => { + const state = { ...initialState, hasFetchedSuggestions: true } const newState = actions.TOGGLE_EXPANDED(state, true) expect(newState.isExpanded).toBe(true) expect(newState.areSuggestionsVisible).toBe(true) + expect(newState.hasFetchedSuggestions).toBe(false) const collapsed = actions.TOGGLE_EXPANDED(state, false) expect(collapsed.isExpanded).toBe(false) expect(collapsed.areSuggestionsVisible).toBe(false) + expect(collapsed.hasFetchedSuggestions).toBe(false) }) it('SET_KEYBOARD_FOCUS_WITHIN sets focus and shows suggestions', () => { @@ -47,23 +49,26 @@ describe('search state actions', () => { expect(newState.value).toBe('test') }) - it('UPDATE_SUGGESTIONS updates the suggestions array', () => { + it('UPDATE_SUGGESTIONS updates the suggestions array and sets hasFetchedSuggestions', () => { const state = { ...initialState } const suggestions = [{ id: 1 }, { id: 2 }] const newState = actions.UPDATE_SUGGESTIONS(state, suggestions) expect(newState.suggestions).toEqual(suggestions) + expect(newState.hasFetchedSuggestions).toBe(true) }) - it('SHOW_SUGGESTIONS sets areSuggestionsVisible to true', () => { - const state = { ...initialState, areSuggestionsVisible: false } + it('SHOW_SUGGESTIONS sets areSuggestionsVisible to true and resets hasFetchedSuggestions', () => { + const state = { ...initialState, areSuggestionsVisible: false, hasFetchedSuggestions: true } const newState = actions.SHOW_SUGGESTIONS(state) expect(newState.areSuggestionsVisible).toBe(true) + expect(newState.hasFetchedSuggestions).toBe(false) }) - it('HIDE_SUGGESTIONS sets areSuggestionsVisible to false', () => { - const state = { ...initialState, areSuggestionsVisible: true } + it('HIDE_SUGGESTIONS sets areSuggestionsVisible to false and resets hasFetchedSuggestions', () => { + const state = { ...initialState, areSuggestionsVisible: true, hasFetchedSuggestions: true } const newState = actions.HIDE_SUGGESTIONS(state) expect(newState.areSuggestionsVisible).toBe(false) + expect(newState.hasFetchedSuggestions).toBe(false) }) it('SET_SELECTED updates selectedIndex and visibility', () => { diff --git a/src/App/hooks/useLayoutMeasurements.js b/src/App/hooks/useLayoutMeasurements.js index da199a85..707247ca 100755 --- a/src/App/hooks/useLayoutMeasurements.js +++ b/src/App/hooks/useLayoutMeasurements.js @@ -81,7 +81,7 @@ export function useLayoutMeasurements () { // -------------------------------- // 3. Recaluclate CSS vars when elements resize // -------------------------------- - useResizeObserver([bannerRef, mainRef, topRef, actionsRef, footerRef], () => { + useResizeObserver([bannerRef, mainRef, topRef, topLeftColRef, topRightColRef, actionsRef, footerRef], () => { requestAnimationFrame(() => { calculateLayout() }) diff --git a/src/App/hooks/useLayoutMeasurements.test.js b/src/App/hooks/useLayoutMeasurements.test.js index 552fffc3..4280ef10 100644 --- a/src/App/hooks/useLayoutMeasurements.test.js +++ b/src/App/hooks/useLayoutMeasurements.test.js @@ -114,7 +114,7 @@ describe('useLayoutMeasurements', () => { const { layoutRefs } = setup() renderHook(() => useLayoutMeasurements()) expect(useResizeObserver).toHaveBeenCalledWith( - [layoutRefs.bannerRef, layoutRefs.mainRef, layoutRefs.topRef, layoutRefs.actionsRef, layoutRefs.footerRef], + [layoutRefs.bannerRef, layoutRefs.mainRef, layoutRefs.topRef, layoutRefs.topLeftColRef, layoutRefs.topRightColRef, layoutRefs.actionsRef, layoutRefs.footerRef], expect.any(Function) ) layoutRefs.appContainerRef.current.style.setProperty.mockClear() From c65cb63fd7afbbb34807f7757f7e192972db5eb2 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 3 Mar 2026 09:41:18 +0000 Subject: [PATCH 4/8] Map events reinstated and new methods added --- docs/api.md | 32 ++++++++++- providers/beta/esri/src/appEvents.js | 2 - providers/beta/esri/src/esriProvider.js | 16 +----- providers/beta/esri/src/mapEvents.js | 8 ++- providers/maplibre/src/maplibreProvider.js | 19 +++---- .../maplibre/src/maplibreProvider.test.js | 4 +- src/App/components/Viewport/MapController.jsx | 3 +- src/App/hooks/useMapProviderOverrides.js | 20 ++++++- src/App/hooks/useMapProviderOverrides.test.js | 53 ++++++++++++++++- src/App/store/MapProvider.jsx | 30 ++-------- src/App/store/MapProvider.test.jsx | 57 ++++++------------- src/App/store/mapActionsMap.js | 11 ++-- src/InteractiveMap/InteractiveMap.js | 18 ++++++ src/InteractiveMap/InteractiveMap.test.js | 12 ++++ src/config/events.js | 12 ++-- 15 files changed, 181 insertions(+), 116 deletions(-) diff --git a/docs/api.md b/docs/api.md index 2f48962d..89d4e9bb 100644 --- a/docs/api.md +++ b/docs/api.md @@ -666,6 +666,36 @@ interactiveMap.toggleButtonState('opt-a', 'pressed', true) --- +### `fitToBounds(bbox)` + +Fit the map view to a bounding box. Safe zone padding is automatically applied so the content remains fully visible. + +| Parameter | Type | Description | +|-----------|------|-------------| +| `bbox` | `[number, number, number, number]` | Bounds as [west, south, east, north] or [minX, minY, maxX, maxY] depending on CRS | + +```js +interactiveMap.fitToBounds([-0.489, 51.28, 0.236, 51.686]) +``` + +--- + +### `setView(opts)` + +Set the map center and zoom. Safe zone padding is automatically applied. + +| Parameter | Type | Description | +|-----------|------|-------------| +| `opts` | `Object` | View options | +| `opts.center` | `[number, number]` | Optional center [lng, lat] or [easting, northing] depending on CRS | +| `opts.zoom` | `number` | Optional zoom level | + +```js +interactiveMap.setView({ center: [-0.1276, 51.5074], zoom: 12 }) +``` + +--- + ## Events Subscribe to events using `interactiveMap.on()` and unsubscribe with `interactiveMap.off()`. @@ -709,8 +739,6 @@ Emitted when the underlying map is ready and initial app state (style and size) | `map` | Object | The underlying map instance (all providers) | | `view` | Object | The map view (ESRI only) | | `crs` | string | The coordinate reference system (e.g. `'EPSG:4326'`) | -| `fitToBounds` | Function | Fit the map to a bounding box | -| `setView` | Function | Set the map center and zoom | | `mapStyleId` | string | The ID of the active map style | | `mapSize` | string | The active map size (`'small'`, `'medium'`, or `'large'`) | diff --git a/providers/beta/esri/src/appEvents.js b/providers/beta/esri/src/appEvents.js index 0f992b99..a46bc896 100644 --- a/providers/beta/esri/src/appEvents.js +++ b/providers/beta/esri/src/appEvents.js @@ -1,5 +1,4 @@ export function attachAppEvents ({ - mapProvider, baseTileLayer, events, eventBus @@ -7,7 +6,6 @@ export function attachAppEvents ({ const handleSetMapStyle = mapStyle => { baseTileLayer.loadStyle(mapStyle.url).then(() => { eventBus.emit(events.MAP_STYLE_CHANGE, { - ...mapProvider.getMapAPI(), mapStyleId: mapStyle.id }) }) diff --git a/providers/beta/esri/src/esriProvider.js b/providers/beta/esri/src/esriProvider.js index 238df1ed..efddac7f 100644 --- a/providers/beta/esri/src/esriProvider.js +++ b/providers/beta/esri/src/esriProvider.js @@ -28,7 +28,9 @@ export default class EsriProvider { } async initMap (config) { - const { container, padding, mapStyle, maxExtent, ...initConfig } = config + const { container, padding, mapStyle, mapSize, maxExtent, ...initConfig } = config + this.mapStyleId = mapStyle?.id + this.mapSize = mapSize const { events, eventBus } = this if (this.setupConfig) { @@ -82,7 +84,6 @@ export default class EsriProvider { // Attach app events and store handles this.appEventHandles = attachAppEvents({ - mapProvider: this, baseTileLayer, events, eventBus @@ -113,17 +114,6 @@ export default class EsriProvider { } } - /** Returns the public API exposed via the map:ready event. */ - getMapAPI () { - return { - map: this.map, - view: this.view, - crs: this.crs, - fitToBounds: this.fitToBounds.bind(this), - setView: this.setView.bind(this) - } - } - // ========================== // Side-effects // ========================== diff --git a/providers/beta/esri/src/mapEvents.js b/providers/beta/esri/src/mapEvents.js index 0a1dfc8d..6bfe8399 100644 --- a/providers/beta/esri/src/mapEvents.js +++ b/providers/beta/esri/src/mapEvents.js @@ -54,7 +54,13 @@ export function attachMapEvents ({ // ready once(() => view.ready).then(() => { if (!destroyed) { - eventBus.emit(events.MAP_PROVIDER_READY, mapProvider.getMapAPI()) + eventBus.emit(events.MAP_READY, { + map: mapProvider.map, + view: mapProvider.view, + mapStyleId: mapProvider.mapStyleId, + mapSize: mapProvider.mapSize, + crs: mapProvider.crs + }) } }) diff --git a/providers/maplibre/src/maplibreProvider.js b/providers/maplibre/src/maplibreProvider.js index ff7ff0b4..d3a317e0 100755 --- a/providers/maplibre/src/maplibreProvider.js +++ b/providers/maplibre/src/maplibreProvider.js @@ -44,7 +44,9 @@ export default class MapLibreProvider { * @returns {Promise} */ async initMap (config) { - const { container, padding, mapStyle, center, zoom, bounds, pixelRatio, ...initConfig } = config + const { container, padding, mapStyle, mapSize, center, zoom, bounds, pixelRatio, ...initConfig } = config + this.mapStyleId = mapStyle?.id + this.mapSize = mapSize const { Map: MaplibreMap } = this.maplibreModule const { events, eventBus } = this @@ -101,17 +103,12 @@ export default class MapLibreProvider { this.labelNavigator = createMapLabelNavigator(map, mapStyle?.mapColorScheme, events, eventBus) }) - this.eventBus.emit(events.MAP_PROVIDER_READY, this.getMapAPI()) - } - - /** Returns the public API exposed via the map:ready event. */ - getMapAPI () { - return { + this.eventBus.emit(events.MAP_READY, { map: this.map, - crs: this.crs, - fitToBounds: this.fitToBounds.bind(this), - setView: this.setView.bind(this) - } + mapStyleId: this.mapStyleId, + mapSize: this.mapSize, + crs: this.crs + }) } /** Destroy the map and clean up resources. */ diff --git a/providers/maplibre/src/maplibreProvider.test.js b/providers/maplibre/src/maplibreProvider.test.js index 3d12de41..c82dd3e5 100644 --- a/providers/maplibre/src/maplibreProvider.test.js +++ b/providers/maplibre/src/maplibreProvider.test.js @@ -59,7 +59,7 @@ describe('MapLibreProvider', () => { const makeProvider = (config = {}) => new MapLibreProvider({ mapFramework: maplibreModule, - events: { MAP_PROVIDER_READY: 'map:providerready' }, + events: { MAP_READY: 'map:ready' }, eventBus, ...config }) @@ -84,7 +84,7 @@ describe('MapLibreProvider', () => { expect(map.setPadding).toHaveBeenCalled() expect(attachMapEvents).toHaveBeenCalled() expect(attachAppEvents).toHaveBeenCalled() - expect(eventBus.emit).toHaveBeenCalledWith('map:providerready', expect.any(Object)) + expect(eventBus.emit).toHaveBeenCalledWith('map:ready', { map, mapStyleId: undefined, mapSize: undefined, crs: undefined }) }) test('initMap: fitBounds called when bounds provided; skipped when absent; null mapStyle → style undefined', async () => { diff --git a/src/App/components/Viewport/MapController.jsx b/src/App/components/Viewport/MapController.jsx index ac860b9c..92b4e596 100755 --- a/src/App/components/Viewport/MapController.jsx +++ b/src/App/components/Viewport/MapController.jsx @@ -38,7 +38,8 @@ export const MapController = ({ mapContainerRef }) => { center: initialState.center, zoom: initialState.zoom, bounds: initialState.bounds, - mapStyle + mapStyle, + mapSize }) }) diff --git a/src/App/hooks/useMapProviderOverrides.js b/src/App/hooks/useMapProviderOverrides.js index 6ba654ff..e8e898d3 100755 --- a/src/App/hooks/useMapProviderOverrides.js +++ b/src/App/hooks/useMapProviderOverrides.js @@ -2,12 +2,13 @@ import { useEffect, useRef } from 'react' import { useConfig } from '../store/configContext.js' import { useApp } from '../store/appContext.js' import { useMap } from '../store/mapContext.js' +import { EVENTS as events } from '../../config/events.js' import { getSafeZoneInset } from '../../utils/getSafeZoneInset.js' import { scalePoints } from '../../utils/scalePoints.js' import { scaleFactor } from '../../config/appConfig.js' export const useMapProviderOverrides = () => { - const { mapProvider } = useConfig() + const { mapProvider, eventBus } = useConfig() const { dispatch: appDispatch, layoutRefs } = useApp() const { mapSize } = useMap() @@ -67,4 +68,21 @@ export const useMapProviderOverrides = () => { mapProvider.setView = originalSetView } }, [mapProvider, appDispatch, layoutRefs, mapSize]) + + // Forward public API events to the (overridden) mapProvider methods so that + // interactiveMap.fitToBounds() and interactiveMap.setView() respect safe zone padding. + useEffect(() => { + if (!mapProvider || !eventBus) return undefined + + const handleFitToBounds = (bbox) => mapProvider.fitToBounds(bbox) + const handleSetView = (opts) => mapProvider.setView(opts) + + eventBus.on(events.MAP_FIT_TO_BOUNDS, handleFitToBounds) + eventBus.on(events.MAP_SET_VIEW, handleSetView) + + return () => { + eventBus.off(events.MAP_FIT_TO_BOUNDS, handleFitToBounds) + eventBus.off(events.MAP_SET_VIEW, handleSetView) + } + }, [mapProvider, eventBus]) } diff --git a/src/App/hooks/useMapProviderOverrides.test.js b/src/App/hooks/useMapProviderOverrides.test.js index dc645f85..72440731 100644 --- a/src/App/hooks/useMapProviderOverrides.test.js +++ b/src/App/hooks/useMapProviderOverrides.test.js @@ -22,15 +22,21 @@ const setup = (overrides = {}) => { setPadding: jest.fn(), ...overrides.mapProvider } + const capturedHandlers = {} + const eventBus = { + on: jest.fn((event, handler) => { capturedHandlers[event] = handler }), + off: jest.fn(), + ...overrides.eventBus + } - useConfig.mockReturnValue({ mapProvider, ...overrides.config }) + useConfig.mockReturnValue({ mapProvider, eventBus, ...overrides.config }) useApp.mockReturnValue({ dispatch, layoutRefs, ...overrides.app }) useMap.mockReturnValue({ mapSize: 'md', ...overrides.map }) getSafeZoneInset.mockReturnValue({ top: 10, right: 5, bottom: 15, left: 5 }) scalePoints.mockReturnValue({ top: 20, right: 10, bottom: 30, left: 10 }) - return { dispatch, layoutRefs, mapProvider } + return { dispatch, layoutRefs, mapProvider, eventBus, capturedHandlers } } describe('useMapProviderOverrides', () => { @@ -133,4 +139,47 @@ describe('useMapProviderOverrides', () => { expect(mapProvider.fitToBounds).not.toBe(firstOverride) }) + + test('subscribes to MAP_FIT_TO_BOUNDS and MAP_SET_VIEW on eventBus', () => { + const { eventBus } = setup() + renderHook(() => useMapProviderOverrides()) + + expect(eventBus.on).toHaveBeenCalledWith('map:fittobounds', expect.any(Function)) + expect(eventBus.on).toHaveBeenCalledWith('map:setview', expect.any(Function)) + }) + + test('MAP_FIT_TO_BOUNDS event forwards bbox to mapProvider.fitToBounds', () => { + const { mapProvider, capturedHandlers } = setup() + const originalFitToBounds = mapProvider.fitToBounds + renderHook(() => useMapProviderOverrides()) + + capturedHandlers['map:fittobounds']([0, 0, 1, 1]) + + expect(originalFitToBounds).toHaveBeenCalledWith([0, 0, 1, 1]) + }) + + test('MAP_SET_VIEW event forwards opts to mapProvider.setView', () => { + const { mapProvider, capturedHandlers } = setup() + const originalSetView = mapProvider.setView + renderHook(() => useMapProviderOverrides()) + + capturedHandlers['map:setview']({ center: [1, 2], zoom: 10 }) + + expect(originalSetView).toHaveBeenCalledWith({ center: [1, 2], zoom: 10 }) + }) + + test('unsubscribes from MAP_FIT_TO_BOUNDS and MAP_SET_VIEW on unmount', () => { + const { eventBus } = setup() + const { unmount } = renderHook(() => useMapProviderOverrides()) + + unmount() + + expect(eventBus.off).toHaveBeenCalledWith('map:fittobounds', expect.any(Function)) + expect(eventBus.off).toHaveBeenCalledWith('map:setview', expect.any(Function)) + }) + + test('skips event subscriptions when eventBus is null', () => { + setup({ config: { eventBus: null } }) + expect(() => renderHook(() => useMapProviderOverrides())).not.toThrow() + }) }) diff --git a/src/App/store/MapProvider.jsx b/src/App/store/MapProvider.jsx index 2a3e5a8f..88d202cf 100755 --- a/src/App/store/MapProvider.jsx +++ b/src/App/store/MapProvider.jsx @@ -9,13 +9,10 @@ export const MapProvider = ({ options, children }) => { const [state, dispatch] = useReducer(reducer, initialState(options)) const { eventBus } = options - const mapProviderAPIRef = useRef(null) - const hasEmittedMapReadyRef = useRef(false) const isMapSizeInitialisedRef = useRef(false) - const handleProviderReady = (mapProviderAPI) => { - mapProviderAPIRef.current = mapProviderAPI - dispatch({ type: 'SET_MAP_READY', payload: mapProviderAPI }) + const handleMapReady = () => { + dispatch({ type: 'SET_MAP_READY' }) } const handleInitMapStyles = (mapStyles) => { @@ -37,33 +34,19 @@ export const MapProvider = ({ options, children }) => { // Listen to eventBus and update state useEffect(() => { - eventBus.on(events.MAP_PROVIDER_READY, handleProviderReady) + eventBus.on(events.MAP_READY, handleMapReady) eventBus.on(events.MAP_INIT_MAP_STYLES, handleInitMapStyles) eventBus.on(events.MAP_SET_STYLE, handleSetMapStyle) eventBus.on(events.MAP_SET_SIZE, handleSetMapSize) return () => { - eventBus.off(events.MAP_PROVIDER_READY, handleProviderReady) + eventBus.off(events.MAP_READY, handleMapReady) eventBus.off(events.MAP_INIT_MAP_STYLES, handleInitMapStyles) eventBus.off(events.MAP_SET_STYLE, handleSetMapStyle) eventBus.off(events.MAP_SET_SIZE, handleSetMapSize) } }, []) - // Emit consumer-facing map:ready once provider, mapStyle and mapSize are all settled. - // Fires exactly once — the ref guard prevents re-emission if state later changes. - useEffect(() => { - if (!state.isMapReady || !state.mapStyle || !state.mapSize || hasEmittedMapReadyRef.current) { - return - } - hasEmittedMapReadyRef.current = true - eventBus.emit(events.MAP_READY, { - ...mapProviderAPIRef.current, - mapStyleId: state.mapStyle.id, - mapSize: state.mapSize - }) - }, [state.isMapReady, state.mapStyle, state.mapSize]) - // Emit map:sizechange when mapSize changes, skipping the initial value. useEffect(() => { if (!state.mapSize) { @@ -73,10 +56,7 @@ export const MapProvider = ({ options, children }) => { isMapSizeInitialisedRef.current = true return } - eventBus.emit(events.MAP_SIZE_CHANGE, { - ...mapProviderAPIRef.current, - mapSize: state.mapSize - }) + eventBus.emit(events.MAP_SIZE_CHANGE, { mapSize: state.mapSize }) }, [state.mapSize]) // Persist mapStyle and mapSize in localStorage diff --git a/src/App/store/MapProvider.test.jsx b/src/App/store/MapProvider.test.jsx index 1d5295db..c61c74e7 100755 --- a/src/App/store/MapProvider.test.jsx +++ b/src/App/store/MapProvider.test.jsx @@ -71,15 +71,15 @@ describe('MapProvider', () => { expect(contextValue).toHaveProperty('isMapReady') }) - test('subscribes to MAP_PROVIDER_READY instead of MAP_READY', () => { + test('subscribes to MAP_READY (not MAP_PROVIDER_READY)', () => { render(
Child
 ) - expect(mockEventBus.on).toHaveBeenCalledWith('map:providerready', expect.any(Function)) - expect(mockEventBus.on).not.toHaveBeenCalledWith('map:ready', expect.any(Function)) + expect(mockEventBus.on).toHaveBeenCalledWith('map:ready', expect.any(Function)) + expect(mockEventBus.on).not.toHaveBeenCalledWith('map:providerready', expect.any(Function)) }) test('subscribes and unsubscribes to eventBus', () => { @@ -89,65 +89,40 @@ describe('MapProvider', () => { ) - expect(mockEventBus.on).toHaveBeenCalledWith('map:providerready', expect.any(Function)) + expect(mockEventBus.on).toHaveBeenCalledWith('map:ready', expect.any(Function)) expect(mockEventBus.on).toHaveBeenCalledWith('map:initmapstyles', expect.any(Function)) expect(mockEventBus.on).toHaveBeenCalledWith('map:setstyle', expect.any(Function)) expect(mockEventBus.on).toHaveBeenCalledWith('map:setsize', expect.any(Function)) // Trigger handlers → covers reducer calls act(() => { - capturedHandlers['map:providerready']({ map: {} }) + capturedHandlers['map:ready']() capturedHandlers['map:setstyle']({ id: 'style1' }) capturedHandlers['map:setsize']('300x300') capturedHandlers['map:initmapstyles']([{ id: 'style1' }]) }) }) - test('emits consumer map:ready with enriched payload once provider, mapStyle and mapSize are settled', () => { - render( - -
Child
- - ) - - const mapProviderAPI = { map: {}, crs: 'EPSG:4326', fitToBounds: jest.fn(), setView: jest.fn() } - const mapStyle = { id: 'outdoor' } - - act(() => { - capturedHandlers['map:providerready'](mapProviderAPI) - capturedHandlers['map:initmapstyles']([mapStyle]) - }) - - expect(mockEventBus.emit).toHaveBeenCalledWith('map:ready', expect.objectContaining({ - map: mapProviderAPI.map, - crs: mapProviderAPI.crs, - mapStyleId: mapStyle.id, - mapSize: '100x100' - })) - }) + test('dispatches SET_MAP_READY when MAP_READY fires', () => { + let contextValue + const Child = () => { + contextValue = React.useContext(MapContext) + return null + } - test('emits consumer map:ready only once even if state changes after', () => { render( -
Child
+ ) - const mapProviderAPI = { map: {} } - const mapStyle = { id: 'outdoor' } + expect(contextValue.isMapReady).toBe(false) act(() => { - capturedHandlers['map:providerready'](mapProviderAPI) - capturedHandlers['map:initmapstyles']([mapStyle]) - }) - - // Trigger a size change after ready - act(() => { - capturedHandlers['map:setsize']('large') + capturedHandlers['map:ready']() }) - const mapReadyCalls = mockEventBus.emit.mock.calls.filter(([event]) => event === 'map:ready') - expect(mapReadyCalls).toHaveLength(1) + expect(contextValue.isMapReady).toBe(true) }) test('emits map:sizechange when mapSize changes after initial value', () => { @@ -218,7 +193,7 @@ describe('MapProvider', () => { ) act(() => { - capturedHandlers['map:providerready']({ map: {} }) + capturedHandlers['map:ready']() capturedHandlers['map:setstyle']({ id: 'style2' }) capturedHandlers['map:setsize']('400x400') }) diff --git a/src/App/store/mapActionsMap.js b/src/App/store/mapActionsMap.js index 7166ae96..51e66f80 100755 --- a/src/App/store/mapActionsMap.js +++ b/src/App/store/mapActionsMap.js @@ -7,13 +7,10 @@ const mergePayload = (state, payload) => ({ ...payload }) -const setMapReady = (state, mapProvider) => { - return { - ...state, - isMapReady: true, - mapProvider - } -} +const setMapReady = (state) => ({ + ...state, + isMapReady: true +}) const setMapStyle = (state, payload) => { return { diff --git a/src/InteractiveMap/InteractiveMap.js b/src/InteractiveMap/InteractiveMap.js index 9543a57c..e1ab0348 100755 --- a/src/InteractiveMap/InteractiveMap.js +++ b/src/InteractiveMap/InteractiveMap.js @@ -404,4 +404,22 @@ export default class InteractiveMap { addControl (id, config) { this.eventBus.emit(events.APP_ADD_CONTROL, { id, config }) } + + /** + * Fit the map view to a bounding box, respecting the safe zone padding. + * + * @param {[number, number, number, number]} bbox - Bounds as [west, south, east, north] or [minX, minY, maxX, maxY] depending on the crs. + */ + fitToBounds (bbox) { + this.eventBus.emit(events.MAP_FIT_TO_BOUNDS, bbox) + } + + /** + * Set the map center and zoom, respecting the safe zone padding. + * + * @param {{ center?: [number, number], zoom?: number }} opts - View options. + */ + setView (opts) { + this.eventBus.emit(events.MAP_SET_VIEW, opts) + } } diff --git a/src/InteractiveMap/InteractiveMap.test.js b/src/InteractiveMap/InteractiveMap.test.js index a0180d4e..ae207db8 100755 --- a/src/InteractiveMap/InteractiveMap.test.js +++ b/src/InteractiveMap/InteractiveMap.test.js @@ -509,4 +509,16 @@ describe('InteractiveMap Public API Methods', () => { { id: 'btn-1', prop: 'disabled', value: true } ) }) + + it('fitToBounds emits MAP_FIT_TO_BOUNDS with bbox', () => { + const bbox = [-0.489, 51.28, 0.236, 51.686] + map.fitToBounds(bbox) + expect(map.eventBus.emit).toHaveBeenCalledWith('map:fittobounds', bbox) + }) + + it('setView emits MAP_SET_VIEW with opts', () => { + const opts = { center: [-0.1276, 51.5074], zoom: 12 } + map.setView(opts) + expect(map.eventBus.emit).toHaveBeenCalledWith('map:setview', opts) + }) }) diff --git a/src/config/events.js b/src/config/events.js index f2534d59..57a95ba7 100644 --- a/src/config/events.js +++ b/src/config/events.js @@ -93,6 +93,10 @@ export const EVENTS = { MAP_SET_SIZE: 'map:setsize', /** @internal Set pixel ratio. Payload: pixelRatio */ MAP_SET_PIXEL_RATIO: 'map:setpixelratio', + /** @internal Fit the map to a bounding box. Payload: [west, south, east, north] */ + MAP_FIT_TO_BOUNDS: 'map:fittobounds', + /** @internal Set the map center and zoom. Payload: { center: [number, number], zoom?: number } */ + MAP_SET_VIEW: 'map:setview', // ============================================ // Map responses (advanced / subscribe) @@ -101,12 +105,6 @@ export const EVENTS = { /** @internal Emitted when map styles are initialized. */ MAP_INIT_MAP_STYLES: 'map:initmapstyles', - /** - * @internal Emitted by the map provider when the underlying map view is ready. - * Consumed by MapProvider to compose and emit the consumer-facing MAP_READY event. - */ - MAP_PROVIDER_READY: 'map:providerready', - /** * Emitted when the map style has finished loading. * Payload: `{ mapStyleId: string }` @@ -128,8 +126,6 @@ export const EVENTS = { * - `map` — the underlying map instance * - `view` — the map view (ESRI SDK only) * - `crs` — coordinate reference system string (e.g. `'EPSG:4326'`) - * - `fitToBounds` — function to fit the map to a bounding box - * - `setView` — function to set the map center and zoom * - `mapStyleId` — the ID of the active map style (e.g. `'outdoor'`, `'dark'`) * - `mapSize` — the active map size string (e.g. `'small'`, `'medium'`, `'large'`) * From 9d8fab88ee618d69e79f565c8815144ccc7260f7 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 3 Mar 2026 10:01:19 +0000 Subject: [PATCH 5/8] fitToBounds extended to take features --- docs/api.md | 21 ++- package-lock.json | 66 ++++++++- package.json | 1 + providers/beta/esri/src/esriProvider.js | 5 +- providers/beta/esri/src/utils/coords.js | 34 ++++- providers/beta/esri/src/utils/coords.test.js | 126 ++++++++++++++++++ providers/maplibre/src/maplibreProvider.js | 9 +- .../maplibre/src/maplibreProvider.test.js | 13 ++ providers/maplibre/src/utils/spatial.js | 11 ++ providers/maplibre/src/utils/spatial.test.js | 12 ++ src/InteractiveMap/InteractiveMap.js | 4 +- src/types.js | 4 +- 12 files changed, 289 insertions(+), 17 deletions(-) create mode 100644 providers/beta/esri/src/utils/coords.test.js diff --git a/docs/api.md b/docs/api.md index 89d4e9bb..51728722 100644 --- a/docs/api.md +++ b/docs/api.md @@ -666,16 +666,31 @@ interactiveMap.toggleButtonState('opt-a', 'pressed', true) --- -### `fitToBounds(bbox)` +### `fitToBounds(bounds)` -Fit the map view to a bounding box. Safe zone padding is automatically applied so the content remains fully visible. +Fit the map view to a bounding box or GeoJSON geometry. Safe zone padding is automatically applied so the content remains fully visible. | Parameter | Type | Description | |-----------|------|-------------| -| `bbox` | `[number, number, number, number]` | Bounds as [west, south, east, north] or [minX, minY, maxX, maxY] depending on CRS | +| `bounds` | `[number, number, number, number]` | Bounds as [west, south, east, north] or [minX, minY, maxX, maxY] depending on CRS | +| `bounds` | `object` | A GeoJSON Feature, FeatureCollection, or geometry — bbox is computed automatically | ```js +// Flat bbox interactiveMap.fitToBounds([-0.489, 51.28, 0.236, 51.686]) + +// GeoJSON Feature +interactiveMap.fitToBounds({ + type: 'Feature', + geometry: { type: 'Point', coordinates: [-0.1276, 51.5074] }, + properties: {} +}) + +// GeoJSON FeatureCollection +interactiveMap.fitToBounds({ + type: 'FeatureCollection', + features: [featureA, featureB] +}) ``` --- diff --git a/package-lock.json b/package-lock.json index 1dbfba10..1a80ed2c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "@docusaurus/theme-common": "^3.9.2", "@easyops-cn/docusaurus-search-local": "^0.55.0", "@turf/area": "^7.2.0", + "@turf/bbox": "^7.3.4", "@turf/bearing": "^7.3.3", "@turf/boolean-disjoint": "^7.3.3", "@turf/boolean-valid": "^7.2.0", @@ -9110,11 +9111,40 @@ } }, "node_modules/@turf/bbox": { - "version": "7.3.3", + "version": "7.3.4", + "resolved": "https://registry.npmjs.org/@turf/bbox/-/bbox-7.3.4.tgz", + "integrity": "sha512-D5ErVWtfQbEPh11yzI69uxqrcJmbPU/9Y59f1uTapgwAwQHQztDWgsYpnL3ns8r1GmPWLP8sGJLVTIk2TZSiYA==", "license": "MIT", "dependencies": { - "@turf/helpers": "7.3.3", - "@turf/meta": "7.3.3", + "@turf/helpers": "7.3.4", + "@turf/meta": "7.3.4", + "@types/geojson": "^7946.0.10", + "tslib": "^2.8.1" + }, + "funding": { + "url": "https://opencollective.com/turf" + } + }, + "node_modules/@turf/bbox/node_modules/@turf/helpers": { + "version": "7.3.4", + "resolved": "https://registry.npmjs.org/@turf/helpers/-/helpers-7.3.4.tgz", + "integrity": "sha512-U/S5qyqgx3WTvg4twaH0WxF3EixoTCfDsmk98g1E3/5e2YKp7JKYZdz0vivsS5/UZLJeZDEElOSFH4pUgp+l7g==", + "license": "MIT", + "dependencies": { + "@types/geojson": "^7946.0.10", + "tslib": "^2.8.1" + }, + "funding": { + "url": "https://opencollective.com/turf" + } + }, + "node_modules/@turf/bbox/node_modules/@turf/meta": { + "version": "7.3.4", + "resolved": "https://registry.npmjs.org/@turf/meta/-/meta-7.3.4.tgz", + "integrity": "sha512-tlmw9/Hs1p2n0uoHVm1w3ugw1I6L8jv9YZrcdQa4SH5FX5UY0ATrKeIvfA55FlL//PGuYppJp+eyg/0eb4goqw==", + "license": "MIT", + "dependencies": { + "@turf/helpers": "7.3.4", "@types/geojson": "^7946.0.10", "tslib": "^2.8.1" }, @@ -9276,6 +9306,21 @@ "url": "https://opencollective.com/turf" } }, + "node_modules/@turf/boolean-valid/node_modules/@turf/bbox": { + "version": "7.3.3", + "resolved": "https://registry.npmjs.org/@turf/bbox/-/bbox-7.3.3.tgz", + "integrity": "sha512-1zNO/JUgDp0N+3EG5fG7+8EolE95OW1LD8ur0hRP0JK+lRyN0gAvJT7n1I9pu/NIqTa8x/zXxGRc1dcOdohYkg==", + "license": "MIT", + "dependencies": { + "@turf/helpers": "7.3.3", + "@turf/meta": "7.3.3", + "@types/geojson": "^7946.0.10", + "tslib": "^2.8.1" + }, + "funding": { + "url": "https://opencollective.com/turf" + } + }, "node_modules/@turf/circle": { "version": "7.3.3", "dev": true, @@ -9357,6 +9402,21 @@ "url": "https://opencollective.com/turf" } }, + "node_modules/@turf/geojson-rbush/node_modules/@turf/bbox": { + "version": "7.3.3", + "resolved": "https://registry.npmjs.org/@turf/bbox/-/bbox-7.3.3.tgz", + "integrity": "sha512-1zNO/JUgDp0N+3EG5fG7+8EolE95OW1LD8ur0hRP0JK+lRyN0gAvJT7n1I9pu/NIqTa8x/zXxGRc1dcOdohYkg==", + "license": "MIT", + "dependencies": { + "@turf/helpers": "7.3.3", + "@turf/meta": "7.3.3", + "@types/geojson": "^7946.0.10", + "tslib": "^2.8.1" + }, + "funding": { + "url": "https://opencollective.com/turf" + } + }, "node_modules/@turf/helpers": { "version": "7.3.3", "license": "MIT", diff --git a/package.json b/package.json index e0f567a5..e99a92d9 100755 --- a/package.json +++ b/package.json @@ -197,6 +197,7 @@ "@docusaurus/theme-common": "^3.9.2", "@easyops-cn/docusaurus-search-local": "^0.55.0", "@turf/area": "^7.2.0", + "@turf/bbox": "^7.3.4", "@turf/bearing": "^7.3.3", "@turf/boolean-disjoint": "^7.3.3", "@turf/boolean-valid": "^7.2.0", diff --git a/providers/beta/esri/src/esriProvider.js b/providers/beta/esri/src/esriProvider.js index efddac7f..2a54289b 100644 --- a/providers/beta/esri/src/esriProvider.js +++ b/providers/beta/esri/src/esriProvider.js @@ -9,7 +9,7 @@ import { attachAppEvents } from './appEvents.js' import { attachMapEvents } from './mapEvents.js' import { getAreaDimensions, getCardinalMove, getPaddedExtent } from './utils/spatial.js' import { queryVectorTileFeatures } from './utils/query.js' -import { getExtentFromFlatCoords, getPointFromFlatCoords } from './utils/coords.js' +import { getExtentFromFlatCoords, getPointFromFlatCoords, getBboxFromGeoJSON } from './utils/coords.js' import { cleanDOM } from './utils/esriFixes.js' export default class EsriProvider { @@ -141,7 +141,8 @@ export default class EsriProvider { } fitToBounds (bounds) { - this.view.goTo(getExtentFromFlatCoords(bounds), { duration: defaults.DELAY }) + const extent = Array.isArray(bounds) ? getExtentFromFlatCoords(bounds) : getBboxFromGeoJSON(bounds) + this.view.goTo(extent, { duration: defaults.DELAY }) } setPadding (padding) { diff --git a/providers/beta/esri/src/utils/coords.js b/providers/beta/esri/src/utils/coords.js index 5961688a..b77c613e 100644 --- a/providers/beta/esri/src/utils/coords.js +++ b/providers/beta/esri/src/utils/coords.js @@ -23,7 +23,39 @@ const getPointFromFlatCoords = (coords) => { : undefined } +const collectCoords = (obj, acc) => { + if (!obj) return + if (obj.type === 'FeatureCollection') { + obj.features.forEach(f => collectCoords(f, acc)) + } else if (obj.type === 'Feature') { + collectCoords(obj.geometry, acc) + } else if (obj.type === 'GeometryCollection') { + obj.geometries.forEach(g => collectCoords(g, acc)) + } else { + const flatten = (coords) => { + if (typeof coords[0] === 'number') acc.push(coords) + else coords.forEach(flatten) + } + flatten(obj.coordinates) + } +} + +/** + * Get an ESRI Extent from any GeoJSON object (Feature, FeatureCollection, or geometry). + * + * @param {object} geojson - GeoJSON Feature, FeatureCollection, or geometry + * @returns {import('@arcgis/core/geometry/Extent.js').default} + */ +const getBboxFromGeoJSON = (geojson) => { + const points = [] + collectCoords(geojson, points) + const xs = points.map(p => p[0]) + const ys = points.map(p => p[1]) + return getExtentFromFlatCoords([Math.min(...xs), Math.min(...ys), Math.max(...xs), Math.max(...ys)]) +} + export { getExtentFromFlatCoords, - getPointFromFlatCoords + getPointFromFlatCoords, + getBboxFromGeoJSON } \ No newline at end of file diff --git a/providers/beta/esri/src/utils/coords.test.js b/providers/beta/esri/src/utils/coords.test.js new file mode 100644 index 00000000..70e9757f --- /dev/null +++ b/providers/beta/esri/src/utils/coords.test.js @@ -0,0 +1,126 @@ +import { getExtentFromFlatCoords, getPointFromFlatCoords, getBboxFromGeoJSON } from './coords.js' + +jest.mock('@arcgis/core/geometry/Extent.js', () => + jest.fn().mockImplementation((opts) => ({ ...opts, type: 'extent' })) +) +jest.mock('@arcgis/core/geometry/Point.js', () => + jest.fn().mockImplementation((opts) => ({ ...opts, type: 'point' })) +) + +describe('coords utils', () => { + describe('getExtentFromFlatCoords', () => { + test('returns an Extent with correct xmin/ymin/xmax/ymax', () => { + const result = getExtentFromFlatCoords([1, 2, 3, 4]) + expect(result.xmin).toBe(1) + expect(result.ymin).toBe(2) + expect(result.xmax).toBe(3) + expect(result.ymax).toBe(4) + expect(result.spatialReference).toEqual({ wkid: 27700 }) + }) + + test('returns undefined for falsy input', () => { + expect(getExtentFromFlatCoords(null)).toBeUndefined() + expect(getExtentFromFlatCoords(undefined)).toBeUndefined() + }) + }) + + describe('getPointFromFlatCoords', () => { + test('returns a Point with correct x/y', () => { + const result = getPointFromFlatCoords([100, 200]) + expect(result.x).toBe(100) + expect(result.y).toBe(200) + expect(result.spatialReference).toEqual({ wkid: 27700 }) + }) + + test('returns undefined for falsy input', () => { + expect(getPointFromFlatCoords(null)).toBeUndefined() + }) + }) + + describe('getBboxFromGeoJSON', () => { + test('Point feature → extent wrapping that single point', () => { + const feature = { + type: 'Feature', + geometry: { type: 'Point', coordinates: [300000, 100000] }, + properties: {} + } + const result = getBboxFromGeoJSON(feature) + expect(result.xmin).toBe(300000) + expect(result.ymin).toBe(100000) + expect(result.xmax).toBe(300000) + expect(result.ymax).toBe(100000) + }) + + test('LineString feature → tight bbox around all coords', () => { + const feature = { + type: 'Feature', + geometry: { + type: 'LineString', + coordinates: [[100, 200], [300, 50], [500, 400]] + }, + properties: {} + } + const result = getBboxFromGeoJSON(feature) + expect(result.xmin).toBe(100) + expect(result.ymin).toBe(50) + expect(result.xmax).toBe(500) + expect(result.ymax).toBe(400) + }) + + test('Polygon feature → bbox around all rings', () => { + const feature = { + type: 'Feature', + geometry: { + type: 'Polygon', + coordinates: [[[0, 0], [10, 0], [10, 5], [0, 5], [0, 0]]] + }, + properties: {} + } + const result = getBboxFromGeoJSON(feature) + expect(result.xmin).toBe(0) + expect(result.ymin).toBe(0) + expect(result.xmax).toBe(10) + expect(result.ymax).toBe(5) + }) + + test('FeatureCollection → bbox spanning all features', () => { + const fc = { + type: 'FeatureCollection', + features: [ + { type: 'Feature', geometry: { type: 'Point', coordinates: [100, 200] }, properties: {} }, + { type: 'Feature', geometry: { type: 'Point', coordinates: [500, 800] }, properties: {} } + ] + } + const result = getBboxFromGeoJSON(fc) + expect(result.xmin).toBe(100) + expect(result.ymin).toBe(200) + expect(result.xmax).toBe(500) + expect(result.ymax).toBe(800) + }) + + test('GeometryCollection → bbox spanning all geometries', () => { + const gc = { + type: 'GeometryCollection', + geometries: [ + { type: 'Point', coordinates: [10, 20] }, + { type: 'LineString', coordinates: [[50, 5], [100, 90]] } + ] + } + const result = getBboxFromGeoJSON(gc) + expect(result.xmin).toBe(10) + expect(result.ymin).toBe(5) + expect(result.xmax).toBe(100) + expect(result.ymax).toBe(90) + }) + + test('result has correct spatialReference from getExtentFromFlatCoords', () => { + const feature = { + type: 'Feature', + geometry: { type: 'Point', coordinates: [1, 2] }, + properties: {} + } + const result = getBboxFromGeoJSON(feature) + expect(result.spatialReference).toEqual({ wkid: 27700 }) + }) + }) +}) diff --git a/providers/maplibre/src/maplibreProvider.js b/providers/maplibre/src/maplibreProvider.js index d3a317e0..8eda891a 100755 --- a/providers/maplibre/src/maplibreProvider.js +++ b/providers/maplibre/src/maplibreProvider.js @@ -7,7 +7,7 @@ import { DEFAULTS, supportedShortcuts } from './defaults.js' import { cleanCanvas, applyPreventDefaultFix } from './utils/maplibreFixes.js' import { attachMapEvents } from './mapEvents.js' import { attachAppEvents } from './appEvents.js' -import { getAreaDimensions, getCardinalMove, getResolution, getPaddedBounds } from './utils/spatial.js' +import { getAreaDimensions, getCardinalMove, getBboxFromGeoJSON, getResolution, getPaddedBounds } from './utils/spatial.js' import { createMapLabelNavigator } from './utils/labels.js' import { updateHighlightedFeatures } from './utils/highlightFeatures.js' import { queryFeatures } from './utils/queryFeatures.js' @@ -175,12 +175,13 @@ export default class MapLibreProvider { } /** - * Fit map view to the specified bounds [west, south, east, north]. + * Fit map view to the specified bounds or GeoJSON geometry. * - * @param {[number, number, number, number]} bounds - Bounds as [west, south, east, north]. + * @param {[number, number, number, number] | object} bounds - Bounds as [west, south, east, north], or a GeoJSON Feature, FeatureCollection, or geometry. Bbox is computed from GeoJSON using @turf/bbox. */ fitToBounds (bounds) { - this.map.fitBounds(bounds, { duration: DEFAULTS.animationDuration }) + const bbox = Array.isArray(bounds) ? bounds : getBboxFromGeoJSON(bounds) + this.map.fitBounds(bbox, { duration: DEFAULTS.animationDuration }) } /** diff --git a/providers/maplibre/src/maplibreProvider.test.js b/providers/maplibre/src/maplibreProvider.test.js index c82dd3e5..7b6bbf2b 100644 --- a/providers/maplibre/src/maplibreProvider.test.js +++ b/providers/maplibre/src/maplibreProvider.test.js @@ -19,6 +19,7 @@ jest.mock('./appEvents.js', () => ({ attachAppEvents: jest.fn() })) jest.mock('./utils/spatial.js', () => ({ getAreaDimensions: jest.fn(() => '400m by 750m'), getCardinalMove: jest.fn(() => 'north'), + getBboxFromGeoJSON: jest.fn(() => [-1, 50, 1, 52]), getResolution: jest.fn(() => 10), getPaddedBounds: jest.fn(() => [[0, 0], [1, 1]]) })) @@ -157,6 +158,18 @@ describe('MapLibreProvider', () => { expect(map.setPadding).toHaveBeenCalledWith({ top: 5 }) }) + test('fitToBounds accepts GeoJSON: computes bbox via getBboxFromGeoJSON', async () => { + const { getBboxFromGeoJSON } = require('./utils/spatial.js') + const p = makeProvider() + await doInitMap(p) + const feature = { type: 'Feature', geometry: { type: 'Point', coordinates: [1, 52] }, properties: {} } + + p.fitToBounds(feature) + + expect(getBboxFromGeoJSON).toHaveBeenCalledWith(feature) + expect(map.fitBounds).toHaveBeenCalledWith([-1, 50, 1, 52], { duration: 400 }) + }) + test('getCenter, getZoom, getBounds return formatted values', async () => { const p = makeProvider() await doInitMap(p) diff --git a/providers/maplibre/src/utils/spatial.js b/providers/maplibre/src/utils/spatial.js index 8b641afe..01efd737 100755 --- a/providers/maplibre/src/utils/spatial.js +++ b/providers/maplibre/src/utils/spatial.js @@ -1,5 +1,6 @@ // src/utils/spatial.js import LatLon from 'geodesy/latlon-spherical.js' +import turfBbox from '@turf/bbox' // ----------------------------------------------------------------------------- // Internal (not exported) @@ -186,9 +187,19 @@ const getPaddedBounds = (LngLatBounds, map) => { } +/** + * Get a flat bbox [west, south, east, north] from any GeoJSON object + * (Feature, FeatureCollection, or geometry). + * + * @param {object} geojson - GeoJSON Feature, FeatureCollection, or geometry + * @returns {[number, number, number, number]} + */ +const getBboxFromGeoJSON = (geojson) => turfBbox(geojson) + export { getAreaDimensions, getCardinalMove, + getBboxFromGeoJSON, spatialNavigate, getResolution, getPaddedBounds, diff --git a/providers/maplibre/src/utils/spatial.test.js b/providers/maplibre/src/utils/spatial.test.js index cfc242bc..c7d14be9 100644 --- a/providers/maplibre/src/utils/spatial.test.js +++ b/providers/maplibre/src/utils/spatial.test.js @@ -7,6 +7,8 @@ jest.mock('geodesy/latlon-spherical.js', () => })) ) +jest.mock('@turf/bbox', () => jest.fn(() => [-1, 50, 1, 52])) + describe('spatial utils', () => { test('formatDimension hits all branches', () => { @@ -93,4 +95,14 @@ describe('spatial utils', () => { expect(bounds.sw).toBeDefined() expect(bounds.ne).toBeDefined() }) + + test('getBboxFromGeoJSON delegates to @turf/bbox and returns flat bbox array', () => { + const turfBbox = require('@turf/bbox') + const feature = { type: 'Feature', geometry: { type: 'Point', coordinates: [1, 52] }, properties: {} } + + const result = spatial.getBboxFromGeoJSON(feature) + + expect(turfBbox).toHaveBeenCalledWith(feature) + expect(result).toEqual([-1, 50, 1, 52]) + }) }) \ No newline at end of file diff --git a/src/InteractiveMap/InteractiveMap.js b/src/InteractiveMap/InteractiveMap.js index e1ab0348..a97f55d7 100755 --- a/src/InteractiveMap/InteractiveMap.js +++ b/src/InteractiveMap/InteractiveMap.js @@ -406,9 +406,9 @@ export default class InteractiveMap { } /** - * Fit the map view to a bounding box, respecting the safe zone padding. + * Fit the map view to a bounding box or GeoJSON geometry, respecting the safe zone padding. * - * @param {[number, number, number, number]} bbox - Bounds as [west, south, east, north] or [minX, minY, maxX, maxY] depending on the crs. + * @param {[number, number, number, number] | object} bbox - Bounds as [west, south, east, north] or [minX, minY, maxX, maxY] depending on the crs, or a GeoJSON Feature, FeatureCollection, or geometry. */ fitToBounds (bbox) { this.eventBus.emit(events.MAP_FIT_TO_BOUNDS, bbox) diff --git a/src/types.js b/src/types.js index 9afef7bc..0d7527ca 100644 --- a/src/types.js +++ b/src/types.js @@ -203,8 +203,8 @@ * @property {(offset: [number, number]) => void} panBy * Pan map by pixel offset [x, y]. Positive x pans right, positive y pans down. * - * @property {(bounds: [number, number, number, number]) => void} fitToBounds - * Fit map view to the specified bounds [west, south, east, north] or [minX, minY, maxX, maxY] depending on the crs of the map provider. + * @property {(bounds: [number, number, number, number] | object) => void} fitToBounds + * Fit map view to the specified bounds [west, south, east, north] or [minX, minY, maxX, maxY] depending on the crs, or a GeoJSON Feature, FeatureCollection, or geometry. * * @property {(padding: { top?: number, bottom?: number, left?: number, right?: number }) => void} setPadding * Set map padding as pixel insets from the top, bottom, left and right edges of the map. From 015d600d3f317549345859be3b73809eda9f96d3 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 3 Mar 2026 10:44:26 +0000 Subject: [PATCH 6/8] Sonar fixes --- plugins/search/src/components/Form/Form.jsx | 18 ++++++++++-------- src/App/hooks/useMapProviderOverrides.js | 4 +++- src/App/renderer/mapButtons.js | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/plugins/search/src/components/Form/Form.jsx b/plugins/search/src/components/Form/Form.jsx index 8f00c79c..563e9080 100755 --- a/plugins/search/src/components/Form/Form.jsx +++ b/plugins/search/src/components/Form/Form.jsx @@ -2,6 +2,14 @@ import { useEffect } from 'react' import { Suggestions } from '../Suggestions/Suggestions' +const getResultMessage = (count) => + count === 0 ? 'No results available' : `${count} result${count === 1 ? '' : 's'} available` + +const getFormStyle = (pluginConfig, pluginState, appState) => ({ + display: pluginConfig.expanded || pluginState.isExpanded ? 'flex' : undefined, + ...(appState.breakpoint !== 'mobile' && pluginConfig?.width && { width: pluginConfig.width }), +}) + export const Form = ({ id, pluginState, @@ -20,10 +28,7 @@ export const Form = ({ if (!areSuggestionsVisible || !hasFetchedSuggestions) { return } - const count = suggestions.length - const plural = count === 1 ? '' : 's' - const message = count === 0 ? 'No results available' : `${count} result${plural} available` - services.announce(message) + services.announce(getResultMessage(suggestions.length)) }, [suggestions, hasFetchedSuggestions]) const classNames = [ @@ -39,10 +44,7 @@ export const Form = ({ id={`${id}-search-form`} role="search" className={classNames} - style={{ - display: pluginConfig.expanded || pluginState.isExpanded ? 'flex' : undefined, - ...(appState.breakpoint !== 'mobile' && pluginConfig?.width && { width: pluginConfig.width }), - }} + style={getFormStyle(pluginConfig, pluginState, appState)} aria-controls={`${id}-viewport`} onSubmit={(e) => events.handleSubmit(e, appState, pluginState)} > diff --git a/src/App/hooks/useMapProviderOverrides.js b/src/App/hooks/useMapProviderOverrides.js index e8e898d3..dfef9fbd 100755 --- a/src/App/hooks/useMapProviderOverrides.js +++ b/src/App/hooks/useMapProviderOverrides.js @@ -72,7 +72,9 @@ export const useMapProviderOverrides = () => { // Forward public API events to the (overridden) mapProvider methods so that // interactiveMap.fitToBounds() and interactiveMap.setView() respect safe zone padding. useEffect(() => { - if (!mapProvider || !eventBus) return undefined + if (!mapProvider || !eventBus) { + return undefined + } const handleFitToBounds = (bbox) => mapProvider.fitToBounds(bbox) const handleSetView = (opts) => mapProvider.setView(opts) diff --git a/src/App/renderer/mapButtons.js b/src/App/renderer/mapButtons.js index 92f5d1b9..eb9b19f3 100755 --- a/src/App/renderer/mapButtons.js +++ b/src/App/renderer/mapButtons.js @@ -225,7 +225,7 @@ function mapButtons ({ slot, appState, appConfig, evaluateProp }) { type: 'group', order: groupOrder, element: ( -
+
{/* NOSONAR - div with role="group" is correct for a button group */} {sorted.map(btn => renderButton({ btn, appState, appConfig, evaluateProp }))}
) From 515ecc445d251bffa4dc44a88d221b5b71bec804 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 3 Mar 2026 10:57:27 +0000 Subject: [PATCH 7/8] Eval test removed --- plugins/search/src/components/Form/Form.jsx | 9 +++- providers/maplibre/src/index.js | 18 +++----- providers/maplibre/src/index.test.js | 49 ++++++++++++++------- 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/plugins/search/src/components/Form/Form.jsx b/plugins/search/src/components/Form/Form.jsx index 563e9080..6ce9210f 100755 --- a/plugins/search/src/components/Form/Form.jsx +++ b/plugins/search/src/components/Form/Form.jsx @@ -2,8 +2,13 @@ import { useEffect } from 'react' import { Suggestions } from '../Suggestions/Suggestions' -const getResultMessage = (count) => - count === 0 ? 'No results available' : `${count} result${count === 1 ? '' : 's'} available` +const getResultMessage = (count) => { + if (count === 0) { + return 'No results available' + } + const plural = count === 1 ? 'result' : 'results' + return `${count} ${plural} available` +} const getFormStyle = (pluginConfig, pluginState, appState) => ({ display: pluginConfig.expanded || pluginState.isExpanded ? 'flex' : undefined, diff --git a/providers/maplibre/src/index.js b/providers/maplibre/src/index.js index ff628497..adad9c32 100755 --- a/providers/maplibre/src/index.js +++ b/providers/maplibre/src/index.js @@ -7,23 +7,15 @@ import { getWebGL } from './utils/detectWebgl.js' /** - * Checks whether the browser supports modern ES2020 syntax - * (optional chaining `?.` and nullish coalescing `??`), which - * Chrome 80+ supports. Safe to use in ES5 bootstrap code. + * Checks whether the browser supports the ES2020+ feature set required by + * MapLibre, using `String.prototype.replaceAll` as a proxy. This method + * landed in Chrome 85 / Firefox 77 / Safari 13.1 — the same cohort that + * supports optional chaining `?.` and nullish coalescing `??`. * * @returns {boolean} true if modern syntax is supported, false otherwise */ function supportsModernMaplibre() { - try { - // Try compiling ES2020 syntax dynamically - new Function('var x = null ?? 5; var y = ({a:1})?.a;') - return true - } - catch (e) { - // Exception intentionally ignored; returns false for unsupported syntax - void e // NOSONAR - return false - } + return typeof ''.replaceAll === 'function' } /** diff --git a/providers/maplibre/src/index.test.js b/providers/maplibre/src/index.test.js index 4c83c95a..60ef8fd4 100644 --- a/providers/maplibre/src/index.test.js +++ b/providers/maplibre/src/index.test.js @@ -9,10 +9,18 @@ describe('createMapLibreProvider', () => { beforeEach(() => { getWebGL.mockReturnValue({ isEnabled: true, error: null }) + + // Ensure modern support by default + String.prototype.replaceAll = jest.fn() + }) + + afterEach(() => { + jest.restoreAllMocks() }) - test('checkDeviceCapabilities: WebGL enabled, no IE → isSupported true, no error', () => { + test('checkDeviceCapabilities: WebGL enabled, modern browser, no IE → isSupported true', () => { const result = createMapLibreProvider().checkDeviceCapabilities() + expect(result.isSupported).toBe(true) expect(result.error).toBeFalsy() expect(getWebGL).toHaveBeenCalledWith(['webgl2', 'webgl1']) @@ -20,41 +28,52 @@ describe('createMapLibreProvider', () => { test('checkDeviceCapabilities: WebGL disabled → isSupported false, returns webGL error', () => { getWebGL.mockReturnValue({ isEnabled: false, error: 'WebGL not supported' }) + const result = createMapLibreProvider().checkDeviceCapabilities() + expect(result.isSupported).toBe(false) expect(result.error).toBe('WebGL not supported') }) test('checkDeviceCapabilities: IE detected → error is IE message', () => { - Object.defineProperty(document, 'documentMode', { get: () => 11, configurable: true }) + Object.defineProperty(document, 'documentMode', { + get: () => 11, + configurable: true + }) + try { const result = createMapLibreProvider().checkDeviceCapabilities() expect(result.error).toBe('Internet Explorer is not supported') } finally { - Object.defineProperty(document, 'documentMode', { get: () => undefined, configurable: true }) + Object.defineProperty(document, 'documentMode', { + get: () => undefined, + configurable: true + }) } }) - test('supportsModernMaplibre returns false when Function constructor throws → isSupported false', () => { - const RealFunction = global.Function - global.Function = function () { throw new SyntaxError('unsupported') } - try { - const result = createMapLibreProvider().checkDeviceCapabilities() - expect(result.isSupported).toBe(false) - } finally { - global.Function = RealFunction - } + test('checkDeviceCapabilities: no replaceAll support → isSupported false', () => { + delete String.prototype.replaceAll + + const result = createMapLibreProvider().checkDeviceCapabilities() + + expect(result.isSupported).toBe(false) }) - test('load returns MapProvider, mapFramework, and mapProviderConfig with config spread', async () => { + test('load returns MapProvider, mapFramework, and merged mapProviderConfig', async () => { const result = await createMapLibreProvider({ tileSize: 512 }).load() + expect(result.MapProvider).toBeDefined() expect(result.mapFramework).toBeDefined() - expect(result.mapProviderConfig).toEqual({ tileSize: 512, crs: 'EPSG:4326' }) + expect(result.mapProviderConfig).toEqual({ + tileSize: 512, + crs: 'EPSG:4326' + }) }) test('load uses empty default config', async () => { const { mapProviderConfig } = await createMapLibreProvider().load() + expect(mapProviderConfig).toEqual({ crs: 'EPSG:4326' }) }) -}) +}) \ No newline at end of file From c7f6fbaef415b21e539cc31a14e9fbb60647171b Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 3 Mar 2026 11:06:08 +0000 Subject: [PATCH 8/8] Typo fix --- docs/api/panel-definition.md | 4 ++-- docs/plugins/plugin-manifest.md | 2 +- plugins/beta/datasets/src/manifest.js | 6 ++--- plugins/beta/map-styles/src/manifest.js | 6 ++--- plugins/beta/use-location/src/manifest.js | 6 ++--- src/App/components/Panel/Panel.jsx | 4 ++-- src/App/components/Panel/Panel.test.jsx | 28 +++++++++++------------ src/App/renderer/mapButtons.js | 4 ++-- src/App/renderer/mapButtons.test.js | 8 +++---- src/App/store/appActionsMap.js | 4 ++-- src/App/store/appActionsMap.test.js | 8 +++---- src/config/appConfig.js | 8 +++---- src/types.js | 4 ++-- 13 files changed, 46 insertions(+), 46 deletions(-) diff --git a/docs/api/panel-definition.md b/docs/api/panel-definition.md index e1c67e78..07e1e9b0 100644 --- a/docs/api/panel-definition.md +++ b/docs/api/panel-definition.md @@ -83,7 +83,7 @@ Each breakpoint (`mobile`, `tablet`, `desktop`) accepts the following properties The [slot](./slots.md) where the panel should appear at this breakpoint. Slots are named regions in the UI layout. -### `dismissable` +### `dismissible` **Type:** `boolean` Whether the panel can be dismissed (closed) by the user. When `false` and `open` is `true`, the panel is always visible at this breakpoint and any associated panel-toggle button is automatically suppressed. @@ -96,7 +96,7 @@ Whether the panel is exclusive. An exclusive panel will hide other panels when i ### `open` **Type:** `boolean` -Whether the panel is open. When `true` and combined with `dismissable: false`, the panel is always visible at this breakpoint and will be restored automatically when the breakpoint is entered. +Whether the panel is open. When `true` and combined with `dismissible: false`, the panel is always visible at this breakpoint and will be restored automatically when the breakpoint is entered. ### `showLabel` **Type:** `boolean` diff --git a/docs/plugins/plugin-manifest.md b/docs/plugins/plugin-manifest.md index c110e3ad..d9840e1f 100644 --- a/docs/plugins/plugin-manifest.md +++ b/docs/plugins/plugin-manifest.md @@ -92,7 +92,7 @@ const InitComponent = ({ context }) => { Panel definitions to register in the UI. -Panels come with pre-built behaviour and styling, including headings, dismissable states, and modal overlays. Supports responsive breakpoint configuration. +Panels come with pre-built behaviour and styling, including headings, dismissible states, and modal overlays. Supports responsive breakpoint configuration. See [PanelDefinition](../api/panel-definition.md) for full details. diff --git a/plugins/beta/datasets/src/manifest.js b/plugins/beta/datasets/src/manifest.js index d845a5b3..9a6d6658 100755 --- a/plugins/beta/datasets/src/manifest.js +++ b/plugins/beta/datasets/src/manifest.js @@ -24,18 +24,18 @@ export const manifest = { mobile: { slot: 'bottom', modal: true, - dismissable: true + dismissible: true }, tablet: { slot: 'inset', - dismissable: true, + dismissible: true, exclusive: true, width: '300px' }, desktop: { slot: 'inset', modal: false, - dismissable: true, + dismissible: true, exclusive: true, width: '320px' }, diff --git a/plugins/beta/map-styles/src/manifest.js b/plugins/beta/map-styles/src/manifest.js index 32995517..34954279 100755 --- a/plugins/beta/map-styles/src/manifest.js +++ b/plugins/beta/map-styles/src/manifest.js @@ -11,19 +11,19 @@ export const manifest = { mobile: { slot: 'bottom', modal: true, - dismissable: true + dismissible: true }, tablet: { slot: 'inset', modal: true, width: '400px', - dismissable: true + dismissible: true }, desktop: { slot: 'inset', modal: true, width: '400px', - dismissable: true + dismissible: true }, render: MapStyles // will be wrapped automatically }], diff --git a/plugins/beta/use-location/src/manifest.js b/plugins/beta/use-location/src/manifest.js index b229ba3f..1905f799 100755 --- a/plugins/beta/use-location/src/manifest.js +++ b/plugins/beta/use-location/src/manifest.js @@ -33,19 +33,19 @@ export const manifest = { mobile: { slot: 'banner', open: false, - dismissable: true, + dismissible: true, modal: true }, tablet: { slot: 'banner', open: false, - dismissable: true, + dismissible: true, modal: true }, desktop: { slot: 'banner', open: false, - dismissable: true, + dismissible: true, modal: true }, render: UseLocation diff --git a/src/App/components/Panel/Panel.jsx b/src/App/components/Panel/Panel.jsx index 996ead96..1548fa7b 100755 --- a/src/App/components/Panel/Panel.jsx +++ b/src/App/components/Panel/Panel.jsx @@ -8,9 +8,9 @@ import { Icon } from '../Icon/Icon' const computePanelState = (bpConfig, triggeringElement) => { const isAside = bpConfig.slot === 'side' && bpConfig.open && !bpConfig.modal - const isDialog = !isAside && bpConfig.dismissable + const isDialog = !isAside && bpConfig.dismissible const isModal = bpConfig.modal === true - const isDismissable = bpConfig.dismissable !== false + const isDismissable = bpConfig.dismissible !== false const shouldFocus = Boolean(isModal || triggeringElement) const buttonContainerEl = bpConfig.slot.endsWith('button') ? triggeringElement?.parentNode : undefined return { isAside, isDialog, isModal, isDismissable, shouldFocus, buttonContainerEl } diff --git a/src/App/components/Panel/Panel.test.jsx b/src/App/components/Panel/Panel.test.jsx index 8efca61c..c2cfc008 100755 --- a/src/App/components/Panel/Panel.test.jsx +++ b/src/App/components/Panel/Panel.test.jsx @@ -32,7 +32,7 @@ describe('Panel', () => { const renderPanel = (config = {}, props = {}) => { const panelConfig = { - desktop: { slot: 'side', open: true, dismissable: false, modal: false, showLabel: true }, + desktop: { slot: 'side', open: true, dismissible: false, modal: false, showLabel: true }, ...config } return render() @@ -48,17 +48,17 @@ describe('Panel', () => { }) it('renders visually hidden label when showLabel=false', () => { - renderPanel({ desktop: { slot: 'side', open: true, dismissable: false, modal: false, showLabel: false } }) + renderPanel({ desktop: { slot: 'side', open: true, dismissible: false, modal: false, showLabel: false } }) expect(screen.getByText('Settings')).toHaveClass('im-u-visually-hidden') }) - it('applies offset class to body when showLabel=false and dismissable', () => { - renderPanel({ desktop: { slot: 'side', dismissable: true, open: false, showLabel: false } }) + it('applies offset class to body when showLabel=false and dismissible', () => { + renderPanel({ desktop: { slot: 'side', dismissible: true, open: false, showLabel: false } }) expect(screen.getByRole('dialog').querySelector('.im-c-panel__body')).toHaveClass('im-c-panel__body--offset') }) it('applies width style if provided', () => { - renderPanel({ desktop: { slot: 'side', dismissable: true, open: true, width: '300px' } }) + renderPanel({ desktop: { slot: 'side', dismissible: true, open: true, width: '300px' } }) expect(screen.getByRole('complementary')).toHaveStyle({ width: '300px' }) }) @@ -81,23 +81,23 @@ describe('Panel', () => { }) describe('role and aria attributes', () => { - it('renders region role for non-dismissable panels', () => { + it('renders region role for non-dismissible panels', () => { renderPanel() expect(screen.getByRole('region')).toBeInTheDocument() }) - it('renders dialog role for dismissable non-aside panels', () => { - renderPanel({ desktop: { slot: 'side', dismissable: true, open: false } }) + it('renders dialog role for dismissible non-aside panels', () => { + renderPanel({ desktop: { slot: 'side', dismissible: true, open: false } }) expect(screen.getByRole('dialog')).toBeInTheDocument() }) - it('renders complementary role for dismissable aside panels', () => { - renderPanel({ desktop: { slot: 'side', open: true, dismissable: true } }) + it('renders complementary role for dismissible aside panels', () => { + renderPanel({ desktop: { slot: 'side', open: true, dismissible: true } }) expect(screen.getByRole('complementary')).toBeInTheDocument() }) it('sets aria-modal and tabIndex for modal dialogs', () => { - renderPanel({ desktop: { slot: 'overlay', dismissable: true, modal: true } }) + renderPanel({ desktop: { slot: 'overlay', dismissible: true, modal: true } }) const dialog = screen.getByRole('dialog') expect(dialog).toHaveAttribute('aria-modal', 'true') expect(dialog).toHaveAttribute('tabIndex', '-1') @@ -110,7 +110,7 @@ describe('Panel', () => { const triggeringElement = { focus: focusMock, parentNode: document.createElement('div') } renderPanel( - { desktop: { slot: 'top-button', dismissable: true, open: false } }, + { desktop: { slot: 'top-button', dismissible: true, open: false } }, { props: { triggeringElement } } ) @@ -124,7 +124,7 @@ describe('Panel', () => { const triggeringElement = { focus: focusMock, parentNode: document.createElement('div') } renderPanel( - { desktop: { slot: 'overlay', dismissable: true, modal: true } }, + { desktop: { slot: 'overlay', dismissible: true, modal: true } }, { props: { triggeringElement } } ) @@ -133,7 +133,7 @@ describe('Panel', () => { }) it('falls back to viewportRef focus when no triggeringElement', () => { - renderPanel({ desktop: { slot: 'side', dismissable: true, open: false } }) + renderPanel({ desktop: { slot: 'side', dismissible: true, open: false } }) fireEvent.click(screen.getByRole('button', { name: 'Close Settings' })) expect(layoutRefs.viewportRef.current.focus).toHaveBeenCalled() diff --git a/src/App/renderer/mapButtons.js b/src/App/renderer/mapButtons.js index eb9b19f3..80e95e74 100755 --- a/src/App/renderer/mapButtons.js +++ b/src/App/renderer/mapButtons.js @@ -32,10 +32,10 @@ function getMatchingButtons ({ appState, buttonConfig, slot, evaluateProp }) { return false } - // Skip panel-toggle buttons when the panel is non-dismissable (always visible) at this breakpoint + // Skip panel-toggle buttons when the panel is non-dismissible (always visible) at this breakpoint if (config.panelId) { const panelBpConfig = appState.panelConfig?.[config.panelId]?.[breakpoint] - if (panelBpConfig?.open === true && panelBpConfig?.dismissable === false) { + if (panelBpConfig?.open === true && panelBpConfig?.dismissible === false) { return false } } diff --git a/src/App/renderer/mapButtons.test.js b/src/App/renderer/mapButtons.test.js index d022b919..ffacd856 100755 --- a/src/App/renderer/mapButtons.test.js +++ b/src/App/renderer/mapButtons.test.js @@ -139,14 +139,14 @@ describe('mapButtons module', () => { expect(getMatchingButtons({ buttonConfig: config, slot: 'header', appState, evaluateProp }).length).toBe(2) }) - it('filters out panel-toggle button when panel is open and non-dismissable at current breakpoint', () => { - const state = { ...appState, panelConfig: { myPanel: { desktop: { open: true, dismissable: false } } } } + it('filters out panel-toggle button when panel is open and non-dismissible at current breakpoint', () => { + const state = { ...appState, panelConfig: { myPanel: { desktop: { open: true, dismissible: false } } } } const config = { b1: { ...baseBtn, panelId: 'myPanel' } } expect(getMatchingButtons({ buttonConfig: config, slot: 'header', appState: state, evaluateProp }).length).toBe(0) }) - it('includes panel-toggle button when panel is dismissable at current breakpoint', () => { - const state = { ...appState, panelConfig: { myPanel: { desktop: { open: true, dismissable: true } } } } + it('includes panel-toggle button when panel is dismissible at current breakpoint', () => { + const state = { ...appState, panelConfig: { myPanel: { desktop: { open: true, dismissible: true } } } } const config = { b1: { ...baseBtn, panelId: 'myPanel' } } expect(getMatchingButtons({ buttonConfig: config, slot: 'header', appState: state, evaluateProp }).length).toBe(1) }) diff --git a/src/App/store/appActionsMap.js b/src/App/store/appActionsMap.js index 3487dd83..57afc570 100755 --- a/src/App/store/appActionsMap.js +++ b/src/App/store/appActionsMap.js @@ -78,13 +78,13 @@ const setBreakpoint = (state, payload) => { ? buildOpenPanels(state, lastPanelId, payload.breakpoint, state.openPanels[lastPanelId]?.props || {}) : {} - // Restore panels that are non-dismissable and always open at the new breakpoint + // Restore panels that are non-dismissible and always open at the new breakpoint const panelConfig = state.panelConfig || state.panelRegistry.getPanelConfig() const persistentPanels = Object.fromEntries( Object.entries(panelConfig) .filter(([panelId, config]) => { const bpConfig = config[payload.breakpoint] - return bpConfig?.open === true && bpConfig?.dismissable === false && !transitionedOpenPanels[panelId] + return bpConfig?.open === true && bpConfig?.dismissible === false && !transitionedOpenPanels[panelId] }) .map(([panelId]) => [panelId, state.openPanels[panelId] || { props: {} }]) ) diff --git a/src/App/store/appActionsMap.test.js b/src/App/store/appActionsMap.test.js index 2a64e507..1a4e9a11 100755 --- a/src/App/store/appActionsMap.test.js +++ b/src/App/store/appActionsMap.test.js @@ -14,7 +14,7 @@ describe('actionsMap full coverage', () => { panel1: { desktop: { exclusive: true, modal: false, open: true }, mobile: { exclusive: true, modal: false } }, panel2: { desktop: { exclusive: false, modal: true }, mobile: { exclusive: false, modal: true } }, panel3: { desktop: { exclusive: false, modal: false }, mobile: { exclusive: false, modal: false } }, - panel4: { desktop: { open: true, dismissable: false }, mobile: { open: true, dismissable: true } } + panel4: { desktop: { open: true, dismissible: false }, mobile: { open: true, dismissible: true } } } state = { @@ -275,19 +275,19 @@ describe('actionsMap full coverage', () => { expect(result.isFullscreen).toBe(true) }) - test('SET_BREAKPOINT restores non-dismissable open panel at new breakpoint', () => { + test('SET_BREAKPOINT restores non-dismissible open panel at new breakpoint', () => { const tmp = { ...state, openPanels: {} } const result = actionsMap.SET_BREAKPOINT(tmp, { breakpoint: 'desktop', behaviour: 'responsive', hybridWidth: null, maxMobileWidth: 640 }) expect(result.openPanels.panel4).toBeDefined() }) - test('SET_BREAKPOINT does not force-open a non-dismissable panel where it is dismissable', () => { + test('SET_BREAKPOINT does not force-open a non-dismissible panel where it is dismissible', () => { const tmp = { ...state, openPanels: {} } const result = actionsMap.SET_BREAKPOINT(tmp, { breakpoint: 'mobile', behaviour: 'responsive', hybridWidth: null, maxMobileWidth: 640 }) expect(result.openPanels.panel4).toBeUndefined() }) - test('SET_BREAKPOINT preserves existing props when restoring a non-dismissable panel', () => { + test('SET_BREAKPOINT preserves existing props when restoring a non-dismissible panel', () => { const props = { myProp: 'value' } const tmp = { ...state, openPanels: { panel4: { props } } } const result = actionsMap.SET_BREAKPOINT(tmp, { breakpoint: 'desktop', behaviour: 'responsive', hybridWidth: null, maxMobileWidth: 640 }) diff --git a/src/config/appConfig.js b/src/config/appConfig.js index 7082d84c..6606e560 100755 --- a/src/config/appConfig.js +++ b/src/config/appConfig.js @@ -3,7 +3,7 @@ import { KeyboardHelp } from '../App/components/KeyboardHelp/KeyboardHelp.jsx' const keyboardBasePanelSlots = { slot: 'middle', open: false, - dismissable: true, + dismissible: true, modal: true } @@ -115,21 +115,21 @@ export const defaultPanelConfig = { mobile: { slot: 'bottom', open: true, - dismissable: true, + dismissible: true, modal: false, showLabel: true }, tablet: { slot: 'inset', open: true, - dismissable: true, + dismissible: true, modal: false, showLabel: true }, desktop: { slot: 'inset', open: true, - dismissable: true, + dismissible: true, modal: false, showLabel: true }, diff --git a/src/types.js b/src/types.js index 0d7527ca..e97ae153 100644 --- a/src/types.js +++ b/src/types.js @@ -37,7 +37,7 @@ * * @typedef {Object} PanelBreakpointConfig * - * @property {boolean} [dismissable] + * @property {boolean} [dismissible] * Whether panel can be dismissed. When `false` and `open` is `true`, the panel is always visible at this * breakpoint and any associated panel-toggle button is automatically suppressed. * @@ -45,7 +45,7 @@ * Whether panel is exclusive. An exclusive panel will hide other panels when it is visible. * * @property {boolean} [open] - * Whether the panel is open. When `true` and combined with `dismissable: false`, the panel is always visible at this + * Whether the panel is open. When `true` and combined with `dismissible: false`, the panel is always visible at this * breakpoint and will be restored automatically when the breakpoint is entered. * * @property {boolean} [showLabel]