Split the confirmation footer by variants pt2 by VickyStash · Pull Request #100041 · Expensify/App

VickyStash · 2026-09-01T13:43:55Z

Explanation of Change

Refactor with no behavior change.
Follows #99578, which turned MoneyRequestConfirmationListFooter into a dispatcher with per-type variants and migrated per-diem and time footers. This one migrates the rest and deletes the fallback.

Six new variants:

DistanceMapFooter
DistanceManualFooter
DistanceOdometerFooter
ScanFooter
ManualFooter
InvoiceFooter

Each renders only the sections its expense type reaches.

Fixed Issues

$ #99449
PROPOSAL: N/A

Tests

Verify that no errors appear in the JS console

This is a refactor with no intended behavior change, so testing is regression-focused: every expense type must render and submit exactly as before.

Please, verify the confirmation page looks and works same way as before in the next scenariosu:

Manual expense creation.
Scan expense creation. Verify the compact layout with the receipt thumbnail, that "Show more" expands the remaining fields, and that submitting works.
Distance expense via the map creation.
Distance expense via manual entry.
Distance expense via odometer.
Create an invoice. Verify the sender row renders above the fields.
Submitting untracked expense from self DM.

Offline tests

Same, as in Tests section

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

Verify that no errors appear in the JS console

Same, as in Tests section

PR Author Checklist

Screenshots/Videos

Manual

manual.mp4

Scan

scan.mp4

Distance -> map

distance_map.mp4

Distance -> manual

distance_manual.mp4

Distance -> odometr

distance_odometr.mp4

Invoice

invoice.mp4

Submit from self dm

submit.mp4

codecov · 2026-09-01T13:47:49Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
...onents/MoneyRequestConfirmationFields/Provider.tsx	`100.00% <100.00%> (ø)`
...mponents/MoneyRequestConfirmationFields/context.ts	`80.00% <ø> (ø)`
src/components/MoneyRequestConfirmationList.tsx	`94.20% <ø> (ø)`
...eyRequestConfirmationList/sections/AmountField.tsx	`44.44% <100.00%> (ø)`
...RequestConfirmationList/sections/DistanceField.tsx	`70.00% <100.00%> (+1.57%)`	⬆️
...stConfirmationList/sections/InvoiceSenderField.tsx	`93.54% <100.00%> (+0.44%)`	⬆️
...RequestConfirmationList/sections/MerchantField.tsx	`75.00% <100.00%> (ø)`
...oneyRequestConfirmationList/sections/RateField.tsx	`80.64% <100.00%> (+0.64%)`	⬆️
...MoneyRequestConfirmationList/sections/selectors.ts	`83.33% <ø> (ø)`
...ieldGroups/detailsFields/DistanceDetailsFields.tsx	`100.00% <100.00%> (ø)`
... and 11 more
... and 12 files with indirect coverage changes

VickyStash · 2026-09-01T14:26:28Z

@codex review

VickyStash · 2026-09-01T14:26:39Z

@MelvinBot review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bf73a117e2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

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".

MelvinBot · 2026-09-01T14:50:02Z

🤖 Code review: split the confirmation footer by variants pt2

One thing I'd fix before merge — the amount field's autofocus is silently dropped. Everything else is robustness/style, and I confirmed several things that look risky in the diff are actually fine.

🔴 `autoFocus` is no longer passed to `AmountField`

The deleted TransactionDetailsFields computed shouldAutoFocusAmountField = !canUseTouchScreen() and passed autoFocus={shouldAutoFocusAmountField} to AmountField. Neither ManualDetailsFields.tsx:36-45 nor DistanceDetailsFields passes it, so it falls back to its autoFocus = false default. After this PR nothing in the footer tree passes it:

$ grep -rn "autoFocus\|canUseTouchScreen" src/components/MoneyRequestConfirmationListFooter/
(no matches)

That disables the focus effect at AmountField.tsx:117-130, so in the new manual expense flow the amount input no longer regains focus when the participant picker closes on non-touch devices.

The giveaway that this was accidental: ManualDetailsFields still threads isParticipantPickerVisible into AmountField (L43), but that prop exists only to gate the autoFocus effect — so it is now inert too.

Fix: pass autoFocus={!canUseTouchScreen()} in ManualDetailsFields. Distance and time don't need it (isAmountFieldDisabled already covers both). If dropping it everywhere was intentional, then delete the autoFocus prop and the isParticipantPickerVisible threading rather than leaving them half-wired.

🟡 The router's branch order is load-bearing but undocumented

index.tsx:19-49 checks isTypeInvoice before the distance and scan branches. An invoice that was also a distance or scan request would silently lose the map, the Distance field, the Rate field, and compact mode.

I traced it and neither combination is reachable today — the Distance tab is SPLIT-only (IOURequestStartPage.tsx:318) and shouldUseTab excludes invoice (L71), so iouRequestType never becomes DISTANCE*/SCAN for an invoice. So it's not a bug — but this file's correctness now rests on an invariant three files away, and the PR deletes the JSDoc that used to explain footer selection. Please add a comment saying why invoice wins.

🟡 New files have no header description

Seven new files, none with a header comment. The two files this PR deletes (TransactionDetailsFields/index.tsx, InvoiceSenderSection.tsx) and DefaultFooter all had one explaining their role, and our standards ask non-trivial new files to start with a description. The refactor reads as a net loss of "why" documentation.

🟢 Checked and fine

