fix: Restore all matching items selected label - #99579
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@mkhutornyi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
garrettmknight
left a comment
There was a problem hiding this comment.
Looks good from a product perspective.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
ReviewThe core change is correct — for 1. The button spins and is disabled while showing the all-matching label
Why it's disabled, and suggested fix
const isAllMatchingItemsCountLoading =
!shouldShowAllMatchingItemsSelected && areAllMatchingItemsSelected && typeof allMatchingItemsCount !== 'number' && !isOffline && !hasSearchErrors;This is inherited from #95902 rather than introduced here, but that PR needed the count for the label in more cases than this one does. Since you're touching this branch anyway, dropping the pointless wait is cheap. 2.
|
| Step | Status | Observation |
|---|---|---|
| 1. Navigate to Spend > Expenses | ✅ pass | Expenses tab loaded, showing the "No expenses yet" empty state |
| 2. Open bulk-selection menu, choose "Select all" | No rows and no select-all checkbox render on an empty list | |
| 3. Button shows "All matching items selected" | No bulk actions button exists | |
| 4. Uncheck one expense | No expenses to uncheck | |
| 5. Button shows exact selected count | Not reachable | |
| 6. Recheck that expense | Not reachable | |
| 7. Button returns to all-matching label | Not reachable | |
| 8. "Select all on this page" | Not reachable | |
| 9. Button shows numeric count | Not reachable | |
| 10. Spend > Reports unchanged | Reports tab also showed "No reports yet" | |
| 11. No JS console errors | ✅ pass | No error banners or broken layout across the navigation |
Evidence (2)
Spend > Expenses empty state - no bulk-selection UI
Spend > Reports empty state - no bulk-selection UI
Regression huntNo functional regression found in the changed code. The label logic is sound and self-contained: 1. The new label is invisible when it first appears
Not a regression — const isAllMatchingItemsCountLoading =
!shouldShowAllMatchingItemsSelected && areAllMatchingItemsSelected && typeof allMatchingItemsCount !== 'number' && !isOffline && !hasSearchErrors;Chain
2. The label now leaks a pre-existing selection-state bugGating on Repro: grouped view ( The rendered number is the same as on Traced code path (static analysis, Jest could not be run in this environment)
Everything else I checked is symmetric and safe: flat row uncheck/recheck, group uncheck/recheck while still collapsed, "Select all on this page" (clears the flag in the same commit), and query/filter/tab changes ( I wrote a Jest case to confirm step 4 empirically but could not execute Jest here, so this is a code trace, not a test run. Worth adding as a real test in Two lower-severity variants flip the label the other way — reconcile pruning an exclusion whose row left the results (already asserted at 3. Long-locale layout is unchecked"172 selected" (12 chars) becomes "All matching items selected" (27 en, 49 fr, ~45 el). The wide bulk-actions button is content-sized with no truncation — Clean
Test run — partialI created 3 manual expenses so the list rendered. The all-matching path is unreachable at that size: the "Select all" popover is gated on
Evidence (5)Spend > Expenses with 3 rows and select-all checkbox
Bulk-actions button showing 3 selected
Bulk-actions dropdown menu open
Button showing 2 selected after unchecking one row
Spend > Reports empty state
|
|
Found a regression in the PR and fixed it, stil testing some new flows and will finish by end of the day 😃 |
|
@Valforte and @mkhutornyi Done from my side.You can merge it, Thanks! |
| delete excludedTransactions[key]; | ||
| } | ||
| if (!Object.hasOwn(prevState.selectedTransactions, key) && transaction.isSelectedViaGroup && transaction.groupKey) { | ||
| delete excludedTransactions[transaction.groupKey]; |
There was a problem hiding this comment.
I just want to make sure I'm understanding this right, this is here so that if a user Select All Matching, the remove a transaction, when they re select the transaction it treats as Select All Matching is applyed again?
There was a problem hiding this comment.
Yes. All-matching mode remains active; this removes the stale parent-group exclusion when the group is reselected, allowing the button to return to “All matching items selected.” This is covered by a regression test.
|
@Valforte All yours, Thanks! |
|
🚧 Valforte has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Valforte in version: 9.4.74-0 🚀
|
|
🤖 Help site changes are required. Draft PR: #100628 This PR restored the All matching items selected label on the selection button in Spend > Expenses. The help site never documented that label, and three step lists told members to select a button named Selected, which does not exist — the selection button always shows either a count (3 selected) or All matching items selected. Files updated in the docs PR
Label verification and scope decisionsWhere the labels came from. The Melvin web test account has no expenses ("No expenses yet"), so the bulk-selection UI does not render and the labels could not be captured live. The live UI did confirm the navigation labels Spend and Expenses. The remaining strings were read from
Worth a second look: the menu appears only when Left out on purpose. The restored label only applies to expenses, so Spend > Reports wording was not touched. Separately, Guidelines applied. The @emkhalid, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |





Explanation of Change
This PR restores the “All matching items selected” label for Spend > Expenses after selecting all matching expenses.
When one or more expenses are excluded, the button continues to show the exact numeric selection count. Rechecking all excluded expenses restores the all-matching label.
This change only affects the button label. It does not change selection state, totals, pagination, exports, or Spend > Reports behavior.
The required translation was also restored in every supported locale, with focused unit-test coverage.
Fixed Issues
$ #99567
PROPOSAL:
Tests
Precondition: Use an account with more than one page of expenses.
Offline tests
QA Steps
Same as Tests and Offline tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
android-native-fix.mov
Android: mWeb Chrome
android-web-fix.mov
iOS: Native
ios-native-fix.mov
iOS: mWeb Safari
ios-web-fix.mov
MacOS: Chrome / Safari
macos-fix.mov