Skip to content

fix: debounce list update announcements to prevent race conditions during rapid filtering #7745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mustafajw07 wants to merge 5 commits into
base: main
Choose a base branch
from
+38 −28
Open

fix: debounce list update announcements to prevent race conditions during rapid filtering #7745

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
737cd3b
fix: debounce list update announcements to prevent race conditions du…
mustafajw07 Apr 10, 2026
b6d17cb
fix: debounce list update announcements in useAnnouncements to preven…
mustafajw07 Apr 10, 2026
81610c9
Merge branch 'main' into feature/7607-A11y
mustafajw07 Apr 10, 2026
ae34e23
Merge branch 'main' into feature/7611-A11y
francinelucca Apr 10, 2026
5fa2176
refactor: update announcement delay to 300ms in useAnnouncements hook
mustafajw07 Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/use-announcements.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Debounce list update announcements in `useAnnouncements` to prevent race conditions during rapid filtering. This ensures screen readers don't overwhelm users with intermediate states during fast typing.
60 changes: 32 additions & 28 deletions packages/react/src/FilteredActionList/useAnnouncements.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,40 +105,44 @@ export const useAnnouncements = (
function announceListUpdates() {
if (isFirstRender) return // ignore on first render as announceInitialFocus will also announce

liveRegion?.clear() // clear previous announcements
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
}
Comment on lines +109 to +115
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@francinelucca francinelucca Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense, perhaps we should remove the delay for this function in lieu of the added debounce

Copy link
Copy Markdown
Author

@mustafajw07 mustafajw07 Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the changes in pr, please validate


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(', ')
if (usingRovingTabindex) {
const announcementText = `${items.length} item${items.length > 1 ? 's' : ''} available, ${selectedItems} selected.`

announce(announcementText, {
delayMs,
from: liveRegion ? liveRegion : undefined, // announce will create a liveRegion if it doesn't find one
from: liveRegion ? liveRegion : undefined,
})
})
}
} else {
// give @primer/behaviors a moment to update active-descendant
window.requestAnimationFrame(() => {
const activeItem = getItemWithActiveDescendant(listContainerRef, items)
Comment on lines +125 to +127
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The debounced callback schedules a requestAnimationFrame but the effect cleanup only clears the timeout. If items changes after the timeout fires but before the RAF runs, the RAF callback can still announce using stale items/active-descendant state, reintroducing the race this change is trying to prevent. Consider capturing the RAF id and cancelling it in the cleanup (e.g. store rafId outside the callbacks and call cancelAnimationFrame when cleaning up).

Copilot uses AI. Check for mistakes.
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)
},
[
announce,
Expand Down
Loading