fix: debounce list update announcements to prevent race conditions during rapid filtering#7745
fix: debounce list update announcements to prevent race conditions during rapid filtering#7745mustafajw07 wants to merge 5 commits intoprimer:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 5fa2176 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
mustafajw07
had a problem deploying
to
github-pages
GitHub Actions
Failure
There was a problem hiding this comment.
Pull request overview
Updates FilteredActionList/SelectPanel screen reader announcements to avoid excessive, competing list update announcements during rapid filtering by introducing debouncing in useAnnouncements.
Changes:
- Debounces
announceListUpdatesinuseAnnouncementsusingsetTimeoutwith effect cleanup. - Adds a changeset for a patch release describing the accessibility fix.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/FilteredActionList/useAnnouncements.tsx | Wrapes list update announcements in a timeout and adds cleanup to reduce rapid-fire announcements. |
| .changeset/use-announcements.md | Patch changeset documenting the debounced list announcements behavior change. |
Copilot's findings
Comments suppressed due to low confidence (3)
packages/react/src/FilteredActionList/useAnnouncements.tsx:142
- The cleanup only clears the
setTimeout, but in theactive-descendantbranch arequestAnimationFramecallback is scheduled after the timeout fires. If the effect re-runs/unmounts after the timeout callback executes but before the rAF runs, the stale rAF can still announce outdated state. Consider tracking the rAF id (and callingcancelAnimationFramein cleanup) or using acancelledflag checked in both callbacks.
// give @primer/behaviors a moment to update active-descendant
window.requestAnimationFrame(() => {
const activeItem = getItemWithActiveDescendant(listContainerRef, items)
if (!activeItem) return
const {index, text, selected} = activeItem
const announcementText = [
`List updated`,
`Focused item: ${text}`,
`${selected ? 'selected' : 'not selected'}`,
`${index + 1} of ${items.length}`,
].join(', ')
announce(announcementText, {
delayMs,
from: liveRegion ? liveRegion : undefined, // announce will create a liveRegion if it doesn't find one
})
})
}
packages/react/src/FilteredActionList/useAnnouncements.tsx:113
messageis optional, but this branch interpolatesmessage?.title/message?.descriptioninto a template string. Ifmessageis undefined, screen readers will announce "undefined. undefined" whenitems.length === 0 && !loading. Please guard onmessagebeing present (or provide a fallback string) before announcing.
if (items.length === 0 && !loading) {
announce(`${message?.title}. ${message?.description}`, {delayMs})
return
packages/react/src/FilteredActionList/useAnnouncements.tsx:145
- This PR introduces debouncing semantics for list update announcements, but existing tests only assert the final message after typing. Please add a test that simulates rapid filtering (e.g., multiple quick
typecalls or fake timers) and asserts that intermediate announcements are not emitted (only the final state is announced).
const timeoutId = window.setTimeout(() => {
liveRegion?.clear() // clear previous announcements
if (items.length === 0 && !loading) {
announce(`${message?.title}. ${message?.description}`, {delayMs})
return
}
if (usingRovingTabindex) {
const announcementText = `${items.length} item${items.length > 1 ? 's' : ''} available, ${selectedItems} selected.`
announce(announcementText, {
delayMs,
from: liveRegion ? liveRegion : undefined,
})
} else {
// give @primer/behaviors a moment to update active-descendant
window.requestAnimationFrame(() => {
const activeItem = getItemWithActiveDescendant(listContainerRef, items)
if (!activeItem) return
const {index, text, selected} = activeItem
const announcementText = [
`List updated`,
`Focused item: ${text}`,
`${selected ? 'selected' : 'not selected'}`,
`${index + 1} of ${items.length}`,
].join(', ')
announce(announcementText, {
delayMs,
from: liveRegion ? liveRegion : undefined, // announce will create a liveRegion if it doesn't find one
})
})
}
}, delayMs)
return () => window.clearTimeout(timeoutId)
- Files reviewed: 2/2 changed files
- Comments generated: 1
| const timeoutId = window.setTimeout(() => { | ||
| liveRegion?.clear() // clear previous announcements | ||
|
|
||
| if (items.length === 0 && !loading) { | ||
| announce(`${message?.title}. ${message?.description}`, {delayMs}) | ||
| return | ||
| } | ||
| if (items.length === 0 && !loading) { | ||
| announce(`${message?.title}. ${message?.description}`, {delayMs}) | ||
| return | ||
| } |
There was a problem hiding this comment.
delayMs is already passed to announce(...) to intentionally delay announcements (see file-level comment). Using the same delayMs as the setTimeout debounce means list updates are delayed twice (timeout + announce delay), likely ~1000ms instead of the previous 500ms. Consider separating debounce timing from the announcement delay (e.g., debounceMs for the outer timeout, and keep delayMs only for announce, or vice-versa).
This issue also appears in the following locations of the same file:
- line 108
- line 111
- line 124
There was a problem hiding this comment.
I think this makes sense, perhaps we should remove the delay for this function in lieu of the added debounce
There was a problem hiding this comment.
Updated the changes in pr, please validate
francinelucca
had a problem deploying
to
github-pages
GitHub Actions
Failure
| const timeoutId = window.setTimeout(() => { | ||
| liveRegion?.clear() // clear previous announcements | ||
|
|
||
| if (items.length === 0 && !loading) { | ||
| announce(`${message?.title}. ${message?.description}`, {delayMs}) | ||
| return | ||
| } | ||
| if (items.length === 0 && !loading) { | ||
| announce(`${message?.title}. ${message?.description}`, {delayMs}) | ||
| return | ||
| } |
There was a problem hiding this comment.
I think this makes sense, perhaps we should remove the delay for this function in lieu of the added debounce
Closes #7611
Overview
This PR fixes an accessibility issue where screen reader announcements were firing excessively due to a lack of debouncing during rapid list filtering. By wrapping the
announceListUpdateslogic inuseAnnouncementswith asetTimeoutand providing a cleanup function, list announcements are now properly debounced, preventing race conditions. This ensures users using assistive technologies aren't overwhelmed with stale or conflicting announcements when typing quickly in aSelectPanelorFilteredActionList.Changelog
New
(None)
Changed
useAnnouncementsto debounce screen reader announcements natively by wrapping the list updates in asetTimeout.clearTimeoutcleanup function inside ofannounceListUpdatesto clear pending timeout effects.Removed
(None)
Rollout strategy
Testing & Reviewing
useAnnouncements(e.g.FilteredActionListorSelectPanel).Merge checklist