diff --git a/.changeset/mosaic-user-button-action-feedback.md b/.changeset/mosaic-user-button-action-feedback.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-user-button-action-feedback.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/ui/src/mosaic/hooks/__tests__/useSpinDelay.test.ts b/packages/ui/src/mosaic/hooks/__tests__/useSpinDelay.test.ts index 66052c26835..3ac3f63839e 100644 --- a/packages/ui/src/mosaic/hooks/__tests__/useSpinDelay.test.ts +++ b/packages/ui/src/mosaic/hooks/__tests__/useSpinDelay.test.ts @@ -76,6 +76,27 @@ describe('useSpinDelay', () => { expect(result.current).toBeNull(); }); + // Direct feedback on a click has nothing to debounce, so a zero delay must not cost a timer's + // worth of render passes before the spinner appears. + it('surfaces the value in the same pass when there is no delay to wait out', async () => { + const { result, rerender } = render(null, { delay: 0, minDuration: 200 }); + await act(() => rerender({ value: 'a' })); + + expect(result.current).toBe('a'); + }); + + it('still holds a zero-delay value for minDuration', async () => { + const { result, rerender } = render(null, { delay: 0, minDuration: 200 }); + await act(() => rerender({ value: 'a' })); + await act(() => rerender({ value: null })); + + await advance(199); + expect(result.current).toBe('a'); + + await advance(1); + expect(result.current).toBeNull(); + }); + it('swaps to a new value immediately when one replaces another mid-show', async () => { const { result, rerender } = render(null, { delay: 500, minDuration: 200 }); await act(() => rerender({ value: 'a' })); diff --git a/packages/ui/src/mosaic/hooks/useSpinDelay.ts b/packages/ui/src/mosaic/hooks/useSpinDelay.ts index b847c0bc517..dac6bc682b7 100644 --- a/packages/ui/src/mosaic/hooks/useSpinDelay.ts +++ b/packages/ui/src/mosaic/hooks/useSpinDelay.ts @@ -1,7 +1,7 @@ import { useEffect, useRef, useState } from 'react'; export interface SpinDelayOptions { - /** Wait this long before showing the value, so quick actions never flash a spinner. */ + /** Wait this long before showing the value, so quick actions never flash a spinner. `0` shows it straight away. */ delay?: number; /** Once shown, keep the value up at least this long, so the spinner never flickers off. */ minDuration?: number; @@ -25,11 +25,17 @@ export function useSpinDelay(value: T | null, options: SpinDelayOptions = {}) const shownAt = useRef(0); useEffect(() => { - // Nothing showing yet: arm a timer so the value only surfaces if it outlasts `delay`. + // Nothing showing yet: arm a timer so the value only surfaces if it outlasts `delay`. With no + // delay there is nothing to outlast, so it surfaces in this pass rather than a timer's. if (shown === null) { if (value === null) { return; } + if (delay <= 0) { + shownAt.current = Date.now(); + setShown(value); + return; + } const timer = setTimeout(() => { shownAt.current = Date.now(); setShown(value); diff --git a/packages/ui/src/mosaic/user-button/__tests__/user-button.integration.test.tsx b/packages/ui/src/mosaic/user-button/__tests__/user-button.integration.test.tsx new file mode 100644 index 00000000000..67dca43dc6b --- /dev/null +++ b/packages/ui/src/mosaic/user-button/__tests__/user-button.integration.test.tsx @@ -0,0 +1,485 @@ +import type * as SharedReact from '@clerk/shared/react'; +import { render, screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { MosaicProvider } from '../../MosaicProvider'; +import type { UserButtonProps } from '../user-button'; +import { UserButton } from '../user-button'; + +// End-to-end wiring test for the connected UserButton: it renders the real view through the real +// controller against a mocked Clerk, then drives the real popover DOM. Unlike the controller test +// (controller -> Clerk), this proves the layers compose — including the container's +// close-on-success: one-shot actions close the popover, navigations leave it open. + +interface FakeUser { + id: string; + firstName: string | null; + lastName: string | null; + username: string | null; + primaryEmailAddress: { emailAddress: string } | null; + imageUrl: string; + organizationMemberships: unknown[]; + createOrganizationEnabled: boolean; +} + +interface FakeSession { + id: string; + user: FakeUser; +} + +interface FakeList { + data: unknown[]; + count: number; + hasNextPage: boolean; + isLoading: boolean; + revalidate: ReturnType; +} + +let isUserLoaded: boolean; +let isSessionLoaded: boolean; +let isOrgLoaded: boolean; +let user: FakeUser | null; +let session: { id: string; checkAuthorization: ReturnType } | null; +let organization: { id: string; name: string; imageUrl: string; membersCount: number } | null; +let userMemberships: FakeList; +let userInvitations: FakeList; +let userSuggestions: FakeList; +let signedInSessions: FakeSession[]; +let pagingRef: ReturnType; +let singleSessionMode: boolean; + +let setActive: ReturnType; +let signOut: ReturnType; +let navigate: ReturnType; +let openUserProfile: ReturnType; +let openOrganizationProfile: ReturnType; +let openCreateOrganization: ReturnType; +let openInviteMembers: ReturnType; + +vi.mock('@clerk/shared/react', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + useUser: () => ({ isLoaded: isUserLoaded, user }), + useSession: () => ({ isLoaded: isSessionLoaded, session }), + useOrganization: () => ({ isLoaded: isOrgLoaded, organization }), + useClerk: () => ({ + navigate, + setActive, + signOut, + openUserProfile, + openOrganizationProfile, + openCreateOrganization, + openInviteMembers, + buildUserProfileUrl: () => '/user-profile', + buildOrganizationProfileUrl: () => '/org-profile', + buildCreateOrganizationUrl: () => '/create-org', + buildSignInUrl: () => '/sign-in', + buildAfterSignOutUrl: () => '/after-sign-out', + buildAfterMultiSessionSingleSignOutUrl: () => '/after-single-sign-out', + client: { signedInSessions }, + __internal_environment: { + displayConfig: { afterSwitchSessionUrl: '/after-switch' }, + authConfig: { singleSessionMode }, + }, + }), + }; +}); + +// Stubbed at the same seam as the controller test: the in-view helper is the controller's whole +// fetch boundary, so `ref` doubles as the assertion that the paging sentinel mounted. +vi.mock('../../../hooks/useOrganizationListInView', () => ({ + useOrganizationListInView: () => ({ userMemberships, userInvitations, userSuggestions, ref: pagingRef }), +})); + +function acceptable(id: string, orgId: string, orgName: string, status: 'pending' | 'accepted' = 'pending') { + return { + id, + status, + accept: vi.fn().mockResolvedValue(undefined), + publicOrganizationData: { id: orgId, name: orgName, imageUrl: '' }, + }; +} + +function membership(orgId: string, name: string, membersCount: number) { + return { organization: { id: orgId, name, imageUrl: '', membersCount } }; +} + +function list(data: unknown[], count: number, hasNextPage = false, isLoading = false): FakeList { + return { data, count, hasNextPage, isLoading, revalidate: vi.fn().mockResolvedValue(undefined) }; +} + +/** A promise whose settling is controlled by the test, to hold an async action in flight. */ +function createDeferred() { + let resolve: () => void = () => {}; + let reject: (reason?: unknown) => void = () => {}; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +beforeEach(() => { + isUserLoaded = true; + isSessionLoaded = true; + isOrgLoaded = true; + user = { + id: 'user_1', + firstName: 'Alice', + lastName: 'Smith', + username: 'alice', + primaryEmailAddress: { emailAddress: 'alice@example.com' }, + imageUrl: 'https://img/alice', + organizationMemberships: [{ id: 'orgmem_1' }], + createOrganizationEnabled: true, + }; + session = { id: 'sess_1', checkAuthorization: vi.fn().mockReturnValue(true) }; + organization = { id: 'org_1', name: 'Acme', imageUrl: '', membersCount: 3 }; + userMemberships = list([membership('org_1', 'Acme', 3), membership('org_9', 'Other', 1)], 2); + userInvitations = list([acceptable('inv_1', 'org_3', 'Gamma')], 1); + userSuggestions = list([acceptable('sug_1', 'org_2', 'Beta')], 1); + pagingRef = vi.fn(); + singleSessionMode = false; + signedInSessions = [ + { id: 'sess_1', user }, + { + id: 'sess_2', + user: { + id: 'user_2', + firstName: 'Bob', + lastName: 'Jones', + username: null, + primaryEmailAddress: { emailAddress: 'bob@example.com' }, + imageUrl: 'https://img/bob', + organizationMemberships: [], + createOrganizationEnabled: true, + }, + }, + ]; + setActive = vi.fn().mockResolvedValue(undefined); + signOut = vi.fn().mockResolvedValue(undefined); + navigate = vi.fn().mockResolvedValue(undefined); + openUserProfile = vi.fn(); + openOrganizationProfile = vi.fn(); + openCreateOrganization = vi.fn(); + openInviteMembers = vi.fn(); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +function renderUserButton(props: UserButtonProps = {}) { + return render( + + {/* The button portals its popup out, so this host holds only what it renders in place. */} +
+ +
+ , + ); +} + +const host = () => screen.getByTestId('host'); +const trigger = () => screen.getByRole('button', { name: /Open account menu/ }); +const popup = () => screen.queryByRole('dialog', { name: 'Account' }); +const spinner = () => popup()?.querySelector('.cl-spinner') ?? null; + +async function open() { + const act = userEvent.setup(); + await act.click(trigger()); + expect(popup()).toBeInTheDocument(); + return act; +} + +// Alice has a username, so that is what identifies her row; Bob has none and falls back to email. +const accountMenu = () => screen.getByRole('button', { name: 'Actions for alice' }); + +/** Opens the `⋯` on the active account's row and clicks one of its actions. */ +async function accountAction(act: ReturnType, label: string) { + await act.click(accountMenu()); + await act.click(await screen.findByRole('menuitem', { name: label })); +} + +describe('UserButton (connected)', () => { + it('renders a non-interactive placeholder while the controller is loading', () => { + isUserLoaded = false; + renderUserButton(); + + expect(host()).not.toBeEmptyDOMElement(); + expect(screen.queryByRole('button')).toBeNull(); + }); + + it('renders nothing when there is no active user', () => { + user = null; + renderUserButton(); + expect(host()).toBeEmptyDOMElement(); + }); + + it('renders the trigger and keeps the popover closed until clicked', () => { + renderUserButton(); + expect(trigger()).toBeInTheDocument(); + expect(popup()).toBeNull(); + }); + + it('opens the popover on trigger click', async () => { + renderUserButton(); + await open(); + + expect(screen.getByRole('button', { name: 'Other' })).toBeInTheDocument(); + expect(accountMenu()).toBeInTheDocument(); + }); + + it('selecting an organization calls setActive without a redirect by default and closes the popover', async () => { + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Other' })); + + expect(setActive).toHaveBeenCalledWith({ organization: 'org_9', redirectUrl: undefined }); + await waitFor(() => expect(popup()).toBeNull()); + }); + + it('leaving the active organization for the personal workspace clears it', async () => { + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Personal account' })); + + expect(setActive).toHaveBeenCalledWith({ organization: null, redirectUrl: undefined }); + await waitFor(() => expect(popup()).toBeNull()); + }); + + it('drops the personal workspace where the app hides it, leaving the organizations', async () => { + renderUserButton({ hidePersonal: true }); + await open(); + + expect(screen.queryByText('Personal account')).toBeNull(); + expect(screen.getByRole('button', { name: 'Other' })).toBeInTheDocument(); + }); + + // `mode` is the view's own prop; this only proves the connected component hands it down, since + // the account-only surface is otherwise indistinguishable from an account with no organizations. + it('forwards mode to the view, so an account-only surface lists no organizations', async () => { + renderUserButton({ mode: 'user' }); + await open(); + + expect(screen.queryByRole('button', { name: 'Other' })).toBeNull(); + expect(screen.getByRole('button', { name: 'bob@example.com' })).toBeInTheDocument(); + }); + + it('switching to another account calls setActive with the session and stays open', async () => { + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'bob@example.com' })); + + expect(setActive).toHaveBeenCalledWith({ session: 'sess_2', redirectUrl: '/after-switch' }); + await waitFor(() => expect(spinner()).toBeNull()); + expect(popup()).toBeInTheDocument(); + }); + + it('signing out of the active account calls signOut with its session id', async () => { + renderUserButton(); + const act = await open(); + + await accountAction(act, 'Sign out'); + + // Another account stays signed in, so this is a single sign out, not a full one. + expect(signOut).toHaveBeenCalledWith({ sessionId: 'sess_1', redirectUrl: '/after-single-sign-out' }); + }); + + it('signing out of all accounts calls signOut with the after-sign-out url', async () => { + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Sign out of all accounts' })); + + expect(signOut).toHaveBeenCalledWith({ redirectUrl: '/after-sign-out' }); + }); + + it('accepting an invitation accepts it, revalidates, and stays open', async () => { + renderUserButton(); + const act = await open(); + const invitation = userInvitations.data[0] as ReturnType; + + await act.click(screen.getByRole('button', { name: 'Accept' })); + + await waitFor(() => expect(invitation.accept).toHaveBeenCalledTimes(1)); + expect(userInvitations.revalidate).toHaveBeenCalledTimes(1); + await waitFor(() => expect(spinner()).toBeNull()); + expect(popup()).toBeInTheDocument(); + }); + + it('accepting a suggestion accepts it, revalidates, and stays open', async () => { + renderUserButton(); + const act = await open(); + const suggestion = userSuggestions.data[0] as ReturnType; + + await act.click(screen.getByRole('button', { name: 'Join' })); + + await waitFor(() => expect(suggestion.accept).toHaveBeenCalledTimes(1)); + expect(userSuggestions.revalidate).toHaveBeenCalledTimes(1); + await waitFor(() => expect(spinner()).toBeNull()); + expect(popup()).toBeInTheDocument(); + }); + + it('drops add-account and sign-out-of-all in single-session mode', async () => { + singleSessionMode = true; + signedInSessions = signedInSessions.slice(0, 1); + renderUserButton(); + const act = await open(); + + expect(screen.queryByRole('button', { name: 'Sign out of all accounts' })).toBeNull(); + expect(screen.queryByLabelText('Account actions')).toBeNull(); + await act.click(accountMenu()); + expect(screen.queryByRole('menuitem', { name: 'Add account' })).toBeNull(); + }); + + it('managing the account opens the UserProfile modal and closes the popover', async () => { + renderUserButton(); + const act = await open(); + + await accountAction(act, 'Manage account'); + + expect(openUserProfile).toHaveBeenCalled(); + expect(navigate).not.toHaveBeenCalled(); + await waitFor(() => expect(popup()).toBeNull()); + }); + + it('inviting members opens the InviteMembers modal and closes the popover', async () => { + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Invite' })); + + expect(openInviteMembers).toHaveBeenCalled(); + expect(navigate).not.toHaveBeenCalled(); + await waitFor(() => expect(popup()).toBeNull()); + }); + + it('creating an organization opens the modal and closes the popover', async () => { + renderUserButton(); + const act = await open(); + + await accountAction(act, 'Create organization'); + + expect(openCreateOrganization).toHaveBeenCalled(); + expect(navigate).not.toHaveBeenCalled(); + await waitFor(() => expect(popup()).toBeNull()); + }); + + it('creating an organization navigates instead when a URL routes it', async () => { + renderUserButton({ createOrganizationUrl: '/new-org' }); + const act = await open(); + + await accountAction(act, 'Create organization'); + + expect(navigate).toHaveBeenCalledWith('/new-org'); + expect(openCreateOrganization).not.toHaveBeenCalled(); + await waitFor(() => expect(popup()).toBeNull()); + }); + + it('leaves create-organization out of the account menu for a user who cannot open one', async () => { + user = { ...(user as FakeUser), createOrganizationEnabled: false }; + renderUserButton(); + const act = await open(); + await act.click(accountMenu()); + + // The menu is still there; it is only this one item that has nothing to offer. + expect(await screen.findByRole('menuitem', { name: 'Manage account' })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: 'Create organization' })).toBeNull(); + }); + + it('spins the clicked affordance and stands every other one down while an action is in flight', async () => { + const deferred = createDeferred(); + setActive.mockReturnValueOnce(deferred.promise); + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Other' })); + + // Every one of these is a network round trip, so there is nothing to debounce: the click gets + // its spinner in the same pass rather than after a delay window. + expect(spinner()).toBeInTheDocument(); + // A stood-down row stays a button, disabled. Dropping it to a static row would remount it, + // and with it the avatar it carries. + expect(screen.getByRole('button', { name: 'Sign out of all accounts' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'bob@example.com' })).toBeDisabled(); + expect(popup()).toBeInTheDocument(); + + deferred.resolve(); + await waitFor(() => expect(popup()).toBeNull()); + }); + + // `setActive` swaps the active organization while its promise is still in flight, so live data + // would rearrange the surface under the pointer: the header renaming itself, the check jumping + // rows, and Invite appearing or leaving as the permission is re-read. + it('holds the surface on the data it started with until the action settles', async () => { + const deferred = createDeferred(); + setActive.mockReturnValueOnce(deferred.promise); + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Other' })); + organization = { id: 'org_9', name: 'Other', imageUrl: '', membersCount: 1 }; + + // Any re-render now reads the swapped organization; the surface must not follow it. + await waitFor(() => expect(spinner()).toBeInTheDocument()); + const surface = popup(); + if (!surface) { + throw new Error('expected the popover to be open'); + } + // Still the organization the surface opened on: heading it and listed under it, unclickable. + expect(within(surface).getAllByText('Acme')).toHaveLength(2); + expect(screen.queryByRole('button', { name: 'Acme' })).toBeNull(); + + deferred.resolve(); + await waitFor(() => expect(popup()).toBeNull()); + }); + + it('replaces the accept button with a spinner while a suggestion is being joined', async () => { + const deferred = createDeferred(); + const suggestion = userSuggestions.data[0] as ReturnType; + suggestion.accept.mockReturnValueOnce(deferred.promise); + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Join' })); + + expect(spinner()).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Join' })).toBeNull(); + + deferred.resolve(); + await waitFor(() => expect(spinner()).toBeNull()); + expect(popup()).toBeInTheDocument(); + }); + + it('keeps the popover open and clears busy state when an action rejects', async () => { + const deferred = createDeferred(); + setActive.mockReturnValueOnce(deferred.promise); + renderUserButton(); + const act = await open(); + + await act.click(screen.getByRole('button', { name: 'Other' })); + expect(spinner()).toBeInTheDocument(); + + deferred.reject(new Error('setActive failed')); + + await waitFor(() => expect(spinner()).toBeNull(), { timeout: 2000 }); + expect(popup()).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Sign out of all accounts' })).toBeEnabled(); + }); + + // The view decides whether to mount the sentinel at all; this is the wiring that carries the + // in-view ref from the paginated lists, through the controller, to it. + it('hands the paging sentinel to the in-view ref when a list has a next page', async () => { + userMemberships = list([membership('org_1', 'Acme', 3)], 1, true); + renderUserButton(); + await open(); + + expect(pagingRef).toHaveBeenCalledWith(expect.any(HTMLElement)); + }); +}); diff --git a/packages/ui/src/mosaic/user-button/user-button.tsx b/packages/ui/src/mosaic/user-button/user-button.tsx index c7d479b4cde..199be30e0d6 100644 --- a/packages/ui/src/mosaic/user-button/user-button.tsx +++ b/packages/ui/src/mosaic/user-button/user-button.tsx @@ -3,32 +3,51 @@ import { useState } from 'react'; import { useSpinDelay } from '../hooks/useSpinDelay'; -import { type UserButtonControllerOptions, useUserButtonController } from './user-button.controller'; +import type { UserButtonController, UserButtonControllerOptions } from './user-button.controller'; +import { useUserButtonController } from './user-button.controller'; import type { UserButtonRootProps, UserButtonTriggerProps } from './user-button.view'; import { userButtonBusyKeys, UserButtonTriggerSkeleton, UserButtonView } from './user-button.view'; export type UserButtonProps = UserButtonControllerOptions & UserButtonTriggerProps & - Pick; + Pick; + +/** The one action in flight: which affordance owns it, and what the surface froze on to run it. */ +interface PendingAction { + key: string; + snapshot: Extract; +} /** * The connected UserButton: reads live Clerk data through `useUserButtonController` and renders * the presentational `UserButtonView`. Owns the popover open state and the single in-flight action: - * it marks the clicked affordance busy (spinner + disables the rest), closes the popover only when - * the action resolves, and clears busy state (leaving the popover open) if it rejects. Actions that - * open another surface (manage/create navigations) leave the popover as-is. + * it marks the clicked affordance busy (spinner + disables the rest), holds the surface still on + * the data the action started from, closes the popover only when the action resolves, and clears + * busy state (leaving the popover open) if it rejects. Actions that hand off to another surface + * (managing, inviting, creating, adding an account) close the popover on the way out. */ -export function UserButton({ renderTriggerLabel, renderPlanBadge, modePriority, ...options }: UserButtonProps = {}) { +export function UserButton({ + renderTriggerLabel, + renderPlanBadge, + mode, + modePriority, + ...options +}: UserButtonProps = {}) { const controller = useUserButtonController(options); const [open, setOpen] = useState(false); - const [pendingKey, setPendingKey] = useState(null); + const [action, setAction] = useState(null); - // Hold the spinner off for quick actions and steady it once shown. Re-entry is still guarded on - // the immediate `pendingKey`; only the view's feedback is delayed. - const displayPendingKey = useSpinDelay(pendingKey); + // Every action here is a network round trip, so there is nothing to debounce and the click gets + // its spinner at once. The hook is still what steadies it, holding it up long enough to read. + const displayPendingKey = useSpinDelay(action?.key ?? null, { delay: 0 }); if (controller.status === 'loading') { - return ; + return ( + + ); } if (controller.status !== 'ready') { @@ -48,16 +67,30 @@ export function UserButton({ renderTriggerLabel, renderPlanBadge, modePriority, ) => fn ? (...args: Args) => { - if (pendingKey) { + if (action) { return; } - setPendingKey(keyFor(...args)); + setAction({ key: keyFor(...args), snapshot: controller }); void Promise.resolve(fn(...args)) .then(closeOnSuccess ? close : () => {}, () => {}) - .finally(() => setPendingKey(null)); + .finally(() => setAction(null)); + } + : undefined; + + // A modal or another page takes over from here, so there is nothing left for the popover to show; + // left up, it would sit over the very surface it just opened. + const handOff = (fn: (() => void) | undefined) => + fn + ? () => { + close(); + fn(); } : undefined; + // `setActive` swaps the active organization while its promise is still in flight, so the live + // controller would rearrange the popup mid-action: the header renaming itself, the check jumping + // rows, Invite coming and going as the permission is re-read. Rendering the snapshot the action + // started from holds it all still, and the result lands in one step when the action settles. const { status: _status, onSelectOrganization, @@ -66,14 +99,20 @@ export function UserButton({ renderTriggerLabel, renderPlanBadge, modePriority, onSignOutAll, onAcceptSuggestion, onAcceptInvitation, + onManageAccount, + onManageOrganization, + onInviteMembers, + onCreateOrganization, + onAddAccount, ...data - } = controller; + } = action?.snapshot ?? controller; return ( ); }