fix: migrate live region to useAnnouncer#2267
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR updates Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20736aaf-604e-454d-9239-3f38ee2d4368
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
app/app.vueapp/pages/search.vuepackage.json
| () => ({ | ||
| 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 }) => { | ||
| 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 { |
There was a problem hiding this comment.
Watch the committed search state here, not the raw input.
Line 624 watches query.value, but the results come from committedQuery. When instant search is off, typing a new term can re-announce counts or “no results” for the previous result set. Lines 643 and 655 also read preferredPageSize.value and isRelevanceSort.value, but neither is in the source object, so page-size-only or sort-only changes can miss an announcement.
💡 Proposed fix
watch(
() => ({
rateLimited: isRateLimited.value,
searchStatus: status.value,
count: displayResults.value.length,
- searchQuery: query.value,
+ searchQuery: committedQuery.value,
mode: viewMode.value,
pagMode: paginationMode.value,
+ isRelevanceSort: isRelevanceSort.value,
+ pageSize: preferredPageSize.value,
total: effectiveTotal.value,
}),
- ({ rateLimited, searchStatus, count, searchQuery, mode, pagMode, total }) => {
+ ({ rateLimited, searchStatus, count, searchQuery, mode, pagMode, isRelevanceSort, pageSize, total }) => {
if (rateLimited) {
announcePolite($t('search.rate_limited'))
return
}
if (searchStatus === 'pending') {
cancelPendingAnnouncements()
return
}
if (count > 0) {
if (mode === 'table' || pagMode === 'paginated') {
- const pSize = Math.min(preferredPageSize.value, total)
+ const pSize = Math.min(pageSize, total)
announcePolite(
$t(
'filters.count.showing_paginated',
{
pageSize: pSize.toString(),
count: $n(total),
},
total,
),
)
- } else if (isRelevanceSort.value) {
+ } else if (isRelevanceSort) {
announcePolite(
$t(
'search.found_packages',
{ count: $n(visibleResults.value?.total ?? 0) },
visibleResults.value?.total ?? 0,
|
Did you close this because our Nuxt version isn't new enough (I think)? Or another reason? @RYGRIT |
@ghostdevv My failure to synchronize the code in a timely manner caused a conflict in the pnpm-lock file. After merging, this triggered all CI errors. Therefore, I plan to resubmit the PR. |
2 participants
🔗 Linked issue
follow up #1812
🧭 Context
nuxt/nuxt#34318
📚 Description