Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/configure-sso-modal-close-button-height.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix the close button in the organization profile's SSO configuration wizard growing the modal header taller than its other content.
43 changes: 42 additions & 1 deletion packages/ui/src/components/ConfigureSSO/ConfigureSSOHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { Flex, useLocalizations } from '@/customizables';
import { useSafeLayoutEffect } from '@clerk/shared/react';

import { descriptors, Flex, Icon, useLocalizations } from '@/customizables';
import { IconButton } from '@/elements/IconButton';
import { useUnsafeModalContext } from '@/elements/Modal';
import { useUnsafeProfileCardCloseButton } from '@/elements/ProfileCard';
import { Close } from '@/icons';
import { mqu } from '@/styledSystem';

import { ProfileCardHeader } from './elements/ProfileCard';
Expand All @@ -12,6 +18,19 @@ type ConfigureSSOHeaderProps = {
export const ConfigureSSOHeader = ({ title }: ConfigureSSOHeaderProps): JSX.Element => {
const { activeSteps, currentIndex, goToStep } = useWizard();
const { t } = useLocalizations();
const { toggle } = useUnsafeModalContext();
const { setHeaderOwnsCloseButton } = useUnsafeProfileCardCloseButton();
const isModal = Boolean(toggle);

// In modal mode the header renders its own close button, so the card's shared
// absolute overlay hides on desktop. Lifecycle registration only.
useSafeLayoutEffect(() => {
if (!isModal) {
return;
}
setHeaderOwnsCloseButton?.(true);
return () => setHeaderOwnsCloseButton?.(false);
}, [isModal, setHeaderOwnsCloseButton]);

// Breadcrumb membership = labelled steps. `select-provider` has no label, so
// it is absent from the visual stepper while remaining a real navigable step.
Expand Down Expand Up @@ -45,6 +64,28 @@ export const ConfigureSSOHeader = ({ title }: ConfigureSSOHeaderProps): JSX.Elem
})}
</Stepper>
</Flex>

