Fix - Reports - New expense is not briefly highlighted in the list after expense is submitted by FitseTLT · Pull Request #64111 · Expensify/App

FitseTLT · 2025-06-12T22:07:25Z

Details

Fixed Issues

$ #59449
PROPOSAL: #59449 (comment)

Tests

Go to Reports
Open FAB > Create expense > Manual
Enter amount > Next
Select a user and submit the expense to them
The new expense will be briefly highlighted in Expenses list after the expense is submitted

Verify that no errors appear in the JS console

Offline tests

Same as above

QA Steps

Same as above

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

2025-06-13.20-28-09.mp4

Android: mWeb Chrome

2025-06-13.20-26-18.mp4

iOS: Native

2025-09-12.23-40-45.mp4

iOS: mWeb Safari

2025-06-13.20-22-49.mp4

MacOS: Chrome / Safari

2025-06-13.01-05-51.mp4

MacOS: Desktop

2025-06-13.20-21-46.mp4

melvin-bot · 2025-06-12T22:07:30Z

@QichenZhu 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]

QichenZhu · 2025-06-20T16:56:16Z

            // Perform the scrolling action
-            ref.scrollToIndex(indexOfNewItem);
+            if (indexOfNewItem === data.length - 1) {
+                // Scrolling to last item needs a delay to work due to FlatList internal bug.


Is there a chance this is caused by the reason in #61278 (comment)? If so, it's probably not a FlatList bug since we're not using it the recommended way. But if it turns out to be an upstream bug, let's add an issue link so we can remove this workaround once it's fixed upstream.

May be but I don't see the log warning in my console.

The warning was suppressed by #61392.

App/src/components/Search/SearchList.tsx

Line 86 in 02e37d7

const onScrollToIndexFailed = () => {};

Yeah exactly @QichenZhu I proved that onScrollToIndexFailed is called in this case. So do you suggest another method that delaying the call?

I don't think we're on the right track to fix this. The codebase has changed a lot since your proposal, so we might need to dig deeper into the root cause and try a different solution. Please let me know if you're still interested. Thanks!

@QichenZhu It is now working for that specific case it was failing (where the item is added to the end of the list) without any workaround delay. Can you check it ?

2025-07-16.17-47-43.mp4

QichenZhu · 2025-08-30T09:26:30Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  Expenses created offline are greyed out and not highlighted, but I think that's fine.
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified there are no new alerts related to the canBeMissing param for useOnyx
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
We already have unit tests, but they can't catch this bug, probably because React render timing behaves differently in real vs. test environments.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android-native.webm

Android: mWeb Chrome

android-web.webm

iOS: HybridApp

ios-native.mov

iOS: mWeb Safari

ios-web.mov

MacOS: Chrome / Safari

highlight-old-expenses.mov

MacOS: Desktop

mac-desktop.mov

QichenZhu

@FitseTLT Thanks! This is a nice improvement compared to main branch. Just two things:

Expenses created offline are greyed out and not highlighted. Is this expected?
Screenshot/video for iOS Native is missing.

JS00001 · 2025-09-10T17:39:02Z

@QichenZhu Whats the issue? I'm having trouble following the thread

QichenZhu · 2025-09-11T06:23:04Z

@JS00001, this PR works better than the main branch, except for the three issues listed in my previous comments:

#64111 (review):

Expenses created offline are greyed out and not highlighted. Is this expected?

Screenshot/video for iOS Native is missing.

and #64111 (comment):

The above still applies. I also noticed one more minor issue: both new and old expenses are highlighted.

I think we've hit the limits of this path, and this might be the best outcome we can achieve with it. I'm not sure whether it meets the repo standard and we should proceed and merge it.

FitseTLT · 2025-09-12T20:48:21Z

@JS00001, this PR works better than the main branch, except for the three issues listed in my previous comments:

#64111 (review):

Expenses created offline are greyed out and not highlighted. Is this expected?

Yes, it is intentional. This is the code that is preventing it

App/src/hooks/useSearchHighlightAndScroll.ts

Lines 88 to 91 in f6a223b

    
           if (!isFocused || isOffline) { 
        
               hasPendingSearchRef.current = true; 
        
               return; 
        
           }

Screenshot/video for iOS Native is missing.

Added

and #64111 (comment):

The above still applies. I also noticed one more minor issue: both new and old expenses are highlighted.

I think we've hit the limits of this path, and this might be the best outcome we can achieve with it. I'm not sure whether it meets the repo standard and we should proceed and merge it.

Yeah this is a bit out of scope and the first time I tried to solve this problem I tried to use the transaction created prop but unfortunately it only holds the date data, not a timestamp, so unfortunately we cannot precisely tell a transaction is an older one but it is an edge case problem.

JS00001 · 2025-09-15T13:08:06Z

Expenses created offline are greyed out and not highlighted. Is this expected?

Yes that is expected

Screenshot/video for iOS Native is missing.

Looks like this was added

The above still applies. I also noticed one more minor issue: both new and old expenses are highlighted.

I dont think we should merge this with this issue existing. Let me look into the issue a bit more

JS00001 · 2025-09-15T13:09:03Z

@FitseTLT Can you explain the issue a bit more and why other expenses are getting highlighted?

FitseTLT · 2025-09-24T19:00:25Z

@FitseTLT Can you explain the issue a bit more and why other expenses are getting highlighted?

Yeah @JS00001 This is because hasNewItemsRef is set to true on the creation of new expense. So when the user scrolls down new items will be added and as the hasNewItemsRef will not be reset it will allow this new old expenses to be highlighted. I have updated the solution from using a boolean ref to transactionIDs list ref so that it will hold the new transaction ids and check if the new items added are really new. Result:

2025-09-24.20-04-46.mp4

The other inconsistent bug I am facing is regarding viewOffset here

App/src/components/Search/SearchList/index.tsx

Line 270 in 5c596dd

    
           listRef.current.scrollToIndex({index, animated, viewOffset: variables.contentHeaderHeight});

the documentation says positive value will push it down to the view port but from my testing positive value is pushing it out of view and negative value causing the vice versa. I did change the value to negative on a previous commit but after some time when merging main the switched back. So I have switched to using viewPosition:0 to put the item on top of the view I think this one is more safe and as the bug is internal flatlist one this is the best solution I suggest. Result:

Web:

2025-09-24.21-37-06.mp4

Android:

2025-09-24.21-54-02.mp4

JS00001 · 2025-09-25T18:04:24Z

I think that looks good, @QichenZhu Could you please re-review?

QichenZhu · 2025-09-26T05:05:49Z

Not trying to nitpick, but I'm still seeing old expenses being highlighted.

I don't think it's related to the highlighting logic. It seems more like an issue with virtual scrolling.

Maybe we should live with it for now and ask an agency for help.

highlight-old-expenses.mov

JS00001 · 2025-09-26T11:55:57Z

Not trying to nitpick, but I'm still seeing old expenses being highlighted.

I don't think this is a nitpick, and I dont think we should merge this as is. @trjExpensify Any thoughts on this? This PR is supposed to highlight new expenses, but its sometimes highlighting more than one expense, even ones that arent highlighted. I dont think the pros of merging this outweigh the cons

trjExpensify · 2025-09-26T12:51:49Z

Yeah, I agree with you.

codecov · 2025-09-26T12:59:09Z

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.

Files with missing lines	Coverage Δ
src/components/Search/SearchList/index.tsx	`0.00% <0.00%> (ø)`
src/components/Search/index.tsx	`0.00% <0.00%> (ø)`
src/hooks/useSearchHighlightAndScroll.ts	`64.06% <66.66%> (-0.74%)`	⬇️
... and 24 files with indirect coverage changes

FitseTLT · 2025-09-26T14:31:17Z

Not trying to nitpick, but I'm still seeing old expenses being highlighted.

I don't think it's related to the highlighting logic. It seems more like an issue with virtual scrolling.

Maybe we should live with it for now and ask an agency for help.

highlight-old-expenses.mov

This one has nothing to do with useSearchHighlightAndScroll I debugged that we are only setting newSearchResultKey for the new item and even shouldHighlight is only true for the new item useAnimatedHighlightStyle

App/src/hooks/useAnimatedHighlightStyle/index.ts

Lines 48 to 50 in 9490950

    
           export default function useAnimatedHighlightStyle({ 
        
               borderRadius, 
        
               shouldHighlight,

I think this is an advanced internal bug of the list component and I don't see how it is related to the scope of this PR.

JS00001 · 2025-09-26T14:32:08Z

@FitseTLT Is this bug on prod right now?

FitseTLT · 2025-09-26T14:47:41Z

@FitseTLT Is this bug on prod right now?

Nope On main the highlighting logic is broken so highlighting doesn't work for the new item too.

JS00001 · 2025-09-26T14:58:42Z

Yeah I agree the bug could exist but isnt visible. However, I dont think we should be releasing 1 improvement alongside 1 added bug

FitseTLT · 2025-09-30T12:46:42Z

Yeah I agree the bug could exist but isnt visible. However, I dont think we should be releasing 1 improvement alongside 1 added bug

I understand your concern. Then, I think we should get someone from expert team for the last bug.

JS00001 · 2025-10-10T16:45:37Z

@FitseTLT Can you please lay out the bug and the steps to repro it, and what you would need from them, so I can ask for their opinion on it?

FitseTLT · 2025-10-10T20:18:42Z

Test Steps:
Precondition: use an account with many expenses already created

Switch to this branch
Go to Reports > Expenses
Apply a filter like currency (ensure there are a lot of results with that filter)
create a new expense with that currency in (4)
when the new item is added to the search and highlighted quickly scroll down

Bug: you will see another old search item from bottom also gets highlighted briefly See. the video in #64111 (comment)

Note: we have debugged that it has nothing to do with the useSearchHighlightAndScroll and we ensured that the hook only sets the proper new item as the newSearchResultKey

App/src/hooks/useSearchHighlightAndScroll.ts

Line 270 in 62f4869

return {newSearchResultKey, handleSelectionListScroll, newTransactions};

we suspect some FlatList internal bug is causing it but we are not sure.

2025-10-10.23-33-23.mp4

JS00001 · 2025-10-24T14:43:11Z

sorry for the holdup, asked here: https://expensify.slack.com/archives/C08CZDJFJ77/p1761316978655769

revert change back to calling handle scroll on ref

dd90569

FitseTLT requested a review from a team as a code owner June 12, 2025 22:07

melvin-bot Bot requested a review from QichenZhu June 12, 2025 22:07

melvin-bot Bot removed the request for review from a team June 12, 2025 22:07

FitseTLT mentioned this pull request Jun 13, 2025

Fix - Reports - New expense is not briefly highlighted in the list after expense is submitted #60282

Closed

50 tasks

Merge branch 'main' into fix-scroll-to-new-item-search

ed708c4

QichenZhu mentioned this pull request Jun 16, 2025

[$250 Reports - New expense is not briefly highlighted in the list after expense is submitted #59449

Closed

8 tasks

FitseTLT added 5 commits June 17, 2025 15:08

Merge branch 'main' into fix-scroll-to-new-item-search

7727465

Merge branch 'main' into fix-scroll-to-new-item-search

d5f8689

added data as a dependency

c8d3323

gave a bit delay to scroll to last item

88f1666

increase delay

70b80dd

QichenZhu reviewed Jun 20, 2025

View reviewed changes

FitseTLT added 12 commits June 25, 2025 16:50

Merge branch 'main' into fix-scroll-to-new-item-search

04e1b92

Merge branch 'main' into fix-scroll-to-new-item-search

cbdebc3

Merge branch 'main' into fix-scroll-to-new-item-search

ea15c3d

Merge branch 'main' into fix-scroll-to-new-item-search

54d1709

Merge branch 'main' into fix-scroll-to-new-item-search

bacb913

Merge branch 'main' into fix-scroll-to-new-item-search

d8cfb93

minor fixes

13bf564

Merge branch 'main' into fix-scroll-to-new-item-search

601fc12

Merge branch 'main' into fix-scroll-to-new-item-search

fefadee

Merge branch 'main' into fix-scroll-to-new-item-search

2e72ccd

fix hasNewItemsRef resetting logic

162eeaf

Merge branch 'main' into fix-scroll-to-new-item-search

981a4a6

FitseTLT requested a review from a team as a code owner August 29, 2025 12:04

QichenZhu approved these changes Aug 30, 2025

View reviewed changes

melvin-bot Bot requested a review from JS00001 August 30, 2025 09:46

FitseTLT added 2 commits September 12, 2025 21:03

Merge branch 'main' into fix-scroll-to-new-item-search

f6a223b

minor update

ee40b97

Merge branch 'main' into fix-scroll-to-new-item-search

0f3cf03

FitseTLT added 3 commits September 17, 2025 00:17

Merge branch 'main' into fix-scroll-to-new-item-search

3f72dfc

Merge branch 'main' into fix-scroll-to-new-item-search

5c596dd

fix bug of old items being highlighted

41c3a89

QichenZhu approved these changes Sep 26, 2025

View reviewed changes

Merge branch 'main' into fix-scroll-to-new-item-search

f296398

Merge branch 'main' into fix-scroll-to-new-item-search

8996805

FitseTLT mentioned this pull request Oct 29, 2025

[$250] Reports - Not seen the newly expense briefly highlighted if reports tab having many expenses #71422

Closed

8 tasks

JS00001 closed this Dec 12, 2025

Conversation

FitseTLT commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Jun 12, 2025

Uh oh!

QichenZhu Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

FitseTLT Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

QichenZhu Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

FitseTLT Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

QichenZhu Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FitseTLT Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

QichenZhu commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

QichenZhu left a comment

Choose a reason for hiding this comment

Uh oh!

JS00001 commented Sep 10, 2025

Uh oh!

QichenZhu commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FitseTLT commented Sep 12, 2025

Uh oh!

JS00001 commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JS00001 commented Sep 15, 2025

Uh oh!

FitseTLT commented Sep 24, 2025

Uh oh!

JS00001 commented Sep 25, 2025

Uh oh!

QichenZhu commented Sep 26, 2025

Uh oh!

JS00001 commented Sep 26, 2025

Uh oh!

trjExpensify commented Sep 26, 2025

Uh oh!

codecov Bot commented Sep 26, 2025 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

FitseTLT commented Sep 26, 2025

Uh oh!

JS00001 commented Sep 26, 2025

Uh oh!

FitseTLT commented Sep 26, 2025

Uh oh!

JS00001 commented Sep 26, 2025

Uh oh!

FitseTLT commented Sep 30, 2025

Uh oh!

JS00001 commented Oct 10, 2025

Uh oh!

FitseTLT commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

FitseTLT commented Jun 12, 2025 •

edited

Loading

QichenZhu Jun 30, 2025 •

edited

Loading

QichenZhu commented Aug 30, 2025 •

edited

Loading

QichenZhu commented Sep 11, 2025 •

edited

Loading

JS00001 commented Sep 15, 2025 •

edited

Loading

codecov Bot commented Sep 26, 2025 •

edited by github-actions Bot

Loading

FitseTLT commented Oct 10, 2025 •

edited

Loading