feat(a11y): spotlight announcement #6795
Conversation
WalkthroughThis PR adds singular and plural i18n keys for search results across multiple locales, introduces a MAX_SEARCH_RESULTS cap in search logic, and implements accessibility announcements announcing the number of search results (or none) in the Rooms list search hook. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Search UI
participant Search as Search Logic
participant A11y as Accessibility
User->>UI: Enter query & tap Search
UI->>Search: Trigger search request
Search->>Search: Accumulate local results (stop at MAX_SEARCH_RESULTS=20)
alt reached MAX_SEARCH_RESULTS
Search-->>UI: Return partial results (count up to 20)
else completed search
Search-->>UI: Return full results (count)
end
UI->>A11y: announceSearchResultsForAccessibility(count)
alt count < 1
A11y-->>User: Announce "No_results_found"
else count == 1
A11y-->>User: Announce "One_result_found"
else count > 1
A11y-->>User: Announce "Search_Results_found" (with count)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
experimental_ios_build
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
upload_experimental_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.67.0.107710 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNSbNPwagDruErKMs7M_vdBjYYhB2GHiCnTEPWjerssEFhgowFkZ36t7PJ99bM_DToMzaItJ3s3JRbfkmigV |
|
iOS Build Available Rocket.Chat Experimental 4.67.0.107711 |
OtavioStasiak
changed the title
OtavioStasiak
changed the title
OtavioStasiak
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
app/i18n/locales/en.json (1)
762-762: Optional: Add copy for capped result sets.If search short-circuits at MAX_SEARCH_RESULTS, consider an extra key to avoid misleading counts, e.g., "Search_Results_shown_limited": "{{count}} results shown. More available."
Confirm whether app/lib/methods/search.ts enforces a cap and whether announcements should reflect that.
app/i18n/locales/it.json (1)
410-410: LGTM; optional punctuation consistency.Both translations read well. Consider aligning trailing punctuation with nearby “No_results_found” (no period) for consistency across result messages.
Also applies to: 521-521
app/i18n/locales/ru.json (1)
500-500: Russian plural forms need locale-aware handling.
- “Найден один результат.” is fine.
- “Найдено {{count}} результатов.” is incorrect for 2–4 (should be “результата”). Recommend switching to pluralization keys supported by your i18n layer (e.g., i18next) instead of a single catch‑all plural.
Option A (preferred, locale-aware keys):
- Search_Results_found_one: Найден {{count}} результат.
- Search_Results_found_few: Найдено {{count}} результата.
- Search_Results_found_many: Найдено {{count}} результатов.
- Search_Results_found_other: Найдено {{count}} результата.
Then call t('Search_Results_found', { count }). If the stack can’t do this now, consider deferring shipping to Russian until pluralization is wired.
Would you confirm which i18n library/version is in use and whether plural rules are enabled for ru?Also applies to: 625-625
app/i18n/locales/hu.json (1)
545-545: Optional style tightening.Current strings are correct. For brevity/consistency with common UI phrasing, consider:
- One_result_found: “1 találat.”
- Search_Results_found: “{{count}} találat.”
No behavior change required.
Also applies to: 687-687
app/lib/methods/search.ts (1)
15-15: MAX_SEARCH_RESULTS currently doesn’t actually cap or meaningfully short‑circuit resultsRight now:
MAX_SEARCH_RESULTSis set to 20, but the only usage is theif (data.length >= MAX_SEARCH_RESULTS) return data;.- The remote
spotlightcall only happens whensearchText && localSearchData.length < 7, sodata.lengthis always< 7in the only path where remote search runs.- The only scenarios where
data.lengthcould be>= 20are ones where remote search is already skipped (e.g., empty search text with many local suggestions), so behavior is effectively unchanged and the constant doesn’t enforce a global cap.If the intent is to genuinely cap search results and avoid overloading screen readers, consider:
- Returning at most
MAX_SEARCH_RESULTSitems from the function, and- Optionally skipping the remote call once that cap is reached.
One minimal adjustment that enforces a hard cap without changing call patterns too much:
- const data: TSearch[] = localSearchData; - - if (data.length >= MAX_SEARCH_RESULTS) { - return data; - } + const data: TSearch[] = localSearchData; @@ - debounce = null; - return data; + debounce = null; + return data.slice(0, MAX_SEARCH_RESULTS); @@ - return data; + return data.slice(0, MAX_SEARCH_RESULTS);If you also want to avoid the remote
spotlightcall when we already have enough local results, you could tighten the condition around the remote call instead (or in addition), but the main gap right now is that the constant doesn’t actually influence the returned size.Also applies to: 139-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (27)
app/i18n/locales/ar.json(2 hunks)app/i18n/locales/bn-IN.json(2 hunks)app/i18n/locales/cs.json(2 hunks)app/i18n/locales/de.json(2 hunks)app/i18n/locales/en.json(2 hunks)app/i18n/locales/es.json(2 hunks)app/i18n/locales/fi.json(2 hunks)app/i18n/locales/fr.json(2 hunks)app/i18n/locales/hi-IN.json(2 hunks)app/i18n/locales/hu.json(2 hunks)app/i18n/locales/it.json(2 hunks)app/i18n/locales/ja.json(2 hunks)app/i18n/locales/nl.json(2 hunks)app/i18n/locales/nn.json(2 hunks)app/i18n/locales/no.json(2 hunks)app/i18n/locales/pt-BR.json(2 hunks)app/i18n/locales/pt-PT.json(2 hunks)app/i18n/locales/ru.json(2 hunks)app/i18n/locales/sl-SI.json(2 hunks)app/i18n/locales/sv.json(2 hunks)app/i18n/locales/ta-IN.json(2 hunks)app/i18n/locales/te-IN.json(2 hunks)app/i18n/locales/tr.json(2 hunks)app/i18n/locales/zh-CN.json(2 hunks)app/i18n/locales/zh-TW.json(2 hunks)app/lib/methods/search.ts(2 hunks)app/views/RoomsListView/hooks/useSearch.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/views/RoomsListView/hooks/useSearch.ts (3)
app/lib/methods/search.ts (1)
search(116-190)app/lib/methods/helpers/debounce.ts (1)
useDebounce(27-29)app/containers/RoomItem/interfaces.ts (1)
IRoomItem(166-169)
🔇 Additional comments (18)
app/i18n/locales/hi-IN.json (1)
544-544: LGTM for Hindi strings.Singular/plural read naturally and placeholders are correct.
Also applies to: 686-686
app/i18n/locales/pt-PT.json (1)
354-354: LGTM for pt-PT.Copy is idiomatic; plural uses {{count}} as expected.
Also applies to: 429-429
app/i18n/locales/es.json (1)
269-269: LGTM for Spanish.Strings are clear; variables are correct.
Also applies to: 343-343
app/i18n/locales/zh-CN.json (1)
376-376: LGTM for zh-CN.Punctuation and {{count}} placeholder are correct.
Also applies to: 485-485
app/i18n/locales/ja.json (1)
348-348: LGTM for Japanese.Natural phrasing; singular without vars, plural with {{count}}.
Also applies to: 431-431
app/i18n/locales/no.json (1)
572-572: LGTM for Norwegian Bokmål.Correct forms; {{count}} present in plural.
Also applies to: 722-722
app/i18n/locales/en.json (1)
604-604: LGTM for English.Keys match intended usage (singular vs plural with {{count}}).
Also applies to: 762-762
app/i18n/locales/fr.json (1)
470-470: LGTM.Accurate French and correct placeholder usage.
Also applies to: 592-592
app/i18n/locales/nl.json (1)
470-470: LGTM.Correct Dutch wording and placeholder usage.
Also applies to: 592-592
app/i18n/locales/sv.json (1)
509-509: Search result translations look consistent and correctKey names match existing conventions, the
{{count}}placeholder is correctly used, and the wording aligns with the existingNo_results_foundstring. No issues spotted.Also applies to: 647-647
app/i18n/locales/bn-IN.json (1)
544-544: Bengali search-result strings are correctly wiredBoth keys follow existing naming, use
{{count}}correctly, and read consistently with the existing “no results” message. Nothing to change here.Also applies to: 686-686
app/i18n/locales/cs.json (1)
582-582: Czech result messages integrate cleanlyNew keys follow the existing i18n pattern, with correct
{{count}}handling and natural phrasing for singular/plural summaries. Looks good.Also applies to: 734-734
app/i18n/locales/zh-TW.json (1)
393-393: Traditional Chinese search-result strings are correctKey names, use of the
{{count}}placeholder, and wording all match existing zh-TW style and the “no results” message. No issues.Also applies to: 503-503
app/i18n/locales/fi.json (1)
510-510: Finnish localization for search results is consistentBoth messages are grammatically fine, reuse the
{{count}}token correctly, and align with the existing “no results” copy. All good.Also applies to: 648-648
app/i18n/locales/de.json (1)
533-533: German singular/plural search messages are well-formedThe new keys follow existing naming, German phrasing is natural, and
{{count}}is correctly interpolated. No adjustments needed.Also applies to: 673-673
app/i18n/locales/pt-BR.json (1)
592-592: PT‑BR result-count translations and placeholders look good“Um resultado encontrado.” and “{{count}} resultados encontrados.” are natural Brazilian Portuguese and correctly use the
{{count}}placeholder, matching howSearch_Results_foundis consumed inuseSearch.Also applies to: 743-743
app/i18n/locales/tr.json (1)
393-393: Turkish search‑result translations are correct and consistent“Bir sonuç bulundu.” and “{{count}} sonuç bulundu.” are natural Turkish, and the
{{count}}placeholder matches how the hook passescountintoSearch_Results_found.Also applies to: 504-504
app/views/RoomsListView/hooks/useSearch.ts (1)
2-2: A11y announcements for search results are wired correctlyThe new
announceSearchResultsForAccessibilityhelper usesAccessibilityInfo.announceForAccessibilitywith the localizedNo_results_found,One_result_found, andSearch_Results_foundmessages, and is invoked once per debounced search completion. This cleanly covers 0/1/many cases without impacting existing search behavior.Also applies to: 7-7, 63-71, 78-78
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
OtavioStasiak
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
OtavioStasiak
requested a deployment
to
official_android_build
GitHub Actions
Waiting
OtavioStasiak
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
Proposed changes
Issue(s)
https://rocketchat.atlassian.net/browse/MA-121
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Bug Fixes