diff --git a/web/app/components/header/account-setting/members-page/__tests__/index.spec.tsx b/web/app/components/header/account-setting/members-page/__tests__/index.spec.tsx index e16ba348259..29cae8e0a68 100644 --- a/web/app/components/header/account-setting/members-page/__tests__/index.spec.tsx +++ b/web/app/components/header/account-setting/members-page/__tests__/index.spec.tsx @@ -1,7 +1,7 @@ import type { AppContextValue } from '@/context/app-context' import type { Role } from '@/models/access-control' 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 { vi } from 'vitest' import { createMockProviderContextValue } from '@/__mocks__/provider-context' @@ -24,6 +24,10 @@ const renderMembersPage = () => renderWithSystemFeatures(, { 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 => ({ id: 'role-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.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(screen.getByTestId('member-row-1').children[2])!.toHaveClass('min-w-0', 'grow') + expect(getMemberDetailsButton('1').children[0])!.toHaveClass('w-65', 'shrink-0') + expect(getMemberDetailsButton('1').children[2])!.toHaveClass('min-w-0', 'grow') }) it('should render plural roles column header when RBAC is enabled', () => { @@ -495,12 +499,26 @@ describe('MembersPage', () => { 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 () => { const user = userEvent.setup() renderMembersPage() - await user.click(screen.getByTestId('member-row-2')) + await user.click(getMemberDetailsButton('2')) expect(screen.getByText('Member Details Modal'))!.toBeInTheDocument() expect(screen.getByTestId('details-member-name'))!.toHaveTextContent('Admin User') @@ -514,8 +532,8 @@ describe('MembersPage', () => { renderMembersPage() - const row = screen.getByTestId('member-row-2') - row.focus() + const detailsButton = getMemberDetailsButton('2') + detailsButton.focus() await user.keyboard('{Enter}') expect(screen.getByText('Member Details Modal'))!.toBeInTheDocument() @@ -526,7 +544,7 @@ describe('MembersPage', () => { renderMembersPage() - await user.click(screen.getByTestId('member-row-1')) + await user.click(getMemberDetailsButton('1')) expect(screen.getByTestId('details-can-assign'))!.toHaveTextContent('false') }) @@ -543,7 +561,7 @@ describe('MembersPage', () => { renderMembersPage() - await user.click(screen.getByTestId('member-row-2')) + await user.click(getMemberDetailsButton('2')) expect(screen.getByTestId('details-can-assign'))!.toHaveTextContent('false') }) @@ -553,7 +571,7 @@ describe('MembersPage', () => { renderMembersPage() - await user.click(screen.getByTestId('member-row-2')) + await user.click(getMemberDetailsButton('2')) await user.click(screen.getByRole('button', { name: 'Submit Member Roles' })) 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' })) expect(mockUpdateRolesOfMember).toHaveBeenCalledWith({ diff --git a/web/app/components/header/account-setting/members-page/member-details-modal/__tests__/index.spec.tsx b/web/app/components/header/account-setting/members-page/member-details-modal/__tests__/index.spec.tsx index c87e3f60e91..71acdd68e3e 100644 --- a/web/app/components/header/account-setting/members-page/member-details-modal/__tests__/index.spec.tsx +++ b/web/app/components/header/account-setting/members-page/member-details-modal/__tests__/index.spec.tsx @@ -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', () => { @@ -189,9 +189,7 @@ describe('MemberDetailsModal', () => { />, ) - await user.click(screen.getByRole('button', { name: /Custom role/i })) - - await user.click(screen.getByRole('menuitem', { name: /common\.operation\.remove/i })) + await user.click(screen.getByRole('button', { name: /common\.operation\.remove.*Custom role/i })) expect(handleAssignSubmit).not.toHaveBeenCalled() diff --git a/web/app/components/header/account-setting/members-page/member-details-modal/permission-role-chip.tsx b/web/app/components/header/account-setting/members-page/member-details-modal/permission-role-chip.tsx index e9456c18f66..a634d0a381b 100644 --- a/web/app/components/header/account-setting/members-page/member-details-modal/permission-role-chip.tsx +++ b/web/app/components/header/account-setting/members-page/member-details-modal/permission-role-chip.tsx @@ -2,17 +2,11 @@ import { cn } from '@langgenius/dify-ui/cn' import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from '@langgenius/dify-ui/dropdown-menu' -import { - PreviewCard, - PreviewCardContent, - PreviewCardTrigger, -} from '@langgenius/dify-ui/preview-card' -import { memo, useState } from 'react' + Popover, + PopoverContent, + PopoverTrigger, +} from '@langgenius/dify-ui/popover' +import { memo } from 'react' import { Trans, useTranslation } from 'react-i18next' type PermissionRoleChipProps = { @@ -33,9 +27,8 @@ const PermissionRoleChip = ({ className, }: PermissionRoleChipProps) => { const { t } = useTranslation() - const [open, setOpen] = useState(false) const permissions = permissionKeys - const canOpenMenu = !isOwner && !!onRemove + const canRemoveRole = !isOwner && !!onRemove const permissionLabels = permissions .map(key => t(key, { ns: 'permissionKeys', @@ -44,42 +37,42 @@ const PermissionRoleChip = ({ .join(', ') const hasPermissionLabels = permissionLabels.length > 0 - const chipClassName = 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', - canOpenMenu && 'cursor-pointer hover:bg-background-section-burn focus-visible:ring-1 focus-visible:ring-components-input-border-active focus-visible:outline-hidden', - open && 'bg-background-section-burn', + const chipRootClassName = cn( + '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', + 'hover:bg-background-section-burn has-[[data-popup-open]]:bg-background-section-burn', + 'has-[:focus-visible]:ring-2 has-[:focus-visible]:ring-state-accent-solid', className, ) - const chipContent = ( - {label} - ) + const removeLabel = `${t('operation.remove', { ns: 'common' })} ${label}` - const chip = canOpenMenu - ? ( + const chip = ( + + + {label} + + )} + /> + {canRemoveRole && ( onRemove?.(roleId)} > - {chipContent} + - ) - : ( - - {chipContent} - - ) + )} + + ) const permissionSummary = ( @@ -109,53 +102,17 @@ const PermissionRoleChip = ({ ) - const chipWithPermissions = ( - - - - {permissionSummary} - - - ) - - if (!canOpenMenu) - return chipWithPermissions - - const menuTrigger = ( - - } /> - - {permissionSummary} - - - ) - return ( - - {menuTrigger} - + {chip} + - onRemove?.(roleId)} - > - - {t('operation.remove', { ns: 'common' })} - - - + {permissionSummary} + + ) } diff --git a/web/app/components/header/account-setting/members-page/member-menu.tsx b/web/app/components/header/account-setting/members-page/member-menu.tsx index 133d16c7d5c..e5757489337 100644 --- a/web/app/components/header/account-setting/members-page/member-menu.tsx +++ b/web/app/components/header/account-setting/members-page/member-menu.tsx @@ -10,7 +10,6 @@ import { AlertDialogDescription, AlertDialogTitle, } from '@langgenius/dify-ui/alert-dialog' -import { cn } from '@langgenius/dify-ui/cn' import { DropdownMenu, DropdownMenuContent, @@ -108,26 +107,17 @@ const MemberMenu = ({ onTransferOwnership?.() }, [onTransferOwnership]) - const stopPropagationOnClick = useCallback((e: React.MouseEvent) => { - e.stopPropagation() - }, []) - - const stopPropagationOnKeyDown = useCallback((e: React.KeyboardEvent) => { - if (e.key === 'Enter' || e.key === ' ') - e.stopPropagation() - }, []) - if (!canAssignRoles && !canRemove && !showTransferOwnership) return null return ( - + )} diff --git a/web/app/components/header/account-setting/members-page/member-row.tsx b/web/app/components/header/account-setting/members-page/member-row.tsx index 2886a45700e..d7047eb2929 100644 --- a/web/app/components/header/account-setting/members-page/member-row.tsx +++ b/web/app/components/header/account-setting/members-page/member-row.tsx @@ -1,7 +1,7 @@ 'use client' -import type { KeyboardEvent } from 'react' import type { Member } from '@/models/common' import { Avatar } from '@langgenius/dify-ui/avatar' +import { cn } from '@langgenius/dify-ui/cn' import { memo, useCallback } from 'react' import { useTranslation } from 'react-i18next' import { useFormatTimeFromNow } from '@/hooks/use-format-time-from-now' @@ -41,57 +41,60 @@ const MemberRow = ({ onOpenDetails(member) }, [member, onOpenDetails]) - const handleRowKeyDown = useCallback((e: KeyboardEvent) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - openDetails() - } - }, [openDetails]) - return ( - - - - - {member.name} - {member.status === 'pending' && ( - - {t('members.pending', { ns: 'common' })} - - )} - {isCurrentUser && ( - - {t('members.you', { ns: 'common' })} - - )} - - {member.email} - - - - {formatTimeFromNow(Number((member.last_active_at || member.created_at)) * 1000)} - + + + + + + {member.name} + {member.status === 'pending' && ( + + {t('members.pending', { ns: 'common' })} + + )} + {isCurrentUser && ( + + {t('members.you', { ns: 'common' })} + + )} + + {member.email} + + + + {formatTimeFromNow(Number((member.last_active_at || member.created_at)) * 1000)} + + + + + - {canManage && ( { const overflow = roleNames.slice(max) return ( - + {visible.map(role => ( ))} @@ -44,7 +44,7 @@ const RoleBadges = ({ roleNames, max = 2, className }: RoleBadgesProps) => { {`+${overflow.length}`} )} - + ) }