From 543f5f2fd4d4243462ac3c04df90a455c33da346 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Wed, 15 Jul 2026 13:47:22 +0100 Subject: [PATCH] Screen reader priority message fixes --- src/App/controls/keyboardActions.js | 6 +- src/App/controls/keyboardActions.test.js | 8 +- src/App/hooks/useMapAnnouncements.js | 2 +- src/App/hooks/useMapAnnouncements.test.js | 8 +- src/services/announcer.js | 52 ++++++++++--- src/services/announcer.test.js | 90 +++++++++++++++-------- src/services/hints.js | 2 +- src/services/hints.test.js | 4 +- 8 files changed, 115 insertions(+), 57 deletions(-) diff --git a/src/App/controls/keyboardActions.js b/src/App/controls/keyboardActions.js index 4bbe21fc1..bc6e61181 100755 --- a/src/App/controls/keyboardActions.js +++ b/src/App/controls/keyboardActions.js @@ -46,7 +46,7 @@ export const createKeyboardActions = (mapProvider, announce, { const place = await reverseGeocode(mapProvider.getZoom(), coord) const area = mapProvider.getAreaDimensions?.() const message = area ? `${place}. Covering ${area}.` : `${place}.` - announce(message, 'core') + announce(message, 'action') }, highlightNextLabel: (e) => { @@ -54,7 +54,7 @@ export const createKeyboardActions = (mapProvider, announce, { return } const label = mapProvider.highlightNextLabel(e.key) - announce(label, 'core') + announce(label, 'action') }, highlightLabelAtCenter: () => { @@ -62,7 +62,7 @@ export const createKeyboardActions = (mapProvider, announce, { return } const label = mapProvider.highlightLabelAtCenter() - announce(label, 'core') + announce(label, 'action') }, clearSelection: () => mapProvider?.clearHighlightedLabel?.() diff --git a/src/App/controls/keyboardActions.test.js b/src/App/controls/keyboardActions.test.js index b2f84bbbe..5ae89c5e8 100644 --- a/src/App/controls/keyboardActions.test.js +++ b/src/App/controls/keyboardActions.test.js @@ -131,7 +131,7 @@ describe('getInfo', () => { const { actions, announce } = makeActions() await actions.getInfo() expect(reverseGeocode).toHaveBeenCalledWith(ZOOM_LEVEL, { lng: 1, lat: 2 }) - expect(announce).toHaveBeenCalledWith('London. Covering 5km².', 'core') + expect(announce).toHaveBeenCalledWith('London. Covering 5km².', 'action') }) test('announces result without area when getAreaDimensions is absent', async () => { @@ -139,7 +139,7 @@ describe('getInfo', () => { const { actions, mapProvider, announce } = makeActions() delete mapProvider.getAreaDimensions await actions.getInfo() - expect(announce).toHaveBeenCalledWith('Paris.', 'core') + expect(announce).toHaveBeenCalledWith('Paris.', 'action') }) }) @@ -148,7 +148,7 @@ describe('label actions', () => { const { actions, mapProvider, announce } = makeActions() mapProvider.highlightNextLabel.mockReturnValue('Label A') actions.highlightNextLabel({ key: 'A' }) - expect(announce).toHaveBeenCalledWith('Label A', 'core') + expect(announce).toHaveBeenCalledWith('Label A', 'action') }) test('highlightNextLabel is no-op when readMapText is false', () => { @@ -162,7 +162,7 @@ describe('label actions', () => { const { actions, mapProvider, announce } = makeActions() mapProvider.highlightLabelAtCenter.mockReturnValue('Center Label') actions.highlightLabelAtCenter() - expect(announce).toHaveBeenCalledWith('Center Label', 'core') + expect(announce).toHaveBeenCalledWith('Center Label', 'action') }) test('highlightLabelAtCenter is no-op when readMapText is false', () => { diff --git a/src/App/hooks/useMapAnnouncements.js b/src/App/hooks/useMapAnnouncements.js index 16224a061..8015ea504 100755 --- a/src/App/hooks/useMapAnnouncements.js +++ b/src/App/hooks/useMapAnnouncements.js @@ -50,7 +50,7 @@ export function useMapAnnouncements () { const message = resolveMessage(previous, current, mapProvider) if (message) { - announce(message, 'core') + announce(message, 'action') } } diff --git a/src/App/hooks/useMapAnnouncements.test.js b/src/App/hooks/useMapAnnouncements.test.js index f5450c541..a6e3c60da 100644 --- a/src/App/hooks/useMapAnnouncements.test.js +++ b/src/App/hooks/useMapAnnouncements.test.js @@ -77,7 +77,7 @@ describe('useMapAnnouncements', () => { direction: 'north', areaDimensions: { width: 100, height: 100 } }) - expect(announce).toHaveBeenCalledWith('Moved north', 'core') + expect(announce).toHaveBeenCalledWith('Moved north', 'action') }) test('announces zoomed message when only zoom changes', () => { @@ -97,7 +97,7 @@ describe('useMapAnnouncements', () => { to: 12, areaDimensions: { width: 100, height: 100 } }) - expect(announce).toHaveBeenCalledWith('Zoomed in', 'core') + expect(announce).toHaveBeenCalledWith('Zoomed in', 'action') }) test('announces no change message when neither center nor zoom changes', () => { @@ -114,7 +114,7 @@ describe('useMapAnnouncements', () => { center: [0, 0], zoom: 10 }) - expect(announce).toHaveBeenCalledWith('No change', 'core') + expect(announce).toHaveBeenCalledWith('No change', 'action') }) test('announces new area message when both center and zoom change', () => { @@ -132,7 +132,7 @@ describe('useMapAnnouncements', () => { zoom: 12, areaDimensions: { width: 100, height: 100 } }) - expect(announce).toHaveBeenCalledWith('New area', 'core') + expect(announce).toHaveBeenCalledWith('New area', 'action') }) test('detects center change correctly for lat or lng', () => { diff --git a/src/services/announcer.js b/src/services/announcer.js index 38eacfe74..2a4df6609 100755 --- a/src/services/announcer.js +++ b/src/services/announcer.js @@ -1,10 +1,33 @@ // services/announcer.js import { debounce } from '../utils/debounce.js' +/** + * Live-region announcer for screen readers. + * + * Priority model — the axis that matters is "did the user ask for this?", not + * which part of the code sent it: + * + * - 'action' Direct response to a deliberate user keypress (pan/zoom result, + * Alt+I reverse-geocode, a plugin shortcut result, search results). + * The latest action always wins: rapid actions are debounced down + * to the most recent, and an action is never blocked by an ambient + * message. This is the default. + * + * - 'ambient' Passive/system-initiated message the user did not directly ask + * for (e.g. the "Press Shift+? for keyboard controls" hint shown on + * focus). Ambient messages yield: they are skipped while a recent + * action result is holding priority so they can never clobber what + * the user actually requested. + */ export function createAnnouncer (mapStatusRef) { const CLEAR_DELAY = 100 const DEBOUNCE_DELAY = 500 - let priorityLock = false + // How long an action result keeps priority over ambient messages. Covers the + // 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 + + let actionHoldTimer = null // Core function to write to the live region const setLiveRegion = (msg) => { @@ -22,27 +45,34 @@ export function createAnnouncer (mapStatusRef) { }, CLEAR_DELAY) } - // Debounced announcer to group rapid events + // Debounced announcer to group rapid action events down to the latest one const debouncedAnnounce = debounce(setLiveRegion, DEBOUNCE_DELAY) + // Hold (or refresh) action priority so ambient messages yield for a window + const holdActionPriority = () => { + clearTimeout(actionHoldTimer) + actionHoldTimer = setTimeout(() => { + actionHoldTimer = null + }, ACTION_HOLD_DELAY) + } + // Public announce function - const announce = (msg, type = 'core') => { + const announce = (msg, kind = 'action') => { if (!msg) { return } - if (type === 'plugin') { - priorityLock = true + if (kind === 'ambient') { + // Yield to a recent user-action result rather than clobber it + if (actionHoldTimer) { + return + } setLiveRegion(msg) return } - if (priorityLock) { - // skip this one but release lock for next time - priorityLock = false - return - } - + // Action: latest deliberate user action wins and is always read + holdActionPriority() debouncedAnnounce(msg) } diff --git a/src/services/announcer.test.js b/src/services/announcer.test.js index ee2cd289b..391539d3f 100755 --- a/src/services/announcer.test.js +++ b/src/services/announcer.test.js @@ -18,7 +18,9 @@ describe('createAnnouncer', () => { it('does nothing if msg is falsy', () => { announce(null) expect(mapStatusRef.current.textContent).toBe('') - announce(undefined, 'core') + announce(undefined, 'action') + expect(mapStatusRef.current.textContent).toBe('') + announce('', 'ambient') expect(mapStatusRef.current.textContent).toBe('') }) @@ -31,7 +33,7 @@ describe('createAnnouncer', () => { }) it('returns early if mapStatusRef.current becomes null during setTimeout', () => { - announce('Test message', 'plugin') + announce('Test message', 'ambient') expect(mapStatusRef.current.textContent).toBe('') // cleared immediately mapStatusRef.current = null @@ -39,51 +41,77 @@ describe('createAnnouncer', () => { expect(() => jest.advanceTimersByTime(100)).not.toThrow() }) - it('sets textContent for a plugin message immediately', () => { - announce('Plugin message', 'plugin') + it('sets textContent for an ambient message immediately (after clear)', () => { + announce('Hint message', 'ambient') expect(mapStatusRef.current.textContent).toBe('') // cleared immediately jest.advanceTimersByTime(100) - expect(mapStatusRef.current.textContent).toBe('Plugin message') + expect(mapStatusRef.current.textContent).toBe('Hint message') }) - it('sets textContent for a core message after debounce + clear', () => { - announce('Core message', 'core') - expect(mapStatusRef.current.textContent).toBe('') // cleared immediately + it('sets textContent for an action message after debounce + clear', () => { + announce('Action message', 'action') + expect(mapStatusRef.current.textContent).toBe('') - // advance only CLEAR_DELAY should still be empty due to debounce + // advance only CLEAR_DELAY: still empty, debounce has not fired yet jest.advanceTimersByTime(100) expect(mapStatusRef.current.textContent).toBe('') - // advance DEBOUNCE_DELAY to trigger actual announcement + // advance DEBOUNCE_DELAY to trigger the actual announcement + jest.advanceTimersByTime(500) + expect(mapStatusRef.current.textContent).toBe('Action message') + }) + + it('defaults to an action message when no kind is given', () => { + announce('Default message') + jest.advanceTimersByTime(100) + // action is debounced, so nothing yet + expect(mapStatusRef.current.textContent).toBe('') jest.advanceTimersByTime(500) - expect(mapStatusRef.current.textContent).toBe('Core message') + expect(mapStatusRef.current.textContent).toBe('Default message') }) - it('respects priority lock when plugin message is sent', () => { - // plugin sets lock - announce('Plugin message', 'plugin') + it('lets an action always announce even after an ambient message (first-time fix)', () => { + // ambient hint shown first (e.g. keyboard-controls hint on focus) + announce('Keyboard hint', 'ambient') jest.advanceTimersByTime(100) - expect(mapStatusRef.current.textContent).toBe('Plugin message') - - // core message should be skipped due to lock - announce('Core message', 'core') - jest.advanceTimersByTime(600) // DEBOUNCE_DELAY + CLEAR_DELAY - expect(mapStatusRef.current.textContent).toBe('Plugin message') - - // next core message should work - announce('Next core message', 'core') - jest.advanceTimersByTime(100) // clear first - jest.advanceTimersByTime(500) // debounce - expect(mapStatusRef.current.textContent).toBe('Next core message') + expect(mapStatusRef.current.textContent).toBe('Keyboard hint') + + // user action must not be blocked by the earlier ambient message + announce('Reverse geocode result', 'action') + jest.advanceTimersByTime(600) // debounce + clear + expect(mapStatusRef.current.textContent).toBe('Reverse geocode result') }) - it('plugin messages always set priority lock and show immediately', () => { - announce('Plugin 1', 'plugin') + it('gives a recent action priority: an ambient message is skipped', () => { + announce('Map moved', 'action') + jest.advanceTimersByTime(600) // debounce + clear + expect(mapStatusRef.current.textContent).toBe('Map moved') + + // ambient arrives within the action hold window and must be ignored + announce('Keyboard hint', 'ambient') jest.advanceTimersByTime(100) - expect(mapStatusRef.current.textContent).toBe('Plugin 1') + expect(mapStatusRef.current.textContent).toBe('Map moved') + }) + + it('lets an ambient message through once the action hold window expires', () => { + announce('Map moved', 'action') + jest.advanceTimersByTime(600) + expect(mapStatusRef.current.textContent).toBe('Map moved') - announce('Plugin 2', 'plugin') + // wait out the full ACTION_HOLD_DELAY (1000ms from the action call) + jest.advanceTimersByTime(400) + + announce('Keyboard hint', 'ambient') jest.advanceTimersByTime(100) - expect(mapStatusRef.current.textContent).toBe('Plugin 2') + expect(mapStatusRef.current.textContent).toBe('Keyboard hint') + }) + + it('latest action wins: rapid actions announce only the most recent', () => { + announce('First', 'action') + jest.advanceTimersByTime(200) // still within debounce window + announce('Second', 'action') // resets the debounce + + jest.advanceTimersByTime(600) // let the second one land + expect(mapStatusRef.current.textContent).toBe('Second') }) }) diff --git a/src/services/hints.js b/src/services/hints.js index 194dad7dc..58b61474f 100644 --- a/src/services/hints.js +++ b/src/services/hints.js @@ -32,7 +32,7 @@ export function createHints (announce) { clearTimer() current = { html } notify() - announce(announceText ?? stripHtml(html), 'plugin') + announce(announceText ?? stripHtml(html), 'ambient') if (duration > 0) { timer = setTimeout(dismiss, duration) } diff --git a/src/services/hints.test.js b/src/services/hints.test.js index cf5303419..0509880b3 100644 --- a/src/services/hints.test.js +++ b/src/services/hints.test.js @@ -25,12 +25,12 @@ describe('show', () => { it('calls announce with stripped html', () => { hints.show('Enter to select') - expect(announce).toHaveBeenCalledWith('Enter to select', 'plugin') + expect(announce).toHaveBeenCalledWith('Enter to select', 'ambient') }) it('calls announce with custom text when announce option is provided', () => { hints.show('Alt+K help', { announce: 'Press Alt+K for keyboard controls' }) - expect(announce).toHaveBeenCalledWith('Press Alt+K for keyboard controls', 'plugin') + expect(announce).toHaveBeenCalledWith('Press Alt+K for keyboard controls', 'ambient') }) it('replaces an existing hint and resets the timer', () => {