-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix/77987 - Standardize product behavior using "Global Create" #78965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
trjExpensify
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jmills created this parent issue 👍
|
@dmkt9 Please resolve the conflict. |
|
@dmkt9 Please resolve the conflict again. |
|
@dmkt9 Please double-check why you are disabling ESLint with // eslint-disable-next-line. Let’s try to find an alternative solution to avoid disabling ESLint. If there is truly no other option and we must disable it, please make sure there is a clear and valid reason for doing so. |
| if (manualHighlightTransactionIDs.has(id)) { | ||
| return true; | ||
| } | ||
| if (!triggeredByHookRef.current || !hasNewItemsRef.current) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why we check this condition here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition means I only moved the check from the outer if into the filter condition of currentTransactionIDs to ensure that manualHighlightTransactionIDs are merged without breaking the original logic.
As for why there are two conditions, you can also notice that the useSearchHighlightAndScroll hook only highlights items that it itself fetched from the server.
|
Bug Screen.Recording.2026-01-09.at.22.52.29.movI think it should redirect to Reports → Expenses. |
|
@dmkt9 please resolve the conflict as well |
@huult Which If it’s |
|
@dmkt9 Please resolve the conflict as well. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-01-22.at.16.36.52.mp4Android: mWeb ChromeScreen.Recording.2026-01-22.at.16.39.03.mp4iOS: HybridAppScreen.Recording.2026-01-22.at.16.29.45.mp4iOS: mWeb SafariScreen.Recording.2026-01-22.at.16.32.41.mp4MacOS: Chrome / SafariScreen.Recording.2026-01-21.at.11.05.59.mp4 |
@dmkt9 Could you record a video and ask @trjExpensify to confirm this behavior?
I see your screenshot. When creating an expense in a workspace from the mobile FAB, it redirects to the Inbox. This behavior is not consistent with the web version. Could you double-check this, and if it’s still unclear, please ask for confirmation? |
@huult I'm fine with changing the behavior of Hi @trjExpensify, with the Quick Scan button we also need to ensure the behavior is the same as when we create an expense from the Global Create button, right?
|
@huult I don’t quite understand this point. Do you mean that our PR behaves inconsistently when creating on mobile and web? |
|
@trjExpensify Please help review the product behavior changes. Let us know if you have any questions. |
huult
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
I'll let @trjExpensify review the product changes first and then I can do a final review of the code if everything looks good! |
|
@dmkt9 please resolve the conflict! |
With regards to the navigation changes? Yeah. |
|
@huult I've just updated this PR. When you get a chance, please take a look. Thanks. |

Explanation of Change
Standardize navigation immediately after an expense is created using the "Global Create" button.
Fixed Issues
$ #77987
PROPOSAL: #77987 (comment)
Tests
Same as QA Steps
Offline tests
No tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
android.native.workspace.mp4
android.native.track.mp4
android.native.perdiem.mp4
android.native.distance.mp4
android.native.invoice.mp4
Android: mWeb Chrome
android.mweb.mp4
iOS: Native
ios.native.mp4
iOS: mWeb Safari
ios.mweb.mp4
MacOS: Chrome / Safari
mac.web.mp4