{...errorState} is equivalent to the four explicit props — ErrorState is exactly {shouldDisplayFieldError, formError, clearFormErrors, setFormError}.
receiptStitchError kept only in DistanceOdometerFooter — correct, it originates from useOdometerReceiptStitcher.
Dropping isCompactMode/compactControls from the non-scan variants — correct, isCompactMode = !showMoreFields && isScanRequest && !isInLandscapeMode.
Dropping the fieldVisibility.distance / .rate guards inside DistanceDetailsFields — safe, both equal isDistanceRequest.
distanceRateCurrency dropped from ManualDetailsFields' AmountField — no effect, only read when isDistanceRequest.
TimeFields still rendered, via the pre-existing TimeDetailsFields.
shouldShowDistanceMap is not orphaned — still used by DistanceMapSection and ReceiptSection.

Tests

Only DistanceField.test.tsx was updated, and the Provider-wrapping change there is a clean improvement. But the new router in index.tsx is the whole point of the PR and has no test — a mis-ordered or missing branch drops fields with no type error and no test failure. A small table-driven test asserting each flag combination selects the expected variant would lock in the behavior this PR is asserting.

The PR body is also still the empty template: no explanation of change, no Tests or QA steps, no screenshots, checklist unchecked.

Duplication

The five new variants are largely identical ConfirmationFieldList prop threading. I read that as the deliberate tradeoff of splitting by variant (explicit over DRY) so I'm not asking you to change it — just flagging that a reviewer may push back on it.

🤖 Web smoke test: ⚠️ partial

I drove the PR branch on dev NewDot to confirm the two main variants still render. Manual and distance confirmation pages both render their expected fields; I could not check the JS console because the web driver doesn't expose it.

Step	Status
Manual expense confirmation page shows Amount, Merchant, and Description fields `is visible` passed for label="Amount, €5.00", label="Merchant", label="Description" on the Confirm details page reached via + > Create expense > Manual.	✅
Distance expense confirmation page shows route map, Distance field, and Rate field `is visible` passed for role=region label="Map", label="Distance, 0.51 kilometers", label="Rate, €0.3 / kilometer" on the Confirm details page reached via Distance > Map tab with start/stop addresses set.	✅
JS console errors/warnings during both flows Not checkable: the web driver has no console inspection command. No error banners, toasts, or broken screens appeared in either flow, but that is not equivalent to a console check.	❌

Note the smoke test only covers the manual and distance-map variants. Scan, odometer, invoice, and the compact-mode path are untested, and the autofocus regression above would not show up in either flow I ran.

view run

…rs-pt2

…rs-pt2 # Conflicts: # src/components/MoneyRequestConfirmationList/sections/InvoiceSenderField.tsx

VickyStash · 2026-09-02T07:34:32Z

@codex review

chatgpt-codex-connector · 2026-09-02T07:40:34Z

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 0ff12e019d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

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".

VickyStash · 2026-09-02T07:41:35Z

@MelvinBot review

MelvinBot · 2026-09-02T08:08:35Z

🤖 Code review round 2: split the confirmation footer by variants pt2

No behavior regression found. I traced all eight router branches and every prop each variant threads, and the refactor is equivalent to the old DefaultFooter + TransactionDetailsFields pair for every reachable flag combination. The autoFocus fix from round 1 is genuinely correct, and the added comments closed most of the documentation gap.

npm run test and npm run typecheck both pass locally (38 tests across the 6 relevant suites, zero type errors).

Two things worth addressing before merge, both about the new structure rather than the behavior it produces.

🟡 The router is the point of this PR and has no test coverage

tests/ui/MoneyRequestReportFooter.tsx is the only test that renders the router, and its providerProps omit every mode flag — isTypeInvoice, isPerDiemRequest, isTimeRequest, isDistanceRequest, isManualDistanceRequest, isOdometerDistanceRequest, isScanRequest. They all take their false default, so all five tests fall through to the residual return <ManualFooter {...props} /> at index.tsx:60. Seven of eight branches never render.

And all five assertions target the same shared element:

const reportItem = screen.getByTestId('menu-item-Report');

menu-item-Report comes from SettingsFields inside ConfirmationFieldList, which every variant renders — so even a deliberately mis-wired router would still pass.

This matters more than usual because this PR deleted a guard rather than moving it. DistanceMapSection used to self-guard with if (!isDistanceRequest || isManualDistanceRequest || isOdometerDistanceRequest) return null;. That invariant now lives only in the fall-through order at index.tsx:44-54, where reordering three lines silently drops the map with no type error and no test failure.

Ask: the harness in that file is already 90% of the work. Parameterize providerProps over the eight flag combinations and assert one variant-unique marker each — the map section, the invoice sender row, the odometer stitch error, the compact flex1 wrapper.

🟡 The footer shape now hinges on optional `= false` booleans

Every mode flag in Provider.tsx:95-102 is optional with a false default. Before this PR, the invoice guard read iouType !== CONST.IOU.TYPE.INVOICE off the required iouType field. Now isTypeInvoice — a separate optional boolean that can legally be false while iouType === INVOICE — gates the entire InvoiceFooter including the sender row.

Not reachable today (MoneyRequestConfirmationList passes both from the same expression), but the invariant went from enforced-by-type to enforced-by-convention. Deriving isTypeInvoice from the already-required iouType inside the router, as the deleted InvoiceSenderSection did, removes one way to get a silently wrong footer for free.

Related: the Omit<> variant prop types read like a safety net but aren't one. InvoiceFooterProps omits distanceData/isCompactMode, but the router spreads the full props object, so if invoice ever gains a scan or distance tab the footer drops those fields with no compile error.

