fix: improve members role chip accessibility (#38037)

This commit is contained in:
yyh 2026-06-27 02:40:03 +08:00 committed by GitHub
parent 446b3962c1
commit 17bee5fb32
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 126 additions and 160 deletions

View File

@ -1,7 +1,7 @@
import type { AppContextValue } from '@/context/app-context' import type { AppContextValue } from '@/context/app-context'
import type { Role } from '@/models/access-control' import type { Role } from '@/models/access-control'
import type { ICurrentWorkspace, Member } from '@/models/common' import type { ICurrentWorkspace, Member } from '@/models/common'
import { screen } from '@testing-library/react' import { screen, within } from '@testing-library/react'
import userEvent from '@testing-library/user-event' import userEvent from '@testing-library/user-event'
import { vi } from 'vitest' import { vi } from 'vitest'
import { createMockProviderContextValue } from '@/__mocks__/provider-context' import { createMockProviderContextValue } from '@/__mocks__/provider-context'
@ -24,6 +24,10 @@ const renderMembersPage = () => renderWithSystemFeatures(<MembersPage />, {
systemFeatures: { is_email_setup: true }, systemFeatures: { is_email_setup: true },
}) })
const getMemberDetailsButton = (memberId: string) => within(screen.getByTestId(`member-row-${memberId}`)).getByRole('button', {
name: /members\.memberDetails\.openAria/i,
})
const createRole = (overrides: Partial<Role>): Role => ({ const createRole = (overrides: Partial<Role>): Role => ({
id: 'role-1', id: 'role-1',
tenant_id: 'tenant-1', tenant_id: 'tenant-1',
@ -220,8 +224,8 @@ describe('MembersPage', () => {
expect(screen.getByText('common.members.name', { selector: '.system-xs-medium-uppercase' }))!.toHaveClass('w-65', 'shrink-0') expect(screen.getByText('common.members.name', { selector: '.system-xs-medium-uppercase' }))!.toHaveClass('w-65', 'shrink-0')
expect(screen.getByText('common.members.role', { selector: '.system-xs-medium-uppercase' }))!.toHaveClass('min-w-0', 'grow') expect(screen.getByText('common.members.role', { selector: '.system-xs-medium-uppercase' }))!.toHaveClass('min-w-0', 'grow')
expect(screen.getByTestId('member-row-1').children[0])!.toHaveClass('w-65', 'shrink-0') expect(getMemberDetailsButton('1').children[0])!.toHaveClass('w-65', 'shrink-0')
expect(screen.getByTestId('member-row-1').children[2])!.toHaveClass('min-w-0', 'grow') expect(getMemberDetailsButton('1').children[2])!.toHaveClass('min-w-0', 'grow')
}) })
it('should render plural roles column header when RBAC is enabled', () => { it('should render plural roles column header when RBAC is enabled', () => {
@ -495,12 +499,26 @@ describe('MembersPage', () => {
expect(screen.getByText('Admin'))!.toBeInTheDocument() expect(screen.getByText('Admin'))!.toBeInTheDocument()
}) })
it('should expose member details as a native row button without nesting member actions', () => {
renderMembersPage()
const row = screen.getByTestId('member-row-2')
const detailsButton = getMemberDetailsButton('2')
const memberMenu = within(row).getByTestId('member-menu')
expect(row).not.toHaveAttribute('role', 'button')
expect(row).not.toHaveClass('hover:bg-state-base-hover')
expect(detailsButton).toHaveAttribute('type', 'button')
expect(detailsButton).toHaveClass('hover:bg-state-base-hover', 'focus-visible:bg-state-base-hover')
expect(detailsButton).not.toContainElement(memberMenu)
})
it('should open member details modal when a member row is clicked', async () => { it('should open member details modal when a member row is clicked', async () => {
const user = userEvent.setup() const user = userEvent.setup()
renderMembersPage() renderMembersPage()
await user.click(screen.getByTestId('member-row-2')) await user.click(getMemberDetailsButton('2'))
expect(screen.getByText('Member Details Modal'))!.toBeInTheDocument() expect(screen.getByText('Member Details Modal'))!.toBeInTheDocument()
expect(screen.getByTestId('details-member-name'))!.toHaveTextContent('Admin User') expect(screen.getByTestId('details-member-name'))!.toHaveTextContent('Admin User')
@ -514,8 +532,8 @@ describe('MembersPage', () => {
renderMembersPage() renderMembersPage()
const row = screen.getByTestId('member-row-2') const detailsButton = getMemberDetailsButton('2')
row.focus() detailsButton.focus()
await user.keyboard('{Enter}') await user.keyboard('{Enter}')
expect(screen.getByText('Member Details Modal'))!.toBeInTheDocument() expect(screen.getByText('Member Details Modal'))!.toBeInTheDocument()
@ -526,7 +544,7 @@ describe('MembersPage', () => {
renderMembersPage() renderMembersPage()
await user.click(screen.getByTestId('member-row-1')) await user.click(getMemberDetailsButton('1'))
expect(screen.getByTestId('details-can-assign'))!.toHaveTextContent('false') expect(screen.getByTestId('details-can-assign'))!.toHaveTextContent('false')
}) })
@ -543,7 +561,7 @@ describe('MembersPage', () => {
renderMembersPage() renderMembersPage()
await user.click(screen.getByTestId('member-row-2')) await user.click(getMemberDetailsButton('2'))
expect(screen.getByTestId('details-can-assign'))!.toHaveTextContent('false') expect(screen.getByTestId('details-can-assign'))!.toHaveTextContent('false')
}) })
@ -553,7 +571,7 @@ describe('MembersPage', () => {
renderMembersPage() renderMembersPage()
await user.click(screen.getByTestId('member-row-2')) await user.click(getMemberDetailsButton('2'))
await user.click(screen.getByRole('button', { name: 'Submit Member Roles' })) await user.click(screen.getByRole('button', { name: 'Submit Member Roles' }))
expect(mockUpdateRolesOfMember).toHaveBeenCalledWith({ expect(mockUpdateRolesOfMember).toHaveBeenCalledWith({
@ -575,7 +593,7 @@ describe('MembersPage', () => {
}, },
}) })
await user.click(screen.getByTestId('member-row-2')) await user.click(getMemberDetailsButton('2'))
await user.click(screen.getByRole('button', { name: 'Submit Member Roles' })) await user.click(screen.getByRole('button', { name: 'Submit Member Roles' }))
expect(mockUpdateRolesOfMember).toHaveBeenCalledWith({ expect(mockUpdateRolesOfMember).toHaveBeenCalledWith({

View File

@ -145,11 +145,11 @@ describe('MemberDetailsModal', () => {
/>, />,
) )
expect(screen.queryByRole('button', { name: /Custom role/i })).not.toBeInTheDocument() expect(screen.getByRole('button', { name: /^Custom role$/i })).toBeInTheDocument()
await user.click(screen.getByText('Custom role')) await user.click(screen.getByRole('button', { name: /^Custom role$/i }))
expect(screen.queryByRole('menuitem', { name: /common\.operation\.remove/i })).not.toBeInTheDocument() expect(screen.queryByRole('button', { name: /common\.operation\.remove/i })).not.toBeInTheDocument()
}) })
it('should not show role removal controls when role assignment is not allowed', () => { it('should not show role removal controls when role assignment is not allowed', () => {
@ -189,9 +189,7 @@ describe('MemberDetailsModal', () => {
/>, />,
) )
await user.click(screen.getByRole('button', { name: /Custom role/i })) await user.click(screen.getByRole('button', { name: /common\.operation\.remove.*Custom role/i }))
await user.click(screen.getByRole('menuitem', { name: /common\.operation\.remove/i }))
expect(handleAssignSubmit).not.toHaveBeenCalled() expect(handleAssignSubmit).not.toHaveBeenCalled()

View File

@ -2,17 +2,11 @@
import { cn } from '@langgenius/dify-ui/cn' import { cn } from '@langgenius/dify-ui/cn'
import { import {
DropdownMenu, Popover,
DropdownMenuContent, PopoverContent,
DropdownMenuItem, PopoverTrigger,
DropdownMenuTrigger, } from '@langgenius/dify-ui/popover'
} from '@langgenius/dify-ui/dropdown-menu' import { memo } from 'react'
import {
PreviewCard,
PreviewCardContent,
PreviewCardTrigger,
} from '@langgenius/dify-ui/preview-card'
import { memo, useState } from 'react'
import { Trans, useTranslation } from 'react-i18next' import { Trans, useTranslation } from 'react-i18next'
type PermissionRoleChipProps = { type PermissionRoleChipProps = {
@ -33,9 +27,8 @@ const PermissionRoleChip = ({
className, className,
}: PermissionRoleChipProps) => { }: PermissionRoleChipProps) => {
const { t } = useTranslation() const { t } = useTranslation()
const [open, setOpen] = useState(false)
const permissions = permissionKeys const permissions = permissionKeys
const canOpenMenu = !isOwner && !!onRemove const canRemoveRole = !isOwner && !!onRemove
const permissionLabels = permissions const permissionLabels = permissions
.map(key => t(key, { .map(key => t(key, {
ns: 'permissionKeys', ns: 'permissionKeys',
@ -44,42 +37,42 @@ const PermissionRoleChip = ({
.join(', ') .join(', ')
const hasPermissionLabels = permissionLabels.length > 0 const hasPermissionLabels = permissionLabels.length > 0
const chipClassName = cn( const chipRootClassName = cn(
'inline-flex h-6 max-w-full items-center rounded-full border-[0.5px] border-components-panel-border-subtle bg-background-body p-1 system-xs-regular text-text-primary shadow-xs transition-colors outline-none', 'inline-flex h-6 max-w-full min-w-0 items-center gap-1 rounded-full border-[0.5px] border-components-panel-border-subtle bg-background-body px-1.5 py-0.5 system-xs-medium text-text-primary shadow-xs transition-colors',
canOpenMenu && 'cursor-pointer hover:bg-background-section-burn focus-visible:ring-1 focus-visible:ring-components-input-border-active focus-visible:outline-hidden', 'hover:bg-background-section-burn has-[[data-popup-open]]:bg-background-section-burn',
open && 'bg-background-section-burn', 'has-[:focus-visible]:ring-2 has-[:focus-visible]:ring-state-accent-solid',
className, className,
) )
const chipContent = ( const removeLabel = `${t('operation.remove', { ns: 'common' })} ${label}`
<span className="min-w-0 truncate px-1 leading-4">{label}</span>
)
const chip = canOpenMenu const chip = (
? ( <span className={chipRootClassName}>
<PopoverTrigger
openOnHover
delay={300}
closeDelay={200}
render={(
<button
type="button"
className="min-w-0 truncate rounded-sm border-none bg-transparent p-0 text-start leading-4 outline-hidden"
>
{label}
</button>
)}
/>
{canRemoveRole && (
<button <button
type="button" type="button"
className={chipClassName} aria-label={removeLabel}
aria-label={t('members.memberDetails.roleActionsAria', { className="flex size-3.5 shrink-0 items-center justify-center rounded-full border-none bg-transparent p-0 text-text-tertiary outline-hidden hover:bg-state-base-hover-alt hover:text-text-secondary focus-visible:bg-state-base-hover-alt focus-visible:text-text-secondary"
ns: 'common', onClick={() => onRemove?.(roleId)}
role: label,
defaultValue: 'Open actions for {{role}} role',
})}
data-testid="permission-role-chip"
data-role-id={roleId}
> >
{chipContent} <span aria-hidden className="i-ri-close-line size-3" />
</button> </button>
) )}
: ( </span>
<span )
className={chipClassName}
data-testid="permission-role-chip"
data-role-id={roleId}
>
{chipContent}
</span>
)
const permissionSummary = ( const permissionSummary = (
<div className="flex w-58 flex-col gap-1 px-4 py-3.5"> <div className="flex w-58 flex-col gap-1 px-4 py-3.5">
@ -109,53 +102,17 @@ const PermissionRoleChip = ({
</div> </div>
) )
const chipWithPermissions = (
<PreviewCard>
<PreviewCardTrigger render={chip} />
<PreviewCardContent
placement="bottom-start"
sideOffset={8}
popupClassName="overflow-hidden border-components-panel-border bg-components-tooltip-bg p-0 shadow-lg backdrop-blur-[5px]"
>
{permissionSummary}
</PreviewCardContent>
</PreviewCard>
)
if (!canOpenMenu)
return chipWithPermissions
const menuTrigger = (
<PreviewCard>
<DropdownMenuTrigger render={<PreviewCardTrigger render={chip} />} />
<PreviewCardContent
placement="bottom-start"
sideOffset={8}
popupClassName="overflow-hidden border-components-panel-border bg-components-tooltip-bg p-0 shadow-lg backdrop-blur-[5px]"
>
{permissionSummary}
</PreviewCardContent>
</PreviewCard>
)
return ( return (
<DropdownMenu open={open} onOpenChange={setOpen}> <Popover>
{menuTrigger} {chip}
<DropdownMenuContent <PopoverContent
placement="bottom-start" placement="bottom-start"
sideOffset={4} sideOffset={8}
popupClassName="w-[236px] rounded-xl p-1" popupClassName="overflow-hidden border-components-panel-border bg-components-tooltip-bg p-0 shadow-lg backdrop-blur-[5px]"
> >
<DropdownMenuItem {permissionSummary}
variant="destructive" </PopoverContent>
className="h-8 gap-2 rounded-lg px-2 py-1 system-sm-regular" </Popover>
onClick={() => onRemove?.(roleId)}
>
<span aria-hidden className="i-ri-delete-bin-line size-4 shrink-0" />
{t('operation.remove', { ns: 'common' })}
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
) )
} }

View File

@ -10,7 +10,6 @@ import {
AlertDialogDescription, AlertDialogDescription,
AlertDialogTitle, AlertDialogTitle,
} from '@langgenius/dify-ui/alert-dialog' } from '@langgenius/dify-ui/alert-dialog'
import { cn } from '@langgenius/dify-ui/cn'
import { import {
DropdownMenu, DropdownMenu,
DropdownMenuContent, DropdownMenuContent,
@ -108,26 +107,17 @@ const MemberMenu = ({
onTransferOwnership?.() onTransferOwnership?.()
}, [onTransferOwnership]) }, [onTransferOwnership])
const stopPropagationOnClick = useCallback((e: React.MouseEvent) => {
e.stopPropagation()
}, [])
const stopPropagationOnKeyDown = useCallback((e: React.KeyboardEvent<HTMLDivElement>) => {
if (e.key === 'Enter' || e.key === ' ')
e.stopPropagation()
}, [])
if (!canAssignRoles && !canRemove && !showTransferOwnership) if (!canAssignRoles && !canRemove && !showTransferOwnership)
return null return null
return ( return (
<div role="presentation" onClick={stopPropagationOnClick} onKeyDown={stopPropagationOnKeyDown}> <div role="presentation">
<DropdownMenu open={open} onOpenChange={setOpen}> <DropdownMenu open={open} onOpenChange={setOpen}>
<DropdownMenuTrigger <DropdownMenuTrigger
render={( render={(
<ActionButton <ActionButton
size="l" size="l"
className={cn(open && 'bg-state-base-hover')} className="data-popup-open:bg-state-base-hover"
aria-label={t('members.memberActions', { ns: 'common', defaultValue: 'Member actions' })} aria-label={t('members.memberActions', { ns: 'common', defaultValue: 'Member actions' })}
/> />
)} )}

View File

@ -1,7 +1,7 @@
'use client' 'use client'
import type { KeyboardEvent } from 'react'
import type { Member } from '@/models/common' import type { Member } from '@/models/common'
import { Avatar } from '@langgenius/dify-ui/avatar' import { Avatar } from '@langgenius/dify-ui/avatar'
import { cn } from '@langgenius/dify-ui/cn'
import { memo, useCallback } from 'react' import { memo, useCallback } from 'react'
import { useTranslation } from 'react-i18next' import { useTranslation } from 'react-i18next'
import { useFormatTimeFromNow } from '@/hooks/use-format-time-from-now' import { useFormatTimeFromNow } from '@/hooks/use-format-time-from-now'
@ -41,57 +41,60 @@ const MemberRow = ({
onOpenDetails(member) onOpenDetails(member)
}, [member, onOpenDetails]) }, [member, onOpenDetails])
const handleRowKeyDown = useCallback((e: KeyboardEvent<HTMLDivElement>) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault()
openDetails()
}
}, [openDetails])
return ( return (
<div <div
role="button"
tabIndex={0}
data-testid={`member-row-${member.id}`} data-testid={`member-row-${member.id}`}
aria-label={t('members.memberDetails.openAria', { className="relative border-b border-divider-subtle"
ns: 'common',
name: member.name,
defaultValue: 'Open member details for {{name}}',
})}
className="flex cursor-pointer border-b border-divider-subtle hover:bg-state-base-hover focus-visible:bg-state-base-hover focus-visible:outline-hidden"
onClick={openDetails}
onKeyDown={handleRowKeyDown}
> >
<div className="flex w-65 shrink-0 items-center px-3 py-2"> <button
<Avatar avatar={member.avatar_url} size="sm" className="mr-2" name={member.name} /> type="button"
<div className=""> aria-label={t('members.memberDetails.openAria', {
<div className="system-sm-medium text-text-secondary"> ns: 'common',
{member.name} name: member.name,
{member.status === 'pending' && ( defaultValue: 'Open member details for {{name}}',
<span className="ml-1 system-xs-medium text-text-warning"> })}
{t('members.pending', { ns: 'common' })} className={cn(
</span> 'flex w-full min-w-0 cursor-pointer bg-transparent text-left hover:bg-state-base-hover focus-visible:bg-state-base-hover focus-visible:outline-hidden',
)} canManage && 'pr-12',
{isCurrentUser && ( )}
<span className="system-xs-regular text-text-tertiary"> onClick={openDetails}
{t('members.you', { ns: 'common' })} >
</span> <span className="flex w-65 shrink-0 items-center px-3 py-2">
)} <Avatar avatar={member.avatar_url} size="sm" className="mr-2" name={member.name} />
</div> <span className="min-w-0">
<div className="system-xs-regular text-text-tertiary">{member.email}</div> <span className="block system-sm-medium text-text-secondary">
</div> {member.name}
</div> {member.status === 'pending' && (
<div className="flex w-30 shrink-0 items-center py-2 system-sm-regular text-text-secondary"> <span className="ml-1 system-xs-medium text-text-warning">
{formatTimeFromNow(Number((member.last_active_at || member.created_at)) * 1000)} {t('members.pending', { ns: 'common' })}
</div> </span>
)}
{isCurrentUser && (
<span className="system-xs-regular text-text-tertiary">
{t('members.you', { ns: 'common' })}
</span>
)}
</span>
<span className="block system-xs-regular text-text-tertiary">{member.email}</span>
</span>
</span>
<span className="flex w-30 shrink-0 items-center py-2 system-sm-regular text-text-secondary">
{formatTimeFromNow(Number((member.last_active_at || member.created_at)) * 1000)}
</span>
<span
className="flex min-w-0 grow items-center gap-2 px-3"
role="presentation"
>
<RoleBadges
className="grow"
roleNames={roleNames}
/>
</span>
</button>
<div <div
className="flex min-w-0 grow items-center gap-2 px-3" className="absolute inset-y-0 right-0 flex items-center px-3"
role="presentation" role="presentation"
> >
<RoleBadges
className="grow"
roleNames={roleNames}
/>
{canManage && ( {canManage && (
<MemberMenu <MemberMenu
member={member} member={member}

View File

@ -33,7 +33,7 @@ const RoleBadges = ({ roleNames, max = 2, className }: RoleBadgesProps) => {
const overflow = roleNames.slice(max) const overflow = roleNames.slice(max)
return ( return (
<div className={cn('flex min-w-0 items-center gap-1', className)}> <span className={cn('flex min-w-0 items-center gap-1', className)}>
{visible.map(role => ( {visible.map(role => (
<RoleBadge key={role} label={role} /> <RoleBadge key={role} label={role} />
))} ))}
@ -44,7 +44,7 @@ const RoleBadges = ({ roleNames, max = 2, className }: RoleBadgesProps) => {
{`+${overflow.length}`} {`+${overflow.length}`}
</span> </span>
)} )}
</div> </span>
) )
} }