From 9d6674f6a49d361c368b604dc3fc22d7347b2b29 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Wed, 5 Aug 2026 17:05:17 +0100 Subject: [PATCH 1/3] Hit screen reader and styling fixes --- demo/js/draw.js | 7 +++ src/App/components/Hints/Hints.module.scss | 9 +-- src/scss/settings/_dimensions.scss | 3 + src/services/announcer.js | 24 +++++++- src/services/announcer.test.js | 68 ++++++++++++++++++++++ src/services/hints.js | 3 + src/services/hints.test.js | 19 ++++++ 7 files changed, 128 insertions(+), 5 deletions(-) diff --git a/demo/js/draw.js b/demo/js/draw.js index 210932ee..dfc95a4b 100755 --- a/demo/js/draw.js +++ b/demo/js/draw.js @@ -164,6 +164,13 @@ const interactiveMap = new InteractiveMap('map', { interactiveMap.on('app:ready', function (e) { // console.log('app:ready') + interactiveMap.addPanel('banner', { + label: 'Hello', + html: 'Alert', + mobile: { slot: 'banner' }, + tablet: { slot: 'banner' }, + desktop: { slot: 'banner' } + }) }) diff --git a/src/App/components/Hints/Hints.module.scss b/src/App/components/Hints/Hints.module.scss index 067a13df..3b3f8611 100644 --- a/src/App/components/Hints/Hints.module.scss +++ b/src/App/components/Hints/Hints.module.scss @@ -5,9 +5,12 @@ .im-o-hints { position: absolute; bottom: var(--hint-bottom, var(--primary-gap)); - left: 50%; - transform: translateX(-50%); + left: 0; + right: 0; + margin-inline: auto; z-index: 1001; + width: fit-content; + max-width: min(var(--hint-max-width), calc(100% - (2 * var(--primary-gap)))); } // =================================================== @@ -22,8 +25,6 @@ } .im-c-hints__hint { - text-wrap: nowrap; - color: var(--tooltip-foreground-color); background-color: var(--tooltip-background-color); border-radius: var(--tooltip-border-radius); diff --git a/src/scss/settings/_dimensions.scss b/src/scss/settings/_dimensions.scss index 8d78b914..eaca7744 100755 --- a/src/scss/settings/_dimensions.scss +++ b/src/scss/settings/_dimensions.scss @@ -31,6 +31,9 @@ --tooltip-font-size: 14px; --tooltip-border-radius: 3px; + // Hints + --hint-max-width: 320px; + // Panels --min-panel-width: 260px; --panel-close-button-gap: 4px; diff --git a/src/services/announcer.js b/src/services/announcer.js index 2a4df660..d4c2b6f1 100755 --- a/src/services/announcer.js +++ b/src/services/announcer.js @@ -26,8 +26,17 @@ export function createAnnouncer (mapStatusRef) { // debounce + clear write (600ms) plus a short buffer so a hint firing right // after a user action can't stomp the action's message before it is read. const ACTION_HOLD_DELAY = 1000 + // Invisible, unpronounced marker appended to alternate repeats of the same + // message. Some screen readers (VoiceOver in particular) de-dupe against the + // last utterance they spoke for a live region, not just the DOM's final + // state — so a genuine ''-then-text mutation can still be silently skipped + // if the text matches what was already read. Toggling this marker in keeps + // consecutive identical announcements from ever being byte-identical. + const REPEAT_MARKER = '\u200B' let actionHoldTimer = null + let lastAnnouncedMsg = null + let repeatMarkerOn = false // Core function to write to the live region const setLiveRegion = (msg) => { @@ -41,7 +50,9 @@ export function createAnnouncer (mapStatusRef) { if (!mapStatusRef.current) { return } - mapStatusRef.current.textContent = msg + repeatMarkerOn = msg === lastAnnouncedMsg ? !repeatMarkerOn : false + lastAnnouncedMsg = msg + mapStatusRef.current.textContent = repeatMarkerOn ? msg + REPEAT_MARKER : msg }, CLEAR_DELAY) } @@ -76,5 +87,16 @@ export function createAnnouncer (mapStatusRef) { debouncedAnnounce(msg) } + // Blanks the live region without announcing anything. Used when a message is + // dismissed (e.g. a hint toast auto-dismissing) so a later identical message + // starts from a genuinely empty region — otherwise clear-then-set-same-text + // nets out to no change and some screen readers skip the re-announcement. + announce.clear = () => { + if (!mapStatusRef?.current) { + return + } + mapStatusRef.current.textContent = '' + } + return announce } diff --git a/src/services/announcer.test.js b/src/services/announcer.test.js index 391539d3..e0f96e73 100755 --- a/src/services/announcer.test.js +++ b/src/services/announcer.test.js @@ -106,6 +106,74 @@ describe('createAnnouncer', () => { expect(mapStatusRef.current.textContent).toBe('Keyboard hint') }) + it('exposes a clear() that blanks the live region without announcing', () => { + announce('Hint message', 'ambient') + jest.advanceTimersByTime(100) + expect(mapStatusRef.current.textContent).toBe('Hint message') + + announce.clear() + expect(mapStatusRef.current.textContent).toBe('') + }) + + it('clear() is a no-op when mapStatusRef.current is null', () => { + const announceWithNull = createAnnouncer({ current: null }) + expect(() => announceWithNull.clear()).not.toThrow() + }) + + it('a repeated identical ambient message re-announces after clear() blanks stale text', () => { + // First announcement: region starts empty, so clear-then-set is a real transition. + announce('Same message', 'ambient') + jest.advanceTimersByTime(100) + expect(mapStatusRef.current.textContent).toBe('Same message') + + // Something (e.g. a hint dismissing) blanks the region before the retry, the way + // hints.dismiss() now does — without this, clear-then-set-same-text nets to no + // visible change and some screen readers skip the re-announcement. + announce.clear() + expect(mapStatusRef.current.textContent).toBe('') + + announce('Same message', 'ambient') + jest.advanceTimersByTime(100) + // Repeat marker kicks in because this is a repeat of the last *spoken* message, + // regardless of the clear() in between — see the dedicated marker tests below. + expect(mapStatusRef.current.textContent).toBe('Same message' + '\u200B') + }) + + it('alternates an invisible marker on consecutive identical messages so the text is never byte-identical twice in a row', () => { + // VoiceOver has been observed to de-dupe against the last utterance it spoke for a + // live region, not just the DOM's final state — so a real ''-then-text mutation can + // still be silently skipped if the text matches what it already read out. Repeats + // must therefore differ at the string level even though they sound identical. + announce('Same message', 'ambient') + jest.advanceTimersByTime(100) + const first = mapStatusRef.current.textContent + expect(first).toBe('Same message') + + announce('Same message', 'ambient') + jest.advanceTimersByTime(100) + const second = mapStatusRef.current.textContent + expect(second).not.toBe(first) + expect(second).toBe('Same message' + '\u200B') + + announce('Same message', 'ambient') + jest.advanceTimersByTime(100) + const third = mapStatusRef.current.textContent + expect(third).not.toBe(second) + expect(third).toBe(first) // toggles back to the plain form + }) + + it('resets the repeat marker once a different message interrupts a run of repeats', () => { + announce('A', 'ambient') + jest.advanceTimersByTime(100) + announce('A', 'ambient') + jest.advanceTimersByTime(100) + expect(mapStatusRef.current.textContent).toBe('A' + '\u200B') + + announce('B', 'ambient') + jest.advanceTimersByTime(100) + expect(mapStatusRef.current.textContent).toBe('B') // fresh message, no marker + }) + it('latest action wins: rapid actions announce only the most recent', () => { announce('First', 'action') jest.advanceTimersByTime(200) // still within debounce window diff --git a/src/services/hints.js b/src/services/hints.js index 58b61474..6216eee9 100644 --- a/src/services/hints.js +++ b/src/services/hints.js @@ -25,6 +25,9 @@ export function createHints (announce) { clearTimer() current = null notify() + // Blank the live region so a later identical hint re-announces instead of + // silently netting-out against the stale text left by this dismissal. + announce.clear?.() } const show = (html, options = {}) => { diff --git a/src/services/hints.test.js b/src/services/hints.test.js index 0509880b..40ab8c84 100644 --- a/src/services/hints.test.js +++ b/src/services/hints.test.js @@ -85,6 +85,25 @@ describe('dismiss', () => { it('is safe to call when no hint is active', () => { expect(() => hints.dismiss()).not.toThrow() }) + + it('blanks the live region via announce.clear so a later identical hint re-announces', () => { + announce.clear = jest.fn() + hints.show('hello') + hints.dismiss() + expect(announce.clear).toHaveBeenCalled() + }) + + it('is safe to call when announce has no clear method', () => { + hints.show('hello') + expect(() => hints.dismiss()).not.toThrow() + }) + + it('blanks the live region on auto-dismiss too, not just explicit dismiss', () => { + announce.clear = jest.fn() + hints.show('hello', { duration: 3000 }) + jest.advanceTimersByTime(3000) + expect(announce.clear).toHaveBeenCalled() + }) }) // ─── subscribe / unsubscribe ────────────────────────────────────────────────── From 303b9a90fc5a4808cb95dcc6fb2e7adaef82ab7b Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Wed, 5 Aug 2026 17:31:18 +0100 Subject: [PATCH 2/3] show/dismiss hint public api methods --- docs/api.md | 30 +++++++++++++ src/App/hooks/useHintsAPI.js | 30 +++++++++++++ src/App/hooks/useHintsAPI.test.js | 51 +++++++++++++++++++++++ src/App/renderer/PluginInits.jsx | 4 ++ src/App/renderer/PluginInits.test.jsx | 8 +++- src/InteractiveMap/InteractiveMap.js | 20 +++++++++ src/InteractiveMap/InteractiveMap.test.js | 7 ++++ src/config/events.js | 4 ++ src/types.js | 12 ++++++ 9 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 src/App/hooks/useHintsAPI.js create mode 100644 src/App/hooks/useHintsAPI.test.js diff --git a/docs/api.md b/docs/api.md index d66e2caa..bc0784d8 100644 --- a/docs/api.md +++ b/docs/api.md @@ -763,6 +763,36 @@ See [ControlDefinition](./api/control-definition.md) for configuration options. --- +### `showHint(text, options?)` + +Show a toast hint, announced to screen readers. Replaces any hint currently showing and restarts its dismiss timer — there is only ever one hint visible at a time. + +| Parameter | Type | Description | +|-----------|------|-------------| +| `text` | `string` | Hint text. May contain simple HTML (e.g. ``) | +| `options.duration` | `number` | Auto-dismiss delay in milliseconds. Pass `0` to persist until `dismissHint()` is called. Default: `4000` | +| `options.announce` | `string` | Optional plain-text override for the screen-reader announcement. Defaults to `text` with any HTML tags stripped | + +```js +interactiveMap.showHint('Press Enter to select') + +// Persist until explicitly dismissed +interactiveMap.showHint('Draw mode active', { duration: 0 }) +interactiveMap.dismissHint() +``` + +--- + +### `dismissHint()` + +Dismiss the active toast hint, if any. No-op if no hint is showing. Mainly useful for hints shown with `{ duration: 0 }`, which otherwise persist indefinitely. + +```js +interactiveMap.dismissHint() +``` + +--- + ### `setContinueEnabled(enabled)` Enable or disable the Continue button added by [`backAndContinue`](#backandcontinue). Use this for imperative control — for example, enabling Continue after an async operation or in response to an external event. For reactive state-derived conditions, prefer the `continueEnabledWhen` function in `backAndContinue` instead. diff --git a/src/App/hooks/useHintsAPI.js b/src/App/hooks/useHintsAPI.js new file mode 100644 index 00000000..2db8e082 --- /dev/null +++ b/src/App/hooks/useHintsAPI.js @@ -0,0 +1,30 @@ +import { useEffect } from 'react' +import { EVENTS as events } from '../../config/events.js' +import { useService } from '../store/serviceContext.js' + +/** + * Wires the public showHint()/dismissHint() API onto the hints service. + * The hints service already owns its own subscriber list (see Hints.jsx), + * so this just forwards eventBus commands to it — no React state involved. + */ +export const useHintsAPI = () => { + const { eventBus, hints } = useService() + + useEffect(() => { + const handleShowHint = ({ text, options } = {}) => { + if (!text) { + return + } + hints.show(text, options) + } + const handleDismissHint = () => hints.dismiss() + + eventBus.on(events.APP_SHOW_HINT, handleShowHint) + eventBus.on(events.APP_DISMISS_HINT, handleDismissHint) + + return () => { + eventBus.off(events.APP_SHOW_HINT, handleShowHint) + eventBus.off(events.APP_DISMISS_HINT, handleDismissHint) + } + }, [eventBus, hints]) +} diff --git a/src/App/hooks/useHintsAPI.test.js b/src/App/hooks/useHintsAPI.test.js new file mode 100644 index 00000000..7079f5ca --- /dev/null +++ b/src/App/hooks/useHintsAPI.test.js @@ -0,0 +1,51 @@ +import { renderHook, act } from '@testing-library/react' +import { useHintsAPI } from './useHintsAPI.js' +import { useService } from '../store/serviceContext.js' + +jest.mock('../store/serviceContext.js') + +const makeEventBus = () => { + const handlers = {} + return { + on: jest.fn((event, handler) => { handlers[event] = handler }), + off: jest.fn(), + emit: (event, payload) => handlers[event]?.(payload), + _handlers: handlers + } +} + +describe('useHintsAPI', () => { + let mockEventBus, mockHints + + beforeEach(() => { + mockEventBus = makeEventBus() + mockHints = { show: jest.fn(), dismiss: jest.fn() } + useService.mockReturnValue({ eventBus: mockEventBus, hints: mockHints }) + }) + + it('calls hints.show with text and options on app:showhint', () => { + renderHook(() => useHintsAPI()) + act(() => mockEventBus.emit('app:showhint', { text: 'Press Enter to select', options: { duration: 2000 } })) + expect(mockHints.show).toHaveBeenCalledWith('Press Enter to select', { duration: 2000 }) + }) + + it('ignores app:showhint with no text', () => { + renderHook(() => useHintsAPI()) + act(() => mockEventBus.emit('app:showhint', {})) + act(() => mockEventBus.emit('app:showhint')) + expect(mockHints.show).not.toHaveBeenCalled() + }) + + it('calls hints.dismiss on app:dismisshint', () => { + renderHook(() => useHintsAPI()) + act(() => mockEventBus.emit('app:dismisshint')) + expect(mockHints.dismiss).toHaveBeenCalled() + }) + + it('unsubscribes on unmount', () => { + const { unmount } = renderHook(() => useHintsAPI()) + unmount() + expect(mockEventBus.off).toHaveBeenCalledWith('app:showhint', expect.any(Function)) + expect(mockEventBus.off).toHaveBeenCalledWith('app:dismisshint', expect.any(Function)) + }) +}) diff --git a/src/App/renderer/PluginInits.jsx b/src/App/renderer/PluginInits.jsx index fb78d0f8..356e5ac5 100755 --- a/src/App/renderer/PluginInits.jsx +++ b/src/App/renderer/PluginInits.jsx @@ -3,6 +3,7 @@ import React, { useEffect } from 'react' import { withPluginContexts } from './pluginWrapper.js' import { withPluginApiContexts, usePluginApiState } from './pluginApiWrapper.js' import { useInterfaceAPI } from '../hooks/useInterfaceAPI.js' +import { useHintsAPI } from '../hooks/useHintsAPI.js' import { useApp } from '../store/appContext.js' import { useConfig } from '../store/configContext.js' import { useEvaluateProp } from '../hooks/useEvaluateProp.js' @@ -53,6 +54,9 @@ export const PluginInits = () => { // Add button, panel and control API methods (Needs to be top-level) useInterfaceAPI() + // Wire the showHint()/dismissHint() public API onto the hints service + useHintsAPI() + // Evaluate reactive button states globally const evaluateProp = useEvaluateProp() useButtonStateEvaluator(evaluateProp) diff --git a/src/App/renderer/PluginInits.test.jsx b/src/App/renderer/PluginInits.test.jsx index 443f66b5..c60dfca8 100755 --- a/src/App/renderer/PluginInits.test.jsx +++ b/src/App/renderer/PluginInits.test.jsx @@ -6,6 +6,7 @@ import { useButtonStateEvaluator } from '../hooks/useButtonStateEvaluator.js' import { withPluginApiContexts } from './pluginApiWrapper.js' import { withPluginContexts } from './pluginWrapper.js' import { useInterfaceAPI } from '../hooks/useInterfaceAPI.js' +import { useHintsAPI } from '../hooks/useHintsAPI.js' import { useApp } from '../store/appContext.js' import { useConfig } from '../store/configContext.js' @@ -29,6 +30,10 @@ jest.mock('../hooks/useInterfaceAPI.js', () => ({ useInterfaceAPI: jest.fn() })) +jest.mock('../hooks/useHintsAPI.js', () => ({ + useHintsAPI: jest.fn() +})) + jest.mock('../hooks/useEvaluateProp.js', () => ({ useEvaluateProp: jest.fn(() => (x) => x) })) @@ -59,10 +64,11 @@ describe('PluginInits', () => { useConfig.mockReturnValue({ pluginRegistry: pluginRegistryMock }) }) - it('calls useButtonStateEvaluator and useInterfaceAPI on render', () => { + it('calls useButtonStateEvaluator, useInterfaceAPI and useHintsAPI on render', () => { render() expect(useButtonStateEvaluator).toHaveBeenCalled() expect(useInterfaceAPI).toHaveBeenCalled() + expect(useHintsAPI).toHaveBeenCalled() }) it('renders nothing when no plugins registered', () => { diff --git a/src/InteractiveMap/InteractiveMap.js b/src/InteractiveMap/InteractiveMap.js index d423c71d..9a802c11 100755 --- a/src/InteractiveMap/InteractiveMap.js +++ b/src/InteractiveMap/InteractiveMap.js @@ -3,6 +3,7 @@ /** * @typedef {import('../types.js').ButtonDefinition} ButtonDefinition * @typedef {import('../types.js').ControlDefinition} ControlDefinition + * @typedef {import('../types.js').HintOptions} HintOptions * @typedef {import('../types.js').InteractiveMapConfig} InteractiveMapConfig * @typedef {import('../types.js').MarkerOptions} MarkerOptions * @typedef {import('../types.js').PanelDefinition} PanelDefinition @@ -481,6 +482,25 @@ export default class InteractiveMap { this.eventBus.emit(events.APP_ADD_CONTROL, { id, config }) } + /** + * Show a toast hint, announced to screen readers via the live region. + * Replaces any hint currently showing and restarts its dismiss timer. + * + * @param {string} text - Hint text. May contain simple HTML (e.g. ``). + * @param {HintOptions} [options] - Optional hint behaviour. + */ + showHint (text, options) { + this.eventBus.emit(events.APP_SHOW_HINT, { text, options }) + } + + /** + * Dismiss the active toast hint, if any. No-op if no hint is showing. + * Mainly useful for hints shown with `{ duration: 0 }`, which otherwise persist indefinitely. + */ + dismissHint () { + this.eventBus.emit(events.APP_DISMISS_HINT) + } + /** * Fit the map view to a bounding box or GeoJSON geometry, respecting the safe zone padding. * diff --git a/src/InteractiveMap/InteractiveMap.test.js b/src/InteractiveMap/InteractiveMap.test.js index 04e2345b..053b7fea 100755 --- a/src/InteractiveMap/InteractiveMap.test.js +++ b/src/InteractiveMap/InteractiveMap.test.js @@ -579,4 +579,11 @@ describe('InteractiveMap — Public API Methods', () => { expect(map.eventBus.emitWhenReady).toHaveBeenCalledWith('map:fittobounds', bbox) expect(map.eventBus.emitWhenReady).toHaveBeenCalledWith('map:setview', { center, zoom: 12 }) }) + + it('showHint and dismissHint emit correct events', () => { + map.showHint('Press Enter to select', { duration: 2000 }) + map.dismissHint() + expect(map.eventBus.emit).toHaveBeenCalledWith('app:showhint', { text: 'Press Enter to select', options: { duration: 2000 } }) + expect(map.eventBus.emit).toHaveBeenCalledWith('app:dismisshint') + }) }) diff --git a/src/config/events.js b/src/config/events.js index 7b8210cf..25328f25 100644 --- a/src/config/events.js +++ b/src/config/events.js @@ -47,6 +47,10 @@ export const EVENTS = { APP_HIDE_PANEL: 'app:hidepanel', /** @internal Add a control. Payload: { id, config } */ APP_ADD_CONTROL: 'app:addcontrol', + /** @internal Show a toast hint, announced to screen readers. Payload: { text, options } */ + APP_SHOW_HINT: 'app:showhint', + /** @internal Dismiss the active toast hint, if any. */ + APP_DISMISS_HINT: 'app:dismisshint', // ============================================ // App responses (end-user / subscribe) diff --git a/src/types.js b/src/types.js index 05dce5d9..2b32be2e 100644 --- a/src/types.js +++ b/src/types.js @@ -525,6 +525,18 @@ * */ +/** + * Options for a toast hint shown via `showHint()`. + * + * @typedef {Object} HintOptions + * + * @property {number} [duration=4000] + * Auto-dismiss delay in milliseconds. Pass `0` to persist until `dismissHint()` is called. + * + * @property {string} [announce] + * Plain-text override for the screen-reader announcement. Defaults to `text` with any HTML tags stripped. + */ + /** * Defines a panel that can be rendered in the UI at various breakpoints. * From bb37c3fadac48f0ad5b15a0e14528ef4704fa88c Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Thu, 6 Aug 2026 09:40:21 +0100 Subject: [PATCH 3/3] Public API methods added --- demo/js/index.js | 1 + docs/api.md | 2 + src/App/hooks/useHintsAPI.js | 39 +++++++++++- src/App/hooks/useHintsAPI.test.js | 98 ++++++++++++++++++++++++++++++- src/services/announcer.js | 17 +++++- src/services/announcer.test.js | 66 ++++++++++++++++++++- 6 files changed, 217 insertions(+), 6 deletions(-) diff --git a/demo/js/index.js b/demo/js/index.js index 64a4c4e6..f222fff4 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -330,6 +330,7 @@ const interactiveMap = new InteractiveMap('map', { interactiveMap.on('app:ready', function (e) { // console.log('app:ready') + interactiveMap.showHint('My hint', { duration: 0 }) }) interactiveMap.on('map:ready', function (e) { diff --git a/docs/api.md b/docs/api.md index bc0784d8..c9bad470 100644 --- a/docs/api.md +++ b/docs/api.md @@ -787,6 +787,8 @@ interactiveMap.dismissHint() Dismiss the active toast hint, if any. No-op if no hint is showing. Mainly useful for hints shown with `{ duration: 0 }`, which otherwise persist indefinitely. +The active hint is also dismissed automatically when the user presses Escape anywhere within this map instance. With multiple map instances on a page, Escape only dismisses the hint belonging to the instance the keypress originated in. + ```js interactiveMap.dismissHint() ``` diff --git a/src/App/hooks/useHintsAPI.js b/src/App/hooks/useHintsAPI.js index 2db8e082..56e1cb3f 100644 --- a/src/App/hooks/useHintsAPI.js +++ b/src/App/hooks/useHintsAPI.js @@ -1,6 +1,7 @@ -import { useEffect } from 'react' +import { useEffect, useRef } from 'react' import { EVENTS as events } from '../../config/events.js' import { useService } from '../store/serviceContext.js' +import { useApp } from '../store/appContext.js' /** * Wires the public showHint()/dismissHint() API onto the hints service. @@ -9,6 +10,8 @@ import { useService } from '../store/serviceContext.js' */ export const useHintsAPI = () => { const { eventBus, hints } = useService() + const { layoutRefs } = useApp() + const isHintActiveRef = useRef(false) useEffect(() => { const handleShowHint = ({ text, options } = {}) => { @@ -27,4 +30,38 @@ export const useHintsAPI = () => { eventBus.off(events.APP_DISMISS_HINT, handleDismissHint) } }, [eventBus, hints]) + + // Escape dismisses the active hint when the keypress originates inside this + // map instance. The viewport and features listbox already dismiss hints on + // Escape within their own narrower focus scope (useKeyboardHint.js / + // useFeatureFocus.js) — this covers hints shown via the public showHint() + // API from anywhere else in the same map's UI (e.g. a plugin button). + // + // Listening on document (rather than the map's own container) and checking + // containment ourselves, rather than a container-scoped listener, is + // deliberate: with multiple map instances on one page, each has its own + // hints service and its own isHintActiveRef, so without the containment + // check an Escape press anywhere on the host page would dismiss a hint on + // every instance that happened to have one showing — including maps the + // user isn't even looking at. + useEffect(() => { + return hints.subscribe((hint) => { + isHintActiveRef.current = Boolean(hint) + }) + }, [hints]) + + useEffect(() => { + const handleKeyDown = (e) => { + if (e.key !== 'Escape' || !isHintActiveRef.current) { + return + } + const container = layoutRefs.appContainerRef?.current + if (container && !container.contains(e.target)) { + return + } + hints.dismiss() + } + document.addEventListener('keydown', handleKeyDown) + return () => document.removeEventListener('keydown', handleKeyDown) + }, [hints, layoutRefs]) } diff --git a/src/App/hooks/useHintsAPI.test.js b/src/App/hooks/useHintsAPI.test.js index 7079f5ca..6a8824d5 100644 --- a/src/App/hooks/useHintsAPI.test.js +++ b/src/App/hooks/useHintsAPI.test.js @@ -1,8 +1,10 @@ import { renderHook, act } from '@testing-library/react' import { useHintsAPI } from './useHintsAPI.js' import { useService } from '../store/serviceContext.js' +import { useApp } from '../store/appContext.js' jest.mock('../store/serviceContext.js') +jest.mock('../store/appContext.js') const makeEventBus = () => { const handlers = {} @@ -14,13 +16,48 @@ const makeEventBus = () => { } } +const makeHints = () => { + let subscriber = null + return { + show: jest.fn(), + dismiss: jest.fn(), + subscribe: jest.fn((fn) => { + subscriber = fn + return () => { subscriber = null } + }), + _emit: (hint) => subscriber?.(hint) + } +} + +const pressEscape = (target) => { + target.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })) +} + describe('useHintsAPI', () => { - let mockEventBus, mockHints + let mockEventBus, mockHints, containerEl, insideEl, outsideEl beforeEach(() => { mockEventBus = makeEventBus() - mockHints = { show: jest.fn(), dismiss: jest.fn() } + mockHints = makeHints() useService.mockReturnValue({ eventBus: mockEventBus, hints: mockHints }) + + // Simulates this map instance's root DOM (layoutRefs.appContainerRef), + // plus an element outside it — standing in for a second map instance, + // or unrelated content elsewhere on the host page. + containerEl = document.createElement('div') + insideEl = document.createElement('button') + containerEl.appendChild(insideEl) + document.body.appendChild(containerEl) + + outsideEl = document.createElement('button') + document.body.appendChild(outsideEl) + + useApp.mockReturnValue({ layoutRefs: { appContainerRef: { current: containerEl } } }) + }) + + afterEach(() => { + containerEl.remove() + outsideEl.remove() }) it('calls hints.show with text and options on app:showhint', () => { @@ -48,4 +85,61 @@ describe('useHintsAPI', () => { expect(mockEventBus.off).toHaveBeenCalledWith('app:showhint', expect.any(Function)) expect(mockEventBus.off).toHaveBeenCalledWith('app:dismisshint', expect.any(Function)) }) + + describe('Escape key', () => { + it('dismisses the active hint when Escape originates inside this map instance', () => { + renderHook(() => useHintsAPI()) + act(() => mockHints._emit({ html: 'Press Enter to select' })) + + act(() => pressEscape(insideEl)) + expect(mockHints.dismiss).toHaveBeenCalled() + }) + + it('does not dismiss when Escape originates outside this map instance', () => { + // e.g. a second map instance on the page, or unrelated host-page content + renderHook(() => useHintsAPI()) + act(() => mockHints._emit({ html: 'Press Enter to select' })) + + act(() => pressEscape(outsideEl)) + expect(mockHints.dismiss).not.toHaveBeenCalled() + }) + + it('does nothing on Escape when no hint is showing', () => { + renderHook(() => useHintsAPI()) + act(() => pressEscape(insideEl)) + expect(mockHints.dismiss).not.toHaveBeenCalled() + }) + + it('stops reacting to Escape once the hint has been dismissed', () => { + renderHook(() => useHintsAPI()) + act(() => mockHints._emit({ html: 'Press Enter to select' })) + act(() => mockHints._emit(null)) // hints service reports no active hint + + act(() => pressEscape(insideEl)) + expect(mockHints.dismiss).not.toHaveBeenCalled() + }) + + it('falls back to dismissing when appContainerRef is not yet available', () => { + useApp.mockReturnValue({ layoutRefs: { appContainerRef: { current: null } } }) + renderHook(() => useHintsAPI()) + act(() => mockHints._emit({ html: 'Press Enter to select' })) + + act(() => pressEscape(outsideEl)) + expect(mockHints.dismiss).toHaveBeenCalled() + }) + + it('removes the keydown listener on unmount', () => { + const addSpy = jest.spyOn(document, 'addEventListener') + const removeSpy = jest.spyOn(document, 'removeEventListener') + + const { unmount } = renderHook(() => useHintsAPI()) + expect(addSpy).toHaveBeenCalledWith('keydown', expect.any(Function)) + + unmount() + expect(removeSpy).toHaveBeenCalledWith('keydown', expect.any(Function)) + + addSpy.mockRestore() + removeSpy.mockRestore() + }) + }) }) diff --git a/src/services/announcer.js b/src/services/announcer.js index d4c2b6f1..82d72adc 100755 --- a/src/services/announcer.js +++ b/src/services/announcer.js @@ -19,7 +19,7 @@ import { debounce } from '../utils/debounce.js' * action result is holding priority so they can never clobber what * the user actually requested. */ -export function createAnnouncer (mapStatusRef) { +export function createAnnouncer (mapStatusRef, { startupGraceDelay = 1000 } = {}) { const CLEAR_DELAY = 100 const DEBOUNCE_DELAY = 500 // How long an action result keeps priority over ambient messages. Covers the @@ -33,6 +33,17 @@ export function createAnnouncer (mapStatusRef) { // if the text matches what was already read. Toggling this marker in keeps // consecutive identical announcements from ever being byte-identical. const REPEAT_MARKER = '\u200B' + // A screen reader may still be building its accessibility tree for a + // newly-loaded page — the live region's DOM node can exist and mutate + // correctly while the AT hasn't registered it yet, so an announcement fired + // this soon after boot (e.g. a consumer calling showHint() on 'app:ready') + // can be silently missed even though nothing here did anything wrong. Give + // any announcement inside this window from creation an extra head start; + // announcements after it behave exactly as before, with zero extra delay. + // Configurable (default 1000ms) so tests can set it to 0 and not have to + // account for it in every unrelated timing assertion. + const STARTUP_GRACE_DELAY = startupGraceDelay + const startedAt = Date.now() let actionHoldTimer = null let lastAnnouncedMsg = null @@ -44,6 +55,8 @@ export function createAnnouncer (mapStatusRef) { return } + const startupDelay = Math.max(0, STARTUP_GRACE_DELAY - (Date.now() - startedAt)) + // Clear first (for SR to re-announce) mapStatusRef.current.textContent = '' setTimeout(() => { @@ -53,7 +66,7 @@ export function createAnnouncer (mapStatusRef) { repeatMarkerOn = msg === lastAnnouncedMsg ? !repeatMarkerOn : false lastAnnouncedMsg = msg mapStatusRef.current.textContent = repeatMarkerOn ? msg + REPEAT_MARKER : msg - }, CLEAR_DELAY) + }, CLEAR_DELAY + startupDelay) } // Debounced announcer to group rapid action events down to the latest one diff --git a/src/services/announcer.test.js b/src/services/announcer.test.js index e0f96e73..ba4c8c70 100755 --- a/src/services/announcer.test.js +++ b/src/services/announcer.test.js @@ -8,7 +8,9 @@ describe('createAnnouncer', () => { beforeEach(() => { mapStatusRef = { current: { textContent: '' } } - announce = createAnnouncer(mapStatusRef) + // startupGraceDelay: 0 — these tests exercise everything except the startup + // grace window itself, which has its own dedicated describe block below. + announce = createAnnouncer(mapStatusRef, { startupGraceDelay: 0 }) }) afterEach(() => { @@ -183,3 +185,65 @@ describe('createAnnouncer', () => { expect(mapStatusRef.current.textContent).toBe('Second') }) }) + +describe('createAnnouncer — startup grace delay', () => { + // A consumer calling showHint() the instant the app is ready (e.g. on + // 'app:ready') can fire before the screen reader has finished registering + // the live region in its own accessibility tree, even though the DOM node + // already exists and mutates correctly. These tests use a small explicit + // window (rather than the real 1000ms default) purely so assertions don't + // need to advance unrealistic amounts of fake time. + const GRACE = 300 + let mapStatusRef + let announce + + beforeEach(() => { + mapStatusRef = { current: { textContent: '' } } + announce = createAnnouncer(mapStatusRef, { startupGraceDelay: GRACE }) + }) + + afterEach(() => { + jest.clearAllTimers() + }) + + it('delays an announcement fired immediately after creation by the full grace window', () => { + announce('Ready', 'ambient') + jest.advanceTimersByTime(100) // normal CLEAR_DELAY only + expect(mapStatusRef.current.textContent).toBe('') // still withheld + + jest.advanceTimersByTime(GRACE) // the rest of the grace window + expect(mapStatusRef.current.textContent).toBe('Ready') + }) + + it('only waits out the remaining grace window, not the full window again', () => { + jest.advanceTimersByTime(200) // partway through the grace window already + announce('Ready', 'ambient') + + // Remaining grace (100) + CLEAR_DELAY (100) = 200 total from this point + jest.advanceTimersByTime(199) + expect(mapStatusRef.current.textContent).toBe('') + + jest.advanceTimersByTime(1) + expect(mapStatusRef.current.textContent).toBe('Ready') + }) + + it('adds no extra delay once the grace window has fully elapsed', () => { + jest.advanceTimersByTime(GRACE) + announce('Later', 'ambient') + + jest.advanceTimersByTime(100) // normal CLEAR_DELAY only + expect(mapStatusRef.current.textContent).toBe('Later') + }) + + it('defaults to a 1000ms grace window when not configured', () => { + const defaultAnnounce = createAnnouncer(mapStatusRef) + defaultAnnounce('Ready', 'ambient') + + // 1000ms grace + 100ms CLEAR_DELAY = 1100 total + jest.advanceTimersByTime(1099) + expect(mapStatusRef.current.textContent).toBe('') + + jest.advanceTimersByTime(1) + expect(mapStatusRef.current.textContent).toBe('Ready') + }) +})