Skip to content

fix: migrate live region to useAnnouncer #2268

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

Draft
RYGRIT wants to merge 1 commit into
base: main
Choose a base branch
from
+73 −76
Draft

fix: migrate live region to useAnnouncer #2268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions app/app.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ if (import.meta.client) {

<AppHeader :show-logo="!isHomepage" />

<NuxtAnnouncer />

<NuxtRouteAnnouncer v-slot="{ message }">
{{ route.name === 'search' ? `${$t('search.title_packages')} - npmx` : message }}
</NuxtRouteAnnouncer>
Expand Down
147 changes: 71 additions & 76 deletions app/pages/search.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@
}
onBeforeUnmount(() => {
updateUrlPage.cancel()
cancelPendingAnnouncements()
})

// Update URL when page changes from scrolling
Expand Down Expand Up @@ -584,94 +585,92 @@
})

// -----------------------------------
// Live region debouncing logic
// Live region announcements
// -----------------------------------
const isMobile = useIsMobile()
const { polite } = useAnnouncer()

Check failure on line 591 in app/pages/search.vue

View workflow job for this annotation

GitHub Actions / 💪 Type check 

Cannot find name 'useAnnouncer'.

// Evaluate the text that should be announced to screen readers
const rawLiveRegionMessage = computed(() => {
if (isRateLimited.value) {
return $t('search.rate_limited')
}

// If status is pending, no update phrase needed yet
if (status.value === 'pending') {
return ''
}

if (visibleResults.value && displayResults.value.length > 0) {
if (viewMode.value === 'table' || paginationMode.value === 'paginated') {
const pSize = Math.min(preferredPageSize.value, effectiveTotal.value)

return $t(
'filters.count.showing_paginated',
{
pageSize: pSize.toString(),
count: $n(effectiveTotal.value),
},
effectiveTotal.value,
)
}

if (isRelevanceSort.value) {
return $t(
'search.found_packages',
{ count: $n(visibleResults.value.total) },
visibleResults.value.total,
)
}
const announcePoliteDesktop = debounce((message: string) => {
polite(message)
}, 250)

return $t(
'search.found_packages_sorted',
{ count: $n(effectiveTotal.value) },
effectiveTotal.value,
)
}
const announcePoliteMobile = debounce((message: string) => {
polite(message)
}, 700)

if (status.value === 'success' || status.value === 'error') {
if (displayResults.value.length === 0 && query.value) {
return $t('search.no_results', { query: query.value })
}
function announcePolite(message: string) {
if (isMobile.value) {
announcePoliteDesktop.cancel()
announcePoliteMobile(message)
return
}

return ''
})

const debouncedLiveRegionMessage = ref('')

const updateLiveRegionMobile = debounce((val: string) => {
debouncedLiveRegionMessage.value = val
}, 700)
announcePoliteMobile.cancel()
announcePoliteDesktop(message)
}

const updateLiveRegionDesktop = debounce((val: string) => {
debouncedLiveRegionMessage.value = val
}, 250)
function cancelPendingAnnouncements() {
announcePoliteDesktop.cancel()
announcePoliteMobile.cancel()
}
Comment on lines +612 to +615
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let me locate and examine the search.vue file
fd -t f "search.vue" --exec wc -l {} \;

Repository: npmx-dev/npmx.dev

Length of output: 125

🏁 Script executed:

# Read the specific lines mentioned in the review
cat -n app/pages/search.vue | sed -n '600,630p'

Repository: npmx-dev/npmx.dev

Length of output: 1069

🏁 Script executed:

# Search for where announcePoliteDesktop and announcePoliteMobile are defined/used
rg "announcePoliteDesktop|announcePoliteMobile" app/pages/search.vue -B 5 -A 5

Repository: npmx-dev/npmx.dev

Length of output: 770

🏁 Script executed:

# Check the entire file structure to understand the component
head -100 app/pages/search.vue

Repository: npmx-dev/npmx.dev

Length of output: 3001

🏁 Script executed:

# Find where cancelPendingAnnouncements is called
rg "cancelPendingAnnouncements" app/pages/search.vue -B 3 -A 3

Repository: npmx-dev/npmx.dev

Length of output: 645

🏁 Script executed:

# Look at the entire watch block that announces results
cat -n app/pages/search.vue | sed -n '618,750p'

Repository: npmx-dev/npmx.dev

Length of output: 5801

Ensure screen reader announcements are triggered for repeated identical messages.

When cancelPendingAnnouncements() cancels the debounces, subsequent calls to announcePolite() with identical message text will not produce a fresh announcer update. Since multiple searches can generate the same announcement text (e.g., two searches each finding 100 results), the announcer state needs to be reset to trigger the screen reader update. Either reset the announcer or vary the message before reusing it.


// Announce search results changes to screen readers
watch(
rawLiveRegionMessage,
newVal => {
if (!newVal) {
updateLiveRegionMobile.cancel()
updateLiveRegionDesktop.cancel()
debouncedLiveRegionMessage.value = ''
() => ({
rateLimited: isRateLimited.value,
searchStatus: status.value,
count: displayResults.value.length,
searchQuery: query.value,
mode: viewMode.value,
pagMode: paginationMode.value,
total: effectiveTotal.value,
}),
({ rateLimited, searchStatus, count, searchQuery, mode, pagMode, total }) => {

Check warning on line 628 in app/pages/search.vue

View workflow job for this annotation

GitHub Actions / 🤖 Autofix code

eslint(no-shadow) 

'searchQuery' is already declared in the upper scope.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Rename searchQuery to avoid shadowing the outer scope variable.

The static analysis tool correctly flags that searchQuery in the watcher's destructuring shadows the outer searchQuery from useGlobalSearch() on line 49. Rename this to something like currentQuery or snapshotQuery for clarity.

🛠️ Proposed fix 
-  ({ rateLimited, searchStatus, count, searchQuery, mode, pagMode, total }) => {
+  ({ rateLimited, searchStatus, count, searchQuery: currentQuery, mode, pagMode, total }) => {
     if (rateLimited) {
       announcePolite($t('search.rate_limited'))
       return
     }
     // ... rest of callback
     } else if (searchStatus === 'success' || searchStatus === 'error') {
-      if (searchQuery) {
-        announcePolite($t('search.no_results', { query: searchQuery }))
+      if (currentQuery) {
+        announcePolite($t('search.no_results', { query: currentQuery }))
       } else {
         cancelPendingAnnouncements()
       }
     }
🧰 Tools
🪛 GitHub Check: 🤖 Autofix code

[warning] 628-628: eslint(no-shadow)
'searchQuery' is already declared in the upper scope.

if (rateLimited) {
announcePolite($t('search.rate_limited'))
return
}

if (isMobile.value) {
updateLiveRegionDesktop.cancel()
updateLiveRegionMobile(newVal)
} else {
updateLiveRegionMobile.cancel()
updateLiveRegionDesktop(newVal)
// Don't announce while searching
if (searchStatus === 'pending') {
cancelPendingAnnouncements()
return
}

if (count > 0) {
if (mode === 'table' || pagMode === 'paginated') {
const pSize = Math.min(preferredPageSize.value, total)

announcePolite(
$t(
'filters.count.showing_paginated',
{
pageSize: pSize.toString(),
count: $n(total),
},
total,
),
)
} else if (isRelevanceSort.value) {
announcePolite(
$t(
'search.found_packages',
{ count: $n(visibleResults.value?.total ?? 0) },
visibleResults.value?.total ?? 0,
),
)
} else {
announcePolite($t('search.found_packages_sorted', { count: $n(total) }, total))
}
} else if (searchStatus === 'success' || searchStatus === 'error') {
if (searchQuery) {
announcePolite($t('search.no_results', { query: searchQuery }))
} else {
cancelPendingAnnouncements()
}
}
},
{ immediate: true },
)

onBeforeUnmount(() => {
updateLiveRegionMobile.cancel()
updateLiveRegionDesktop.cancel()
})
</script>

<template>
Expand Down Expand Up @@ -910,10 +909,6 @@
:package-scope="packageScope"
:can-publish-to-scope="canPublishToScope"
/>

<div role="status" class="sr-only">
{{ debouncedLiveRegionMessage }}
</div>
</main>
</template>

Expand Down
Loading