Reports - Expense created while on "Reports", gets highlighted again when opened.#89525
Conversation
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.
|
| // Clear consumed manual highlight flags synchronously so subsequent detect runs | ||
| // don't re-highlight the same IDs. A previous timer-based reset effect lost a race | ||
| // with the reset of `newSearchResultKeys` (its cleanup kept canceling the timer | ||
| // before it could fire), leaving flags stuck at `true` and causing every later add | ||
| // to re-animate every prior expense. |
There was a problem hiding this comment.
imo this comment is a little too verbose and I don't love documenting a previous approach that isn't here now. same in tests/unit/useSearchHighlightAndScrollTest.ts -> the new comment is unnecessary
|
hello @truph01, how's the review coming along? |
|
@truph01 can you prioritize this pr please? |
|
@truph01 sorry it is not clear to me if you are available to review this 😂 |
|
Thank you |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-05-12.at.10.25.16.movAndroid: mWeb Chromeoutput.mp4iOS: HybridAppoutput.mp4iOS: mWeb SafariScreen.Recording.2026-05-12.at.10.15.12.movMacOS: Chrome / Safarioutput.mp4 |
| // Used to skip the deferral that would otherwise hide the freshly-added row from | ||
| // FlashList during the RHP dismiss transition, which would prevent the highlight | ||
| // animation from ever firing on it. | ||
| const newSearchResultKeysRef = useRef(newSearchResultKeys); |
There was a problem hiding this comment.
We can consider returning a hasQueuedHighlights flag directly from the useSearchHighlightAndScroll hook instead of making the consumer mirror state into its own ref:
// In useSearchHighlightAndScroll
return { newSearchResultKeys, handleSelectionListScroll, newTransactions, hasQueuedHighlights: newSearchResultKeys !== null && newSearchResultKeys.size > 0 };
Then use a ref for that boolean in Search/index.tsx — it would be cleaner and keep the highlight logic encapsulated.
There was a problem hiding this comment.
yeah, seems valid, implemented
|
@borys3kk PR looks good, I just have one minor comment. |
looking into it now |
…y-first-expense-is-highlighted
|
@truph01 this pr should be good to go assuming that merging main fixed the performance tests |
There was a problem hiding this comment.
Pull request overview
Fixes the “new expense row highlights again after being opened” behavior on the Reports/Spend (Search) experience by ensuring highlight state is consumed deterministically and by avoiding a focus-time deferral that can prevent the highlighted row from mounting in time.
Changes:
useSearchHighlightAndScroll: clears consumed manual highlight flags synchronously during the “detect new items” effect (removing the prior timer-based reset that could lose races).Search/index.tsx: addshasQueuedHighlightsfrom the hook and uses a ref to let the post-createuseFocusEffectskip the skeleton/deferral when a highlight is queued.- Unit test: removes the timer wait now that the highlight-flag reset is no longer timer-based.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/unit/useSearchHighlightAndScrollTest.ts | Updates the manual-highlight test to no longer wait for a timer-based reset. |
| src/hooks/useSearchHighlightAndScroll.ts | Consumes/clears manual highlight flags inline when detected; exposes hasQueuedHighlights. |
| src/components/Search/index.tsx | Skips focus-time skeleton deferral when highlights are queued to ensure the row mounts in time. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
🚧 @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.3.74-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is an internal bug fix for the expense highlight animation on the Search/Spend tab — it corrects a race condition so only the newly created expense row flashes instead of re-animating all previous rows. The changes are limited to hook logic ( |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.74-7 🚀
|
Explanation of Change
useSearchHighlightAndScroll— replaced the timer-based reset oftransactionIDsToHighlight(which lost a race with thenewSearchResultKeysreset and never fired) with a synchronous Onyx merge inside the detect effect. Stops every prior expense from re-animating on each new add.Search/index.tsx—useFocusEffectnow skips the skeleton-during-transition defer when the highlight hook already queued rows. Otherwise FlashList stays empty for ~1s while the RHP dismiss transition runs, the new row never mounts inside the 300ms highlight window, and the animation never fires for expenses created outside Search.Fixed Issues
$ #81474
PROPOSAL:
Tests
Expenseselected, submit a new expense via the FAB and let the highlight finish animating.Reports(or any other Search sub-tab), then back toExpenses.Offline tests
QA Steps
Same as tests.
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.Screenshots/Videos
Android: Native
Screen.Recording.2026-05-05.at.12.23.20.mov
Android: mWeb Chrome
Screen.Recording.2026-05-05.at.12.26.25.mov
iOS: Native
Screen.Recording.2026-05-05.at.12.11.42.mov
iOS: mWeb Safari
Screen.Recording.2026-05-05.at.12.13.17.mov
MacOS: Chrome / Safari
Screen.Recording.2026-05-04.at.12.39.12.mov