feat(ui): Introduce dialog to reset enterprise connection#8706
Open
iagodahlem wants to merge 19 commits into
Open
feat(ui): Introduce dialog to reset enterprise connection#8706iagodahlem wants to merge 19 commits into
iagodahlem wants to merge 19 commits into
Conversation
🦋 Changeset detectedLatest commit: 65da4f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| const { enterpriseConnection, deleteEnterpriseConnection, setProvider } = useConfigureSSO(); | ||
| const { goToStep } = useWizard(); | ||
|
|
||
| const deleteConnection = useReverification((id: string) => deleteEnterpriseConnection(id)); |
Member
There was a problem hiding this comment.
Do we need reverification here? I'm not sure if all FAPI mutations rely on it
Member
Author
There was a problem hiding this comment.
Keeping it — useReverification wraps every sensitive user.* mutation in @clerk/ui (delete account, delete passkey, etc.), and the pre-refactor inline ResetConnectionForm used it for the same destructive delete call. Skipping it here would diverge from the rest of the component surface.
Member
There was a problem hiding this comment.
Once we refactor those endpoints to the organization scoped instead of the user scope, then I think those won't be needed
iagodahlem
force-pushed
the
iago/orgs-1588-sdk-improve-confirmation-step-with-new-reset-connection
branch
from
929090b to
b190354
Compare
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
API Changes Report
Summary
@clerk/sharedCurrent version: 4.14.0 Subpath
|
Contributor
📝 WalkthroughWalkthroughThis PR refactors the SSO wizard confirmation step from an inline reset confirmation card to a dedicated modal dialog. It adds localization and type keys for the reset dialog and inactive banner, updates Step.Header to accept an inline status badge, introduces ResetConnectionDialog with confirmation validation and an async delete flow that clears the selected provider and rewinds the wizard, restructures the confirmation step layout and styling, and threads a shared contentRef through OrganizationProfile so the dialog portals into the wizard chrome. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/ConfigureSSO/steps/ConfirmationStep.tsx (1)
252-279: 💤 Low valueConsider disabling reset button when organization name is unavailable.
If
organization?.nameis falsy,confirmationValuebecomes an empty string, making the dialog's submit button permanently disabled sincecanSubmit = Boolean(confirmationValue && ...)will be false. Users could open the dialog but would be unable to submit, only cancel.Consider disabling the reset button or hiding the section when organization name is unavailable:
💡 Suggested improvement
const ResetConnectionSection = (): JSX.Element => { const { organization } = useOrganization(); const [isOpen, setIsOpen] = useState(false); + const confirmationValue = organization?.name ?? ''; return ( <ProfileSection.Root title={localizationKeys('configureSSO.confirmation.resetSection.title')} id='resetSso' > <Flex justify='start'> <Button elementDescriptor={descriptors.configureSSOConfirmationResetButton} variant='solid' colorScheme='danger' size='sm' + isDisabled={!confirmationValue} onClick={() => setIsOpen(true)} localizationKey={localizationKeys('configureSSO.confirmation.resetSection.title')} /> </Flex> <ResetConnectionDialog isOpen={isOpen} onClose={() => setIsOpen(false)} - confirmationValue={organization?.name ?? ''} + confirmationValue={confirmationValue} /> </ProfileSection.Root> ); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/ConfigureSSO/steps/ConfirmationStep.tsx` around lines 252 - 279, The ResetConnectionSection opens ResetConnectionDialog with confirmationValue set to organization?.name which can be empty and thus prevents submission; update ResetConnectionSection to disable (or hide) the reset Button when organization?.name is falsy so users cannot open an un-submittable dialog. Specifically, check organization?.name before rendering or pass a disabled prop to the Button rendered in ResetConnectionSection (the Button that calls setIsOpen(true)) and ensure the UI conveys why it’s disabled; alternatively skip rendering the entire ProfileSection.Root when organization?.name is missing to avoid launching ResetConnectionDialog with an empty confirmationValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/ConfigureSSO/ResetConnectionDialog.tsx`:
- Around line 57-64: The placeholder for the form control is using the raw
confirmationValue instead of the localization key; update the useFormControl
call for confirmationField to pass
localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldPlaceholder',
{ name: confirmationValue }) as the placeholder so translators can control
formatting. Locate the confirmationField definition
(useFormControl('deleteConfirmation', ...)) and replace the current placeholder
argument with the localized key while keeping the label using
configureSSO.resetConnectionDialog.confirmationFieldLabel unchanged.
---
Nitpick comments:
In `@packages/ui/src/components/ConfigureSSO/steps/ConfirmationStep.tsx`:
- Around line 252-279: The ResetConnectionSection opens ResetConnectionDialog
with confirmationValue set to organization?.name which can be empty and thus
prevents submission; update ResetConnectionSection to disable (or hide) the
reset Button when organization?.name is falsy so users cannot open an
un-submittable dialog. Specifically, check organization?.name before rendering
or pass a disabled prop to the Button rendered in ResetConnectionSection (the
Button that calls setIsOpen(true)) and ensure the UI conveys why it’s disabled;
alternatively skip rendering the entire ProfileSection.Root when
organization?.name is missing to avoid launching ResetConnectionDialog with an
empty confirmationValue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ae810c5c-535a-41cc-8ed2-11c203be67c1
📒 Files selected for processing (11)
.changeset/cool-boats-burn.mdpackages/localizations/src/en-US.tspackages/shared/src/types/localization.tspackages/ui/src/components/ConfigureSSO/ConfigureSSOContext.tsxpackages/ui/src/components/ConfigureSSO/ResetConnectionDialog.tsxpackages/ui/src/components/ConfigureSSO/__tests__/ResetConnectionDialog.test.tsxpackages/ui/src/components/ConfigureSSO/elements/Step.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfirmationStep.tsxpackages/ui/src/components/OrganizationProfile/OrganizationProfileRoutes.tsxpackages/ui/src/components/OrganizationProfile/OrganizationSelfServeSSOPage.tsxpackages/ui/src/components/OrganizationProfile/index.tsx
Comment on lines
+57
to
+64
| const confirmationField = useFormControl('deleteConfirmation', '', { | ||
| type: 'text', | ||
| label: localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldLabel', { | ||
| name: confirmationValue, | ||
| }), | ||
| isRequired: true, | ||
| placeholder: confirmationValue, | ||
| }); |
Contributor
There was a problem hiding this comment.
Use the localized placeholder key instead of raw organization text.
Line 63 bypasses configureSSO.resetConnectionDialog.confirmationFieldPlaceholder, so translators cannot control placeholder formatting and the new key is effectively unused.
Proposed fix
const confirmationField = useFormControl('deleteConfirmation', '', {
type: 'text',
label: localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldLabel', {
name: confirmationValue,
}),
isRequired: true,
- placeholder: confirmationValue,
+ placeholder: localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldPlaceholder', {
+ name: confirmationValue,
+ }),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const confirmationField = useFormControl('deleteConfirmation', '', { | |
| type: 'text', | |
| label: localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldLabel', { | |
| name: confirmationValue, | |
| }), | |
| isRequired: true, | |
| placeholder: confirmationValue, | |
| }); | |
| const confirmationField = useFormControl('deleteConfirmation', '', { | |
| type: 'text', | |
| label: localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldLabel', { | |
| name: confirmationValue, | |
| }), | |
| isRequired: true, | |
| placeholder: localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldPlaceholder', { | |
| name: confirmationValue, | |
| }), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/ConfigureSSO/ResetConnectionDialog.tsx` around
lines 57 - 64, The placeholder for the form control is using the raw
confirmationValue instead of the localization key; update the useFormControl
call for confirmationField to pass
localizationKeys('configureSSO.resetConnectionDialog.confirmationFieldPlaceholder',
{ name: confirmationValue }) as the placeholder so translators can control
formatting. Locate the confirmationField definition
(useFormControl('deleteConfirmation', ...)) and replace the current placeholder
argument with the localized key while keeping the label using
configureSSO.resetConnectionDialog.confirmationFieldLabel unchanged.
6 tasks
LauraBeatris
approved these changes
Jun 1, 2026
LauraBeatris
reviewed
Jun 1, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/elements/Form.tsx (1)
184-191: ⚡ Quick winAdd an explicit return type to
PlainInput.
PlainInputis part of the exportedFormAPI surface and should declare its return type explicitly.✍️ Proposed fix
-const PlainInput = (props: CommonInputProps & { elementDescriptor?: ElementDescriptor }) => { +const PlainInput = (props: CommonInputProps & { elementDescriptor?: ElementDescriptor }): JSX.Element => { const { elementDescriptor, ...rest } = props; return ( <CommonInputWrapper {...rest}> <Field.Input elementDescriptor={elementDescriptor} /> </CommonInputWrapper> ); };As per coding guidelines, "Always define explicit return types for functions, especially public APIs."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/elements/Form.tsx` around lines 184 - 191, PlainInput currently lacks an explicit return type; update its signature to include one (e.g., annotate PlainInput as returning React.ReactElement or React.ReactElement | null) so the exported Form API has an explicit type. Modify the declaration for PlainInput (the function that accepts CommonInputProps & { elementDescriptor?: ElementDescriptor } and renders CommonInputWrapper and Field.Input) to add the return type (for example: const PlainInput = (props: ...) : React.ReactElement => { ... }), and ensure React is imported if needed for the type.packages/ui/src/elements/FieldControl.tsx (1)
258-270: ⚡ Quick winKeep
elementDescriptorandelementIdoverrides consistent.Line 269 allows descriptor override, but Line 270 always derives
elementIdfromdescriptors.formFieldInput. That can produce mismatched selector classes when a custom descriptor is used.♻️ Proposed fix
type InputElementProps = { elementDescriptor?: ElementDescriptor; + elementId?: ElementId; }; -const InputElement = forwardRef<HTMLInputElement, InputElementProps>(({ elementDescriptor }, ref) => { +const InputElement = forwardRef<HTMLInputElement, InputElementProps>(({ elementDescriptor, elementId }, ref) => { const { t } = useLocalizations(); const formField = useFormField(); const { placeholder, ...inputProps } = sanitizeInputProps(formField); + const resolvedElementDescriptor = elementDescriptor || descriptors.formFieldInput; return ( <Input ref={ref} - elementDescriptor={elementDescriptor || descriptors.formFieldInput} - elementId={descriptors.formFieldInput.setId(formField.fieldId)} + elementDescriptor={resolvedElementDescriptor} + elementId={elementId || descriptors.formFieldInput.setId(formField.fieldId)} {...inputProps} placeholder={t(placeholder)} /> ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/elements/FieldControl.tsx` around lines 258 - 270, The InputElement component allows overriding the elementDescriptor via the elementDescriptor prop but always derives elementId from descriptors.formFieldInput, which can mismatch custom descriptors; update InputElement so elementId is computed from the same descriptor you pass into the Input component (use the resolvedDescriptor variable or the elementDescriptor prop fallback used for elementDescriptor) — e.g., resolve const resolvedDescriptor = elementDescriptor || descriptors.formFieldInput and pass resolvedDescriptor.setId(formField.fieldId) as elementId to Input (keep sanitizeInputProps and ref usage unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ui/src/elements/FieldControl.tsx`:
- Around line 258-270: The InputElement component allows overriding the
elementDescriptor via the elementDescriptor prop but always derives elementId
from descriptors.formFieldInput, which can mismatch custom descriptors; update
InputElement so elementId is computed from the same descriptor you pass into the
Input component (use the resolvedDescriptor variable or the elementDescriptor
prop fallback used for elementDescriptor) — e.g., resolve const
resolvedDescriptor = elementDescriptor || descriptors.formFieldInput and pass
resolvedDescriptor.setId(formField.fieldId) as elementId to Input (keep
sanitizeInputProps and ref usage unchanged).
In `@packages/ui/src/elements/Form.tsx`:
- Around line 184-191: PlainInput currently lacks an explicit return type;
update its signature to include one (e.g., annotate PlainInput as returning
React.ReactElement or React.ReactElement | null) so the exported Form API has an
explicit type. Modify the declaration for PlainInput (the function that accepts
CommonInputProps & { elementDescriptor?: ElementDescriptor } and renders
CommonInputWrapper and Field.Input) to add the return type (for example: const
PlainInput = (props: ...) : React.ReactElement => { ... }), and ensure React is
imported if needed for the type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 213854e4-af71-424a-9336-c3236809da8c
📒 Files selected for processing (6)
packages/ui/src/components/ConfigureSSO/ResetConnectionDialog.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfirmationStep.tsxpackages/ui/src/customizables/elementDescriptors.tspackages/ui/src/elements/FieldControl.tsxpackages/ui/src/elements/Form.tsxpackages/ui/src/internal/appearance.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui/src/components/ConfigureSSO/steps/ConfirmationStep.tsx
- packages/ui/src/components/ConfigureSSO/ResetConnectionDialog.tsx
Standalone modal dialog with type-to-confirm gating. Wraps the destructive delete flow (useReverification + deleteEnterpriseConnection + provider clear + wizard rewind to select-provider). Controlled via isOpen/onClose props with the confirmation value injected by the caller. Widens ConfigureSSOContext.setProvider to accept undefined so the dialog can clear the local provider selection after deleting the connection, and adds the configureSSO.resetConnectionDialog localization keys (and matching type entries in @clerk/shared) for the dialog copy.
Replaces the inline reset confirmation card (Action.Root / Action.Card pattern) with the new ResetConnectionDialog modal. The destructive button now opens the dialog instead of expanding an inline form; the dialog handles type-to-confirm, reverification, deletion, and wizard rewind internally.
…tainer Portals the dialog into the wizard's content container instead of the document body and switches the backdrop to absolute positioning so it stays inside the ConfigureSSO card. Disables the modal-context toggle so the auto-rendered close button is suppressed — the Cancel button is the explicit dismiss for the destructive flow.
Backdrop now uses a transparent fill plus backdrop-filter blur so the wizard chrome shows through without an opaque tint. Card switches to a tighter border radius, start-aligned text, and reduced padding to match the design spec for the inline reset confirmation.
The OrganizationProfile self-serve SSO page was creating its own contentRef that diverged from the ProfileCard content ref. ConfigureSSO needs the same container ref to portal child modals (ResetConnectionDialog) into the wizard chrome instead of the document body, so the SSO page now receives the shared ref from the surrounding routes and forwards it down.
Unifies the SSO Successfully configured title with the status badge, lays out configuration details in a two-column grid, swaps the configure-again and reset connection actions to dedicated outlined and destructive buttons, and adds a sticky inactive banner inside the step footer when the connection is disabled.
Covers the type-to-confirm gating, cancel and reset interactions, and the successful submit chain (delete + provider clear + wizard rewind + onClose).
Lets a step render an inline badge next to the title without crowding the right-aligned children slot. The SSO confirmation step now passes its active status badge through this prop instead of layering its own header component inside the body.
The hand-rolled grid layered onto the confirmation step recreated what ProfileSection.Root already provides for every other profile section. Each detail row now lives inside its own ProfileSection.Root, restoring the profile-section descriptors and consistent responsive collapse while keeping the outlined Configure again button, destructive Reset connection button, status header badge, and inactive footer banner from the recent restyle.
Drops the top border and redundant top padding from the Domain section so it visually pairs with the Enable SSO row above it. Matches the design intent of keeping the connection-state controls (toggle + domain) in a single block, separated from Configuration details and Reset connection by the existing section dividers.
Wraps the confirmation body in Step.Section so it picks up the canonical
section padding instead of hand-rolling padding on Step.Body. Drops the top
border and redundant top padding from the Enable SSO section so it visually
joins the header above. Aligns the Configuration details section title to the
top via centered={false} and folds the Configure again action into the same
grid as the detail rows so it shares their column alignment.
…ion details Drops the hand-rolled grid that paired each detail label with its value and rebuilds the rows on top of ProfileSection.ItemList + ProfileSection.Item. The Configure again action moves below the row list and reuses the section's inset padding so it lines up with the row labels above.
Flips the detail rows from space-between to start justification and gives the label a fixed-width column matching the previous grid minimum so labels stop wrapping when values are long and all three values line up at the same x. Values keep truncating with ellipsis when they overflow.
Widens the Sign on URL label to fit the longest copy without wrapping and sets an explicit gap on the item list so the detail rows space consistently without inheriting the default tight gap from ProfileSection.ItemList.
Folds the per-commit changeset stubs accumulated during this PR into a single generated changeset that describes the whole change end to end.
Wraps the configuration item list and the Configure again action in a Col with a shared gap, lets the items inherit the default space-between layout with a fixed-width label column, and aligns the Issuer label width to the Sign on URL row so labels sit on a consistent column.
Updates the dialog test name to read as a behavioral assertion and removes the verbose confirmation value comment now that the prop name carries the intent on its own.
The Reset connection button only appears on the confirmation step, which itself only renders when the enterprise connection exists, so the missing connection branch is not reachable in practice. Removes the matching test and the stale provider setter JSDoc that no longer describes the flow.
…active banner Adds element descriptors so the reset dialog (card, confirmation input, destructive submit, cancel) and the inactive footer banner on the confirmation step can be targeted by appearance customizations. Surface-level only; no behavior changes.
LauraBeatris
force-pushed
the
iago/orgs-1588-sdk-improve-confirmation-step-with-new-reset-connection
branch
from
3b12c61 to
65da4f8
Compare
LauraBeatris
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements ORGS-1588: a dedicated reset connection dialog and a restyled confirmation step for
<ConfigureSSO />.Summary
<ResetConnectionDialog />— new modal-based, type-to-confirm gated dialog that scopes its backdrop to the wizard's content container viaportalRoot. Wraps the destructive flow behinduseReverification(user.deleteEnterpriseConnection), clears the local provider selection on success, and asks the wizard to navigate back to the provider selection step.Step.Headerwith an inline status badge, grouped Enable SSO + Domain rows, two-column configuration details rendered throughProfileSection.ItemList, outlinedConfigure again, destructiveReset connection, and a sticky info banner inside the step footer when the connection is inactive.Step.Headerbadge prop — newbadge?: ReactNodeprop so a step can render an inline status pill next to its title without crowding the existing right-aligned children slot.<OrganizationProfile />content ref forwarding — propagates the sharedProfileCardcontent ref into<ConfigureSSO />so the dialog portals into the wizard chrome (not the document body) when the component is embedded under<OrganizationProfile />.<ResetConnectionDialog />covering type-to-confirm gating, cancel, and the successful submit chain.The wizard-wide reset entry (a
Step.Footer.Resetcompound part on Verify Domain / Configure / Test) ships in a follow-up PR — see ORGS-1550.Screenshots
Drag-drop the latest captures from the running sandbox here — active + inactive confirmation, plus the reset dialog open.
Test plan
SSO Successfully configured+Inactivebadge inline, grouped Enable SSO + Domain rows, two-column Configuration details with Sign on URL / Issuer / Certificate, outlinedConfigure again, destructive solidReset connection, sticky(i) SSO is inactive and you need to enable it to authenticatebanner in the footer.Activebadge, no inactive banner.Xin the corner.user.deleteEnterpriseConnectionruns and the enterprise connections query is invalidated.<OrganizationProfile />: same behavior; the dialog portals into the wizard chrome, not the page body.pnpm vitest run packages/ui/src/components/ConfigureSSO/__tests__/ResetConnectionDialog.test.tsxgreen.Linear: ORGS-1588