🟢 Nits

index.tsx:44-54 — the distance sub-ordering is the one load-bearing order left uncommented. The deleted DistanceMapSection guard also carried a performance rationale (its two-level structure avoided Onyx subscriptions on flows where the map can never render) that now lives nowhere.
selectors.ts:230 — stale // --- InvoiceSenderSection --- header pointing at the file this PR deletes. invoiceSenderSliceSelector is now consumed by InvoiceSenderField.
types.ts:59,61 — ManualFooterProps and InvoiceFooterProps are character-identical. type InvoiceFooterProps = ManualFooterProps; keeps them from drifting.
The test steps skip numbers 6 and 7 (1,2,3,4,5,8,9), and the author checklist and Screenshots sections are still empty.

Things I checked that turned out fine

autoFocus (round 1's 🔴) — genuinely fixed and equivalent. aef4d61b31 moved !canUseTouchScreen() into the component as shouldAutoFocusOnMount (AmountField.tsx:67) and deleted the prop. Old semantics weren't uniform — TimeDetailsFields never passed it and got the false default — so the new always-on value technically diverges for the time path. It's inert: isAmountFieldDisabled = didConfirm || isReadOnly || shouldShowTimeRequestFields || isDistanceRequest is true there, and the effect returns before autofocusing. Worth knowing that the policy is now the component's rather than the caller's, so future call sites opt in silently.

isParticipantPickerVisible dropped from the distance path. DistanceDetailsFields.tsx:38-47 no longer passes it. The state is reachable (a distance expense from global create can open the picker), and it's harmless only because isDistanceRequest already forces isAmountFieldDisabled true. Cheap to pass through if you want it robust.

Section-by-section equivalence. Order is preserved in every variant. Old was Amount → Merchant → Description → Distance → Rate → Time; ManualDetailsFields keeps Amount → Merchant → Description, DistanceDetailsFields keeps Amount → Description → Distance → Rate. The removed wrapper <View> around the invoice/map sections carried no style, so removing it is layout-neutral in a column container.

Deleted fieldVisibility guards are provably dead in their new homes. fieldVisibility.distance and .rate are both literally isDistanceRequest. shouldShowMerchant contains && !isDistanceRequest. shouldShowTimeRequestFields = isTimeRequest && action === CREATE, which is exactly the TimeFooter branch condition.

Compact-mode plumbing scoped to ScanFooter. Safe because isCompactMode = !showMoreFields && isScanRequest && !isInLandscapeMode (MoneyRequestConfirmationList.tsx:525), and the mode flags are mutually exclusive since they all key off the single transaction.iouRequestType scalar. The defaults are unreachable rather than merely benign — the show-more button only renders behind compactState.isCompactMode.

receiptStitchError scoped to DistanceOdometerFooter. useOdometerReceiptStitcher resets to idle whenever !isOdometerDistanceRequest, so the error is non-null only for odometer.

distanceRateCurrency dropped from ManualDetailsFields. Only read under isDistanceRequest ? ..., which that variant passes as false.

Invoice-first branch order is safe. An invoice is always iouRequestType: 'manual' — IOURequestStartPage.tsx:72 sets shouldUseTab = false for INVOICE and the request type then returns MANUAL. No escape hatch via the receipt empty state either.

Context migration. useConfirmationFields throws if used outside the Provider, so a missing Provider would fail loudly rather than default silently. There is exactly one Provider render site and it wraps the whole footer. Nothing outside the footer tree imports the changed or deleted components.

DistanceField.test.tsx is a clean adaptation. Both assertions are byte-identical to main; the ten deleted defaultProps all land on the same values via Provider defaults. It's marginally stronger on typing. One caveat: it now depends on those defaults rather than stating the flags, so if isReadOnly or isManualDistanceRequest ever defaults to true the test silently exercises a different branch instead of failing.

Standards. All seven new files have header descriptions, none uses useMemo/useCallback/React.memo (correct under React Compiler), no inline styles, no unused props or imports, and npm run lint-changed is clean.

Duplication across the five variants. They're largely identical ConfirmationFieldList prop threading. I read that as the deliberate explicit-over-DRY tradeoff of splitting by variant, so I'm not asking you to change it.

Couldn't verify: web smoke test and React Compiler check

Web smoke test — blocked by infra, twice. The browser tab in this run's session was blank (0 DOM nodes, no network activity, pure white screenshots) from before my first command through the end of the run. Two testers tried independently; both confirmed the dev server and bundles were healthy (curl returns 200, rsbuild reported ready built in 20.6s with no compile errors on your branch) and that the page never painted even the static splash HTML. The session's Chrome process had restarted underneath it without a corresponding navigation. Reload is forbidden in this harness because it drops auth, so no in-session recovery was possible. None of the seven expense-type flows were exercised — that is an infra failure, not a signal about your code.

react-compiler-compliance-check aborted on an empty base-ref env var in this sandbox (fatal: ambiguous argument 'origin/'), so I can't claim compliance either way for the new components. CI is the real gate.

view run

VickyStash · 2026-09-02T10:17:19Z

@MelvinBot review

MelvinBot · 2026-09-02T10:52:57Z

🤖 Code review round 3: split the confirmation footer by variants pt2

Both round-2 asks are addressed, and the new router test is the right shape — I mutation-tested it and it genuinely catches a misroute. npm run typecheck passes; all 27 tests across the 5 relevant suites pass.

One gap left in the new test, and it's the one branch that matters most.

🟡 The new test doesn't lock in the invoice-first ordering

MoneyRequestConfirmationListFooter.test.tsx:108-116 renders the invoice case with an empty mode, so every request-type flag is false and the invoice branch is the only one that can match. That asserts invoice routing works — but not that invoice is checked first, which is the one ordering your own comment at index.tsx:26-27 calls out as load-bearing.

I confirmed this by moving the invoice branch from the top of the router to just above the ManualFooter return. All 10 tests still passed.

One-line fix — give the invoice case a mode that would otherwise route elsewhere:

renderFooter({isScanRequest: true}, CONST.IOU.ACTION.CREATE, CONST.IOU.TYPE.INVOICE);
expect(jest.mocked(InvoiceFooter)).toHaveBeenCalled();
expect(jest.mocked(ScanFooter)).not.toHaveBeenCalled();

That is also a more faithful regression test: the risk you documented isn't "invoice doesn't route", it's "invoice silently loses the map, Distance, Rate, or compact mode if it ever gains a request type".

While you're there, the seven it.each cases assert only toHaveBeenCalled() on the expected variant. Since exactly one variant renders per case that's sufficient today, but adding a .not.toHaveBeenCalled() on the sibling would state the intent — same reason the two action-guard tests at L118-138 already do it, and those two read the best in the file.

🟢 Still open from round 2 (both optional)

index.tsx:44-52 — the distance sub-ordering is the one load-bearing order still uncommented. Less pressing now that the test covers it, but the deleted DistanceMapSection guard also carried a performance rationale (its two-level structure avoided Onyx subscriptions on flows where the map can never render) that still lives nowhere.
types.ts:59,61 — ManualFooterProps and InvoiceFooterProps are still character-identical.

Verified fixed since round 2

isTypeInvoice → iouType === CONST.IOU.TYPE.INVOICE (index.tsx:28). The footer shape no longer hinges on an optional = false boolean that could disagree with iouType. I checked that isTypeInvoice isn't now dead — it's still read by ConfirmationFieldList, useFooterDerivedFlags, useConfirmationSections, and useConfirmationCtaText, so leaving it on the context is correct.

Stale comment fixed — // --- InvoiceSenderField --- in selectors.ts:230.

Router test coverage. All eight branches are now exercised, up from one. I mutation-tested it rather than taking it on faith: reordering the distance branches so bare isDistanceRequest is checked before the manual and odometer sub-branches failed exactly the two expected cases ({isDistanceRequest, isManualDistanceRequest} and {isDistanceRequest, isOdometerDistanceRequest}), so the invariant that replaced the deleted DistanceMapSection guard is genuinely protected now.

Mocking each variant to null and asserting which was called is the right call here — it tests the router's actual job without coupling to what any variant draws. The %s in the it.each title also pretty-prints the mode object ({ isDistanceRequest: true, isManualDistanceRequest: true }), so failures are readable.

Gates. npm run typecheck → passed, zero errors. Tests: 27 passed across MoneyRequestConfirmationListFooter.test.tsx, DistanceField.test.tsx, DistanceRequestController.test.tsx, selectorsTest.ts, and tests/ui/MoneyRequestReportFooter.tsx.

The round-1 and round-2 behavior analysis still stands: no regression found across all eight branches and the props each variant threads.

🤖 Web test steps verification results: ⚠️ partial

4 of the 5 expense-type confirm screens verified correctly on dev NewDot web after this refactor. Scan could not be fully verified because attaching a receipt needs a native file dialog, which the web test harness does not support.

Step	Status
Manual expense: confirmation page renders Amount, Merchant, Description and submits Confirm screen showed Amount=€25, Merchant='Coffee Shop', Description='Team meeting coffee'; clicking Create expense posted the expense into the personal space chat (Coffee Shop, €25.00, Cash).	✅
Scan expense: compact layout with receipt thumbnail; Show more expands remaining fields; submitting works Scan tab rendered its upload UI (Upload receipts / Choose files / forward-to-email / text-to-47777), but attaching an actual receipt requires a native OS file dialog, which agent-device web does not support. No receipt could be attached, so the app never reached the Scan-type confirm screen and the compact layout, Show more, and submit could not be exercised. This is a test-harness limitation, not a confirmed defect in the PR.	❌
Distance expense via Map tab: route map, Distance field, Rate field all render Confirm details page rendered the Mapbox route between the two waypoints plus Distance='51.01 kilometers' and Rate='€0.3 / kilometer' (confirmed via snapshot read-back).	✅
Invoice: confirmation page renders the invoice sender row ABOVE the other fields After enabling Invoices on a test workspace, the Send invoice confirm page rendered 'To' (recipient) then the sender row ('…'s Workspace, Workspace') immediately above Amount/Merchant/Description/Category/Date.	✅
Distance expense via Odometer: odometer reading fields plus Distance and Rate render Odometer entry screen rendered 'Start reading' and 'End reading' fields (computing Total distance: 50 km); the resulting confirm details page rendered Distance='50.00 kilometers' and Rate='€0.3 / kilometer'.	✅

Since Scan is the only variant that reaches compact mode, that path remains unverified end to end — worth a manual pass before merge.

view run

melvin-bot · 2026-09-02T11:03:27Z

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

…rs-pt2

situchan · 2026-09-03T15:15:05Z

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)
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
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 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 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)
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)
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.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
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: mWeb Chrome

