diff --git a/.changeset/mosaic-user-profile-combined.md b/.changeset/mosaic-user-profile-combined.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-user-profile-combined.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/components/profile/index.ts b/packages/mosaic/src/components/profile/index.ts index 39af298aaf2..9f9d3b6f073 100644 --- a/packages/mosaic/src/components/profile/index.ts +++ b/packages/mosaic/src/components/profile/index.ts @@ -2,6 +2,7 @@ export { Profile } from './profile'; export type { ProfileContentProps, ProfileElevation, + ProfileNavGroupProps, ProfileNavItemProps, ProfileNavProps, ProfileContentPanelProps, diff --git a/packages/mosaic/src/components/profile/profile.styles.ts b/packages/mosaic/src/components/profile/profile.styles.ts index a7c1219e9cc..4ef4dcc1eb7 100644 --- a/packages/mosaic/src/components/profile/profile.styles.ts +++ b/packages/mosaic/src/components/profile/profile.styles.ts @@ -189,6 +189,22 @@ export const styles = stylex.create({ minWidth: 0, }, + navGroup: { + gap: NAV_GAP, + display: 'flex', + flexDirection: 'column', + marginBlockStart: { default: null, ':not(:first-child)': space['4'] }, + }, + + navGroupLabel: { + paddingBlock: space['1'], + paddingInline: space['2.5'], + color: colorVars['--cl-color-foreground-secondary'], + fontSize: typeScaleVars['--cl-text-xs-size'], + fontWeight: fontWeightVars['--cl-font-medium'], + lineHeight: typeScaleVars['--cl-text-xs-leading'], + }, + navItem: { borderColor: 'transparent', borderRadius: radiusVars['--cl-radius-md'], diff --git a/packages/mosaic/src/components/profile/profile.test.tsx b/packages/mosaic/src/components/profile/profile.test.tsx index c3d82a62bf8..0e36bedafef 100644 --- a/packages/mosaic/src/components/profile/profile.test.tsx +++ b/packages/mosaic/src/components/profile/profile.test.tsx @@ -1,11 +1,12 @@ import * as stylex from '@stylexjs/stylex'; -import { act, render, screen, waitFor } from '@testing-library/react'; +import { act, isInaccessible, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { UserCircle } from '../../icons/glyphs/user-circle'; import { MosaicProvider } from '../../mosaic-provider'; +import { axe } from '../../primitives/test-utils/axe'; import { Badge } from '../badge'; import { Card } from '../card'; import { Dialog } from '../dialog'; @@ -65,6 +66,30 @@ function renderSurface(props: Partial = {}) { ); } +function GroupedSurface(rootProps: Partial) { + return ( + + Settings + + + Account + Security + + + General + Security + + + + Account page + + + ); +} + function atomsOf(style: stylex.StyleXStyles): string[] { return stylex .props(style) @@ -289,6 +314,42 @@ describe('Profile', () => { }); // The sentinel reports 1px column, 2px select, 3px sheet. + describe('navigation groups', () => { + it('labels each group and describes its tabs by that label', () => { + render(); + + const label = screen.getByText('Acme Inc'); + expect(label).toHaveClass('cl-profile-nav-group-label'); + expect(label.closest('.cl-profile-nav-group')).toContainElement( + screen.getByRole('tab', { name: 'General', description: 'Acme Inc' }), + ); + expect(screen.getByRole('tab', { name: 'Security', description: 'Personal' })).toBeInTheDocument(); + expect(screen.getByRole('tab', { name: 'Security', description: 'Acme Inc' })).toBeInTheDocument(); + }); + + it('keeps the group labels out of the tablist for assistive tech', () => { + render(); + + expect(isInaccessible(screen.getByText('Personal'))).toBe(true); + expect(isInaccessible(screen.getByText('Acme Inc'))).toBe(true); + }); + + it('moves between groups with the arrow keys', async () => { + const user = userEvent.setup(); + render(); + + screen.getByRole('tab', { name: 'Security', description: 'Personal' }).focus(); + await user.keyboard('{ArrowDown}'); + + expect(screen.getByRole('tab', { name: 'General' })).toHaveFocus(); + }); + + it('has no accessibility violations', async () => { + const { container } = render(); + expect((await axe(container)).violations).toEqual([]); + }); + }); + describe('compact', () => { let observe: ((width: number) => void) | null = null; const original = globalThis.ResizeObserver; @@ -367,6 +428,25 @@ describe('Profile', () => { await waitFor(() => expect(screen.queryByRole('listbox')).not.toBeInTheDocument()); }); + it('groups the listbox options under each group label', async () => { + const user = userEvent.setup(); + render( + + + , + ); + act(() => observe?.(2)); + + await user.click(screen.getByRole('combobox', { name: 'Account' })); + + expect(screen.getByRole('group', { name: 'Acme Inc' })).toContainElement( + screen.getByRole('option', { name: 'General' }), + ); + expect(screen.getByRole('group', { name: 'Personal' })).toContainElement( + screen.getByRole('option', { name: 'Account' }), + ); + }); + it('opens the listbox from the keyboard, picks with the arrows, and keeps focus on the title', async () => { const user = userEvent.setup(); function Controlled() { diff --git a/packages/mosaic/src/components/profile/profile.tsx b/packages/mosaic/src/components/profile/profile.tsx index cae4c889539..0f12cbf3db1 100644 --- a/packages/mosaic/src/components/profile/profile.tsx +++ b/packages/mosaic/src/components/profile/profile.tsx @@ -232,6 +232,8 @@ type NavItemMode = 'tab' | 'option'; // `Profile.Nav` children render again as the select's options. const NavItemModeContext = React.createContext('tab'); +const NavGroupLabelContext = React.createContext(undefined); + function BackButton() { const m = useMessages('profile'); return ( @@ -267,7 +269,7 @@ function NavBranding() { } /** - * Children are `Profile.NavItem`s only. Wide, they render as a tablist; compact, as the options of + * Children are `Profile.NavItem`s, or `Profile.NavGroup`s of them. Wide, they render as a tablist; compact, as the options of * the page title's select, or a tablist in a sheet on a phone. */ const Nav = React.forwardRef(function ProfileNav( @@ -340,6 +342,87 @@ const Nav = React.forwardRef(function ProfileNav( ); }); +export interface ProfileNavGroupProps extends MosaicComponentProps<'div'> { + /** Names the group above its items. */ + label: React.ReactNode; +} + +/** + * A labelled run of `Profile.NavItem`s. As tabs, the label describes each item; as options, it names + * a listbox group. + */ +const NavGroup = React.forwardRef(function ProfileNavGroup( + { label, children, xstyle, ...rest }, + ref, +) { + const mode = React.useContext(NavItemModeContext); + const groupProps = mergeStyleProps( + themeProps('profile-nav-group'), + stylex.props(reset.base, styles.navGroup, xstyle), + rest, + ); + const labelProps = mergeStyleProps( + themeProps('profile-nav-group-label'), + stylex.props(reset.base, styles.navGroupLabel), + ); + if (mode === 'option') { + return ( + + {label} + {children} + + ); + } + return ( + + {children} + + ); +}); + +interface TabsNavGroupProps extends Omit { + labelProps: React.ComponentProps<'div'>; +} + +const TabsNavGroup = React.forwardRef(function TabsNavGroup( + { label, labelProps, children, render, ...rest }, + ref, +) { + const labelId = React.useId(); + return ( + + {useRender({ + defaultTagName: 'div', + render, + ref, + props: { + ...rest, + children: ( + <> +
+ {label} +
+ {children} + + ), + }, + })} + + ); +}); + export interface ProfileNavItemProps extends MosaicComponentProps<'button'> { value: string; icon?: React.ReactNode; @@ -355,6 +438,7 @@ const NavItem = React.forwardRef(functio ) { const { compact, closeNav } = useProfileContext('Profile.NavItem'); const mode = React.useContext(NavItemModeContext); + const groupLabelId = React.useContext(NavGroupLabelContext); const styleProps = mergeStyleProps( themeProps('profile-nav-item'), stylex.props(reset.base, styles.navItem, focusOutline.visible, mode === 'option' && styles.navItemOption, xstyle), @@ -405,6 +489,7 @@ const NavItem = React.forwardRef(functio ref={ref} value={value} disabled={disabled} + aria-describedby={groupLabelId} render={render} onClick={event => { onClick?.(event); @@ -596,4 +681,4 @@ const ContentPanel = React.forwardRef( * * ``` */ -export const Profile = { Root, Title, Nav, NavItem, Content, ContentPanel }; +export const Profile = { Root, Title, Nav, NavGroup, NavItem, Content, ContentPanel }; diff --git a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx index ff62d3bb637..289f19ff8ce 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx @@ -1,10 +1,22 @@ import { describe, expect, it } from 'vitest'; -import { getAvailableOrganizationProfilePages } from '../organization-profile.layout'; -import type { OrganizationProfilePages } from '../organization-profile.types'; +import { getAvailableOrganizationProfilePages, resolveOrganizationProfilePages } from '../organization-profile.layout'; +import type { CustomOrganizationProfilePage, OrganizationProfilePages } from '../organization-profile.types'; const general: OrganizationProfilePages['general'] =

General

; +describe('resolveOrganizationProfilePages', () => { + it('keeps the first custom page for a repeated path', () => { + const audit: CustomOrganizationProfilePage = { label: 'Audit', path: 'audit', content: null }; + const otherAudit: CustomOrganizationProfilePage = { label: 'Other audit', path: 'audit', content: null }; + + expect(resolveOrganizationProfilePages(['general'], [audit, otherAudit])).toEqual([ + { id: 'general' }, + { id: 'audit', custom: audit }, + ]); + }); +}); + describe('getAvailableOrganizationProfilePages', () => { it('lists api keys given content', () => { expect(getAvailableOrganizationProfilePages({ general, apiKeys:

API keys

})).toEqual(['general', 'apiKeys']); diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts b/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts index 096c7acfd19..e2d14f244bd 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts +++ b/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts @@ -5,6 +5,7 @@ import { Key } from '../../icons/glyphs/key'; import { SecurityLockSquare } from '../../icons/glyphs/security-lock-square'; import { Users } from '../../icons/glyphs/users'; import { applyOrder } from '../../utils/apply-order'; +import { withUniquePaths } from '../profile-pages/profile-pages.layout'; import type { CustomOrganizationProfilePage, OrganizationProfilePageId, @@ -43,14 +44,15 @@ export function resolveOrganizationProfilePages( customPages: readonly CustomOrganizationProfilePage[] = [], order?: readonly string[], ): OrganizationProfileNavEntry[] { - const customById = new Map(customPages.map(page => [page.path, page])); + const uniqueCustomPages = withUniquePaths(customPages); + const customById = new Map(uniqueCustomPages.map(page => [page.path, page])); const builtInIds = new Set(builtIn); const entries: OrganizationProfileNavEntry[] = [ ...builtIn.map((id): OrganizationProfileNavEntry => { const custom = customById.get(id); return custom ? { id, custom } : { id }; }), - ...customPages.filter(page => !builtInIds.has(page.path)).map(page => ({ id: page.path, custom: page })), + ...uniqueCustomPages.filter(page => !builtInIds.has(page.path)).map(page => ({ id: page.path, custom: page })), ]; return applyOrder(order, entries, entry => entry.id); } diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.tsx index 966da9f76e4..a3b5349aaae 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.tsx @@ -7,6 +7,7 @@ import type { APIKeysPageProps } from '../api-keys/api-keys-table'; import { useOrganizationProfileModel } from './organization-profile.model'; import type { OrganizationProfileOptions } from './organization-profile.provider'; import { OrganizationProfileProvider } from './organization-profile.provider'; +import type { OrganizationProfilePageId, OrganizationProfilePages } from './organization-profile.types'; import type { OrganizationProfileViewProps } from './organization-profile.view'; import { OrganizationProfileView } from './organization-profile.view'; import { OrganizationProfileApiKeysPanel } from './organization-profile-api-keys-panel'; @@ -83,7 +84,7 @@ export function OrganizationProfile({ defaultPage, afterLeaveOrganizationUrl, fallback, - apiKeysProps: { hide: hideAPIKeys = false, ...apiKeysProps } = {}, + apiKeysProps, }: OrganizationProfileProps) { const model = useOrganizationProfileModel(); const [activePage, setActivePage] = useState( @@ -105,15 +106,24 @@ export function OrganizationProfile({ onPageChange={setActivePage} customPages={customPages} pageOrder={pageOrder} - pages={{ - general: , - members: model.pages.includes('members') ? : undefined, - apiKeys: - model.pages.includes('apiKeys') && !hideAPIKeys ? ( - - ) : undefined, - }} + pages={organizationProfilePanels(model.pages, apiKeysProps)} /> ); } + +/** + * The organization's built-in panels, for the pages the instance has turned on. `apiKeysProps.hide` + * leaves the API keys panel out; the rest of `apiKeysProps` goes to it. + */ +export function organizationProfilePanels( + available: readonly OrganizationProfilePageId[], + { hide: hideAPIKeys = false, ...apiKeysProps }: APIKeysPageProps = {}, +): OrganizationProfilePages { + return { + general: , + members: available.includes('members') ? : undefined, + apiKeys: + available.includes('apiKeys') && !hideAPIKeys ? : undefined, + }; +} diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.view.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.view.tsx index 1d4e47e7140..9f55147fe40 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.view.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.view.tsx @@ -2,9 +2,9 @@ import { logger } from '@clerk/shared/logger'; import React from 'react'; import { Icon } from '../../components/icon'; -import type { ProfileRootProps } from '../../components/profile'; -import { Profile } from '../../components/profile'; import { useMessages } from '../../localization'; +import type { ProfilePage, ProfilePagesViewProps } from '../profile-pages/profile-pages.view'; +import { ProfilePagesView } from '../profile-pages/profile-pages.view'; import { getAvailableOrganizationProfilePages, ORGANIZATION_PROFILE_PAGE_ICONS, @@ -16,71 +16,66 @@ import type { OrganizationProfilePages, } from './organization-profile.types'; -export interface OrganizationProfileViewProps extends Omit { - label?: string; - activePage: OrganizationProfilePageId | (string & {}); +/** What the organization's navigation is built from. */ +export interface OrganizationProfilePageListOptions { + /** The built-in pages. One left undefined is left out of the navigation. */ pages: OrganizationProfilePages; + /** Pages of the consumer's own, added to the navigation after the built-ins. */ customPages?: readonly CustomOrganizationProfilePage[]; + /** + * The order the navigation runs in, by id: a built-in page's id, or a custom page's `path`. Ids + * left out keep their default place behind the ones named. + */ pageOrder?: readonly (OrganizationProfilePageId | (string & {}))[]; +} + +export interface OrganizationProfileViewProps + extends + Omit, + OrganizationProfilePageListOptions { + label?: string; + activePage: OrganizationProfilePageId | (string & {}); onPageChange: (page: OrganizationProfilePageId | (string & {})) => void; } export const OrganizationProfileView = React.forwardRef( - function OrganizationProfileView({ activePage, pages, customPages, pageOrder, onPageChange, label, ...rest }, ref) { + function OrganizationProfileView({ pages, customPages, pageOrder, label, ...rest }, ref) { const m = useMessages('organizationProfile'); - const entries = resolveOrganizationProfilePages( - getAvailableOrganizationProfilePages(pages), - customPages, - pageOrder, - ); - const firstPage = entries[0]; - if (!firstPage) { + const list = organizationProfilePageList({ pages, customPages, pageOrder }, m.pages); + if (list.length === 0) { logger.warnOnce('[Clerk] OrganizationProfile has no pages to show.'); } - const resolvedEntry = entries.find(entry => entry.id === activePage) ?? firstPage; - const resolvedPage = resolvedEntry?.id ?? 'general'; return ( - - {label ?? m.label} - - {entries.map(entry => ( - - ) - } - > - {entry.custom ? entry.custom.label : m.pages[entry.id]} - - ))} - - - {entries.map(entry => ( - - {entry.custom ? entry.custom.content : pages[entry.id]} - - ))} - - + /> ); }, ); + +/** The organization's navigation, in order: the built-ins it was given content for, then the consumer's pages. */ +export function organizationProfilePageList( + { pages, customPages, pageOrder }: OrganizationProfilePageListOptions, + labels: Record, +): ProfilePage[] { + return resolveOrganizationProfilePages(getAvailableOrganizationProfilePages(pages), customPages, pageOrder).map( + entry => + entry.custom + ? { id: entry.id, label: entry.custom.label, icon: entry.custom.icon, content: entry.custom.content } + : { + id: entry.id, + label: labels[entry.id], + icon: ( + + ), + content: pages[entry.id], + }, + ); +} diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts new file mode 100644 index 00000000000..96e8f6b5809 --- /dev/null +++ b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts @@ -0,0 +1,15 @@ +import { logger } from '@clerk/shared/logger'; + +export function withUniquePaths(pages: readonly Page[]): Page[] { + const seen = new Set(); + return pages.filter(page => { + if (seen.has(page.path)) { + logger.warnOnce( + `[Clerk] Duplicate path "${page.path}" found in custom pages. This may cause unexpected behavior.`, + ); + return false; + } + seen.add(page.path); + return true; + }); +} diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.view.tsx b/packages/mosaic/src/features/profile-pages/profile-pages.view.tsx new file mode 100644 index 00000000000..710176849c3 --- /dev/null +++ b/packages/mosaic/src/features/profile-pages/profile-pages.view.tsx @@ -0,0 +1,89 @@ +import type { ReactNode } from 'react'; +import React from 'react'; + +import type { ProfileRootProps } from '../../components/profile'; +import { Profile } from '../../components/profile'; + +/** One destination in a profile's navigation, with the page it opens. */ +export interface ProfilePage { + id: string; + label: string; + icon: ReactNode; + content: ReactNode; +} + +/** A run of pages in the navigation. A labelled group is set apart under its label. */ +export interface ProfilePageGroup { + label?: string; + pages: readonly ProfilePage[]; +} + +export interface ProfilePagesViewProps extends Omit { + /** Names the surface, and the dialog it opens in. */ + label: string; + /** The navigation, in order. A group without a label lists its pages without a heading. */ + groups: readonly ProfilePageGroup[]; + /** The open page's id. One the navigation does not list opens the first page instead. */ + activePage: string; + /** Called with a page's id when the user picks it from the navigation. */ + onPageChange: (page: string) => void; +} + +/** A `Profile` built from groups of pages: their navigation, and the active page beside it. */ +export const ProfilePagesView = React.forwardRef(function ProfilePagesView( + { label, groups, activePage, onPageChange, ...rest }, + ref, +) { + const pages = groups.flatMap(group => group.pages); + const activeEntry = pages.find(page => page.id === activePage) ?? pages[0]; + + return ( + + {label} + + {groups.map((group, index) => + group.label === undefined ? ( + + ) : ( + + + + ), + )} + + + {pages.map(page => ( + + {page.content} + + ))} + + + ); +}); + +function NavItems({ pages }: { pages: readonly ProfilePage[] }) { + return pages.map(page => ( + + {page.label} + + )); +} diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx index bec8fb3ac97..d73f1de4307 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx @@ -1,11 +1,18 @@ -import { screen, within } from '@testing-library/react'; +import { screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { beforeAll, describe, expect, it } from 'vitest'; import { page } from 'vitest/browser'; import { openDialog } from '../../../__tests__/feature/dialog'; import { holdRequests, serveFapi } from '../../../__tests__/feature/fake-fapi'; -import { fapiClient, fapiEnvironment, fapiSession, fapiUser } from '../../../__tests__/feature/fapi'; +import { + fapiClient, + fapiEnvironment, + fapiMembership, + fapiOrganization, + fapiSession, + fapiUser, +} from '../../../__tests__/feature/fapi'; import { openMenu } from '../../../__tests__/feature/menu'; import { renderWithClerk } from '../../../__tests__/feature/render'; import { UserProfile } from '../user-profile'; @@ -132,3 +139,146 @@ describe('UserProfile', () => { expect(screen.getByRole('menuitem', { name: 'Reconnect' })).toBeEnabled(); }); }); + +describe('UserProfile with an active organization', () => { + const acme = fapiOrganization({ id: 'org_1', name: 'Acme' }); + + function serveWithOrganization({ + permissions = ['org:sys_memberships:read'], + userAPIKeysEnabled = false, + orgAPIKeysEnabled = false, + activeOrganizationId = acme.id, + }: { + permissions?: string[]; + userAPIKeysEnabled?: boolean; + orgAPIKeysEnabled?: boolean; + activeOrganizationId?: string | null; + } = {}) { + const memberships = [fapiMembership(acme, { permissions })]; + const user = fapiUser({ id: 'user_1', organization_memberships: memberships }); + return serveFapi({ + environment: fapiEnvironment({ + api_keys_settings: { user_api_keys_enabled: userAPIKeysEnabled, orgs_api_keys_enabled: orgAPIKeysEnabled }, + }), + client: fapiClient([fapiSession({ id: 'sess_1', user, last_active_organization_id: activeOrganizationId })]), + memberships, + }); + } + + function tabsDescribedBy(description: string) { + return screen + .getAllByRole('tab') + .filter(tab => tab.getAttribute('aria-describedby') !== null) + .filter(tab => document.getElementById(tab.getAttribute('aria-describedby') ?? '')?.textContent === description) + .map(tab => tab.textContent); + } + + it('lists the organization pages under Organization after the account pages', async () => { + serveWithOrganization(); + await renderWithClerk(); + expect(tabsDescribedBy('Account')).toEqual(['Account', 'Security']); + expect(tabsDescribedBy('Organization')).toEqual(['General', 'Members']); + }); + + it('lists only the organization pages the user may see', async () => { + serveWithOrganization({ permissions: [] }); + await renderWithClerk(); + expect(tabsDescribedBy('Organization')).toEqual(['General']); + }); + + it.each([ + { hide: true, tabs: ['General'] }, + { hide: false, tabs: ['General', 'API Keys'] }, + ])('lists the organization API keys only when they are not hidden: $hide', async ({ hide, tabs }) => { + serveWithOrganization({ orgAPIKeysEnabled: true, permissions: ['org:sys_api_keys:read'] }); + await renderWithClerk(); + expect(tabsDescribedBy('Organization')).toEqual(tabs); + }); + + it('sends the user to afterLeaveOrganizationUrl after leaving the organization', async () => { + const fapi = serveWithOrganization(); + const { navigate } = await renderWithClerk( + , + ); + const user = userEvent.setup(); + const dialog = await openDialog(user, await screen.findByRole('button', { name: 'Leave organization' }), { + role: 'alertdialog', + }); + + await user.type(within(dialog).getByRole('textbox'), 'Acme'); + await user.click(within(dialog).getByRole('button', { name: 'Leave organization' })); + + await waitFor(() => expect(fapi.memberships).toHaveLength(0)); + await waitFor(() => expect(navigate).toHaveBeenCalledWith('/organizations')); + }); + + it('lists only the account pages in user mode', async () => { + serveWithOrganization(); + await renderWithClerk(); + expect(tabNames()).toEqual(['Account', 'Security']); + }); + + it('lists only the account pages without an active organization', async () => { + serveWithOrganization({ activeOrganizationId: null }); + await renderWithClerk(); + expect(tabNames()).toEqual(['Account', 'Security']); + }); + + it('opens on an organization page by its prefixed id', async () => { + serveWithOrganization(); + await renderWithClerk(); + expect(screen.getByRole('tab', { name: 'Members', description: 'Organization' })).toHaveAttribute( + 'aria-selected', + 'true', + ); + expect(screen.getByRole('heading', { level: 3, name: 'Members' })).toBeVisible(); + }); + + it('opens the organization page that shares an account page name', async () => { + serveWithOrganization(); + await renderWithClerk( + Organization security

}], + }} + />, + ); + const user = userEvent.setup(); + + await user.click(screen.getByRole('tab', { name: 'Security', description: 'Organization' })); + expect(screen.getByText('Organization security')).toBeVisible(); + }); + + it('skips an account page whose path is an organization page id', async () => { + serveWithOrganization(); + await renderWithClerk( + Imposter content

}]} + />, + ); + + expect(tabNames()).toEqual(['Account', 'Security', 'General', 'Members']); + expect(screen.getByRole('tab', { name: 'General', description: 'Organization' })).toHaveAttribute( + 'aria-selected', + 'true', + ); + expect(screen.queryByText('Imposter content')).not.toBeInTheDocument(); + }); + + it('passes the organization options to its pages', async () => { + serveWithOrganization(); + await renderWithClerk( + Audit content

}], + pageOrder: ['audit'], + }} + />, + ); + expect(tabsDescribedBy('Organization')).toEqual(['Audit log', 'General', 'Members']); + }); +}); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts index 070f1c16343..0e3b207b2ed 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts @@ -25,6 +25,17 @@ describe('getAvailableUserProfilePages', () => { }); describe('resolveUserProfilePages', () => { + it('keeps the first custom page for a repeated path', () => { + const otherTerms: CustomProfilePage = { label: 'Other terms', path: 'terms', content: null }; + const otherAccount: CustomProfilePage = { label: 'Other account', path: 'account', content: null }; + const account: CustomProfilePage = { label: 'My account', path: 'account', content: null }; + + expect(resolveUserProfilePages(['account'], [account, terms, otherTerms, otherAccount])).toEqual([ + { id: 'account', custom: account }, + { id: 'terms', custom: terms }, + ]); + }); + it('lists the built-ins, then the custom pages by path', () => { expect(resolveUserProfilePages(['account', 'security'], [terms, help])).toEqual([ { id: 'account' }, diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.view.test.tsx index 01c3f22125e..333f6968b00 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.view.test.tsx @@ -194,4 +194,66 @@ describe('UserProfileView', () => { expect(warn).toHaveBeenCalledWith('[Clerk] UserProfile has no pages to show.'); }); + describe('with an organization', () => { + const organization: NonNullable = { + pages: { general:

General content

, apiKeys:

Organization API keys content

}, + }; + + function tabs() { + return screen.getAllByRole('tab').map(tab => tab.textContent); + } + + it('groups the organization pages under Organization, after the account pages', () => { + renderView({ organization }); + + expect(tabs()).toEqual(['Account', 'Security', 'Billing', 'API Keys', 'General', 'API Keys']); + expect(screen.getByRole('tab', { name: 'Security' })).toHaveAccessibleDescription('Account'); + expect(screen.getByRole('tab', { name: 'General' })).toHaveAccessibleDescription('Organization'); + }); + + it('gives the organization pages their own ids', async () => { + const onPageChange = vi.fn(); + const user = userEvent.setup(); + renderView({ organization, onPageChange }); + + await user.click(screen.getByRole('tab', { name: 'General' })); + + expect(onPageChange).toHaveBeenCalledWith('organization/general'); + }); + + it('opens the organization page a shared id names, not the account one', () => { + renderView({ organization, activePage: 'organization/apiKeys' }); + + expect(screen.getByRole('tab', { name: 'API Keys', description: 'Organization' })).toHaveAttribute( + 'aria-selected', + 'true', + ); + expect(screen.getByText('Organization API keys content')).toBeVisible(); + expect(screen.queryByText('API keys content')).not.toBeInTheDocument(); + }); + + it('adds and orders custom pages within the organization group', async () => { + const onPageChange = vi.fn(); + const user = userEvent.setup(); + renderView({ + onPageChange, + organization: { + ...organization, + customPages: [{ label: 'Audit log', path: 'audit', content:

Audit content

}], + pageOrder: ['audit'], + }, + }); + + expect(tabs()).toEqual(['Account', 'Security', 'Billing', 'API Keys', 'Audit log', 'General', 'API Keys']); + await user.click(screen.getByRole('tab', { name: 'Audit log' })); + expect(onPageChange).toHaveBeenCalledWith('organization/audit'); + }); + + it('falls back to the first page once the organization is gone', () => { + renderView({ activePage: 'organization/general' }); + + expect(screen.getByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true'); + expect(document.querySelector('.cl-profile-nav-group')).not.toBeInTheDocument(); + }); + }); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile.layout.ts b/packages/mosaic/src/features/user-profile/user-profile.layout.ts index e56d92b6e68..2410672bf9e 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.layout.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.layout.ts @@ -4,11 +4,15 @@ import { CreditCard } from '../../icons/glyphs/credit-card'; import { ShieldCheck } from '../../icons/glyphs/shield-check'; import { UserCircle } from '../../icons/glyphs/user-circle'; import { applyOrder } from '../../utils/apply-order'; +import { withUniquePaths } from '../profile-pages/profile-pages.layout'; import type { CustomProfilePage, UserProfilePageId, UserProfilePages } from './user-profile.types'; /** The built-in pages in the order the profile lists them before a consumer reorders anything. */ export const USER_PROFILE_PAGE_IDS: readonly UserProfilePageId[] = ['account', 'security', 'billing', 'apiKeys']; +/** Sets an organization page's id apart from an account page's when the profile shows both. */ +export const ORGANIZATION_PAGE_PREFIX = 'organization/'; + export const USER_PROFILE_PAGE_ICONS: Record = { account: UserCircle, security: ShieldCheck, @@ -39,14 +43,15 @@ export function resolveUserProfilePages( customPages: readonly CustomProfilePage[] = [], order?: readonly string[], ): UserProfileNavEntry[] { - const customById = new Map(customPages.map(page => [page.path, page])); + const uniqueCustomPages = withUniquePaths(customPages); + const customById = new Map(uniqueCustomPages.map(page => [page.path, page])); const builtInIds = new Set(builtIn); const entries: UserProfileNavEntry[] = [ ...builtIn.map((id): UserProfileNavEntry => { const custom = customById.get(id); return custom ? { id, custom } : { id }; }), - ...customPages.filter(page => !builtInIds.has(page.path)).map(page => ({ id: page.path, custom: page })), + ...uniqueCustomPages.filter(page => !builtInIds.has(page.path)).map(page => ({ id: page.path, custom: page })), ]; return applyOrder(order, entries, entry => entry.id); } diff --git a/packages/mosaic/src/features/user-profile/user-profile.messages.ts b/packages/mosaic/src/features/user-profile/user-profile.messages.ts index 7f95082be45..fd1b9b141bf 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.messages.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.messages.ts @@ -1,6 +1,10 @@ export const userProfileMessages = { /** Names the surface: its navigation landmark, and the dialog it opens in. */ label: 'User profile', + /** Labels the account's pages when the organization's are listed beside them. */ + accountGroup: 'Account', + /** Labels the organization's pages, listed after the account's. */ + organizationGroup: 'Organization', pages: { account: 'Account', security: 'Security', diff --git a/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts b/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts index b9b9e03e90a..73ea32c6e4e 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts @@ -3,7 +3,7 @@ import type { UserProfileProps } from './user-profile'; export interface UserProfileModalOptions extends Pick< UserProfileProps, - 'customPages' | 'pageOrder' | 'additionalOAuthScopes' | 'apiKeysProps' + 'customPages' | 'pageOrder' | 'additionalOAuthScopes' | 'apiKeysProps' | 'mode' | 'organizationProfileProps' > { page?: UserProfileProps['defaultPage']; } diff --git a/packages/mosaic/src/features/user-profile/user-profile.tsx b/packages/mosaic/src/features/user-profile/user-profile.tsx index 2d5347c1a34..1591a2ace70 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.tsx @@ -4,7 +4,12 @@ import type { ReactNode } from 'react'; import { useState } from 'react'; import type { APIKeysPageProps } from '../api-keys/api-keys-table'; +import type { OrganizationProfileProps } from '../organization-profile/organization-profile'; +import { organizationProfilePanels } from '../organization-profile/organization-profile'; +import { useOrganizationProfileModel } from '../organization-profile/organization-profile.model'; +import { OrganizationProfileProvider } from '../organization-profile/organization-profile.provider'; import { useUserProfilePages } from '../switcher/switcher.pages'; +import type { SwitcherMode } from '../switcher/switcher.types'; import type { UserProfileOptions } from './user-profile.provider'; import { UserProfileProvider } from './user-profile.provider'; import type { UserProfileViewProps } from './user-profile.view'; @@ -14,6 +19,12 @@ import { UserProfileProfilePanel } from './user-profile-profile-panel'; import { UserProfileSecurityPanel } from './user-profile-security-panel'; import { useUserProfileUserModel } from './user-profile-user.model'; +/** + * Which profiles the surface carries. `combined` adds the active organization's pages after the + * account's; `user` shows the account's alone, even when an organization is active. + */ +export type UserProfileMode = Exclude; + /** Everything `` takes: your own pages and their order, the page it opens on, and the OAuth scopes it asks for. */ export interface UserProfileProps extends Pick, Pick { @@ -34,11 +45,26 @@ export interface UserProfileProps * keys a page lists, and `showDescription` adds an optional description when creating a key. */ apiKeysProps?: APIKeysPageProps; + /** + * Which profiles the surface carries: the account's and the active organization's, or the + * account's alone. + * + * @default 'combined' + */ + mode?: UserProfileMode; + /** + * Options for the organization's pages in `combined` mode: your own pages and their order, the API + * keys page, and where leaving the organization lands. + */ + organizationProfileProps?: Pick< + OrganizationProfileProps, + 'customPages' | 'pageOrder' | 'apiKeysProps' | 'afterLeaveOrganizationUrl' + >; } /** * The signed-in user's profile: Account, Security, and API keys when the instance enables them, - * with your own pages alongside. It reads the user from Clerk, so it takes no data. It renders + * with your own pages alongside, and the active organization's pages after them. It reads the user from Clerk, so it takes no data. It renders * `fallback` until Clerk answers, and nothing at all when nobody is signed in. * * @example @@ -54,12 +80,29 @@ export interface UserProfileProps * ``` * * @example - * `defaultPage` picks the page the profile opens on without moving it in the navigation. + * `defaultPage` picks the page the profile opens on without moving it in the navigation. An + * organization page's id is prefixed with `organization/`. * ```tsx * * ``` * * @example + * `mode='user'` leaves the active organization's pages out. + * ```tsx + * + * ``` + * + * @example + * `organizationProfileProps` adds your own pages to the organization's group. + * ```tsx + * , content: }], + * }} + * /> + * ``` + * + * @example * `customPages` adds your own pages; `pageOrder` places them among the built-in ones. * ```tsx * , + security: , + apiKeys: + builtInPages.includes('apiKeys') && !hideAPIKeys ? : undefined, + }, + }; + return ( + {mode === 'user' ? ( + + ) : ( + + )} + + ); +} + +function CombinedUserProfileView({ + organizationProfileProps: { customPages, pageOrder, apiKeysProps, afterLeaveOrganizationUrl } = {}, + ...viewProps +}: UserProfileViewProps & Pick) { + const model = useOrganizationProfileModel(); + + return ( + , - security: , - apiKeys: - builtInPages.includes('apiKeys') && !hideAPIKeys ? ( - - ) : undefined, - }} + {...viewProps} + organization={ + model.status === 'ready' + ? { pages: organizationProfilePanels(model.pages, apiKeysProps), customPages, pageOrder } + : undefined + } /> - + ); } diff --git a/packages/mosaic/src/features/user-profile/user-profile.view.tsx b/packages/mosaic/src/features/user-profile/user-profile.view.tsx index 320a7d8472a..27994a99272 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.view.tsx @@ -2,17 +2,31 @@ import { logger } from '@clerk/shared/logger'; import React from 'react'; import { Icon } from '../../components/icon'; -import type { ProfileRootProps } from '../../components/profile'; -import { Profile } from '../../components/profile'; import { useMessages } from '../../localization'; -import { getAvailableUserProfilePages, resolveUserProfilePages, USER_PROFILE_PAGE_ICONS } from './user-profile.layout'; +import type { OrganizationProfilePageId } from '../organization-profile/organization-profile.types'; +import type { OrganizationProfilePageListOptions } from '../organization-profile/organization-profile.view'; +import { organizationProfilePageList } from '../organization-profile/organization-profile.view'; +import type { ProfilePage, ProfilePageGroup, ProfilePagesViewProps } from '../profile-pages/profile-pages.view'; +import { ProfilePagesView } from '../profile-pages/profile-pages.view'; +import { + getAvailableUserProfilePages, + ORGANIZATION_PAGE_PREFIX, + resolveUserProfilePages, + USER_PROFILE_PAGE_ICONS, +} from './user-profile.layout'; import type { CustomProfilePage, UserProfilePageId, UserProfilePages } from './user-profile.types'; -export interface UserProfileViewProps extends Omit { +export interface UserProfileViewProps extends Omit< + ProfilePagesViewProps, + 'label' | 'groups' | 'activePage' | 'onPageChange' +> { /** Names the surface, and the dialog it opens in. Defaults to the `userProfile.label` message. */ label?: string; - /** The open page: a built-in page's id, or a custom page's `path`. */ - activePage: UserProfilePageId | (string & {}); + /** + * The open page: a built-in page's id, or a custom page's `path`. An organization page's id is + * prefixed with `organization/`. + */ + activePage: UserProfilePageId | `organization/${OrganizationProfilePageId}` | (string & {}); pages: UserProfilePages; /** Pages of the consumer's own, added to the navigation after the built-ins. */ customPages?: readonly CustomProfilePage[]; @@ -21,68 +35,77 @@ export interface UserProfileViewProps extends Omit void; + /** Adds the organization's pages, in a group of their own. */ + organization?: OrganizationProfilePageListOptions; + onPageChange: (page: UserProfileViewProps['activePage']) => void; } /** * The user profile as a `Profile`: the built-in pages the instance has content for, the - * consumer's own pages after them, in the order asked for. An `activePage` the navigation does - * not list falls back to the first one, so a page turned off by the environment cannot leave the - * surface blank. + * consumer's own pages after them, in the order asked for, and the organization's pages in a group + * after those when there is one. An `activePage` the navigation does not list falls back to the + * first one, so a page turned off by the environment cannot leave the surface blank. */ export const UserProfileView = React.forwardRef(function UserProfileView( - { activePage, pages, customPages, pageOrder, onPageChange, label, ...rest }, + { pages, customPages, pageOrder, organization, label, ...rest }, ref, ) { const m = useMessages('userProfile'); - const entries = resolveUserProfilePages(getAvailableUserProfilePages(pages), customPages, pageOrder); - const firstPage = entries[0]; - if (!firstPage) { + const organizationLabels = useMessages('organizationProfile').pages; + const accountCustomPages = customPages?.filter(page => { + if (!page.path.startsWith(ORGANIZATION_PAGE_PREFIX)) { + return true; + } + logger.warnOnce( + `[Clerk] UserProfile custom page "${page.path}" uses the reserved "${ORGANIZATION_PAGE_PREFIX}" prefix and was skipped.`, + ); + return false; + }); + const accountPages = userProfilePageList({ pages, customPages: accountCustomPages, pageOrder }, m.pages); + const groups: ProfilePageGroup[] = organization + ? [ + { label: m.accountGroup, pages: accountPages }, + { + label: m.organizationGroup, + pages: organizationProfilePageList(organization, organizationLabels).map(page => ({ + ...page, + id: `${ORGANIZATION_PAGE_PREFIX}${page.id}`, + })), + }, + ] + : [{ pages: accountPages }]; + if (accountPages.length === 0) { logger.warnOnce('[Clerk] UserProfile has no pages to show.'); } - const resolvedEntry = entries.find(entry => entry.id === activePage) ?? firstPage; - const resolvedPage = resolvedEntry?.id ?? 'account'; return ( - - {label ?? m.label} - - {entries.map(entry => ( - - ) - } - > - {entry.custom ? entry.custom.label : m.pages[entry.id]} - - ))} - - - {entries.map(entry => ( - - {entry.custom ? entry.custom.content : pages[entry.id]} - - ))} - - + /> ); }); + +/** The account's navigation, in order: the built-ins it was given content for, then the consumer's pages. */ +function userProfilePageList( + { pages, customPages, pageOrder }: Pick, + labels: Record, +): ProfilePage[] { + return resolveUserProfilePages(getAvailableUserProfilePages(pages), customPages, pageOrder).map(entry => + entry.custom + ? { id: entry.id, label: entry.custom.label, icon: entry.custom.icon, content: entry.custom.content } + : { + id: entry.id, + label: labels[entry.id], + icon: ( + + ), + content: pages[entry.id], + }, + ); +} diff --git a/packages/mosaic/src/primitives/select/README.md b/packages/mosaic/src/primitives/select/README.md index 3b394a3e46a..f4617d5c905 100644 --- a/packages/mosaic/src/primitives/select/README.md +++ b/packages/mosaic/src/primitives/select/README.md @@ -104,6 +104,8 @@ By default, the selected option visually aligns with the trigger. Disable this f | `Select.Positioner` | `
` | Floating positioned container | | `Select.Popup` | `
` | Visual wrapper for the option list | | `Select.Option` | ` + +
); }