Skip to content

Commit b4008b4

Browse files
committed
refactor(ui): drop wizard onComplete prop for parent fall-through
A nested wizard's terminal goNext now bubbles to the parent wizard's goNext automatically — the nested wizard already holds the parent in context, so the explicit onComplete callback was redundant. Remove onComplete from the Wizard primitive and its callsites (the configure sub-flow, the verify-domain inner flow, and the four SAML provider sub-flows). A top-level wizard with no parent treats a terminal goNext as a no-op.
1 parent 15abeea commit b4008b4

13 files changed

Lines changed: 76 additions & 89 deletions

File tree

packages/ui/src/components/ConfigureSSO/elements/Wizard/Wizard.tsx

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,6 @@ interface RootProps {
2222
* Where `reset()` lands, force-enabled (defaults to the first declared step).
2323
*/
2424
resetStepId?: string;
25-
/**
26-
* Terminal completion hook for the wizard's forward boundary. When the last
27-
* step's `goNext` has nowhere to advance, `onComplete` is invoked instead of
28-
* falling through to a parent wizard. Used by nested ConfigureSSO sub-flows
29-
* to bubble their inner terminal step into the top-level wizard.
30-
*
31-
* NOTE: this is a transitional escape hatch; once nested flows rely purely on
32-
* the parent-wizard fall-through it can be removed.
33-
*/
34-
onComplete?: () => void;
3525
}
3626

3727
const EMPTY_GUARDS: WizardGuards = {};
@@ -85,7 +75,7 @@ const deriveDescriptors = (children: React.ReactNode): WizardStepDescriptor[] =>
8575
return descriptors;
8676
};
8777