iOS: HybridApp

ios.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

web1.mov

web2.mov

situchan · 2026-09-03T15:15:14Z

typescript failing

VickyStash · 2026-09-03T15:18:45Z

typescript failing

@situchan It's from main, not from this PR #100311

…rs-pt2

VickyStash · 2026-09-04T07:45:44Z

@situchan I'll be OOO next week, is it possible to do the review today?

situchan · 2026-09-04T08:53:07Z

yes

situchan

chatgpt-codex-connector · 2026-09-04T10:21:43Z

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 35bba5f908

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

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".

MelvinBot · 2026-09-04T10:53:48Z

🤖 Regression review — no confirmed user-reachable regression

I reconstructed what the old DefaultFooter + TransactionDetailsFields rendered for each expense type and compared it against every new variant, prop by prop. Every dropped gate and hardcoded prop is provably equivalent given the dispatcher's routing. Full no-op proofs are in the collapsed section.

What I would still change before merging are three robustness gaps, not behavior breaks:

1. The invoice branch is checked first, and nothing enforces its invariant

MoneyRequestConfirmationListFooter/index.tsx:28 returns InvoiceFooter before the scan and distance checks. InvoiceFooter passes no compactState and no distanceData, so if an invoice were ever a scan or distance request the page would render broken — not degraded:

