feat(SDK-1129): add HistoricalPaymentsFlow - #2552
Conversation
There was a problem hiding this comment.
This is the one thing that's a bit weird. The components for HistoricalPaymentsFlow are marked alpha but this is live-updating the public PaymentFlow which feels... a bit weird. Should there be a way to mark an alpha feature as opt-in?
There was a problem hiding this comment.
For a similar situation with i9 in employee onboarding, I ended up just going with a prop that was available at the top level of the flow component. LIke withEmployeeI9 defaulted to false on the initial rollout. If you're concerned about releasing pre test fest, that might be a good way to keep it hidden while the test fest and fixes happen. Then when we feel ready we can remove the flag
You could also go more descriptive with the prop like withUnstableInProgressHistoricalPaymentsUseWithCaution or similar
There was a problem hiding this comment.
from slack discussion: removing the PaymentFlow changes and only adding new alpha surface area in this PR
serikjensen
left a comment
serikjensen
left a comment
There was a problem hiding this comment.
Looking good! couple of things
- Would recommend adding the boolean flag prop and defaulting it to false while we're rolling this out pre test fest (as described in slack)
- We should align with Aaron on breadcrumb use, i think we're getting away from that so that could simplify some things in the implementation
- Recommendation to update the state machine to properly re-create on company id change
There was a problem hiding this comment.
For a similar situation with i9 in employee onboarding, I ended up just going with a prop that was available at the top level of the flow component. LIke withEmployeeI9 defaulted to false on the initial rollout. If you're concerned about releasing pre test fest, that might be a good way to keep it hidden while the test fest and fixes happen. Then when we feel ready we can remove the flag
You could also go more descriptive with the prop like withUnstableInProgressHistoricalPaymentsUseWithCaution or similar
| export const historicalPaymentBreadcrumbsNodes: BreadcrumbNodes = { | ||
| createHistoricalPayment: { | ||
| parent: null, | ||
| item: { | ||
| id: 'createHistoricalPayment', | ||
| label: 'breadcrumbLabel', | ||
| namespace: 'Contractor.Payments.CreateHistoricalPayment', | ||
| onNavigate: ((ctx: HistoricalPaymentFlowContextInterface) => ({ | ||
| ...updateBreadcrumbs('createHistoricalPayment', ctx), | ||
| })) as (context: unknown) => unknown, | ||
| }, | ||
| }, | ||
| historicalPaymentSummary: { | ||
| parent: null, | ||
| item: { | ||
| id: 'historicalPaymentSummary', | ||
| label: 'breadcrumbLabel', | ||
| namespace: 'Contractor.Payments.HistoricalPaymentSummary', | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
It's worth touching base with @aaronlee777 on the breadcrumb use here. I think breadcrumbs are generally something we're getting away from. He may also have an alternative for nav recommendation for the flow
| return createMachine( | ||
| 'createHistoricalPayment', | ||
| historicalPaymentMachine, | ||
| (initialContext: HistoricalPaymentFlowContextInterface) => ({ | ||
| ...initialContext, | ||
| ...initialBreadcrumbContext, | ||
| component: CreateHistoricalPaymentContextual, | ||
| companyId, | ||
| }), |
There was a problem hiding this comment.
You might consider placing the machine creation in a useMemo here instead. Ex. if we got a new company id, we'd want to tear down the machine and create a new one
Chains CreateHistoricalPayment -> HistoricalPaymentSummary, mirroring CreatePaymentFlow's shape exactly: two-state machine driven by contractor/historicalPayments/created, lazy useState machine init (not useMemo) so in-flight state survives a host re-render, and a plain zero-transition summary state rather than a robot3 final() state, since this can be mounted standalone and must not dead-end if the host doesn't unmount it after completion. Exported alongside its own breadcrumb labels; not yet reachable from PaymentFlow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ow export Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6343b14 to
16ce490
Compare
2 participants
Summary
HistoricalPaymentFlow, chainingCreateHistoricalPayment->HistoricalPaymentSummary(mirrorsCreatePaymentFlow's shape: two-state machine, lazy-init so in-flight state survives a host re-render, non-final terminal state so the summary screen stays interactive if not unmounted).PaymentFlow: newcontractor/historicalPayments/createevent, a "Record a historical payment" CTA on the payments list, and ahistoricalPaymenthub state that returns tolandingon exit.HistoricalPaymentFlowpublicly (@alpha) alongside its breadcrumb labels, and regenerates the derived API report / docs / i18n types.flows.mov
Test plan
npm run tscnpm run test -- --run src/components/Contractor/Payments(81 tests passing)HistoricalPaymentFlow.test.tsxcovering the create -> summary chain and exitPaymentFlow.test.tsxcovering CTA entry, the full create -> summary -> exit chain through the hub, and re-render resilience mid-flow🤖 Generated with Claude Code