Skip to content

Commit a7f923c

Browse files
author
Iago Dahlem
authored
fix(ui): self-serve SSO follow-ups (a11y + UX) (#8940)
1 parent fcb4651 commit a7f923c

18 files changed

Lines changed: 514 additions & 267 deletions

File tree

‎.changeset/tame-dogs-allow.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
'@clerk/ui': patch
4+
---
5+
6+
Self-serve SSO: restore keyboard-accessible provider selection, mark configuration wizard steps complete based on connection state rather than position, and fix the organization Security page loading state.

‎packages/ui/src/components/ConfigureSSO/ConfigureSSOWizard.tsx‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,25 @@ export const ConfigureSSOWizard = ({ title, forceInitialStep, ...props }: Config
2020

2121
const steps = React.useMemo<WizardStepConfig[]>(
2222
() => [
23-
{ id: 'verify-domain', label: 'Domains' },
24-
// `select-provider` now lives inside `configure` as its first sub-step, so
25-
// reaching `configure` only requires verified domains (fresh start) or an
26-
// existing connection (resume / change-provider).
27-
{ id: 'configure', label: 'Connection', guard: () => allDomainsVerified || c.hasConnection },
28-
{ id: 'test', label: 'Test', guard: () => c.hasMinimumConfiguration || c.isActive },
29-
{ id: 'activate', label: 'Activate', guard: () => c.hasSuccessfulTestRun || c.isActive },
23+
{ id: 'verify-domain', label: 'Domains', isComplete: () => allDomainsVerified },
24+
{
25+
id: 'configure',
26+
label: 'Connection',
27+
isReachable: () => allDomainsVerified || c.hasConnection,
28+
isComplete: () => c.hasMinimumConfiguration || c.isActive,
29+
},
30+
{
31+
id: 'test',
32+
label: 'Test',
33+
isReachable: () => c.hasMinimumConfiguration || c.isActive,
34+
isComplete: () => c.hasSuccessfulTestRun || c.isActive,
35+
},
36+
{
37+
id: 'activate',
38+
label: 'Activate',
39+
isReachable: () => c.hasSuccessfulTestRun || c.isActive,
40+
isComplete: () => c.isActive,
41+
},
3042
],
3143
[c, allDomainsVerified],
3244
);

‎packages/ui/src/components/ConfigureSSO/__tests__/ConfigureSSO.navigation.test.tsx‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,4 +267,58 @@ describe('ConfigureSSO wizard navigation (integration)', () => {
267267
expect(getByText('Test')).toBeInTheDocument();
268268
expect(getByText('Activate')).toBeInTheDocument();
269269
});
270+
271+
// Completion is guard-driven, not positional: re-entering an ACTIVE connection
272+
// shows every stepper step ticked even after navigating BACK to an earlier step.
273+
// A completed bullet renders a checkmark regardless of whether it is also the
274+
// current step — so for a fully active connection all four bullets show checkmarks
275+
// and zero show a digit. Under the old positional logic the steps AFTER current
276+
// would lose their tick and show their numbers again, so this asserts the fix directly.
277+
it('re-entering an active connection keeps every step completed after navigating back', async () => {
278+
const { wrapper, fixtures } = await createFixtures(withAdminOrgUser);
279+
280+
fixtures.clerk.organization?.getEnterpriseConnections.mockResolvedValue([
281+
{ ...configuredConnection, id: 'ent_active', active: true } as any,
282+
]);
283+
fixtures.clerk.organization?.getEnterpriseConnectionTestRuns.mockResolvedValue({
284+
data: [],
285+
total_count: 0,
286+
} as any);
287+
mockVerifiedDomains(fixtures);
288+
289+
const { container, findByText, userEvent } = render(<ConfigureSSO />, { wrapper });
290+
291+
// Mounts on the activate step (active connection short-circuits to it).
292+
await findByText(/sso connection is active/i);
293+
294+
const stepper = () => container.querySelector('.cl-configureSSOStepper') as HTMLElement;
295+
const bulletDigitCount = () =>
296+
Array.from(stepper().querySelectorAll('.cl-configureSSOStepperItemBullet')).filter(el =>
297+
/\d/.test(el.textContent ?? ''),
298+
).length;
299+
const stepperLabels = () =>
300+
Array.from(stepper().querySelectorAll('.cl-configureSSOStepperItemLabel')).map(el => el.textContent);
301+
const stepperButton = (label: string) =>
302+
Array.from(stepper().querySelectorAll<HTMLButtonElement>('.cl-configureSSOStepperItem')).find(
303+
btn => btn.textContent?.trim() === label,
304+
)!;
305+
306+
// The breadcrumb carries all four labels.
307+
expect(stepperLabels()).toEqual(['Domains', 'Connection', 'Test', 'Activate']);
308+
309+
// On the terminal step all four steps are complete, so all four bullets show
310+
// checkmarks — including the current 'Activate' step — and zero show a digit.
311+
expect(bulletDigitCount()).toBe(0);
312+
313+
// Navigate BACK to the first step via the breadcrumb. Every step's guard still
314+
// holds for an active connection, so 'Domains' is reachable.
315+
await userEvent.click(stepperButton('Domains'));
316+
317+
// Positional completion would now un-tick Connection/Test/Activate (they sit
318+
// AFTER current) and show their numbers. Guard-driven completion keeps them
319+
// ticked. 'Domains' is also completed, so its bullet shows a checkmark too —
320+
// zero bullets show a digit.
321+
expect(bulletDigitCount()).toBe(0);
322+
expect(stepperLabels()).toEqual(['Domains', 'Connection', 'Test', 'Activate']);
323+
});
270324
});