isCompactMode is computed purely from the transaction (MoneyRequestConfirmationList.tsx:525), so the wrapper and ReceiptSection would both go compact while ConfirmationFieldList renders every field expanded.
The Show more button is the only caller of setShowMoreFields(true), and it needs compactState.isCompactMode. Without it, showMoreFields can never flip — the user is stuck.

I could not find a reachable path today: shouldUseTab is false for invoice so there are no Scan/Distance tabs (IOURequestStartPage.tsx:72), every invoice entry point clears the draft first, and shouldShowReceiptEmptyState excludes invoice so no receipt can be attached from the confirmation page. The old code degraded gracefully here because InvoiceSenderSection self-gated inside DefaultFooter; the new code does not.

Suggested: make the guard self-enforcing — iouType === CONST.IOU.TYPE.INVOICE && !isScanRequest && !isDistanceRequest, or move the invoice check after the scan/distance checks.

2. `hasBelowShowMore` no longer matches what the compact variant renders

fieldVisibility.ts:149-163 still counts flags.rate and flags.time, but ScanFooter is now the only compact-capable variant and it renders ManualDetailsFields, which has neither. No live impact — scan, distance and time are mutually exclusive iouRequestType values, so both flags are always false under ScanFooter. But the file's comment ("the show-more button cannot drift from the fields actually rendered below it") is now false, and a future variant would get a button that reveals nothing with no test catching it.

3. `AmountField` autofocus is now opt-out-by-accident

Removing the autoFocus prop for an internal !canUseTouchScreen() (AmountField.tsx:67) changes TimeDetailsFields from false to !canUseTouchScreen(). That is a no-op today only because isAmountFieldDisabled includes shouldShowTimeRequestFields and short-circuits the effect. Any future caller that isn't disabled silently grabs focus on desktop.

Minor: DetailsFieldsContext still types fieldVisibility as Pick<..., 'amount' | 'distance' | 'rate' | 'merchant' | 'time'> and documents isCompactMode as suppressing "Amount, Rate, Merchant, Time" — no consumer reads distance, rate or time any more.

Test coverage gap

tests/unit/components/MoneyRequestConfirmationListFooter.test.tsx mocks all eight variants, so it asserts the routing table and nothing about rendered output. None of the above would be caught by it.

Verified no-ops (things that look like regressions but aren't)

DistanceField / RateField now unconditional in DistanceDetailsFields (was fieldVisibility.distance and !isCompactMode && fieldVisibility.rate). fieldVisibility.distance === fieldVisibility.rate === isDistanceRequest, and all three distance variants are gated on isDistanceRequest from the same context. isCompactMode requires isScanRequest, which excludes distance.
AmountField isDistanceRequest hardcoded true in DistanceDetailsFields / false in ManualDetailsFields — matches fieldVisibility.distance on every reachable path, and correctly keeps the field disabled and rate-currency-based for distance.
shouldShowTimeRequestFields={false} in both new groups. shouldShowTimeRequestFields = isTimeRequest && action === CREATE (useFooterDerivedFlags.ts:102) — that exact predicate routes to TimeFooter first.
distanceRateCurrency dropped in ManualDetailsFields — AmountField reads it only behind isDistanceRequest, which is false there.
{...errorState} spread — ErrorState is exactly the four props AmountField needs; nothing extra, nothing shadowed.
receiptStitchError only in DistanceOdometerFooter — useOdometerReceiptStitcher's error can only leave null via a fail dispatched from the stitching state, and beginStitch requires isOdometerDistanceRequest; flipping the flag off aborts and resets. No other variant can hide a message.
DistanceMapSection's two-level guard removed — the deleted guard was isDistanceRequest && !isManualDistanceRequest && !isOdometerDistanceRequest, which is now exactly the DistanceMapFooter dispatch condition. The Onyx-subscription saving is preserved (better: the component isn't mounted at all). Same for InvoiceSenderSection → InvoiceSenderField.
compactState / showMoreFields omitted in non-scan variants — the ConfirmationFieldList default {isCompactMode: false, setShowMoreFields: noop} equals what DefaultFooter passed, because isCompactMode is scan-only.
toggleHandlers = {} vs toggleHandlers ?? {} — the prop is optional and never null; equivalent.
Removed inner <View> that wrapped InvoiceSenderSection + DistanceMapSection — both self-returned null on every other flow, so it was a zero-height box with default flexShrink: 0.
Ordering preserved — InvoiceSenderField and DistanceMapSection both still render before ReceiptSection.
Merchant omitted from DistanceDetailsFields — shouldShowMerchant already had && !isDistanceRequest.
Prop-to-context migration in DistanceField / RateField / MerchantField / InvoiceSenderField — every moved prop is sourced from the same useConfirmationFields() value the old parent read, and there are no consumers outside the provider.
isParticipantPickerVisible dropped in DistanceDetailsFields — no-op, isAmountFieldDisabled is true for distance so the picker-close effect never runs. Same latent-coupling caveat as item 3.
Component-type swap on tab change now remounts the footer subtree instead of reconciling in place. showMoreFields is safe (it lives in the parent and already resets on transactionID change); the lost state is useCompactReceiptDimensions' cached aspect ratio and container width. Only reachable via a SPLIT Manual→Distance tab switch on a hidden tab, so not user-visible.
No stale imports to TransactionDetailsFields, InvoiceSenderSection or DefaultFooter remain.

