Add bank account filter to Search#90791
Conversation
|
Looks good from a design perspective 👍 |
|
@allgandalf lets do a C+ here. @Eskalifer1 would you mind reviewing? |
|
Sure, i will do it |
|
@allgandalf we just merged #90029, so there'll be some conflicts here |
|
@JS00001 good catch on the chip rendering the raw id. the advanced filters list row was using also merged main to clear the conflicts from #90029, see e423cd5. main renamed |
JS00001
left a comment
There was a problem hiding this comment.
Screen.Recording.2026-05-25.at.11.33.18.AM.mov
No longer works on mobile
| / policyName | ||
| / withdrawalId | ||
| / bankAccount | ||
| / bankAccountColumn |
There was a problem hiding this comment.
should this be bankAccountColumn? Should it not be bankAccount?
There was a problem hiding this comment.
Don't we want the bank account filter to support bank-account and bankAccount?
|
Hi @allgandalf Is there any way to reproduce these changes using the Alberta dataset? It seems like I've met all the prerequisites, but when I try to search, nothing comes up. Am I missing something? 2026-05-26.14.00.08.mov |
Eskalifer1
left a comment
There was a problem hiding this comment.
In “Recent Searches,” the bank's ID is displayed instead of its name
2026-05-26.14.40.34.mov
| type BankAccountSelectorProps = { | ||
| value: string[] | undefined; | ||
| onChange: (bankAccounts: string[]) => void; | ||
| }; | ||
|
|
||
| type BankAccountItem = { | ||
| text: string; | ||
| value: string; | ||
| leftElement: React.JSX.Element; | ||
| }; |
There was a problem hiding this comment.
Let's add comments using /** comment above it */ pattern
| const selectedBankAccounts = bankAccountItems.filter((item) => value.includes(item.value)); | ||
|
|
||
| return ( | ||
| <MultiSelect |
There was a problem hiding this comment.
We need to get the footer from the props and pass it here as footer={footer}.
Without this, we'll run into a problem where the Confirm button won't appear on the page when viewed on mobile devices.
| const selectedBankAccounts = bankAccountItems.filter((item) => value.includes(item.value)); | ||
|
|
||
| return ( | ||
| <MultiSelect |
There was a problem hiding this comment.
We need to get the autoFocus from the props and pass it here as autoFocus={autoFocus}.
Without this, we'll run into a problem where the input field gains focus on a desktop computer, which shouldn't happen.
| const selectedBankAccounts = bankAccountItems.filter((item) => value.includes(item.value)); | ||
|
|
||
| return ( | ||
| <MultiSelect |
There was a problem hiding this comment.
We need to get the selectionListTextInputStyle from the props and pass it here as selectionListTextInputStyle={selectionListTextInputStyle}.
Without this, the styles won't display correctly on desktop
| const selectedBankAccounts = bankAccountItems.filter((item) => value.includes(item.value)); | ||
|
|
||
| return ( | ||
| <MultiSelect |
There was a problem hiding this comment.
Same for selectionListStyle
| const selectedBankAccounts = bankAccountItems.filter((item) => value.includes(item.value)); | ||
|
|
||
| return ( | ||
| <MultiSelect |
There was a problem hiding this comment.
Let's also add searchPlaceholder here
| TAX_RATE: 'tax-rate', | ||
| CARD_ID: 'card', | ||
| FEED: 'feed', | ||
| BANK_ACCOUNT: 'bankAccount', |
There was a problem hiding this comment.
Why do we use camelCase here?
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import MultiSelect from './MultiSelect'; | ||
|
|
||
| type BankAccountSelectorProps = { |
There was a problem hiding this comment.
Let's add SearchFilterSelectionListProps & here
| var peg$e122 = peg$classExpectation([" ", "\t", "\n", "\r", "\xA0", ["a", "z"], ["A", "Z"]], false, false); | ||
| var peg$e123 = peg$classExpectation([","], false, false); | ||
| var peg$e124 = peg$classExpectation([" ", "\t", "\n", "\r", "\xA0", ","], false, false); | ||
| var peg$e29 = peg$literalExpectation("bankAccount", true); |
There was a problem hiding this comment.
I suspect we want to support both forms, just like with all the other search filters, so we need to support both bankAccount and bank-account.
I'm leaving a comment here, but this needs to be fixed everywhere, since all the other filters work the same way.
| {!isLoadingOnyxValue(searchAdvancedFiltersFormResult) && ( | ||
| <SearchMultipleSelectionPicker | ||
| items={bankAccountItems} | ||
| initiallySelectedItems={initiallySelectedItems} | ||
| onSaveSelection={onSaveSelection} | ||
| shouldShowTextInput={bankAccountItems.length >= CONST.STANDARD_LIST_ITEM_LIMIT} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
I think we need to add a loading indicator here. After all, it might confuse the user if they go to the page and don't see anything at first—just a blank screen.
We can use a similar pattern to the one used in SearchFiltersCardPage
Eskalifer1
left a comment
There was a problem hiding this comment.
This may be outside the scope of this PR, However, when the user enters an ID, the dropdown menu displays the same bank that the user just entered by ID (bug 1). After the user presses Enter, the value in the search results is updated correctly, but the dropdown menu still shows only one bank—the one that was selected second (bug 2).
2026-05-26.16.00.53.mov
Eskalifer1
left a comment
There was a problem hiding this comment.
What should we do about the Bank Account column? As far as I understand, we're adding a completely new filter, but the Bank Account column already exists.
|
@codex review |
|
Let's merge main cc: @allgandalf |
|
@MelvinBot We're adding a new filter in this PR. Please review the implementation and let us know if there are any issues, edge cases, or improvements we should address. Also, please look through the history of recently added filters and identify any regressions, missed updates, or recurring issues that happened during previous filter implementations. Then verify whether this PR could introduce any of the same problems, or if there are any related areas we may have overlooked this time. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94e8c346be
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| bankAccount | ||
| = "bankAccount"i { return "bankAccount"; } |
There was a problem hiding this comment.
Accept kebab-case bank-account filter key
Extend the bankAccount key rule to also accept bank-account (like other multi-word filters) so typed queries and deep links using the standard kebab-case style work. As written, bank-account:42 is parsed as a generic keyword expression instead of a bank-account filter, so users who type the hyphenated form won’t actually apply this new filter.
Useful? React with 👍 / 👎.

Explanation of Change
Adds a new
bankAccountfilter on the expense Search advanced filters page so users with multiple settlement bank accounts can scope results to a single bank. Picker source is the user'sbankAccountList, multi-select, displayed as${bankName} xx${last4}to match the existing "Bank account" column on withdrawalID grouping. Filter row is gated to expense type and only renders when the user has at least one bank account. Backend half lands in Expensify/Auth#21649.Fixed Issues
$ #88385
PROPOSAL: #88385 (comment)
Tests
Preconditions (one-time setup):
./script/clitools.sh generator:bankaccount(choose Withdrawal, validated, upgraded). Run twiceSeed data:
./script/clitools.sh generator:report(answer Y to submit and approve)./script/clitools.sh generator:reportFilter checks:
${bankName} xx${last4}with the bank iconbankAccount:<id>(or<id1>,<id2>) and reloading the page rehydrates the selectionbank-account:in the autocomplete bar and confirm the user-friendly suggestions for each bank account appear, alphabetically sortedOffline tests
The filter UI reads from Onyx
bankAccountListso the picker, chip rendering, and autocomplete substitution all work offline once the data has been cached. Saving the filter queues a Search request that will be sent on reconnect. Steps:bankAccountfilterQA Steps
Preconditions:
Steps:
${bankName} xx${last4}with the bank iconbank-account:in the autocomplete bar and confirm the user-friendly suggestions appear, alphabetically sortedPR 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.Screen.Recording.2026-05-23.at.3.31.33.PM.mov