Skip to content

ConfirmationPage composition migration - #103070

Merged
mountiny merged 31 commits into
Expensify:mainfrom
software-mansion-labs:Guccio163/confirmationPage/introduction
Oct 21, 2026
Merged

mountiny merged 31 commits into
Expensify:mainfrom
software-mansion-labs:Guccio163/confirmationPage/introduction

Conversation

@Guccio163

@Guccio163 Guccio163 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

ConfirmationPage is migrated from a single monolithic component to a composition-based API, following the same pattern already established for Button and Header.

  • A new primitive set (ConfirmationPage.Illustration, .Heading, .Description, .PrimaryButton, .SecondaryButton, .Content) lives under src/components/ConfirmationPage/, exposed as a compound component via Object.assign.
  • ConfirmationPageDefault is a composed preset covering the common illustration/heading/description/button shape, for callers that don't need custom composition.
  • Callers that need extra behavior (a secondary button, a CTA, a custom descriptionComponent, or an inline footer error) compose the primitives directly: <ConfirmationPage><ConfirmationPage.Content>…</ConfirmationPage.Content><FixedFooter>…</FixedFooter></ConfirmationPage>.
  • All 33 real call sites of the old monolithic ConfirmationPage are migrated (24 onto ConfirmationPageDefault, the rest onto direct composition where the preset doesn't fit). The old src/components/ConfirmationPage.tsx is deleted — nothing imports it anymore.
  • MergeResultPage's large per-result config object (the most complex caller, with CTA/secondary-button/footer variants per merge-account result) is extracted into a useMergeResultConfirmationConfig hook, so the page component itself only renders.
  • This is a pure composition refactor: no visual or behavioral change for any existing caller. Verified with tsc, lint, spell, React Compiler compliance, and the existing Jest suite (including the UI tests that render these pages) — all clean, no regressions.

Fixed Issues

$ #101224
PROPOSAL:

Tests

Test case 1: ConfirmationPageDefault preset (duplicate-expense review empty state)

  1. Create two expenses with the same merchant, amount, and date (so they're flagged as potential duplicates).
  2. Open one of them and tap Review duplicates, then tap Keep all to resolve the violation.
  3. Open that same expense's thread, tap its header title to open Details, tap Share, then Copy URL. The copied link looks like https://staging.new.expensify.com/r/<reportID> — it only opens the expense thread, not the Review duplicates screen.
  4. Take the reportID from the copied URL and open the review route directly in a browser tab (use the expense thread's reportID in both places):
    https://staging.new.expensify.com/r/<reportID>/duplicates/review/<reportID>
    (On mobile, open the same URL via a deep link, e.g. adb shell am start -n <package>/ExpensifyActivityBase -d <url> on Android or xcrun simctl openurl booted <url> on iOS simulator.)
  5. Verify the "All set!" confirmation screen (firework illustration, heading, "There are no duplicate transactions for review here." description, Got it button) renders correctly — this is DynamicReviewPage's empty-state branch, composed with ConfirmationPageDefault.
  6. Tap the Got it button and verify it navigates back to the expense thread.
  • Verify that no errors appear in the JS console

Offline tests

N/A — this PR only changes how existing UI is composed internally; it doesn't touch data fetching, Onyx, or network logic, so there's no new offline-specific behavior to test.

QA Steps

Same as Tests section above.

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • 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) — N/A, no new user-facing copy was added
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers — N/A, no new pattern: this follows the Button/Header composition pattern already agreed and merged
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected) — all 33 existing ConfirmationPage call sites were migrated and verified, not just a sample
  • If a new CSS style is added I verified that: — N/A, no new style added
    • 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 new assets were added or existing ones were modified, I verified that: — N/A, no new assets
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • 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. — N/A
  • 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. — N/A, no Storybook story exists for ConfirmationPage
  • 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. — N/A, ConfirmationPage is not itself a deeplink target
  • 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: — N/A, no visual change, internal composition only
    • 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 the PR adds or modifies the UI: — no new/changed UI, so the sub-items below weren't run as a dedicated a11y pass; the migration preserves every existing accessibility prop (testID, Button's built-in role/label handling) unchanged
    • I asked an AI agent to review the changes for accessibility issues and addressed its findings.
    • I tested with a screen reader (VoiceOver on macOS) and verified all new/changed elements are reachable with a logical focus order.
    • I verified all new/changed elements have meaningful accessible names and roles.
    • I verified state changes are announced (e.g. checked/unchecked, expanded/collapsed, selected).
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow. — N/A, this is a refactor with no behavior change; existing tests for the migrated pages already cover it and all pass unmodified
  • 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.

Screenshots/Videos

Android: Native
android-native.mp4
Android: mWeb Chrome
iOS: Native
ios-native.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
macos-chrome.mp4