CI is green on this branch apart from Check independent approval, including typecheck, all 8 test jobs, ESLint and React Compiler Compliance.

Caveat: I could not drive NewDot in a browser for this run, so none of the above was confirmed at runtime — this is static analysis only. The seven scenarios in your Tests section still need a manual pass, particularly Scan (compact layout + Show more) and Invoice.

view run

MelvinBot · 2026-09-04T11:14:57Z

🤖 Code quality review

The direction is right, and a few things here are genuinely good: pushing action / iouType / isReadOnly / didConfirm out of the field prop lists into useConfirmationFields(), replacing AmountField's autoFocus prop with an internal !canUseTouchScreen(), deleting the two-level guard wrappers now that the dispatcher owns the gate, and adding a dispatcher test at all.

Four things I'd fix before merge, then some smaller stuff.

1. Two dispatcher lines look symmetric and aren't

index.tsx:44 and :48 both read isDistanceRequest && isXDistanceRequest, but only one conjunct is doing work:

:44 is redundant. isManualDistanceRequest is derived from the same transaction as isDistanceRequest (MoneyRequestConfirmationList.tsx:256-258), and DISTANCE_MANUAL is in isDistanceExpenseType, so it implies it.
:48 is load-bearing. isOdometerDistanceRequest is a prop (MoneyRequestConfirmationList.tsx:122), and three callers pass transactions.some(isOdometerDistanceRequestUtil) over a whole set — DynamicIOURequestStepReport.tsx:232, DynamicIOURequestEditReport.tsx:229, SearchTransactionsChangeReport.tsx:320. It can be true for a non-distance expense.

Drop the redundant conjunct on :44 and add a comment on :48 saying why its check is real. Otherwise a future "simplification" deletes the wrong one and silently misroutes.

2. `isTypeInvoice` and `iouType === INVOICE` are now two sources of truth

The dispatcher branches on iouType === CONST.IOU.TYPE.INVOICE (index.tsx:28) while its direct child reads isTypeInvoice for the same question (ConfirmationFieldList.tsx:82). isTypeInvoice is literally iouType === CONST.IOU.TYPE.INVOICE at MoneyRequestConfirmationList.tsx:275.

The new test already shows the hazard: MoneyRequestConfirmationListFooter.test.tsx:112 renders with iouType={INVOICE} and never sets isTypeInvoice, which defaults to false (Provider.tsx:99). That render says both "this is an invoice" and "this is not." It only passes because the variants are mocked. Derive it in ConfirmationFieldList and drop it from the context.

3. The variants repeat ~72 lines of pure pass-through

DistanceManualFooter is DistanceMapFooter minus one JSX line. InvoiceFooter is ManualFooter plus one. The 8-prop <ConfirmationFieldList> block is written out verbatim in all eight variants, and toggleHandlers = {} appears eight times.

Splitting by expense type is the right call and I would not collapse the variants — but re-threading eight identical props isn't explicitness, it buries the two or three lines that actually differ. Two cheap fixes that keep the structure:

Give toggleHandlers a default in ConfirmationFieldList.tsx:60, exactly as compactState already has at :76. Removes eight lines, no behavior change.
Collect the shared block into one rest object so each variant destructures only what it uses. That takes the variants from ~394 lines to ~240 and makes the difference the majority of each file.

Worth avoiding aboveReceipt / belowReceipt ReactNode slot props as the fix — named slot props are configuration in disguise, and you'd trade one coupling for another.

4. `types.ts` has three identical aliases, and the `Omit` enforces nothing

TimeFooterProps (:49), ManualFooterProps (:59) and InvoiceFooterProps (:61) are character-for-character the same Omit<...>. Seven names, five shapes.

And the subtraction doesn't enforce what a reader will assume. TypeScript's excess-property check only fires on fresh object literals, so <InvoiceFooter {...props} /> with the full props object compiles fine — that's already true on main, where PerDiemFooterProps omits receiptOptions and the dispatcher spreads everything anyway. The Omit only stops the body from destructuring the name. Additive Pick composition would say what each variant needs and collapse the duplicates.

Smaller items

DetailsFieldsContext is stale. :7 still picks 'amount' | 'distance' | 'rate' | 'merchant' | 'time', but only amount and merchant are read now — DistanceDetailsFields renders Distance and Rate unconditionally and TimeDetailsFields renders TimeFields unconditionally. :9's comment ("suppresses ... Amount, Rate, Merchant, Time") is now wrong: only Amount and Merchant, only in ManualDetailsFields.

