Implement stable two-section chat switcher results (local + server)#91282
Implement stable two-section chat switcher results (local + server)#91282MelvinBot wants to merge 27 commits into
Conversation
Split the chat switcher into two fixed sections when a search query is active: 1. "Recent chats" (top) - local/cached results rendered immediately with frozen rank 2. "Search results" (bottom) - server-introduced results shown with loading skeleton Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 920eb98a..f4cba3fb 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -7997,7 +7997,7 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
withdrawalType: 'Auszahlungsart',
recentSearches: 'Letzte Suchen',
recentChats: 'Neueste Chats',
- serverResults: 'Search results',
+ serverResults: 'Suchergebnisse',
searchIn: 'Suchen in',
askConcierge: (message: string) => `Frage Concierge „${message}“`,
searchPlaceholder: 'Nach etwas suchen...',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index b8b54dd9..dab2a75e 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -8026,7 +8026,7 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
withdrawalType: 'Type de retrait',
recentSearches: 'Recherches récentes',
recentChats: 'Discussions récentes',
- serverResults: 'Search results',
+ serverResults: 'Résultats de recherche',
searchIn: 'Rechercher dans',
askConcierge: (message: string) => `Demander à Concierge « ${message} »`,
searchPlaceholder: 'Rechercher quelque chose...',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 3884f5ed..badbba0c 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -7989,7 +7989,7 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`,
withdrawalType: 'Tipo di prelievo',
recentSearches: 'Ricerche recenti',
recentChats: 'Chat recenti',
- serverResults: 'Search results',
+ serverResults: 'Risultati di ricerca',
searchIn: 'Cerca in',
askConcierge: (message: string) => `Chiedi a Concierge “${message}”`,
searchPlaceholder: 'Cerca qualcosa...',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index ab403741..b8bfa568 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -7887,7 +7887,7 @@ ${reportName}
withdrawalType: '出金の種類',
recentSearches: '最近の検索',
recentChats: '最近のチャット',
- serverResults: 'Search results',
+ serverResults: '検索結果',
searchIn: '検索対象',
askConcierge: (message: string) => `Concierge に「${message}」と聞く`,
searchPlaceholder: '何かを検索...',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index f92f1c62..179b8cec 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -7947,7 +7947,7 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`,
withdrawalType: 'Type opname',
recentSearches: 'Recente zoekopdrachten',
recentChats: 'Recente chats',
- serverResults: 'Search results',
+ serverResults: 'Zoekresultaten',
searchIn: 'Zoeken in',
askConcierge: (message: string) => `Vraag Concierge: “${message}”`,
searchPlaceholder: 'Zoek iets...',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 54e1294b..9d2454b3 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -7951,7 +7951,7 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`,
withdrawalType: 'Typ wypłaty',
recentSearches: 'Ostatnie wyszukiwania',
recentChats: 'Ostatnie czaty',
- serverResults: 'Search results',
+ serverResults: 'Wyniki wyszukiwania',
searchIn: 'Szukaj w',
askConcierge: (message: string) => `Zapytaj Concierge: „${message}”`,
searchPlaceholder: 'Wyszukaj coś...',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 17319080..f1001c77 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -7936,7 +7936,7 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`,
withdrawalType: 'Tipo de saque',
recentSearches: 'Pesquisas recentes',
recentChats: 'Chats recentes',
- serverResults: 'Search results',
+ serverResults: 'Resultados da pesquisa',
searchIn: 'Pesquisar em',
askConcierge: (message: string) => `Perguntar ao Concierge “${message}”`,
searchPlaceholder: 'Pesquisar algo...',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 74694c89..a864c2ef 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -7748,7 +7748,7 @@ ${reportName}
withdrawalType: '提款类型',
recentSearches: '最近搜索',
recentChats: '最近聊天',
- serverResults: 'Search results',
+ serverResults: '搜索结果',
searchIn: '搜索范围',
askConcierge: (message: string) => `询问 Concierge“${message}”`,
searchPlaceholder: '搜索内容...',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
All CI checks are passing — there are no code-related failures to fix. Results summary:
No code fix needed. |
|
@MelvinBot pull main |
…onChatSwitcherV2 Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Merged main into this branch. The branch is now up to date. |
|
@MelvinBot you wrongly copied translations. See #91282 (comment) |
Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Fixed the translations for |
| sectionIndex: sectionIndex++, | ||
| customHeader: ( | ||
| }; | ||
| if (isSearchingForReports && serverRows.length === 0) { |
There was a problem hiding this comment.
@MelvinBot when searching for reports and no result, just show simple loading indicator instead of complex skeleton view.
See this design:
And section header with customHeader is never rendered when data length is 0.
Replace OptionsListSkeletonView with ActivityIndicator for the server results loading state. Also fix useFlattenedSections to render customHeader even when section data is empty. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Done. Two changes pushed:
|
The useMemo at line 429 references theme.spinner but theme was not included in the dependency array, causing a React Compiler compliance regression. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Fixed the React Compiler Compliance failure: added missing |
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
@MelvinBot Won't this cause regression in other places? |
|
No, regression risk is low. Only 3 consumers pass
One minor note: |
| @@ -1,5 +1,6 @@ | |||
| import type {ForwardedRef, RefObject} from 'react'; | |||
| import React, {useEffect, useMemo, useRef, useState} from 'react'; | |||
| import {ActivityIndicator} from 'react-native'; | |||
There was a problem hiding this comment.
@MelvinBot this should be imported from @components
There was a problem hiding this comment.
Fixed — switched to the @components/ActivityIndicator wrapper and added reasonAttributes for telemetry.
Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
| if (isSearchingForReports && serverRows.length === 0) { | ||
| bottomSection.customHeader = ( | ||
| <ActivityIndicator | ||
| size="large" |
There was a problem hiding this comment.
| size="large" |
|
@MelvinBot remove bottom loading indicator since we already have loading indicator in search input. 
|
The search input already has its own loading indicator, so the bottom ActivityIndicator in the server results section is redundant. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Removed the bottom loading indicator (ActivityIndicator) from the server results section. The search input already has its own loading indicator, so this was redundant. |
mkhutornyi
left a comment
mkhutornyi
left a comment
There was a problem hiding this comment.
@MelvinBot fix failing checks
| const sectionDataLength = section.data?.length ?? 0; | ||
| itemsTotalCount += sectionDataLength; | ||
| const hasHeader = sectionDataLength > 0 && (section.customHeader ?? section.title); | ||
| const hasHeader = (sectionDataLength > 0 || section.customHeader) && (section.customHeader ?? section.title); |
There was a problem hiding this comment.
@MelvinBot no longer need this change as loading indicator was removed
There was a problem hiding this comment.
Reverted — restored the original hasHeader condition since the loading indicator customHeader on empty sections is gone.
…eList
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
|
Fixed the failing ESLint and React Compiler checks: removed unused |
- Fix unsafe return type in useFilteredOptions mock - Replace `require<typeof import(...)>` with `jest.requireActual<typeof Module>` - Remove non-existent `categoryOptions` property from Options mock - Remove unused variable assignments (aliceEl, bobEl, charlieEl) - Remove unused `getKeywordQueryWithCurrentSearchContext` import lost during merge Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Merged
|
|
making a build so the @Expensify/design team can test the ux of this change |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
I feel like this is testing well to my eye, but would love some more 👀 from @Expensify/design. Also, we're missing the little headers from the original mock, and I kinda think those are important so you somewhat understand why the results are split. 
|
I intentionally removed "Recent chats" title when typed something to match production behavior. Should we restore it back? staging.mov |
|
Let's see what the rest of the @Expensify/design team thinks. |
Yeah, this is my general sentiment as well but I don't feel strongly. But it does seem like the whole point of this was to have separate sections, each denoted by a small heading. |
|
If we add "Recent chats" title, where does non-existing user go to? |
|
Not sure I'm totally following. Mind explaining a bit more? We added the title I'm down to use different words but I do think it's important to have the two sections delineated. |
|
This is non-existing user with default avatar 
Is that fine? |
Agree. The latest looks fine but could we get a video? I think the crucial thing here is to see how it changes as you search |
| localRows.sort((a, b) => (frozenLocalRank.get(a.keyForList ?? '') ?? 0) - (frozenLocalRank.get(b.keyForList ?? '') ?? 0)); | ||
|
|
||
| if (localRows.length > 0 || !isLoadingOptions) { | ||
| pushSection({title: undefined, data: localRows, sectionIndex: sectionIndex++}); |
There was a problem hiding this comment.
| pushSection({title: undefined, data: localRows, sectionIndex: sectionIndex++}); | |
| pushSection({title: translate('search.recentChats'), data: localRows, sectionIndex: sectionIndex++}); |
@MelvinBot ^ and pull main
There was a problem hiding this comment.
Applied the suggestion (added recentChats title to the local section) and merged main.
Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Please generate new build and do final test with real data. |
|
Building now 🚀 |
|
🚧 @shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Seems to work fairly well to me? |
|
@MelvinBot fix failing test and pull main |
…onChatSwitcherV2 Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
The local results section in the two-section chat switcher should not display a section title when a search query is active. Only the server results section should show a 'Search results' header. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Fixed the failing test and merged
|
|
@MelvinBot you wrongly fixed. Revert 3486450 and instead fix unit tests, not in SearchAutocompleteList.tsx |
Reverts the component change from 3486450 that removed the "Recent chats" title from the local section. Instead, updates the unit tests to match the intended behavior: the "Recent chats" header stays visible for the local section even when a search query is active. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Reverted the |
|
I'm getting a bit of this weird autoscroll when typing: CleanShot.2026-05-27.at.19.30.29.mp4Feels like the scroll position shouldn't change but rather just the content to change |
Autoscroll also happens in production. We can fix this separately if it's bug. Otherwise looks good? autoscroll.mov |
8 participants
Explanation of Change
The chat switcher (Cmd+K / search) currently renders all matches in a single section. When
SearchForReportsreturns, the entire list is re-ranked, causing rows to shift under the cursor or keyboard highlight — users accidentally open the wrong chat.This PR splits the chat switcher into two fixed sections when a search query is active:
"Recent chats" (top) — local/cached results rendered immediately. A frozen rank map (
frozenLocalRank) captures the order of locally-known results at the moment the query changes. This map is stored in React state (useState/setState) and is only recomputed whenautocompleteQueryValuechanges, so server-driven Onyx writes that updaterecentReportsOptionsdo not alter the already-captured snapshot. The top section's rows are sorted by their captured rank, ensuring the order cannot change when server data arrives."Search results" (bottom) — server-introduced results that were not in the local snapshot. While
isSearchingForReportsis true and no server results have arrived yet, a loading skeleton is shown. Only this section may reorder when new API data arrives.When the query is empty, the existing single "Recent chats" section behaviour is preserved unchanged.
Fixed Issues
$ #88333
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.AI Tests
npm run typecheck-tsgonpx prettier --write(changed files)npm test -- --testPathPattern="Search"Screenshots/Videos
Android: Native
Screenshots will be added in the reviewer checklist.
Android: mWeb Chrome
Screenshots will be added in the reviewer checklist.
iOS: Native
Screenshots will be added in the reviewer checklist.
iOS: mWeb Safari
Screenshots will be added in the reviewer checklist.
MacOS: Chrome / Safari
Screenshots will be added in the reviewer checklist.