Rename ConfirmationPageButtonBase/SecondaryButton exports to match their
file names, and drop the en dash from the new index.tsx header comment.
@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

…hook

Also rename ConfirmationPage layout/Content and style props to StyleProp
so file names match their exports and style props accept arrays/conditionals.
@Guccio163
Guccio163 marked this pull request as ready for review October 8, 2026 12:02
@Guccio163
Guccio163 requested review from a team as code owners October 8, 2026 12:02
@melvin-bot
melvin-bot Bot requested review from MariaHCD and dylanexpensify and removed request for a team October 8, 2026 12:02
@melvin-bot

melvin-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

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

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@Guccio163

Copy link
Copy Markdown
Contributor Author

@ZhenjaHorbach comments applied, thanks for the review!

@ZhenjaHorbach

Copy link
Copy Markdown
Contributor

Thanks
Will check today

@Guccio163
And we have conflicts 🥲

@Guccio163

Copy link
Copy Markdown
Contributor Author

@ZhenjaHorbach not anymore 😁

@ZhenjaHorbach

ZhenjaHorbach commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Changes look good!
@Guccio163
But let's fix conflicts one more time

…-fork into Guccio163/confirmationPage/introduction
@ZhenjaHorbach

Copy link
Copy Markdown
Contributor

LGTM!

@mountiny mountiny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@mountiny
mountiny merged commit b84ade7 into Expensify:main Oct 21, 2026
37 of 41 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

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

@OSBotify

Copy link
Copy Markdown
Contributor

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

@OSBotify

Copy link
Copy Markdown
Contributor

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

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

@MelvinBot

Copy link
Copy Markdown
Contributor

No help site update needed. This PR refactors how ConfirmationPage is built internally, and it doesn't change any user-facing text, labels, or behavior. No existing article sentence became incorrect.


view run

@m-natarajan

Copy link
Copy Markdown

@Guccio163 @ZhenjaHorbach @MelvinBot All set confirmation screen not displayed. Are we missing anythinh here. Can you please check the video attached?

1-web.mp4

@MelvinBot

Copy link
Copy Markdown
Contributor

This looks like a problem with test steps 3–4, not with this PR. Share → Copy URL copies the expense thread link (/r/<reportID>), not the Review duplicates link. Opening it lands on the thread, so the "All set!" screen never appears.

To reach the screen, open the review route for that expense directly:

https://staging.new.expensify.com/r/<reportID>/duplicates/review/<reportID>

Use the expense thread's reportID in both places. You can take it from the copied URL.

Details
  • The PR didn't change when "All set!" shows. The condition is identical before and after the merge (DynamicReviewPage.tsx:270). It renders only when the expense has no remaining duplicate violation. The PR only swapped ConfirmationPage for ConfirmationPageDefault.
  • Copy URL builds the plain report route (ShareCodePage.tsx:161).
  • The review screen lives at duplicates/review/:reportID, added after the report path (ROUTES.ts:1844).

I couldn't open the attached video, so I'm assuming you followed steps 3–4 as written. If you opened the review URL directly and still didn't see "All set!", please share the URL you used.

Guccio163, you may want to update the Tests/QA steps to use the direct review URL.


view run

@m-natarajan

Copy link
Copy Markdown

@Guccio163 @MelvinBot Hmm.. it's not here page displayed when review route URL is used

Snip - (9) New Expensify - Google Chrome

@MelvinBot

Copy link
Copy Markdown
Contributor

The URL in your screenshot uses the expense report's ID (6630504282605630, "Expense Report 2026-10-21"). The review page needs the ID of the single expense's thread. With a report ID, the page finds no linked expense and shows "Hmm… it's not here". This PR didn't change that.

To get the right ID:

  1. Open the report and click the expense you resolved with Keep all.
  2. Copy the ID after /r/ in that expense's URL.
  3. Open https://staging.new.expensify.com/r/<threadID>/duplicates/review/<threadID>, using that ID in both places.
Details
  • The page gets the expense from the report's parent action (DynamicReviewPage.tsx:94-95). Only an expense thread has a parent action linked to an expense. An expense report's parent is the workspace chat preview, so no expense is found.
  • With no expense found, the page shows not found (DynamicReviewPage.tsx:137). "All set!" shows only when an expense is found and has no duplicates left (DynamicReviewPage.tsx:270).
  • In this file, the PR only swapped ConfirmationPage for ConfirmationPageDefault. It didn't touch the not-found logic.

view run

@OSBotify

Copy link
Copy Markdown
Contributor

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

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

@MelvinBot

Copy link
Copy Markdown
Contributor

No help site update is needed. This PR only refactors how ConfirmationPage is built internally. No user-facing labels, copy, flows, or betas change: no src/languages files are touched, and the same translation keys render on every migrated page.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/grgia in version: 9.5.18-1 🚀

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

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants