Repository navigation
ConfirmationPage - Composition over Configuration (full migration) #101224
Description
Problem
ConfirmationPage is a configuration-style wrapper: 24 flat props on ConfirmationPage.tsx:25-77. Ten of them exist only to configure its two buttons, and seven more are *Style escape hatches that exist because callers can't reach the regions they need to restyle:
// buttons
buttonText, onButtonPress, shouldShowButton, isButtonDisabled, isButtonLoading,
secondaryButtonText, onSecondaryButtonPress, shouldShowSecondaryButton,
isSecondaryButtonDisabled, isSecondaryButtonLoading
// style patches
headingStyle, illustrationStyle, descriptionStyle, ctaStyle, footerStyle,
containerStyle, innerContainerStyleEvery new capability means another should* / is* / on*Press / *Text / *Style prop. The shape doesn't scale to a third button, to button content the wrapper never anticipated (an icon, a DotIndicator, a badge), or to any per-screen layout ConfirmationPage didn't predict. It also already carries two duplicate string-or-node pairs — description / descriptionComponent and cta / ctaComponent — which is the configuration API admitting it needs slots.
feat: add request access button to the domain access restricted screen added two of those props (isSecondaryButtonDisabled, isSecondaryButtonLoading) and ZhenjaHorbach flagged the prop count in review.
This is the same "composition over configuration" problem as Button - Composition over Configuration, one layer up: decomposing Button itself doesn't help callers if the wrapper in between re-flattens the API.
Scope note: this issue originally covered only the 10 button props. Per the follow-up discussion it now covers the full
ConfirmationPagemigration to a compound component.
Proposal
Migrate ConfirmationPage to a compound component. Each region it renders becomes a slot the caller composes, matching the Button.Icon / Button.Text / Button.KeyboardShortcut shape already shipped in Button/index.tsx:29-33.
<ConfirmationPage>
<ConfirmationPage.Illustration src={LottieAnimations.Fireworks} />
<ConfirmationPage.Heading>{translate('domain.accessRestricted.title')}</ConfirmationPage.Heading>
<ConfirmationPage.Description>{translate('domain.accessRestricted.description')}</ConfirmationPage.Description>
<ConfirmationPage.Footer>
<ConfirmationPage.Button
isDisabled={!isRequestPending && (!!hasPendingRequest || isOffline)}
isLoading={isRequestPending}
onPress={requestDomainAdminship}
>
<Button.Text>{translate('domain.accessRestricted.requestAdminAccess')}</Button.Text>
</ConfirmationPage.Button>
</ConfirmationPage.Footer>
</ConfirmationPage>What this buys:
shouldShow*disappears. A slot renders when it's provided.- The
*Styleprops disappear.stylegoes on the subcomponent that owns the region, so nothing has to be plumbed through the wrapper. - The string/node pairs collapse.
description+descriptionComponentbecome one<ConfirmationPage.Description>; same forcta+ctaComponent. - Button content belongs to the caller — an icon, a badge, a
DotIndicator, or a third button all become possible without touchingConfirmationPage. - No visual drift.
ConfirmationPage.Buttonpre-appliesCONST.BUTTON_VARIANT.SUCCESS/CONST.BUTTON_SIZE.LARGE/styles.mt3so 30 screens stay consistent without each caller repeating them. This resolves the sizing/styling trade-off raised in the original write-up: sizing stays owned by the component, not the call site.
Layout logic stays inside ConfirmationPage — the ScrollView + centering container at ConfirmationPage.tsx:111-158 and the FixedFooter at ConfirmationPage.tsx:159-188. Only content and state move out.
Current props → target slots
| Current prop(s) | Target |
|---|---|
illustration, illustrationStyle |
<ConfirmationPage.Illustration src style> (keeps the Lottie / SVG / reduced-motion fallback branch) |
heading, headingStyle |
<ConfirmationPage.Heading style> |
description, descriptionComponent, descriptionStyle |
<ConfirmationPage.Description style> |
cta, ctaComponent, ctaStyle |
<ConfirmationPage.CTA style> |
footerComponent |
<ConfirmationPage.FooterMessage> (the absolutely-positioned region above the footer) |
buttonText, onButtonPress, shouldShowButton, isButtonDisabled, isButtonLoading |
<ConfirmationPage.Button> inside <ConfirmationPage.Footer> |
secondaryButtonText, onSecondaryButtonPress, shouldShowSecondaryButton, isSecondaryButtonDisabled, isSecondaryButtonLoading |
a second <ConfirmationPage.Button> |
footerStyle |
style on <ConfirmationPage.Footer> |
containerStyle, innerContainerStyle |
style on <ConfirmationPage> / <ConfirmationPage.Content> |
Exact subcomponent names are open to bikeshedding in the first PR.
Scope
src/components/ConfirmationPage.tsx— the component itself- 30 call sites under
src/pagesrender<ConfirmationPageonmainand all need migrating
Prop usage across the 30 call sites
shouldShowButton 30 descriptionComponent 5
buttonText 30 innerContainerStyle 5
onButtonPress 30 secondaryButtonText 4
heading 30 onSecondaryButtonPress 4
description 28 headingStyle 3
illustration 19 cta 3
containerStyle 18 ctaComponent 3
descriptionStyle 12 footerComponent 2
illustrationStyle 10 is(Secondary)Button* 1 each
shouldShowSecondaryButton 6
Two things stand out: containerStyle / descriptionStyle / illustrationStyle are used on a third to two-thirds of call sites (the configuration API is leaking), while the four is*Button* props are used exactly once each (they were added for one screen and every other caller pays for them in the type).
The 30 call sites, grouped for batching
domain (4)
src/pages/domain/BaseDomainVerifiedPage.tsxsrc/pages/domain/DomainAccessRestrictedPage.tsxsrc/pages/domain/DomainAddedPage.tsxsrc/pages/domain/DomainAlreadyExistsPage.tsx
settings/Wallet (9)
src/pages/settings/Wallet/AddCardToDigitalWalletPage.tsxsrc/pages/settings/Wallet/InternationalDepositAccount/subPages/Success.tsxsrc/pages/settings/Wallet/PersonalCards/PersonalCardsErrorConfirmation.tsxsrc/pages/settings/Wallet/PersonalCards/steps/SuccessStep.tsxsrc/pages/settings/Wallet/PersonalCards/upgrade/UpgradeConfirmation.tsxsrc/pages/settings/Wallet/ShareBankAccount/ShareBankAccount.tsxsrc/pages/settings/Wallet/SuccessReportCardLost.tsxsrc/pages/settings/Wallet/TransferBalancePage.tsxsrc/pages/settings/Wallet/UpdatePersonalBankAccountPage.tsx
settings/Security (4)
src/pages/settings/Security/LockAccount/FailedToLockAccountPage.tsxsrc/pages/settings/Security/LockAccount/UnlockAccountPage.tsxsrc/pages/settings/Security/MergeAccounts/MergeResultPage.tsxsrc/pages/settings/Security/TwoFactorAuth/SuccessPageBase.tsx
workspace (7)
src/pages/workspace/WorkspaceConfirmationSuccessPage.tsxsrc/pages/workspace/companyCards/WorkspaceCompanyCardsErrorConfirmation.tsxsrc/pages/workspace/downgrade/DowngradeConfirmation.tsxsrc/pages/workspace/members/DynamicWorkspaceOwnerChangeSuccessPage.tsxsrc/pages/workspace/receiptPartners/DynamicInviteReceiptPartnerPolicyPage.tsxsrc/pages/workspace/upgrade/UpgradeConfirmation.tsxsrc/pages/workspace/workflows/WorkspaceWorkflowsPayerSuccessPage.tsx
bank accounts / payments (4)
src/pages/AddPersonalBankAccountPage/substeps/SuccessStep.tsxsrc/pages/EnablePayments/Pay/Activate/ActivateStep.tsxsrc/pages/ReimbursementAccount/ReimbursementAccountPage.tsxsrc/pages/ReimbursementAccount/USD/ConnectBankAccount/ConnectBankAccount.tsx
misc (2)
src/pages/TransactionDuplicate/DynamicReviewPage.tsxsrc/pages/inbox/report/DynamicReportDetailsExportPage.tsx
Migration strategy
Incremental, mirroring the parent issue's approach so nothing lands as one 30-file PR:
- Add the subcomponents alongside the existing props.
ConfirmationPagerenderschildrenwhen present and falls back to the flat-prop rendering otherwise, somainstays green at every step. - Mark all 24 flat props
@deprecatedwith a pointer to the slot that replaces each one. - Migrate the call sites in batches by area (the groups above are a reasonable split — roughly 5 PRs).
- Delete the deprecated props and the fallback branch once no call site uses them.
Once the shape is agreed for ConfirmationPage, run the same audit over the other Button wrappers that flatten button state into their own props.
Context
- Follow-up from the review discussion on feat: add request access button to the domain access restricted screen: feat: add request access button to the domain access restricted screen #100319 (comment)
- Full-migration proposal and the decision to widen this issue: Slack thread
Activity
Guccio163 commentedon Sep 16, 2026
Hi @mountiny and @ZhenjaHorbach, I created a proposal for the full
ConfirmationPagecomposition, which would also handle this case; If it gets accepted we could close this issue ✅Guccio163 commentedon Sep 16, 2026
Basing on this comment, we'll handle the whole ConfirmationPage migration (proposed here) in this issue - @MelvinBot rename it and update the description then
[-]ConfirmationPage - Composition over Configuration for its button props[/-][+]ConfirmationPage - Composition over Configuration (full migration)[/+]MelvinBot commentedon Sep 16, 2026
Renamed to ConfirmationPage - Composition over Configuration (full migration) and rewrote the description to cover the whole component, not just the two buttons.
What changed in the description:
*Styleescape hatches and the two duplicate string-or-node pairs (description/descriptionComponent,cta/ctaComponent).ConfirmationPage.Illustration/.Heading/.Description/.CTA/.Footer/.Button, with a prop → slot mapping table.ConfirmationPage.ButtonkeepsCONST.BUTTON_SIZE.LARGE+ margin, which settles the sizing trade-off the old description left open.containerStyle(18),descriptionStyle(12) andillustrationStyle(10) are used widely — the configuration API is already leaking — while each of the fouris*Button*props is used exactly once.@deprecatedall 24 → migrate in ~5 area PRs → delete the props and the fallback.melvin-bot commentedon Oct 12, 2026
This issue has not been updated in over 15 days. @mountiny, @ZhenjaHorbach, @Guccio163 eroding to Monthly issue.
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!
Guccio163 commentedon Oct 13, 2026
The PR just got review by C+, I'll be addressing the comments soon