Fix multiple expenses appearing in confirmation page#81686
Fix multiple expenses appearing in confirmation page#81686MobileMage wants to merge 1 commit intoExpensify:mainfrom
Conversation
Add allTransactionDrafts to the memo comparison function in PureReportActionItem so the component re-renders when draft transactions change, preventing stale data from showing both expenses on the confirm details page. Fixes Expensify#80592
|
@dominictb 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] |
| prevProps.parentReport?.reportID === nextProps.parentReport?.reportID && | ||
| deepEqual(prevProps.personalDetails, nextProps.personalDetails) && | ||
| deepEqual(prevProps.introSelected, nextProps.introSelected) && | ||
| deepEqual(prevProps.allTransactionDrafts, nextProps.allTransactionDrafts) && |
There was a problem hiding this comment.
❌ PERF-5 (docs)
Deep equality checks recursively compare all nested properties, creating performance overhead that often exceeds the re-render cost they aim to prevent. Using deepEqual on allTransactionDrafts (an OnyxCollection<Transaction>) will compare every property of every transaction in the collection on every render.
Consider comparing only the specific properties or IDs that affect the actionableItemButtons behavior:
// Instead of deep equality on the entire collection:
Object.keys(prevProps.allTransactionDrafts ?? {}).length === Object.keys(nextProps.allTransactionDrafts ?? {}).length &&
Object.keys(prevProps.allTransactionDrafts ?? {}).every(key =>
prevProps.allTransactionDrafts?.[key]?.transactionID === nextProps.allTransactionDrafts?.[key]?.transactionID
)Or if only specific transaction IDs matter for the actionable buttons, track just those IDs rather than the entire collection.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
@aimane-chnaif will review this |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
Explanation of Change
Added
allTransactionDraftsto the memo comparison function inPureReportActionItem. Without this, changes toallTransactionDrafts(e.g., when a draft transaction is created during the "Submit it to someone" flow) don't trigger a re-render, causing theactionableItemButtonsmemo to use stale data. This results inremoveDraftTransactionsnot clearing the previous draft, so both expenses appear on the confirmation page.Fixed Issues
$ #80592
PROPOSAL: #80592 (comment)
Tests
Offline tests
N/A - This is a UI memo/re-render fix that does not involve network requests.
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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
80592-web.mp4