Fix: Splits incorrectly highlighted in Inbox after splitting from Spend page - #94139
Conversation
Scope pendingNewTransactionIDs registration to the dismissModalWithReport path so it only runs when the expense report is actually opened. Splitting from Spend > Expenses navigates back to Search and never mounts the expense report, so the previously-registered flags were never consumed/cleared and incorrectly highlighted rows when the report was later opened from Inbox.
|
@gijoe0295 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] |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 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". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-21.at.00.58.14.movAndroid: mWeb ChromeScreen.Recording.2026-07-04.at.02.24.39.moviOS: HybridAppScreen.Recording.2026-07-21.at.00.34.41.moviOS: mWeb SafariScreen.Recording.2026-07-21.at.00.12.34.movMacOS: Chrome / SafariScreen.Recording.2026-06-30.at.00.35.21.mov |
|
@aswin-s MacOS: Chrome Screen.Recording.2026-06-30.at.01.15.39.mov |
Splitting an expense from the Spend/Search page relied on the auto re-search path in useSearchHighlightAndScroll, which is skipped while offline, so the new split rows never highlighted offline. Register the new split transaction IDs via mergeTransactionIdsHighlightOnSearchRoute (TRANSACTION_IDS_HIGHLIGHT_ON_SEARCH_ROUTE) on the Search-page branch. This highlights the rows optimistically without a server re-search, so it works offline too. Reverse splits and existing children are skipped.
|
@gijoe0295 Fixed the highlights in offline mode. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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". |
| // Register newly created split transaction IDs so they briefly highlight on the Search/Spend page. | ||
| // The Search page reads TRANSACTION_IDS_HIGHLIGHT_ON_SEARCH_ROUTE, which highlights matching rows | ||
| // optimistically without waiting for a server re-search. Unlike the auto-detect path in | ||
| // useSearchHighlightAndScroll (skipped while offline), this makes the highlight work offline too. | ||
| // Reverse splits create no new transactions, and existing children are already in the list, so both are skipped. | ||
| if (isSearchPageTopmostFullScreenRoute && !isReverseSplitOperation) { | ||
| const currentSearchType = getCurrentSearchQueryJSON()?.type; | ||
| if (currentSearchType) { | ||
| const existingChildTransactionIDs = new Set(allChildTransactions.map((tx) => tx?.transactionID).filter(Boolean)); | ||
| const newTransactionIDsToHighlight: Record<string, boolean> = {}; | ||
| for (const splitExpense of splitExpenses) { | ||
| if (!splitExpense.transactionID || existingChildTransactionIDs.has(splitExpense.transactionID)) { | ||
| continue; | ||
| } | ||
| newTransactionIDsToHighlight[splitExpense.transactionID] = true; | ||
| } | ||
| if (!isEmptyObject(newTransactionIDsToHighlight)) { | ||
| mergeTransactionIdsHighlightOnSearchRoute(currentSearchType, newTransactionIDsToHighlight); | ||
| } |
There was a problem hiding this comment.
Avoid deep nested if. Please consider these solution:
- Create helper function
- Early return
For example:
- Helper function to get new split transaction IDs
function getNewSplitTransactionIDs(): string[] {
const existingChildTransactionIDs = new Set(allChildTransactions.map((tx) => tx?.transactionID).filter(Boolean));
const newTransactionIDs: string[] = [];
for (const splitExpense of splitExpenses) {
if (!splitExpense.transactionID || existingChildTransactionIDs.has(splitExpense.transactionID)) {
continue;
}
newTransactionIDs.push(splitExpense.transactionID);
}
return newTransactionIDs;
}
- Register search rows which is highlighted:
function registerSearchRouteHighlight() {
// Register newly created split transaction IDs so they briefly highlight on the Search/Spend page.
// The Search page reads TRANSACTION_IDS_HIGHLIGHT_ON_SEARCH_ROUTE, which highlights matching rows
// optimistically without waiting for a server re-search. Unlike the auto-detect path in
// useSearchHighlightAndScroll (skipped while offline), this makes the highlight work offline too.
// Reverse splits create no new transactions, and existing children are already in the list, so both are skipped.
if (!isSearchPageTopmostFullScreenRoute || isReverseSplitOperation) {
return;
}
const currentSearchType = getCurrentSearchQueryJSON()?.type;
if (!currentSearchType) {
return;
}
const newSplitTransactionIDs = getNewSplitTransactionIDs();
const newTransactionIDsToHighlight = Object.fromEntries(newSplitTransactionIDs.map((transactionID) => [transactionID, true]));
if (isEmptyObject(newTransactionIDsToHighlight)) {
return;
}
mergeTransactionIdsHighlightOnSearchRoute(currentSearchType, newTransactionIDsToHighlight);
}
- Usage:
if (isSearchPageTopmostFullScreenRoute || !params.transactionReport?.parentReportID) {
registerSearchRouteHighlight();
|
@aswin-s Here you go! |
…ghlight helpers Addresses review feedback: dedupe the new-split-ID filtering into a shared getNewSplitTransactionIDs helper reused by both the search-route and report-metadata highlight paths, and flatten the deeply nested search-branch highlight into registerSearchRouteHighlight with early returns.
Screen.Recording.2026-07-14.at.01.44.09.mov |
After splitting an expense, the newly-created row that receives focus on the Search/Spend page never appeared highlighted on the mobile (narrow) layout. The highlight animation renders on the row wrapper, but a focused row paints an opaque background on the pressable in front of it, covering the highlight. Suppress the opaque focus background for the duration of the post-create highlight animation so the highlight is visible on the focused row too.
0baaa50 to
4e6a393
Compare
Initialize the highlight latch from item.shouldAnimateInHighlight so a row that mounts already flagged for highlight suppresses the focus background immediately, instead of only when the flag transitions during a later render. Prevents the opaque focus background from covering the highlight on first mount.
|
@gijoe0295 Fixed the highlight issue on mobile resolution. Simulator.Screen.Recording.-.iPhone.17.Pro.Max.-.2026-07-18.at.20.44.54-1.mov |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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". |
…TransactionListItemNarrow.tsx Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>
|
@mountiny Applied the suggested fix and merged latest main. |
|
🚧 mountiny 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/mountiny in version: 9.4.45-0 🚀
|
|
🤖 Help site review — no changes required. I reviewed the changes in this PR against Expensify's help site files under This PR is an internal bug fix for a transient highlight-animation issue: it moves the The help articles that mention splitting (e.g. Because no help site content is affected, I did not create a docs draft PR. @aswin-s, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
Hi @aswin-s. QA team failed this PR with an original issue 1785179628716.Recording__1265.mp4 |
|
@gijoe0295 The underlying logic was modified while this PR was in review through this PR #94670. Looks like we need to fix the logic again. |
|
Can you please follow up? Thanks |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.4.45-14 🚀
Bundle Size Analysis (Sentry): |
7 participants
Explanation of Change
Follow-up to #92506 (deploy blocker #93967).
When splitting an existing expense, #92506 registered the newly created split transaction IDs for the temporary highlight (
addPendingNewTransactionIDs) before the navigation branch inupdateSplitTransactionsFromSplitExpensesFlow. That registration therefore also ran on the Spend > Expenses (Search) path, where the flow navigates back vianavigateBackToLastSuperWideRHPScreenand the expense report is never opened.addPendingNewTransactionIDswrites{pendingNewTransactionIDs: {[id]: true}}into the report'sREPORT_METADATA. Those flags are only consumed — and only cleared — when the expense report'sMoneyRequestReportActionsListmounts and runsuseNewTransactions. Since splitting from Spend never opens the report, the flags sat stranded in metadata until the user later opened that report from the Inbox, at which point the (no longer new) split rows were incorrectly highlighted.The Spend/Search page has its own highlight mechanism (
useSearchHighlightAndScroll/newSearchResultKeys) and does not rely onREPORT_METADATA.pendingNewTransactionIDs, so its highlight is unaffected.Fix:
SplitTransactionUpdate.ts, move the registration loop below the early-return branches so it runs only on thedismissModalWithReportpath — the single path that actually opens the expense report and therefore consumes and clears the flags on mount. The Search/Spend page, selfDM, and last-transaction (report-emptied) cases all return before the loop, so they no longer polluteREPORT_METADATAwith flags that would never be cleared.skips registration when splitting from the Search/Spend page).Fixed Issues
$ #93967
PROPOSAL: #93967 (comment)
Tests
Offline tests
N/A — split expense saving requires network. The highlight logic is optimistic (fires before the API response), so offline state does not affect it.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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.mp4
Android: mWeb Chrome
android-web.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
ios-web.mp4
MacOS: Chrome / Safari
Mac.mp4