‎packages/ui/src/components/ConfigureSSO/elements/Stepper/Stepper.tsx‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from 'react';
22

3-
import { Box, descriptors, Flex, Icon, SimpleButton, Text, Span } from '@/customizables';
4-
import { ChevronRight, Checkmark } from '@/icons';
3+
import { Box, descriptors, Flex, Icon, SimpleButton, Span, Text } from '@/customizables';
4+
import { Checkmark, ChevronRight } from '@/icons';
55

66
import type { StepperItemProps, StepperProps } from './types';
77

@@ -75,14 +75,14 @@ const Item = ({
7575
width: theme.sizes.$4,
7676
height: theme.sizes.$4,
7777
borderRadius: theme.radii.$circle,
78-
backgroundColor: isCurrent
79-
? theme.colors.$colorForeground
80-
: isCompleted
81-
? theme.colors.$success500
78+
backgroundColor: isCompleted
79+
? theme.colors.$success500
80+
: isCurrent
81+
? theme.colors.$colorForeground
8282
: theme.colors.$colorMutedForeground,
8383
})}
8484
>
85-
{isCompleted && !isCurrent ? (
85+
{isCompleted ? (
8686
<Icon
8787
icon={Checkmark}
8888
size='sm'

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { useWizard, WizardContext } from './WizardContext';
88
interface WizardProps {
99
/** The step graph (see the component doc below). The array IS the graph. */
1010
steps: WizardStepConfig[];
11-
/** Mount here instead of the guard-derived initial step (nested resume). */
11+
/** Mount here instead of the reachability-derived initial step (nested resume). */
1212
initialStepId?: string;
1313
children?: React.ReactNode;
1414
}
@@ -17,19 +17,20 @@ interface WizardProps {
1717
* Generic, declarative, UI-less wizard primitive.
1818
*
1919
* Steps are a body-less config array (`steps`): each entry is one navigable
20-
* position with an `id` + optional inline `guard` / `label`. The graph IS the
21-
* array — known synchronously, no `React.Children` walking, no effect-timed
20+
* position with an `id` + optional inline `isReachable` / `label`. The graph IS
21+
* the array — known synchronously, no `React.Children` walking, no effect-timed
2222
* registration — and feeds a domain-agnostic machine in the same render pass.
2323
* Rendering lives in `children`: chrome is a normal child, each step body is a
2424
* render-only `<Wizard.Match id>`.
2525
*
2626
* The machine is hidden behind `useWizard()`. Conditional flow is expressed by
27-
* each step's inline `guard` (applied uniformly by init / nav / stepper); steps
28-
* are never added or removed, only gated. Inner sub-flows nest another `<Wizard>`
29-
* whose forward boundary falls through to the parent (a nested last-step `goNext`
30-
* advances the parent). A guard-blocked mid-flow `goNext` does not hard-stop: it
31-
* parks a deferred advance that resolves once the next guard becomes satisfied
32-
* while still on the step (abandoned by an explicit `goPrev`/`goToStep`).
27+
* each step's inline `isReachable` (applied uniformly by init / nav / stepper);
28+
* steps are never added or removed, only gated. Inner sub-flows nest another
29+
* `<Wizard>` whose forward boundary falls through to the parent (a nested
30+
* last-step `goNext` advances the parent). An isReachable-blocked mid-flow
31+
* `goNext` does not hard-stop: it parks a deferred advance that resolves once
32+
* the next step becomes reachable while still on the step (abandoned by an
33+
* explicit `goPrev`/`goToStep`).
3334
*/
3435
const WizardRoot = ({ steps, initialStepId, children }: WizardProps): JSX.Element => {
3536
const parentWizard = React.useContext(WizardContext);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const NavButtons = (): JSX.Element => {
2828

2929
describe('<Wizard> + <Wizard.Match>', () => {
3030
it('renders only the active step body and advances on goNext', () => {
31-
const steps: WizardStepConfig[] = [{ id: 'a' }, { id: 'b', guard: () => true }];
31+
const steps: WizardStepConfig[] = [{ id: 'a' }, { id: 'b', isReachable: () => true }];
3232

3333
render(
3434
<Wizard
@@ -56,7 +56,7 @@ describe('<Wizard> + <Wizard.Match>', () => {
5656
});
5757

5858
it('a nested sub-flow terminal goNext advances the PARENT wizard', () => {
59-
const outer: WizardStepConfig[] = [{ id: 'outer-1' }, { id: 'outer-2', guard: () => true }];
59+
const outer: WizardStepConfig[] = [{ id: 'outer-1' }, { id: 'outer-2', isReachable: () => true }];
6060
// Single inner step => inner is immediately terminal; its goNext bubbles.
6161
const inner: WizardStepConfig[] = [{ id: 'inner-only' }];
6262

@@ -99,7 +99,7 @@ describe('<Wizard> + <Wizard.Match>', () => {
9999
});
100100

101101
it('a guard-blocked top-level goNext is a hard stop (no advance)', () => {
102-
const steps: WizardStepConfig[] = [{ id: 'a' }, { id: 'b', guard: () => false }];
102+
const steps: WizardStepConfig[] = [{ id: 'a' }, { id: 'b', isReachable: () => false }];
103103

104104
render(
105105
<Wizard

‎packages/ui/src/components/ConfigureSSO/elements/Wizard/__tests__/reducer.test.ts‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,41 @@
11
import { describe, expect, it } from 'vitest';
22

3-
import { guardHolds, initialState, reduce, type WizardConfig, type WizardState } from '../reducer';
3+
import { isStepReachable, initialState, reduce, type WizardConfig, type WizardState } from '../reducer';
44
import type { WizardStepDescriptor } from '../types';
55

66
const cfg = (descriptors: WizardStepDescriptor[]): WizardConfig => ({ descriptors });
77

88
/**
9-
* A representative monotonic guard set over 4 steps:
10-
* a — entry (no guard, always enterable)
11-
* b — guard: g1
12-
* c — guard: g2
13-
* d — guard: g3
9+
* A representative monotonic reachability set over 4 steps:
10+
* a — entry (no isReachable, always enterable)
11+
* b — isReachable: g1
12+
* c — isReachable: g2
13+
* d — isReachable: g3
1414
* Toggling g1..g3 from all-false to all-true walks the furthest-reachable
1515
* boundary one step at a time. Monotonic by construction (g3 ⇒ g2 ⇒ g1).
1616
*/
1717
const monotonic = (g1: boolean, g2: boolean, g3: boolean): WizardStepDescriptor[] => [
1818
{ id: 'a' },
19-
{ id: 'b', guard: () => g1 },
20-
{ id: 'c', guard: () => g2 },
21-
{ id: 'd', guard: () => g3 },
19+
{ id: 'b', isReachable: () => g1 },
20+
{ id: 'c', isReachable: () => g2 },
21+
{ id: 'd', isReachable: () => g3 },
2222
];
2323

2424
const at = (current: string): WizardState => ({ current, direction: 0, hasNavigated: false });
2525

26-
describe('guardHolds', () => {
27-
it('resolves TRUE when no guard (entry step default flip)', () => {
28-
expect(guardHolds({ id: 'a' })).toBe(true);
26+
describe('isStepReachable', () => {
27+
it('resolves TRUE when no isReachable (entry step default)', () => {
28+
expect(isStepReachable({ id: 'a' })).toBe(true);
2929
});
3030

3131
it('delegates to the inline predicate', () => {
32-
expect(guardHolds({ id: 'b', guard: () => true })).toBe(true);
33-
expect(guardHolds({ id: 'b', guard: () => false })).toBe(false);
32+
expect(isStepReachable({ id: 'b', isReachable: () => true })).toBe(true);
33+
expect(isStepReachable({ id: 'b', isReachable: () => false })).toBe(false);
3434
});
3535
});
3636

3737
describe('initialState — furthest contiguously-reachable step', () => {
38-
it('all guards false but entry → step 0', () => {
38+
it('all isReachable false but entry → step 0', () => {
3939
expect(initialState(cfg(monotonic(false, false, false))).current).toBe('a');
4040
});
4141

@@ -51,7 +51,7 @@ describe('initialState — furthest contiguously-reachable step', () => {
5151
expect(initialState(cfg(monotonic(true, true, true))).current).toBe('d');
5252
});
5353

54-
it('stops at the first gate (does not jump a closed guard)', () => {
54+
it('stops at the first gate (does not jump a closed isReachable)', () => {
5555
// b open, c closed, d open: contiguous run stops at b.
5656
expect(initialState(cfg(monotonic(true, false, true))).current).toBe('b');
5757
});
@@ -80,9 +80,9 @@ describe('reduce — referential identity on every no-op path', () => {
8080
expect(reduce(s, { type: 'NEXT' }, cfg(monotonic(true, true, true)))).toBe(s);
8181
});
8282

83-
it('NEXT blocked by next guard → same ref', () => {
83+
it('NEXT blocked by next isReachable → same ref', () => {
8484
const s = at('a');
85-
// b's guard is false → cannot advance.
85+
// b's isReachable is false → cannot advance.
8686
expect(reduce(s, { type: 'NEXT' }, cfg(monotonic(false, false, false)))).toBe(s);
8787
});
8888

@@ -96,12 +96,12 @@ describe('reduce — referential identity on every no-op path', () => {
9696
expect(reduce(s, { type: 'PREV' }, cfg(monotonic(true, true, true)))).toBe(s);
9797
});
9898

99-
it('PREV blocked by predecessor guard → same ref', () => {
100-
// Non-monotonic on purpose: at c, b's guard is false.
99+
it('PREV blocked by predecessor isReachable → same ref', () => {
100+
// Non-monotonic on purpose: at c, b's isReachable is false.
101101
const steps: WizardStepDescriptor[] = [
102102
{ id: 'a' },
103-
{ id: 'b', guard: () => false },
104-
{ id: 'c', guard: () => true },
103+
{ id: 'b', isReachable: () => false },
104+
{ id: 'c', isReachable: () => true },
105105
];
106106
const s = at('c');
107107
expect(reduce(s, { type: 'PREV' }, cfg(steps))).toBe(s);
@@ -117,9 +117,9 @@ describe('reduce — referential identity on every no-op path', () => {
117117
expect(reduce(s, { type: 'GOTO', step: 'a' }, cfg(monotonic(true, true, true)))).toBe(s);
118118
});
119119

120-
it('GOTO blocked by target guard → same ref', () => {
120+
it('GOTO blocked by target isReachable → same ref', () => {
121121
const s = at('a');
122-
// d's guard is false → cannot jump.
122+
// d's isReachable is false → cannot jump.
123123
expect(reduce(s, { type: 'GOTO', step: 'd' }, cfg(monotonic(true, true, false)))).toBe(s);
124124
});
125125

@@ -130,8 +130,8 @@ describe('reduce — referential identity on every no-op path', () => {
130130
});
131131
});
132132

133-
describe('reduce — NEXT sequential + guard-gated', () => {
134-
it('advances exactly one slot when the next guard holds', () => {
133+
describe('reduce — NEXT sequential + isReachable-gated', () => {
134+
it('advances exactly one slot when the next isReachable holds', () => {
135135
const next = reduce(at('a'), { type: 'NEXT' }, cfg(monotonic(true, false, false)));
136136
expect(next.current).toBe('b');
137137
expect(next.direction).toBe(1);
@@ -144,14 +144,14 @@ describe('reduce — NEXT sequential + guard-gated', () => {
144144
expect(next.current).toBe('b');
145145
});
146146

147-
it('a hard stop mid-flow does not skip ahead to a later open guard', () => {
147+
it('a hard stop mid-flow does not skip ahead to a later open isReachable', () => {
148148
// b open, c closed, d open. From b, NEXT targets c (closed) → no-op.
149149
const s = at('b');
150150
expect(reduce(s, { type: 'NEXT' }, cfg(monotonic(true, false, true)))).toBe(s);
151151
});
152152
});
153153

154-
describe('reduce — PREV positional + guard-gated', () => {
154+
describe('reduce — PREV positional + isReachable-gated', () => {
155155
it('walks exactly one declaration slot back', () => {
156156
const prev = reduce(at('c'), { type: 'PREV' }, cfg(monotonic(true, true, true)));
157157
expect(prev.current).toBe('b');
@@ -165,7 +165,7 @@ describe('reduce — PREV positional + guard-gated', () => {
165165
});
166166
});
167167

168-
describe('reduce — GOTO guard-gated', () => {
168+
describe('reduce — GOTO isReachable-gated', () => {
169169
it('jumps to a reachable target', () => {
170170
const goto = reduce(at('a'), { type: 'GOTO', step: 'c' }, cfg(monotonic(true, true, true)));
171171
expect(goto.current).toBe('c');

0 commit comments

Comments
 (0)