Sibling contracts disagree. DistanceDetailsFields and TimeDetailsFields both take isDescriptionRequired: boolean, while ManualDetailsFields:21 takes the whole requiredFlags bag — including isCategoryRequired, which it never reads. Two scalars would match its siblings. Also TimeDetailsFields is the only one of the three with no description comment, and TimeFooter / PerDiemFooter are the only two variants without one now that six documented ones sit beside them.

A comment points at a function this PR deleted. DistanceMapSection.tsx:19-20 says "the outer gate has already guaranteed..." — that gate was the sibling function in this file, and it now lives three files away in the dispatcher. Reword to name DistanceMapFooter. The three literal true / false arguments below it are also dead configuration now that shouldShowDistanceMap short-circuits on exactly those.

Two dispatcher comments describe the branch not taken. index.tsx:32-33 sits above the PerDiemFooter return but explains what happens when the condition is false; :38-39 does the same for TimeFooter. Reframe each as an explanation of the guard. Separately, the "residual, not a fallback" prose is now written three times — the dispatcher docblock, ManualFooter's header, and two test names. Keep the long version on ManualFooter.

Test gaps, each one line. Mocking all eight variants is right for a routing test. What's missing is the ordering the comments defend: renderFooter({isScanRequest: true}, CREATE, INVOICE) → InvoiceFooter pins invoice-first, and renderFooter({isOdometerDistanceRequest: true}) → ManualFooter pins the load-bearing conjunct from item 1. Adding expect.objectContaining(footerProps) to the existing it.each assertion would also cover the dispatcher's other job — forwarding props — for free. Minor: the {} case at :90 renders as an unhelpful title, and no case names the component it expects.

Folder nesting. fieldGroups/ holds ClassificationFields.tsx and SettingsFields.tsx flat, but the three details groups went into fieldGroups/detailsFields/. Five field groups doesn't earn a subfolder — I'd flatten, and now is the cheap moment since the import paths are already churning. Replacing fieldGroups/TransactionDetailsFields/index.tsx with named files is a clear improvement either way.

I couldn't run the suite locally, so the test observations are from reading. CI is green on this branch apart from Check independent approval.

view run

VickyStash · 2026-09-04T16:06:16Z

Melvin's feedback overview:

The invoice branch is checked first, and nothing enforces its invariant
Suggested: make the guard self-enforcing — iouType === CONST.IOU.TYPE.INVOICE && !isScanRequest && !isDistanceRequest, or move the invoice check after the scan/distance checks.

iouType === CONST.IOU.TYPE.INVOICE can't be scan/distance request, so I think the condition is okay the way it's rigth now.

hasBelowShowMore no longer matches what the compact variant renders
No live impact — scan, distance and time are mutually exclusive iouRequestType values, so both flags are always false under ScanFooter.

I think it's a small code cleanup we can do in the following PRs.

AmountField autofocus is now opt-out-by-accident
... Any future caller that isn't disabled silently grabs focus on desktop.

Since it currently works as expected, I wouldn't consider it's a problem for future callers.

Two dispatcher lines look symmetric and aren't

I think it's okay to keep it as is, considering it's more readable

isTypeInvoice and iouType === INVOICE are now two sources of truth

I think it's a cleanup that can be made in the following PRs.

The variants repeat ~72 lines of pure pass-through

types.ts has three identical aliases, and the Omit enforces nothing

THIS is actually valid, and it will be resolved when we split MoneyRequestConfirmationList.tsx, so every confirmation list will use its footer, and the necessary parameters for this footer will be passed.

VickyStash · 2026-09-04T16:06:38Z

I'll be OOO next week, back on September 14 🌴
If any action is needed before that, please reach to @TMisiukiewicz so he can help to find someone

situchan · 2026-09-04T16:09:57Z

Agree automated reviews are not blockers

…9449-confirmation-footers-pt2

mountiny

LGTM

github-actions · 2026-09-08T09:59:31Z

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

OSBotify · 2026-09-08T10:01:48Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-09-08T10:03:42Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #100041.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-378f5f8-57708e6-cf2cbdb35bf770c9fa8fca7f547e3ff668993dc2/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-378f5f8-57708e6-1ff4c5eaa2a1c0199ccf975bb26b00bd13db2600/index.html

Web 🕸️
https://100041.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-08T19:22:52Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.74-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

MelvinBot · 2026-09-08T19:26:11Z

No help site changes are required, so I did not create a draft PR.

This PR is an internal refactor with no behavior change. It splits MoneyRequestConfirmationListFooter into six per-type variants (DistanceMapFooter, DistanceManualFooter, DistanceOdometerFooter, ScanFooter, ManualFooter, InvoiceFooter) and deletes the shared fallback. Nothing a member sees on the confirmation page changes.

How I checked