88-
const Root = ({ children, guards, initialStepId, resetStepId, onComplete }: RootProps): JSX.Element => {
78+
const Root = ({ children, guards, initialStepId, resetStepId }: RootProps): JSX.Element => {
8979
const parentWizard = React.useContext(WizardContext);
9080

9181
// Synchronous, render-time derivation of the step graph from JSX children.
@@ -98,7 +88,7 @@ const Root = ({ children, guards, initialStepId, resetStepId, onComplete }: Root
9888
[descriptors, guards, resetStepId],
9989
);
10090

101-
const value = useWizardMachine({ config, parentWizard, onComplete, initialStepId });
91+
const value = useWizardMachine({ config, parentWizard, initialStepId });
10292

10393
return <WizardContext.Provider value={value}>{children}</WizardContext.Provider>;
10494
};
@@ -133,8 +123,8 @@ Step.displayName = 'Wizard.Step';
133123
* satisfied step, gate out a step) is expressed declaratively via the named
134124
* `guards` record + the step's `guard` / `enabled` props — never by
135125
* adding/removing children. Inner sub-flows nest another `<Wizard>` inside a
136-
* step body; their forward boundary falls through to the parent wizard (or an
137-
* injected `onComplete`).
126+
* step body; their forward boundary falls through to the parent wizard — a
127+
* nested terminal `goNext` advances the parent automatically.
138128
*/
139129
export const Wizard = Object.assign(Root, {
140130
Step,

packages/ui/src/components/ConfigureSSO/elements/Wizard/__tests__/Wizard.test.tsx

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,8 @@ const NestedInner = (): JSX.Element => {
224224
);
225225
};
226226

227-
const Nested = ({ onComplete }: { onComplete?: () => void }): JSX.Element => (
228-
<Wizard onComplete={onComplete}>
227+
const Nested = (): JSX.Element => (
228+
<Wizard>
229229
<NestedInner />
230230
<Wizard.Step id='i1'>
231231
<div>i1</div>
@@ -236,16 +236,11 @@ const Nested = ({ onComplete }: { onComplete?: () => void }): JSX.Element => (
236236
</Wizard>
237237
);
238238

239-
const NestedBridge = (): JSX.Element => {
240-
const { goNext } = useWizard();
241-
return <Nested onComplete={goNext} />;
242-
};
243-
244239
const ParentWithNested = (): JSX.Element => (
245240
<Wizard>
246241
<Probe />
247242
<Wizard.Step id='outer-1'>
248-
<NestedBridge />
243+
<Nested />
249244
</Wizard.Step>
250245
<Wizard.Step id='outer-2'>
251246
<div>outer-2</div>
@@ -258,13 +253,14 @@ describe('Wizard — nested sub-flows', () => {
258253
render(<ParentWithNested />);
259254

260255
// Parent derives only its own two steps — the nested wizard's steps are
261-
// isolated behind the Bridge component boundary.
256+
// isolated behind the component boundary.
262257
expect(text('active')).toBe('outer-1,outer-2');
263258
expect(text('current')).toBe('outer-1');
264259
expect(screen.getByTestId('inner-current').textContent).toBe('i1');
265260

266-
// Advance the inner wizard to its last step, then once more → bubbles to the
267-
// parent's goNext via onComplete.
261+
// Advance the inner wizard to its last step, then once more → the terminal
262+
// goNext bubbles to the parent's goNext automatically (the nested wizard
263+
// reads the parent from context; no onComplete callback).
268264
click('inner-next');
269265
expect(screen.getByTestId('inner-current').textContent).toBe('i2');
270266
click('inner-next');

packages/ui/src/components/ConfigureSSO/elements/Wizard/__tests__/useWizardMachine.test.tsx

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,37 @@ describe('useWizardMachine', () => {
102102
expect(result.current.current).toBe('select-provider');
103103
});
104104

105-
it('invokes onComplete when goNext has nowhere to advance', () => {
106-
let completed = 0;
105+
it('bubbles a terminal goNext to the parent wizard', () => {
106+
let parentAdvanced = 0;
107+
const parentWizard = { goNext: () => (parentAdvanced += 1) } as unknown as Parameters<
108+
typeof useWizardMachine
109+
>[0]['parentWizard'];
110+
const { result } = renderHook(() =>
111+
useWizardMachine({
112+
config: config({ domainVerified: true, configured: true, tested: true, hasConnection: true }),
113+
parentWizard,
114+
initialStepId: 'confirmation',
115+
}),
116+
);
117+
expect(result.current.current).toBe('confirmation');
118+
119+
act(() => result.current.goNext());
120+
121+
// Nowhere to advance inside this wizard → falls through to the parent.
122+
expect(parentAdvanced).toBe(1);
123+
expect(result.current.current).toBe('confirmation');
124+
});
125+
126+
it('treats a terminal goNext as a no-op when there is no parent', () => {
107127
const { result } = renderMachine(
108128
{ domainVerified: true, configured: true, tested: true, hasConnection: true },
109-
{ onComplete: () => (completed += 1), initialStepId: 'confirmation' },
129+
{ initialStepId: 'confirmation' },
110130
);
111131
expect(result.current.current).toBe('confirmation');
112132

113133
act(() => result.current.goNext());
114134

115-
expect(completed).toBe(1);
135+
// Top-level wizard, no parent to bubble to → stays put, no throw.
116136
expect(result.current.current).toBe('confirmation');
117137
});
118138

packages/ui/src/components/ConfigureSSO/elements/Wizard/reducer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export const reduce = (state: WizardState, event: WizardEvent, config: WizardCon
127127
}
128128
}
129129
// Nothing non-satisfied ahead — stay put (the host falls through to a
130-
// parent wizard / onComplete on referential equality).
130+
// parent wizard on referential equality).
131131
return state;
132132
}
133133

packages/ui/src/components/ConfigureSSO/elements/Wizard/types.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,9 @@ export interface WizardContextValue {
154154
isLastStep: boolean;
155155
/**
156156
* Navigate forward. Walks to the next enabled, non-satisfied step. On the
157-
* last step, invokes the wizard's `onComplete` (if any) or falls through to
158-
* the parent wizard's `goNext`.
157+
* last step, falls through to the parent wizard's `goNext` (so a nested
158+
* sub-flow's terminal step advances the parent); a no-op when there is no
159+
* parent.
159160
*/
160161
goNext: () => void;
161162
/**

packages/ui/src/components/ConfigureSSO/elements/Wizard/useWizardMachine.ts

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,12 @@ import type { WizardActiveStep, WizardContextValue } from './types';
1414
/**
1515
* Inputs the host wizard wires the machine to. The `config` carries the step
1616
* descriptors (derived synchronously from `<Wizard.Step>` children) + the
17-
* named-guards record from `<Wizard guards>`. `parentWizard`, `onComplete`, and
18-
* `initialStepId` support nested sub-flows and resume-from-state.
17+
* named-guards record from `<Wizard guards>`. `parentWizard` and `initialStepId`
18+
* support nested sub-flows and resume-from-state.
1919
*/
2020
interface UseWizardMachineArgs {
2121
config: WizardConfig;
2222
parentWizard: WizardContextValue | null;
23-
onComplete?: () => void;
2423
/**
2524
* Mount on this step instead of the guard-derived initial step. Used by
2625
* linear nested flows that resume from an externally-decided position.
@@ -36,6 +35,14 @@ interface UseWizardMachineArgs {
3635
* `WizardState` and feeds the reducer the CURRENT config (descriptors + guards)
3736
* at dispatch time.
3837
*
38+
* ## Forward boundary: parent fall-through
39+
*
40+
* When a nested wizard's `goNext` lands on a step with nowhere to advance, the
41+
* event bubbles to the parent wizard's `goNext` (read from `parentWizard` in
42+
* context). This is how a nested sub-flow's terminal step advances the
43+
* top-level wizard — no bespoke completion callback. A top-level wizard (no
44+
* parent) treats a terminal `goNext` as a no-op.
45+
*
3946
* ## Why a ref instead of a `useEffect`
4047
*
4148
* The reducer needs the latest descriptors/guards whenever an event is
@@ -47,14 +54,9 @@ interface UseWizardMachineArgs {
4754
* This keeps `dispatch` (and the whole nav surface) stable across renders.
4855
*
4956
* Nothing about ConfigureSSO leaks in: the hook only knows descriptors, named
50-
* guards, a parent wizard, and an optional completion hook.
57+
* guards, and a parent wizard.
5158
*/
52-
export const useWizardMachine = ({
53-
config,
54-
parentWizard,
55-
onComplete,
56-
initialStepId,
57-
}: UseWizardMachineArgs): WizardContextValue => {
59+
export const useWizardMachine = ({ config, parentWizard, initialStepId }: UseWizardMachineArgs): WizardContextValue => {
5860
const isNested = parentWizard !== null;
5961

6062
// Seed lazily so the wizard mounts on the right step in a single render pass:
@@ -71,24 +73,19 @@ export const useWizardMachine = ({
7173
configRef.current = config;
7274

7375
// A stable dispatch that reduces against live config and falls through to the
74-
// parent wizard / onComplete when a forward step has nowhere to go. We read
75-
// the parent + onComplete from refs so dispatch identity stays stable.
76+
// parent wizard when a forward step has nowhere to go. We read the parent from
77+
// a ref so dispatch identity stays stable.
7678
const parentRef = React.useRef(parentWizard);
7779
parentRef.current = parentWizard;
78-
const onCompleteRef = React.useRef(onComplete);
79-
onCompleteRef.current = onComplete;
8080

8181
const dispatch = React.useCallback((event: WizardEvent) => {
8282
setState(prev => {
8383
const next = reduce(prev, event, configRef.current);
8484
// Forward fall-through: NEXT produced no transition (already on the last
85-
// step). Complete into the host (onComplete) or bubble to a parent wizard.
85+
// step). Bubble to the parent wizard so a nested sub-flow's terminal step
86+
// advances the top-level wizard. No parent → terminal NEXT is a no-op.
8687
if (event.type === 'NEXT' && next === prev) {
87-
if (onCompleteRef.current) {
88-
onCompleteRef.current();
89-
} else {
90-
parentRef.current?.goNext();
91-
}
88+
parentRef.current?.goNext();
9289
}
9390
return next;
9491
});

packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/index.tsx

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { descriptors, Flow } from '@/customizables';
44

55
import { useConfigureSSO } from '../../ConfigureSSOContext';
66
import { Step } from '../../elements/Step';
7-
import { useWizard } from '../../elements/Wizard';
87
import type { ProviderType } from '../../types';
98
import {
109
SamlCustomConfigureSteps,
@@ -13,18 +12,7 @@ import {
1312
SamlOktaConfigureSteps,
1413
} from './saml';
1514

16-
/**
17-
* Props every provider sub-flow accepts. Each provider hosts its OWN nested
18-
* `<Wizard>` with statically-declared `<Wizard.Step>` children, so the generic
19-
* engine can derive its step graph at that boundary. `onComplete` bubbles the
20-
* inner terminal step into the top-level wizard (kept for now; a later sub-item
21-
* drops it for plain parent fall-through).
22-
*/
23-
export type ProviderConfigureStepsProps = {
24-
onComplete: () => void;
25-
};
26-
27-
const STEPS_BY_PROVIDER: Record<ProviderType, (props: ProviderConfigureStepsProps) => JSX.Element> = {
15+
const STEPS_BY_PROVIDER: Record<ProviderType, () => JSX.Element> = {
2816
saml_custom: SamlCustomConfigureSteps,
2917
saml_okta: SamlOktaConfigureSteps,
3018
saml_google: SamlGoogleConfigureSteps,
@@ -33,9 +21,6 @@ const STEPS_BY_PROVIDER: Record<ProviderType, (props: ProviderConfigureStepsProp
3321

3422
export const ConfigureStep = (): JSX.Element | null => {
3523
const { provider } = useConfigureSSO();
36-
// Switching provider unmounts/mounts a whole nested wizard as a unit. The
37-
// top-level wizard advances when the inner flow completes, via `onComplete`.
38-
const { goNext } = useWizard();
3924

4025
// Type guard, at this point the provider should have been defined
4126
if (!provider) {
@@ -49,13 +34,17 @@ export const ConfigureStep = (): JSX.Element | null => {
4934
throw new Error(`No steps found for provider: ${provider}`);
5035
}
5136

37+
// Switching provider unmounts/mounts a whole nested wizard as a unit. Each
38+
// provider hosts its own nested `<Wizard>`; when its terminal step calls
39+
// `goNext`, the wizard primitive bubbles it to this top-level wizard (the
40+
// nested wizard's parent in context), advancing `configure` → `test`.
5241
return (
5342
<Flow.Part part='configureCreateApp'>
5443
<Step
5544
elementDescriptor={descriptors.configureSSOStep}
5645
elementId={descriptors.configureSSOStep.setId('configure')}
5746
>
58-
<StepsByProvider onComplete={goNext} />
47+
<StepsByProvider />
5948
</Step>
6049
</Flow.Part>
6150
);

packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlCustomConfigureSteps.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import { useConfigureSSO } from '../../../ConfigureSSOContext';
2525
import { Step } from '../../../elements/Step';
2626
import { useWizard, Wizard } from '../../../elements/Wizard';
2727
import { InnerStepCounter } from '../../../elements/Wizard/InnerStepCounter';
28-
import type { ProviderConfigureStepsProps } from '../index';
2928
import {
3029
applySamlSubmitError,
3130
buildSamlConfigurationPayload,
@@ -37,9 +36,9 @@ import {
3736
type IdpConfigurationMode,
3837
} from './shared/IdentityProviderConfigurationModes';
3938

40-
export const SamlCustomConfigureSteps = ({ onComplete }: ProviderConfigureStepsProps): JSX.Element => {
39+
export const SamlCustomConfigureSteps = (): JSX.Element => {
4140
return (
42-
<Wizard onComplete={onComplete}>
41+
<Wizard>
4342
<Wizard.Step id='create-app'>
4443
<Step.Header
4544
title={localizationKeys('configureSSO.configureStep.samlCustom.mainHeaderTitle')}

packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlGoogleConfigureSteps.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { useConfigureSSO } from '../../../ConfigureSSOContext';
1212
import { Step } from '../../../elements/Step';
1313
import { useWizard, Wizard } from '../../../elements/Wizard';
1414
import { InnerStepCounter } from '../../../elements/Wizard/InnerStepCounter';
15-
import type { ProviderConfigureStepsProps } from '../index';
1615
import {
1716
applySamlSubmitError,
1817
buildSamlConfigurationPayload,
@@ -24,9 +23,9 @@ import {
2423
type IdpConfigurationMode,
2524
} from './shared/IdentityProviderConfigurationModes';
2625

27-
export const SamlGoogleConfigureSteps = ({ onComplete }: ProviderConfigureStepsProps): JSX.Element => {
26+
export const SamlGoogleConfigureSteps = (): JSX.Element => {
2827
return (
29-
<Wizard onComplete={onComplete}>
28+
<Wizard>
3029
<Wizard.Step id='create-app'>
3130
<Step.Header
3231
title={localizationKeys('configureSSO.configureStep.samlGoogle.mainHeaderTitle')}

packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlMicrosoftConfigureSteps.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import { useConfigureSSO } from '../../../ConfigureSSOContext';
2727
import { Step } from '../../../elements/Step';
2828
import { useWizard, Wizard } from '../../../elements/Wizard';
2929
import { InnerStepCounter } from '../../../elements/Wizard/InnerStepCounter';
30-
import type { ProviderConfigureStepsProps } from '../index';
3130
import {
3231
applySamlSubmitError,
3332
buildSamlConfigurationPayload,
@@ -39,9 +38,9 @@ import {
3938
type IdpConfigurationMode,
4039
} from './shared/IdentityProviderConfigurationModes';
4140

42-
export const SamlMicrosoftConfigureSteps = ({ onComplete }: ProviderConfigureStepsProps): JSX.Element => {
41+
export const SamlMicrosoftConfigureSteps = (): JSX.Element => {
4342
return (
44-
<Wizard onComplete={onComplete}>
43+
<Wizard>
4544
<Wizard.Step id='create-app'>
4645
<Step.Header
4746
title={localizationKeys('configureSSO.configureStep.samlMicrosoft.mainHeaderTitle')}

0 commit comments

Comments
 (0)