{isModal && (
<IconButton
elementDescriptor={descriptors.modalCloseButton}
variant='ghost'
aria-label='Close modal'
onClick={toggle}
icon={
<Icon
icon={Close}
size='md'
/>
}
sx={t => ({
color: t.colors.$colorMutedForeground,
// 4px symmetric padding → 24px box that matches the stepper bullets and gives the ghost hover a clean square (not a squished pill).
padding: t.space.$1,
// Desktop-only: mobile falls back to the card's absolute close button.
[mqu.md]: { display: 'none' },
})}
/>
)}
</ProfileCardHeader>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ export const ProfileCardHeader = (props: ProfileCardHeaderProps): JSX.Element =>
elementDescriptor={descriptors.configureSSOHeader}
{...props}
sx={theme => ({
gap: theme.space.$2,
gap: theme.space.$3,
width: '100%',
minHeight: theme.sizes.$13,
padding: theme.space.$5,
// Vertically center the back button, stepper, and (modal) close button.
alignItems: 'center',
borderBottomWidth: theme.borderWidths.$normal,
borderBottomStyle: theme.borderStyles.$solid,
borderBottomColor: theme.colors.$borderAlpha100,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ClerkAPIResponseError } from '@clerk/shared/error';
import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';

import { ModalContext } from '@/elements/Modal';
import { bindCreateFixtures } from '@/test/create-fixtures';
import { render, screen, waitFor } from '@/test/utils';

Expand Down Expand Up @@ -48,6 +49,19 @@ const configuredConnection = (overrides: Record<string, unknown> = {}) =>
const renderPage = (wrapper: React.ComponentType<{ children?: React.ReactNode }>) =>
render(<OrganizationSecurityPage contentRef={{ current: null }} />, { wrapper });

// Renders the page inside a modal context (a `toggle` is provided), mirroring how
// the modal-mode OrganizationProfile owns the close button.
const renderPageInModal = (wrapper: React.ComponentType<{ children?: React.ReactNode }>, toggle: () => void) => {
const FixtureWrapper = wrapper;
const modalValue = { value: { toggle } };
const ModalWrapper = ({ children }: { children?: React.ReactNode }) => (
<FixtureWrapper>
<ModalContext.Provider value={modalValue}>{children}</ModalContext.Provider>
</FixtureWrapper>
);
return render(<OrganizationSecurityPage contentRef={{ current: null }} />, { wrapper: ModalWrapper });
};

describe('OrganizationSecurityPage', () => {
describe('overview states', () => {
it('renders the unconfigured state with a Start configuration action', async () => {
Expand Down Expand Up @@ -295,6 +309,52 @@ describe('OrganizationSecurityPage', () => {
});
});

describe('wizard modal close button', () => {
it('renders a header close button wired to the modal toggle when the wizard is open in a modal', async () => {
const { wrapper, fixtures } = await createFixtures(withSecurityPageFixtures);

fixtures.clerk.organization?.getEnterpriseConnections.mockResolvedValue([]);
fixtures.clerk.organization?.getDomains.mockResolvedValue({ data: [verifiedDomain], total_count: 1 } as any);

const toggle = vi.fn();
const { userEvent } = renderPageInModal(wrapper, toggle);

await userEvent.click(await screen.findByRole('button', { name: 'Start configuration' }));
expect(await screen.findByRole('heading', { name: /add SSO domains/i })).toBeInTheDocument();

// jsdom can't evaluate the desktop/mobile media split, so assert behavior:
// the header exposes a close button that drives the modal's toggle.
const closeButton = screen.getByRole('button', { name: 'Close modal' });
await userEvent.click(closeButton);
expect(toggle).toHaveBeenCalled();
});

it('does not render the wizard close button when the wizard is open outside a modal', async () => {
const { wrapper, fixtures } = await createFixtures(withSecurityPageFixtures);

fixtures.clerk.organization?.getEnterpriseConnections.mockResolvedValue([]);
fixtures.clerk.organization?.getDomains.mockResolvedValue({ data: [verifiedDomain], total_count: 1 } as any);

const { userEvent } = renderPage(wrapper);

await userEvent.click(await screen.findByRole('button', { name: 'Start configuration' }));
expect(await screen.findByRole('heading', { name: /add SSO domains/i })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Close modal' })).not.toBeInTheDocument();
});

it('keeps the overview free of the wizard close button inside a modal', async () => {
const { wrapper, fixtures } = await createFixtures(withSecurityPageFixtures);

fixtures.clerk.organization?.getEnterpriseConnections.mockResolvedValue([]);

const toggle = vi.fn();
renderPageInModal(wrapper, toggle);

expect(await screen.findByRole('button', { name: 'Start configuration' })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Close modal' })).not.toBeInTheDocument();
});
});

describe('actions menu', () => {
it('lists Edit, Deactivate, and Remove for an active connection', async () => {
const { wrapper, fixtures } = await createFixtures(withSecurityPageFixtures);
Expand Down
18 changes: 17 additions & 1 deletion packages/ui/src/elements/ProfileCard/ProfileCardRoot.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { createContextAndHook } from '@clerk/shared/react';
import React from 'react';

import { Box, descriptors, Icon } from '../../customizables';
Expand All @@ -8,9 +9,18 @@ import { Card } from '../Card';
import { IconButton } from '../IconButton';
import { useUnsafeModalContext } from '../Modal';

// Lets a descendant header (the SSO wizard) claim the modal close button so the
// shared absolute overlay can step aside on desktop. No card sets this by default,
// so every existing consumer keeps the absolute button unchanged.
export const [ProfileCardCloseButtonContext, _useProfileCardCloseButton, useUnsafeProfileCardCloseButton] =
createContextAndHook<{ setHeaderOwnsCloseButton: (owns: boolean) => void }>('ProfileCardCloseButtonContext');

export const ProfileCardRoot = React.forwardRef<HTMLDivElement, PropsOfComponent<typeof Card.Root>>((props, ref) => {
const { sx, children, ...rest } = props;
const { toggle } = useUnsafeModalContext();
const [headerOwnsCloseButton, setHeaderOwnsCloseButton] = React.useState(false);
// The setState identity is stable, so empty deps are correct.
const closeButtonCtx = React.useMemo(() => ({ value: { setHeaderOwnsCloseButton } }), []);

return (
<Card.Root
Expand Down Expand Up @@ -42,11 +52,17 @@ export const ProfileCardRoot = React.forwardRef<HTMLDivElement, PropsOfComponent
padding: `${t.space.$1x5} ${t.space.$2}`,
top: t.space.$none,
insetInlineEnd: t.space.$none,
// Mobile always keeps the absolute button; restore the Box's default
// display when the header has hidden it on desktop.
...(headerOwnsCloseButton && { display: 'block' }),
},
zIndex: t.zIndices.$modal,
position: 'absolute',
top: t.space.$2,
insetInlineEnd: t.space.$2,
// A descendant header owns the desktop close button: keep this overlay
// mounted (mobile uses it) but hide it on desktop.
...(headerOwnsCloseButton && { display: 'none' }),
})}
>
<IconButton
Expand All @@ -67,7 +83,7 @@ export const ProfileCardRoot = React.forwardRef<HTMLDivElement, PropsOfComponent
/>
</Box>
)}
{children}
<ProfileCardCloseButtonContext.Provider value={closeButtonCtx}>{children}</ProfileCardCloseButtonContext.Provider>
<Card.Footer
isProfileFooter
sx={{
Expand Down
2 changes: 2 additions & 0 deletions packages/ui/src/elements/ProfileCard/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { ProfileCardContent } from './ProfileCardContent';
import { ProfileCardPage } from './ProfileCardPage';
import { ProfileCardRoot } from './ProfileCardRoot';

export { useUnsafeProfileCardCloseButton } from './ProfileCardRoot';

export const ProfileCard = {
Root: ProfileCardRoot,
Content: ProfileCardContent,
Expand Down
Loading