Decouple footer from transaction prop#92766
Conversation
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ 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". |
|
@ShridharGoel 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeScreen.Recording.2026-06-05.at.8.31.45.PM.moviOS: HybridAppiOS: mWeb SafariScreen.Recording.2026-06-05.at.8.34.59.PM.movMacOS: Chrome / SafariScreen.Recording.2026-06-05.at.8.26.30.PM.mov |
| const {iouType, reportID, isReadOnly, didConfirm} = useConfirmationFields(); | ||
| function InvoiceSenderSection({selectedParticipants}: InvoiceSenderSectionProps) { | ||
| const {iouType, reportID, transactionID, isReadOnly, didConfirm} = useConfirmationFields(); | ||
| const transaction = useTransactionSelector(transactionID, invoiceSenderSliceSelector); |
There was a problem hiding this comment.
Shouldn't we subscribe after the iouType check? Might not create much difference since anyways it would return, but still maybe it can be slightly better for perf?
There was a problem hiding this comment.
Yeah, that makes sense, applied the change. I believe the perf will be better since flows that don't apply to the section now skip the transaction subscription entirely
| function PerDiemSection({isPerDiemRequest, transaction, policy, shouldDisplayFieldError, formError}: PerDiemSectionProps) { | ||
| function PerDiemSection({isPerDiemRequest, policy, shouldDisplayFieldError, formError}: PerDiemSectionProps) { | ||
| const {action, iouType, transactionID, reportID, isReadOnly, didConfirm} = useConfirmationFields(); | ||
| const transaction = useTransactionSelector(transactionID, perDiemSliceSelector); |
| const styles = useThemeStyles(); | ||
| const {iouType, isReadOnly} = useConfirmationFields(); | ||
| const {iouType, transactionID, isReadOnly} = useConfirmationFields(); | ||
| const transaction = useTransactionSelector(transactionID, distanceMapSliceSelector); |
There was a problem hiding this comment.
Same, let me know your thoughts.
|
🚧 @cristipaval has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.4.2-0 🚀
Bundle Size Analysis (Sentry): |
Help site review — no changes requiredI reviewed the changes in this PR against the help site files under Conclusion: No help site updates are needed. This is a pure internal performance refactor. It decouples
Since the help site documents product behavior — which is unchanged here — there's nothing to add or update. Because no documented behavior changed, I did not create a draft help site PR. @OlimpiaZurek, please confirm this assessment — if you believe any documented behavior is affected, let me know and I'll create the help site PR. |
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.4.4-1 🚀
Bundle Size Analysis (Sentry): |
Help site review: No changes requiredI reviewed the changes in this PR and no updates to the help site files under Why: This is a purely internal performance refactor. It changes how the Evidence
Since no documentation updates are needed, I did not create a draft help site PR. @OlimpiaZurek, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR (Note: no help site PR was created because this PR contains no user-facing changes that would require documentation updates. If you believe a docs change is warranted, let me know what behavior to document and I'll create the draft PR.) |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.4-3 🚀
|
Explanation of Change
This PR removes the last layer of full-transaction subscriptions in the
MoneyRequestConfirmationListFootersubtree:ConfirmationFieldListno longer reads the transaction at all. The two hooks it calls (useFooterDerivedFlags,useFooterTagVisibility) now self-subscribe to disjoint narrow slices (5 fields + 1 field).useTransactionSelector. The footer receivesisScanRequest: booleaninstead.Fixed Issues
$ #90385
PROPOSAL:
Tests
Tests
Manual expense: open a chat → + → Create expense → Manual → enter an amount, pick a participant → Next. Verify all fields render on the confirmation page (Amount, Description, Merchant, Category, Date, Tags, Tax, Attendees, toggles, Report).
Scan flow: + → Create expense → Scan → take a photo or pick from gallery. Verify the confirmation page enters compact mode (only required fields + receipt thumbnail). Tap Show more → optional fields appear.
Distance flow: + → Track distance → Manual → add 2 waypoints → Next. Verify the map renders at the bottom of the modal with the route, and Distance + Rate fields display correctly. Try an invalid waypoint → verify the pending/error indicator shows in place of the map.
Per-diem flow (workspace with per-diem rates): + → Create expense → Per diem → pick destination + dates. Verify Time / Rate / Destination fields render. Submit → expense posts with the correct rate and amount.
Invoice flow: + → Send invoice → continue. Verify the Send from row shows your invoicing workspace. Tap it → pick a different workspace → display updates.
Split bill: group chat → + → Split expense → enter amount, pick participants → Next. Edit Merchant and Description individually → verify each saves. Submit → split posts with correct per-person amounts.
Edit existing expense: open one → tap Amount, Description, Merchant, Category, Tag, Tax individually → change each → verify each saves.
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari