Skip to content
Draft
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
7 changes: 7 additions & 0 deletions .changeset/user-button-a11y.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@clerk/ui': patch
'@clerk/shared': patch
'@clerk/localizations': patch
---

Fix UserButton popover accessibility: use `role="dialog"` with grouped actions instead of `role="menu"` with `menuitem` children, fix focus management via floating-ui's interaction system, and add identity-first trigger labels
8 changes: 4 additions & 4 deletions integration/tests/astro/components.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ testAgainstRunningApps({ withPattern: ['astro.node.withCustomRoles'] })('basic f
await u.po.userButton.toHaveVisibleMenuItems([/Custom link/i, /Custom action/i, /Custom click/i]);

// Click custom action and check for custom page availbility
await u.page.getByRole('menuitem', { name: /Custom action/i }).click();
await u.page.getByRole('button', { name: /Custom action/i }).click();
await u.po.userProfile.waitForUserProfileModal();
await expect(u.page.getByRole('heading', { name: 'Custom Terms Page' })).toBeVisible();

Expand All @@ -114,15 +114,15 @@ testAgainstRunningApps({ withPattern: ['astro.node.withCustomRoles'] })('basic f
);
});
});
await u.page.getByRole('menuitem', { name: /Custom click/i }).click();
await u.page.getByRole('button', { name: /Custom click/i }).click();
expect(await eventPromise).toBe('custom_click');

// Trigger the popover again
await u.po.userButton.toggleTrigger();
await u.po.userButton.waitForPopover();

// Click custom link and check navigation
await u.page.getByRole('menuitem', { name: /Custom link/i }).click();
await u.page.getByRole('button', { name: /Custom link/i }).click();
await u.page.waitForAppUrl('/user');
});

Expand All @@ -139,7 +139,7 @@ testAgainstRunningApps({ withPattern: ['astro.node.withCustomRoles'] })('basic f
await u.po.userButton.waitForPopover();

// First item should now be the sign out button
await u.page.getByRole('menuitem').first().click();
await u.page.getByRole('button').first().click();
await u.po.expect.toBeSignedOut();
});

Expand Down
2 changes: 1 addition & 1 deletion integration/tests/sign-out-smoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign out
await u.page.getByRole('link', { name: 'Home' }).click();
await u.page.getByRole('button', { name: 'Open user menu' }).click();

await u.page.getByRole('menuitem', { name: 'Sign out' }).click();
await u.page.getByRole('button', { name: 'Sign out' }).click();
await u.po.expect.toBeSignedOut();
await u.page.getByRole('link', { name: 'Protected', exact: true }).click();
await u.page.waitForURL(url => url.href.includes('/sign-in?redirect_url'));
Expand Down
10 changes: 5 additions & 5 deletions integration/tests/vue/components.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withCustomRoles] })('basic te
await u.po.userButton.toHaveVisibleMenuItems([/Custom link/i, /Custom page/i, /Custom action/i]);

// Click custom action
await u.page.getByRole('menuitem', { name: /Custom action/i }).click();
await u.page.getByRole('button', { name: /Custom action/i }).click();
await expect(u.page.getByText('Is action clicked: true')).toBeVisible();

// Trigger the popover again
await u.po.userButton.toggleTrigger();
await u.po.userButton.waitForPopover();

// Click custom action and check for custom page availbility
await u.page.getByRole('menuitem', { name: /Custom page/i }).click();
await u.page.getByRole('button', { name: /Custom page/i }).click();
await u.po.userProfile.waitForUserProfileModal();
await expect(u.page.getByRole('heading', { name: 'Custom Terms Page' })).toBeVisible();

Expand All @@ -98,7 +98,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withCustomRoles] })('basic te
await u.po.userButton.waitForPopover();

// Click custom link and check navigation
await u.page.getByRole('menuitem', { name: /Custom link/i }).click();
await u.page.getByRole('button', { name: /Custom link/i }).click();
await u.page.waitForAppUrl('/profile');
});

Expand All @@ -115,7 +115,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withCustomRoles] })('basic te
await u.po.userButton.waitForPopover();

// First item should now be the sign out button
await u.page.getByRole('menuitem').first().click();
await u.page.getByRole('button').first().click();
await u.po.expect.toBeSignedOut();
});

Expand All @@ -132,7 +132,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withCustomRoles] })('basic te
// Open UserProfile modal through UserButton
await u.po.userButton.toggleTrigger();
await u.po.userButton.waitForPopover();
await u.page.getByRole('menuitem', { name: /Manage account/i }).click();
await u.page.getByRole('button', { name: /Manage account/i }).click();
await u.po.userProfile.waitForUserProfileModal();

// Verify custom pages and links are visible in the UserProfile
Expand Down
7 changes: 5 additions & 2 deletions packages/localizations/src/en-US.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1454,11 +1454,14 @@ export const enUS: LocalizationResource = {
},
userButton: {
action__addAccount: 'Add account',
action__closeUserMenu: 'Close user menu',
action__closeUserMenu: '{{name}} - Close account panel',
action__manageAccount: 'Manage account',
action__openUserMenu: 'Open user menu',
action__openUserMenu: '{{name}} - Open account panel',
action__signOut: 'Sign out',
action__signOutAll: 'Sign out of all accounts',
label__userButtonPopover: 'Account panel',
label__accountActions: 'Account actions',
label__activeSessions: 'Active sessions',
},
userProfile: {
apiKeysPage: {
Expand Down
7 changes: 5 additions & 2 deletions packages/shared/src/types/localization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -995,8 +995,11 @@ export type __internal_LocalizationResource = {
action__signOut: LocalizationValue;
action__signOutAll: LocalizationValue;
action__addAccount: LocalizationValue;
action__openUserMenu: LocalizationValue;
action__closeUserMenu: LocalizationValue;
action__openUserMenu: LocalizationValue<'name'>;
action__closeUserMenu: LocalizationValue<'name'>;
label__userButtonPopover?: LocalizationValue;
label__accountActions?: LocalizationValue;
label__activeSessions?: LocalizationValue;
};
organizationSwitcher: {
personalWorkspace: LocalizationValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ export const createUserButtonPageObject = (testArgs: { page: EnhancedPage }) =>
menuItems = [menuItems];
}
for (const menuItem of menuItems) {
await expect(page.getByRole('menuitem', { name: menuItem })).toBeVisible();
await expect(page.getByRole('button', { name: menuItem })).toBeVisible();
}
},
triggerSignOut: () => {
return page.getByRole('menuitem', { name: /Sign out$/i }).click();
return page.getByRole('button', { name: /Sign out$/i }).click();
},
triggerManageAccount: () => {
return page.getByRole('menuitem', { name: /Manage account/i }).click();
return page.getByRole('button', { name: /Manage account/i }).click();
},
switchAccount: (emailAddress: string) => {
return page.getByText(emailAddress).click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export const OrganizationListPageList = (props: { onCreateOrganizationClick: ()
<Card.Alert sx={t => ({ margin: `${t.space.$none} ${t.space.$5}` })}>{card.error}</Card.Alert>
<Col elementDescriptor={descriptors.main}>
<PreviewListItems>
<Actions role='menu'>
<Actions>
<PersonalAccountPreview />
{(userMemberships.count || 0) > 0 &&
userMemberships.data?.map(inv => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('OrganizationList', () => {
expect(queryByText('to continue to TestApp')).toBeInTheDocument();
//
expect(queryByText('Personal account')).toBeInTheDocument();
expect(queryByRole('menuitem', { name: 'Create organization' })).toBeInTheDocument();
expect(queryByRole('button', { name: 'Create organization' })).toBeInTheDocument();
});
});

Expand Down Expand Up @@ -78,7 +78,7 @@ describe('OrganizationList', () => {
// Display membership
expect(queryByText('Org1')).toBeInTheDocument();

expect(queryByRole('menuitem', { name: 'Create organization' })).toBeInTheDocument();
expect(queryByRole('button', { name: 'Create organization' })).toBeInTheDocument();
});
});

Expand Down Expand Up @@ -236,7 +236,7 @@ describe('OrganizationList', () => {
expect(queryByRole('button', { name: 'Upload' })).not.toBeInTheDocument();
expect(queryByLabelText(/name/i)).not.toBeInTheDocument();
expect(queryByLabelText(/slug/i)).not.toBeInTheDocument();
await userEvent.click(getByRole('menuitem', { name: 'Create organization' }));
await userEvent.click(getByRole('button', { name: 'Create organization' }));
// Header
expect(queryByRole('heading', { name: /Create organization/i })).toBeInTheDocument();
// Form fields of CreateOrganizationForm
Expand Down Expand Up @@ -266,9 +266,9 @@ describe('OrganizationList', () => {
},
);
await waitFor(async () =>
expect(await findByRole('menuitem', { name: 'Create organization' })).toBeInTheDocument(),
expect(await findByRole('button', { name: 'Create organization' })).toBeInTheDocument(),
);
await userEvent.click(getByRole('menuitem', { name: 'Create organization' }));
await userEvent.click(getByRole('button', { name: 'Create organization' }));
await waitFor(async () => expect(await findByLabelText(/name/i)).toBeInTheDocument());
await userEvent.type(getByLabelText(/name/i), 'new org');
await userEvent.click(getByRole('button', { name: /create organization/i }));
Expand All @@ -290,9 +290,9 @@ describe('OrganizationList', () => {
const { findByRole, getByRole, userEvent, queryByLabelText } = render(<OrganizationList />, { wrapper });

await waitFor(async () =>
expect(await findByRole('menuitem', { name: 'Create organization' })).toBeInTheDocument(),
expect(await findByRole('button', { name: 'Create organization' })).toBeInTheDocument(),
);
await userEvent.click(getByRole('menuitem', { name: 'Create organization' }));
await userEvent.click(getByRole('button', { name: 'Create organization' }));
expect(queryByLabelText(/Name/i)).toBeInTheDocument();
expect(queryByLabelText(/Slug/i)).not.toBeInTheDocument();
});
Expand All @@ -310,9 +310,9 @@ describe('OrganizationList', () => {
const { findByRole, getByRole, userEvent, queryByLabelText } = render(<OrganizationList />, { wrapper });

await waitFor(async () =>
expect(await findByRole('menuitem', { name: 'Create organization' })).toBeInTheDocument(),
expect(await findByRole('button', { name: 'Create organization' })).toBeInTheDocument(),
);
await userEvent.click(getByRole('menuitem', { name: 'Create organization' }));
await userEvent.click(getByRole('button', { name: 'Create organization' }));
expect(queryByLabelText(/Name/i)).toBeInTheDocument();
expect(queryByLabelText(/Slug/i)).toBeInTheDocument();
});
Expand Down Expand Up @@ -445,9 +445,9 @@ describe('OrganizationList', () => {

fixtures.clerk.setActive.mockReturnValue(Promise.resolve());
await waitFor(async () =>
expect(await findByRole('menuitem', { name: 'Create organization' })).toBeInTheDocument(),
expect(await findByRole('button', { name: 'Create organization' })).toBeInTheDocument(),
);
await userEvent.click(getByRole('menuitem', { name: 'Create organization' }));
await userEvent.click(getByRole('button', { name: 'Create organization' }));
await waitFor(async () => expect(await findByLabelText(/name/i)).toBeInTheDocument());
await userEvent.type(getByLabelText(/name/i), 'new org');
await userEvent.click(getByRole('button', { name: /create organization/i }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export const OrganizationSwitcherPopover = React.forwardRef<HTMLDivElement, Orga
padding: `${t.space.$4} ${t.space.$5}`,
})}
/>
<Actions role='menu'>{manageOrganizationButton}</Actions>
<Actions>{manageOrganizationButton}</Actions>
</Flex>
);

Expand All @@ -142,10 +142,7 @@ export const OrganizationSwitcherPopover = React.forwardRef<HTMLDivElement, Orga
{...rest}
>
<PopoverCard.Content elementDescriptor={descriptors.organizationSwitcherPopoverMain}>
<Actions
elementDescriptor={descriptors.organizationSwitcherPopoverActions}
role='menu'
>
<Actions elementDescriptor={descriptors.organizationSwitcherPopoverActions}>
{currentOrg
? selectedOrganizationPreview(currentOrg)
: !hidePersonal && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ const InvitationPreview = withCardStateProvider(
elementDescriptor={descriptors.organizationSwitcherPreviewButton}
icon={SwitchArrowRight}
onClick={acceptedOrganization ? () => onOrganizationClick(acceptedOrganization) : undefined}
role='menuitem'
>
<OrganizationPreview
elementId='organizationSwitcherListedOrganization'
Expand All @@ -205,12 +204,7 @@ const InvitationPreview = withCardStateProvider(

const SwitcherInvitationActions = (props: PropsOfComponent<typeof Flex> & { showBorder: boolean }) => {
const { showBorder, ...restProps } = props;
return (
<Actions
role='menu'
{...restProps}
/>
);
return <Actions {...restProps} />;
};

export const SuggestionPreview = withCardStateProvider((props: OrganizationSuggestionResource) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ export const UserMembershipList = (props: UserMembershipListProps) => {
elementId={descriptors.organizationSwitcherPreviewButton.setId('personal')}
icon={SwitchArrowRight}
onClick={onPersonalWorkspaceClick}
role='menuitem'
>
<PersonalWorkspacePreview
user={userWithoutIdentifiers}
Expand All @@ -99,7 +98,6 @@ export const UserMembershipList = (props: UserMembershipListProps) => {
elementId={descriptors.organizationSwitcherPreviewButton.setId('organization')}
icon={SwitchArrowRight}
onClick={() => onOrganizationClick(organization)}
role='menuitem'
sx={t => ({
border: `0 solid ${t.colors.$borderAlpha100}`,
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ describe('OrganizationSwitcher', () => {
props.setProps({ hidePersonal: true });
const { getByRole, userEvent } = render(<OrganizationSwitcher />, { wrapper });
await userEvent.click(getByRole('button'));
await userEvent.click(getByRole('menuitem'));
await userEvent.click(getByRole('button', { name: /manage/i }));
expect(fixtures.clerk.openOrganizationProfile).toHaveBeenCalled();
});

Expand All @@ -375,7 +375,7 @@ describe('OrganizationSwitcher', () => {
props.setProps({ hidePersonal: true });
const { getByRole, userEvent } = render(<OrganizationSwitcher />, { wrapper });
await userEvent.click(getByRole('button', { name: 'Open organization switcher' }));
await userEvent.click(getByRole('menuitem', { name: 'Create organization' }));
await userEvent.click(getByRole('button', { name: 'Create organization' }));
expect(fixtures.clerk.openCreateOrganization).toHaveBeenCalled();
});

Expand All @@ -391,7 +391,7 @@ describe('OrganizationSwitcher', () => {

const { getByRole, queryByLabelText, userEvent } = render(<OrganizationSwitcher />, { wrapper });
await userEvent.click(getByRole('button', { name: 'Open organization switcher' }));
await userEvent.click(getByRole('menuitem', { name: 'Create organization' }));
await userEvent.click(getByRole('button', { name: 'Create organization' }));
expect(fixtures.clerk.openCreateOrganization).toHaveBeenCalled();
expect(queryByLabelText(/Slug/i)).not.toBeInTheDocument();
});
Expand Down Expand Up @@ -619,7 +619,7 @@ describe('OrganizationSwitcher', () => {
);

await userEvent.click(getByRole('button'));
const manageButton = await waitFor(() => screen.getByRole('menuitem', { name: /manage/i }));
const manageButton = await waitFor(() => screen.getByRole('button', { name: /manage/i }));
await userEvent.click(manageButton);

expect(fixtures.clerk.openOrganizationProfile).toHaveBeenCalledWith(expect.objectContaining({ getContainer }));
Expand Down Expand Up @@ -649,7 +649,7 @@ describe('OrganizationSwitcher', () => {
);

await userEvent.click(getByRole('button', { name: 'Open organization switcher' }));
const createButton = await waitFor(() => screen.getByRole('menuitem', { name: 'Create organization' }));
const createButton = await waitFor(() => screen.getByRole('button', { name: 'Create organization' }));
await userEvent.click(createButton);

expect(fixtures.clerk.openCreateOrganization).toHaveBeenCalledWith(expect.objectContaining({ getContainer }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const ChooseOrganizationScreen = (props: ChooseOrganizationScreenProps) =
<Card.Alert sx={t => ({ margin: `${t.space.$none} ${t.space.$8}` })}>{card.error}</Card.Alert>
<Col elementDescriptor={descriptors.main}>
<OrganizationPreviewListItems elementDescriptor={descriptors.taskChooseOrganizationPreviewItems}>
<Actions role='menu'>
<Actions>
{(userMemberships.count || 0) > 0 &&
userMemberships.data?.map(inv => {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export const SetupMfaStartScreen = withCardStateProvider((props: SetupMfaStartSc
</Flex>
)}
<Actions
role='menu'
elementDescriptor={descriptors.taskSetupMfaMethodSelectionItems}
sx={t => ({
borderTopWidth: t.borderWidths.$normal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ const SmsCodeScreen = withCardStateProvider((props: SmsCodeScreenProps) => {
)}
<Col>
<Actions
role='menu'
elementDescriptor={descriptors.taskSetupMfaPhoneSelectionItems}
sx={t => ({
borderTopWidth: t.borderWidths.$normal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ describe('TaskSetupMFA', () => {
await findByText(/add sms code verification/i);
await findByText(/choose phone number you want to use/i);
await findByText(/\+30 691 1111111/i);
expect(getByRole('menuitem', { name: /add phone number/i })).toBeInTheDocument();
expect(getByRole('button', { name: /add phone number/i })).toBeInTheDocument();
});

it('should show add phone screen when no existing phone numbers', async () => {
Expand Down Expand Up @@ -511,7 +511,7 @@ describe('TaskSetupMFA', () => {
await findByText(/add sms code verification/i);

await act(async () => {
await userEvent.click(getByRole('menuitem', { name: /add phone number/i }));
await userEvent.click(getByRole('button', { name: /add phone number/i }));
});

await findByText(/add phone number/i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const SignInAccountSwitcherInternal = () => {
borderTopColor: t.colors.$borderAlpha100,
})}
>
<Actions role='menu'>
<Actions>
{signedInSessions.map(s => (
<PreviewButton
key={s.id}
Expand Down
Loading
Loading