All 26 changed files are under src/components/ or tests/unit/. No src/languages/*.ts files are touched, so no user-facing copy, field label, or button text changed.
The diff adds no new translate() calls and removes none, so the confirmation page renders the same labels as before.
The linked task, [Refactor] Split IOURequestStepConfirmation, MoneyRequestConfirmationList, and useExpenseSubmission per expense type #99449, is a Task to split IOURequestStepConfirmation, MoneyRequestConfirmationList, and useExpenseSubmission per expense type. It describes no product change.
I reviewed the help site articles that describe the confirmation page — including docs/articles/new-expensify/reports-and-expenses/Distance-Expenses.md — and none of them document the footer structure this PR reorganizes. A refactor with no behavior change cannot make them stale.

Since there is no help site PR, the review and Ready for review steps in your request don't apply here. If a follow-up PR in this series does change what a member sees, tag me on it and I'll draft the docs update then.

view run · view recording

OSBotify · 2026-09-09T20:14:35Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.74-2 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

VickyStash added 2 commits September 1, 2026 15:20

Extract distance footers

aa0f18c

Extract Invoice, Scan and Manual footers

1c20ea6

melvin-bot Bot assigned VickyStash Sep 1, 2026

VickyStash added 4 commits September 1, 2026 16:03

Use params from context

3a65421

Clean up

09422e4

Remove InvoiceSenderSection

b31c552

Clean up pt2

bf73a11

chatgpt-codex-connector Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread ...ponents/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/ManualDetailsFields.tsx

VickyStash added 3 commits September 1, 2026 17:18

Fix autoFocus prop

aef4d61

Add comments

50f45a2

Merge branch 'main' into VickyStash/refactor/99449-confirmation-foote…

37ab8ce

…rs-pt2

VickyStash mentioned this pull request Sep 1, 2026

[Refactor] Split IOURequestStepConfirmation, MoneyRequestConfirmationList, and useExpenseSubmission per expense type #99449

Open

Merge branch 'main' into VickyStash/refactor/99449-confirmation-foote…

0ff12e0

…rs-pt2 # Conflicts: # src/components/MoneyRequestConfirmationList/sections/InvoiceSenderField.tsx

VickyStash added 2 commits September 2, 2026 10:28

Apply minor updates

3f691a7

Add MoneyRequestConfirmationListFooter jest test

0a8efff

VickyStash marked this pull request as ready for review September 2, 2026 11:03

VickyStash requested review from a team as code owners September 2, 2026 11:03

melvin-bot Bot requested review from heyjennahay and situchan September 2, 2026 11:03

melvin-bot Bot removed the request for review from a team September 2, 2026 11:03

Merge branch 'main' into VickyStash/refactor/99449-confirmation-foote…

7118594

…rs-pt2

Merge branch 'main' into VickyStash/refactor/99449-confirmation-foote…

35bba5f

…rs-pt2

situchan approved these changes Sep 4, 2026

View reviewed changes

melvin-bot Bot requested a review from mountiny September 4, 2026 10:18

Merge remote-tracking branch 'origin/main' into VickyStash/refactor/9…

a32c2a8

…9449-confirmation-footers-pt2

mountiny approved these changes Sep 8, 2026

View reviewed changes

mountiny merged commit 378f5f8 into Expensify:main Sep 8, 2026
34 of 35 checks passed

OSBotify mentioned this pull request Sep 8, 2026

Deploy Checklist: New Expensify 2026-09-08 #100624

Closed

69 tasks done

applause-bot Bot mentioned this pull request Sep 9, 2026

[Exploratory] Expense - App keeps loading on distance expense flow created via shortcut. #100713

Open

1 of 7 tasks

This was referenced Sep 17, 2026

Split MoneyRequestConfirmationList #101198

Merged

Split useExpenseSubmission hook #101746

Merged

Conversation

VickyStash commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

VickyStash commented Sep 1, 2026

Uh oh!

VickyStash commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

MelvinBot commented Sep 1, 2026

🔴 autoFocus is no longer passed to AmountField

🟡 The router's branch order is load-bearing but undocumented

🟡 New files have no header description

🟢 Checked and fine

Tests

Duplication

🤖 Web smoke test: ⚠️ partial

Uh oh!

VickyStash commented Sep 2, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 2, 2026

Uh oh!

VickyStash commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026

🟡 The router is the point of this PR and has no test coverage

🟡 The footer shape now hinges on optional = false booleans

🟢 Nits

Uh oh!

VickyStash commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026

🟡 The new test doesn't lock in the invoice-first ordering

🟢 Still open from round 2 (both optional)

🤖 Web test steps verification results: ⚠️ partial

Uh oh!

melvin-bot Bot commented Sep 2, 2026

Uh oh!

situchan commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

situchan commented Sep 3, 2026

Uh oh!

VickyStash commented Sep 3, 2026

Uh oh!

VickyStash commented Sep 4, 2026

Uh oh!

situchan commented Sep 4, 2026

Uh oh!

situchan left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented Sep 4, 2026

Uh oh!

MelvinBot commented Sep 4, 2026

🤖 Regression review — no confirmed user-reachable regression

1. The invoice branch is checked first, and nothing enforces its invariant

2. hasBelowShowMore no longer matches what the compact variant renders

3. AmountField autofocus is now opt-out-by-accident

Test coverage gap

Uh oh!

MelvinBot commented Sep 4, 2026

🤖 Code quality review

VickyStash commented Sep 1, 2026 •

edited

Loading

codecov Bot commented Sep 1, 2026 •

edited by github-actions Bot

Loading

🔴 `autoFocus` is no longer passed to `AmountField`

🟡 The footer shape now hinges on optional `= false` booleans

situchan commented Sep 3, 2026 •

edited

Loading

2. `hasBelowShowMore` no longer matches what the compact variant renders

3. `AmountField` autofocus is now opt-out-by-accident

2. `isTypeInvoice` and `iouType === INVOICE` are now two sources of truth

4. `types.ts` has three identical aliases, and the `Omit` enforces nothing

MelvinBot commented Sep 8, 2026 •

edited

Loading