diff --git a/e2e/access.spec.ts b/e2e/access.spec.ts index 74ee7fb0..a46218da 100644 --- a/e2e/access.spec.ts +++ b/e2e/access.spec.ts @@ -12,7 +12,7 @@ import { searchField, uniqueName, } from './access'; -import { AUDITED_WRITES_SKIP_REASON, LIVE, MOCK_FIXTURES_SKIP_REASON } from './live'; +import { AUDITED_WRITES_SKIP_REASON, LIVE, MOCK_FIXTURES_SKIP_REASON, THEMES } from './live'; const WCAG_TAGS = ['wcag2a', 'wcag2aa', 'wcag21a', 'wcag21aa']; @@ -136,6 +136,167 @@ test.describe('Access page accessibility', () => { }); }); +/** The open dialog (Edit role, Edit group, Delete role). */ +const dialogOf = (page: Page) => page.getByRole('dialog'); + +/** axe on the open dialog only: the page behind it is inert while it is open. */ +async function scanDialog(page: Page) { + return new AxeBuilder({ page }).withTags(WCAG_TAGS).include('[role="dialog"]').analyze(); +} + +/** Opens a role's Edit role dialog from its row. */ +async function openEditRole(page: Page, role: string) { + await page.goto('/access?tab=roles'); + await page.waitForLoadState('networkidle'); + await searchField(page).fill(role); + await page.getByText(role, { exact: true }).click(); + await expect(dialogOf(page)).toBeVisible(); + return dialogOf(page); +} + +for (const theme of THEMES) { + test.describe(`Access page Edit role dialog accessibility: ${theme}`, () => { + test.use({ colorScheme: theme }); + + for (const tab of ['Details', 'Permissions', 'Members']) { + test(`has zero WCAG 2.1 AA violations on the ${tab} tab`, async ({ page }) => { + const dialog = await openEditRole(page, 'ADMIN'); + await dialog.getByRole('tab', { name: tab }).click(); + await expect(dialog.getByRole('tab', { name: tab })).toHaveAttribute( + 'data-state', + 'active', + ); + await expect(dialog.getByText('Loading…').filter({ visible: true })).toHaveCount(0); + + expect((await scanDialog(page)).violations).toEqual([]); + }); + } + }); +} + +test.describe('Access page Edit role dialog reflow', () => { + for (const width of [390, 320]) { + test(`keeps the Permissions tab's feature grid inside the dialog at ${width}px`, async ({ + page, + }) => { + await page.setViewportSize({ width, height: 844 }); + const dialog = await openEditRole(page, 'ADMIN'); + await dialog.getByRole('tab', { name: 'Permissions' }).click(); + const features = dialog.getByText('Allowed features', { exact: true }); + await expect(features).toBeVisible(); + const grid = features.locator('xpath=following-sibling::div[1]'); + const [gridBox, dialogBox] = await Promise.all([grid.boundingBox(), dialog.boundingBox()]); + expect(gridBox && dialogBox).toBeTruthy(); + expect(gridBox!.x + gridBox!.width).toBeLessThanOrEqual(dialogBox!.x + dialogBox!.width); + }); + } +}); + +/** + * These open the mock's non-system Support role and its Engineering group, and + * search the mock's users; a fresh LibreChat has none of them. + */ +test.describe('Access page dialog keyboard and contrast', () => { + test.skip(LIVE, MOCK_FIXTURES_SKIP_REASON); + test.use({ colorScheme: 'light' }); + + test('Delete role gives focus back to its Delete button on Escape and on Cancel', async ({ + page, + }) => { + await page.goto('/access?tab=roles'); + await page.waitForLoadState('networkidle'); + const opener = page.getByRole('button', { name: 'Delete Support', exact: true }); + + for (const close of ['Escape', 'Cancel'] as const) { + await opener.focus(); + await page.keyboard.press('Enter'); + const dialog = dialogOf(page); + await expect(dialog).toBeVisible(); + await expect(dialog.getByRole('button', { name: 'Cancel' })).toBeFocused(); + if (close === 'Escape') await page.keyboard.press('Escape'); + else await dialog.getByRole('button', { name: 'Cancel' }).press('Enter'); + await expect(dialog).not.toBeVisible(); + await expect(opener).toBeFocused(); + } + }); + + test('Delete role has zero WCAG 2.1 AA violations with Delete focused', async ({ page }) => { + await page.goto('/access?tab=roles'); + await page.waitForLoadState('networkidle'); + await page.getByRole('button', { name: 'Delete Support', exact: true }).focus(); + await page.keyboard.press('Enter'); + const confirm = dialogOf(page).getByRole('button', { name: 'Delete', exact: true }); + await confirm.focus(); + await expect(confirm).toBeFocused(); + + expect((await scanDialog(page)).violations).toEqual([]); + }); + + /** Opens Support's Members tab and highlights the first user the search suggests. */ + async function highlightFirstSuggestion(page: Page) { + const dialog = await openEditRole(page, 'Support'); + await dialog.getByRole('tab', { name: 'Members' }).click(); + const search = dialog.getByRole('combobox', { name: /search users to add/i }); + await search.fill('al'); + const first = page.getByRole('listbox').getByRole('option').first(); + await expect(first).toBeVisible(); + await search.press('ArrowDown'); + await expect(first).toHaveAttribute('aria-selected', 'true'); + return { dialog, search, first }; + } + + test('member search: the highlighted suggestion passes axe', async ({ page }) => { + await highlightFirstSuggestion(page); + + const results = await new AxeBuilder({ page }) + .withTags(WCAG_TAGS) + .include('[role="listbox"]') + .analyze(); + expect(results.violations).toEqual([]); + }); + + test('member search: the field points at the highlighted suggestion', async ({ page }) => { + const { search, first } = await highlightFirstSuggestion(page); + + const id = await first.getAttribute('id'); + expect(id).toBeTruthy(); + await expect(search).toHaveAttribute('aria-activedescendant', id ?? ''); + }); + + test('member search: Escape closes the suggestions first, then the dialog', async ({ page }) => { + const { dialog, search } = await highlightFirstSuggestion(page); + + await page.keyboard.press('Escape'); + await expect(page.getByRole('listbox')).toHaveCount(0); + await expect(dialog).toBeVisible(); + await expect(search).toBeFocused(); + + await page.keyboard.press('Escape'); + await expect(dialog).not.toBeVisible(); + }); + + test('a member staged for removal stays legible, says so, and offers a focused Undo', async ({ + page, + }) => { + await page.goto('/access?tab=groups'); + await page.waitForLoadState('networkidle'); + await page.getByText('Engineering', { exact: true }).click(); + const dialog = dialogOf(page); + await dialog.getByRole('tab', { name: 'Members' }).click(); + await dialog.getByRole('button', { name: 'Remove Alice Chen' }).click(); + await expect(dialog.getByText(/^Removing 1 members?$/)).toBeVisible(); + + expect((await scanDialog(page)).violations).toEqual([]); + await expect(dialog.getByRole('button', { name: 'Undo removing Alice Chen' })).toBeFocused(); + await expect(dialog.getByText('Removing 1 member', { exact: true })).toBeVisible(); + await expect(dialog.getByText('Will be removed', { exact: true })).toBeVisible(); + + await page.keyboard.press('Enter'); + await expect(dialog.getByRole('button', { name: 'Remove Alice Chen' })).toBeFocused(); + await expect(dialog.getByText('Will be removed', { exact: true })).toHaveCount(0); + }); +}); + test.describe('Access page tabs', () => { test('defaults to Roles tab', async ({ page }) => { await page.goto('/access'); @@ -236,6 +397,24 @@ test.describe('Access page Create Group dialog', () => { await expect(page.getByText(name, { exact: true })).toBeVisible({ timeout: 5000 }); }); + test('placeholders use the muted text color, not the disabled one', async ({ page }) => { + await page.goto('/access?tab=groups'); + await page.waitForLoadState('networkidle'); + await page.getByRole('button', { name: /create group/i }).click(); + const name = page.locator('#create-group-name'); + await expect(name).toBeVisible(); + + const colors = await name.evaluate((input) => { + const probe = document.createElement('span'); + probe.style.color = 'var(--cui-color-text-muted)'; + document.body.append(probe); + const muted = getComputedStyle(probe).color; + probe.remove(); + return { placeholder: getComputedStyle(input, '::placeholder').color, muted }; + }); + expect(colors.placeholder).toBe(colors.muted); + }); + test('Create Group button is disabled when name is empty', async ({ page }) => { await page.goto('/access?tab=groups'); await page.waitForLoadState('networkidle'); @@ -383,6 +562,51 @@ test.describe('Access page delete', () => { }); }); +/** + * A delete removes its row only when the list refetches, after the dialog has + * closed and given focus back, so focus left on the row's Delete would be lost + * with the row. These delete a role and a group from the keyboard alone. They + * focus the dialog's Delete directly: which button it opens on is checked above. + */ +test.describe('Access page delete from the keyboard', () => { + test.skip(LIVE, AUDITED_WRITES_SKIP_REASON); + const tabs = [ + { tab: 'roles', kind: 'role', create: createRole }, + { tab: 'groups', kind: 'group', create: createGroup }, + ] as const; + let created: { tab: (typeof tabs)[number]['tab']; name: string } | undefined; + + test.afterEach(async ({ page }) => { + if (!created) return; + await deleteFromList(page, created.tab, created.name); + created = undefined; + }); + + for (const { tab, kind, create } of tabs) { + test(`gives focus to the search field once a ${kind} is deleted`, async ({ page }) => { + const name = uniqueName(`Keyboard ${kind}`); + created = { tab, name }; + await create(page, name); + await searchField(page).fill(name); + const opener = page.getByRole('button', { name: `Delete ${name}`, exact: true }); + await expect(opener).toBeVisible(); + + await opener.focus(); + await page.keyboard.press('Enter'); + const dialog = dialogOf(page); + const confirm = dialog.getByRole('button', { name: 'Delete', exact: true }); + await confirm.focus(); + await expect(confirm).toBeFocused(); + await page.keyboard.press('Enter'); + + await expect(dialog).not.toBeVisible(); + await expect(opener).toHaveCount(0); + created = undefined; + await expect(searchField(page)).toBeFocused(); + }); + } +}); + /** * LibreChat keys system grants by role name and its role rename (`renameRole` * in `packages/api/src/admin/roles.ts`) moves only the role's users, so the diff --git a/e2e/grants.spec.ts b/e2e/grants.spec.ts index 3b311c2d..8c6ec2b0 100644 --- a/e2e/grants.spec.ts +++ b/e2e/grants.spec.ts @@ -133,7 +133,9 @@ test.describe('Grants page - Management tab', () => { const adminRow = page.locator('tbody tr').filter({ hasText: 'Admin' }).first(); await adminRow.click(); await expect(page.getByRole('dialog')).toBeVisible({ timeout: 5000 }); - await expect(page.getByRole('dialog').locator('.badge-role')).toBeVisible(); + const badge = page.getByRole('dialog').locator('.badge-role'); + await expect(badge).toBeVisible(); + await expect(badge.getByRole('img')).toHaveCount(0); }); test('dialog cancel discards changes', async ({ page }) => { @@ -166,6 +168,41 @@ test.describe('Grants page - Audit Log tab', () => { await expect(page.getByRole('table')).toBeVisible(); }); + test('Clear dates gives focus to the From date picker', async ({ page }) => { + await page.getByRole('button', { name: 'From', exact: true }).click(); + await page.getByRole('gridcell').filter({ hasText: /^1$/ }).first().click(); + const clear = page.getByRole('button', { name: 'Clear date filters' }); + await clear.focus(); + await page.keyboard.press('Enter'); + + await expect(clear).toHaveCount(0); + await expect(page.locator('#audit-date-from')).toBeFocused(); + await expect(page.locator('#audit-date-from')).toHaveAccessibleName('From'); + }); + + for (const width of [390, 320]) { + test(`the filters wrap at ${width}px: no sideways scroll, Export on screen`, async ({ + page, + }) => { + await page.setViewportSize({ width, height: 844 }); + const filters = page.getByRole('group', { name: 'Filters' }); + await expect(filters).toBeVisible(); + + const overflow = await filters.evaluate((el) => { + let box = el.parentElement; + while (box && getComputedStyle(box).overflowY !== 'auto') box = box.parentElement; + return box ? box.scrollWidth - box.clientWidth : 0; + }); + expect(overflow).toBeLessThanOrEqual(0); + const exportBox = await page + .getByRole('button', { name: /export all matching/i }) + .boundingBox(); + expect(exportBox).not.toBeNull(); + expect(exportBox!.x).toBeGreaterThanOrEqual(0); + expect(exportBox!.x + exportBox!.width).toBeLessThanOrEqual(width); + }); + } + test('empty state when no entries match filters', async ({ page }) => { await searchField(page).fill('zzzznonexistent'); await page.waitForTimeout(500); @@ -220,6 +257,17 @@ test.describe('Grants page - Audit Log tab', () => { } }); + test('Export keeps focus on its button while it runs', async ({ page }) => { + const exportButton = page.getByRole('button', { name: /export all matching/i }); + await exportButton.focus(); + const download = page.waitForEvent('download'); + await page.keyboard.press('Enter'); + await download; + + await expect(exportButton).toBeFocused(); + await expect(exportButton).not.toHaveAttribute('aria-busy'); + }); + test('Export writes the seeded entry the filters match', async ({ page }) => { await searchField(page).fill('Alice'); await page.getByRole('button', { name: /^Revoked$/i }).click(); @@ -262,6 +310,37 @@ test.describe('Grants page - Audit Log tab', () => { await expect(drawer.getByText('Alice Chen').first()).toBeVisible(); await expect(drawer.getByText('read:agents').first()).toBeVisible(); await expect(drawer.getByText(/entry not found/i)).toHaveCount(0); + + const results = await new AxeBuilder({ page }) + .withTags(WCAG_TAGS) + .include('[role="dialog"]') + .analyze(); + expect(results.violations).toEqual([]); + }); + + test('copying a value from the drawer is announced', async ({ page }) => { + await page.context().grantPermissions(['clipboard-read', 'clipboard-write']); + await page.getByRole('button', { name: 'Open audit log entry for USER' }).click(); + const copy = page.getByRole('dialog').locator('button', { hasText: 'read:agents' }); + await copy.focus(); + await page.keyboard.press('Enter'); + + await expect(page.getByRole('status').filter({ hasText: 'Copied' })).toHaveText( + 'Copied capability', + ); + expect(await page.evaluate(() => navigator.clipboard.readText())).toBe('read:agents'); + }); + + test('closing a drawer opened from a permalink gives focus to its row', async ({ page }) => { + const row = page.getByRole('button', { name: 'Open audit log entry for USER' }); + await row.click(); + const entryId = new URL(page.url()).searchParams.get('entryId'); + await page.goto(`/grants?tab=audit-log&entryId=${entryId}`); + await expect(page.getByRole('dialog')).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog')).not.toBeVisible(); + await expect(row).toBeFocused(); }); test('the mock serves one entry by id, as LibreChat does', async ({ request }) => { diff --git a/src/__tests__/buttons.test.ts b/src/__tests__/buttons.test.ts new file mode 100644 index 00000000..e6b44faa --- /dev/null +++ b/src/__tests__/buttons.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest'; +import { themes } from '@clickhouse/click-ui'; +import { contrast, over, parseColor, resolveVar } from '@/test/contrast'; +import { customProperties } from '@/test/styles'; + +/** A danger button (Delete, Clear dates) at rest, hovered, and focused or pressed (`active`). */ +const STATES = ['default', 'hover', 'active'] as const; +/** WCAG 1.4.3 for normal text. */ +const AA = 4.5; + +/** The app's click-ui overrides for both themes, then for one theme only (more specific, so it wins). */ +const SHARED = ':root[data-cui-theme]'; + +const THEMES = [ + ['light', ':root', [SHARED, ":root[data-cui-theme='light']"], themes.light], + ['dark', '.dark', [SHARED, ":root[data-cui-theme='dark']"], themes.dark], +] as const; + +describe('Danger button text contrast', () => { + it.each(THEMES)( + 'meets AA in every state on the page and in a dialog in %s', + (_, selector, blocks, cui) => { + const app = customProperties(selector); + const overrides = new Map(blocks.flatMap((block) => [...customProperties(block)])); + const { text, background } = cui.click.button.basic.color.danger; + const surfaces = { + page: parseColor(resolveVar('var(--cui-color-background-default)', app)), + dialog: parseColor(cui.click.dialog.color.background.default), + }; + + const failing = STATES.flatMap((state) => { + const fg = parseColor( + overrides.get(`--click-button-basic-color-danger-text-${state}`) ?? text[state], + ); + const tint = parseColor( + overrides.get(`--click-button-basic-color-danger-background-${state}`) ?? + background[state], + ); + return Object.entries(surfaces).flatMap(([surface, color]) => { + const ratio = contrast(fg, over(tint, color)); + return ratio >= AA ? [] : [`${state} on ${surface}: ${ratio.toFixed(2)}:1`]; + }); + }); + + expect(failing).toEqual([]); + }, + ); +}); diff --git a/src/components/access/ConfirmDialog.tsx b/src/components/access/ConfirmDialog.tsx index 36d1bdcf..5668ec39 100644 --- a/src/components/access/ConfirmDialog.tsx +++ b/src/components/access/ConfirmDialog.tsx @@ -1,7 +1,21 @@ -import { ConfirmationDialog } from '@clickhouse/click-ui'; +import { useRef } from 'react'; +import { Button, Dialog } from '@clickhouse/click-ui'; import type * as t from '@/types'; -import { useLocalize } from '@/hooks'; +import { useLocalize, useReturnFocus } from '@/hooks'; +import { BusyButton } from '@/components/shared'; +import { isFocusLost } from '@/utils'; +/** + * Asks before a destructive action. It opens on Cancel, so Enter never + * confirms by accident, and the message describes the dialog. Confirm keeps + * it open while `saving` until the caller closes it on success. + * + * Cancel, Escape and the close button give focus back to the control that + * opened it. Closing after Confirm gives it to `returnFocusTo()` instead: the + * action removes the opener's row, but only once the list refetches, after + * the dialog has closed. A close while the action is still saving counts as + * confirmed; one after it failed (the dialog stays open) does not. + */ export function ConfirmDialog({ open, title, @@ -12,28 +26,67 @@ export function ConfirmDialog({ error, onConfirm, onCancel, + returnFocusTo, }: t.ConfirmDialogProps) { const localize = useLocalize(); + const cancelRef = useRef(null); + const confirmedRef = useRef(false); + const returnFocus = useReturnFocus(open, returnFocusTo); + + const cancel = () => { + if (!saving) confirmedRef.current = false; + onCancel(); + }; return ( - { + if (!isOpen) cancel(); + }} > - {error && ( -

- {error} -

- )} - + { + event.preventDefault(); + cancelRef.current?.focus(); + }} + onCloseAutoFocus={(event) => { + if (confirmedRef.current && isFocusLost()) returnFocusTo?.()?.focus(); + confirmedRef.current = false; + returnFocus(event); + }} + className="modal-frost" + > +
+ {error && ( +

+ {error} +

+ )} +
+
+
+ + ); } diff --git a/src/components/access/CreateGroupDialog.tsx b/src/components/access/CreateGroupDialog.tsx index 45506329..8ee45a67 100644 --- a/src/components/access/CreateGroupDialog.tsx +++ b/src/components/access/CreateGroupDialog.tsx @@ -122,7 +122,7 @@ export function CreateGroupDialog({ open, onClose }: t.CreateGroupDialogProps) { onChange={(e) => setName(e.target.value)} placeholder={localize('com_access_group_name_placeholder')} autoFocus - className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-disabled)" + className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-muted)" />
@@ -138,7 +138,7 @@ export function CreateGroupDialog({ open, onClose }: t.CreateGroupDialogProps) { value={description} onChange={(e) => setDescription(e.target.value)} placeholder={localize('com_access_group_desc_placeholder')} - className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-disabled)" + className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-muted)" />
diff --git a/src/components/access/CreateRoleDialog.tsx b/src/components/access/CreateRoleDialog.tsx index 3b96ab92..985a74d2 100644 --- a/src/components/access/CreateRoleDialog.tsx +++ b/src/components/access/CreateRoleDialog.tsx @@ -128,7 +128,7 @@ export function CreateRoleDialog({ open, onClose }: t.CreateRoleDialogProps) { onChange={(e) => setName(e.target.value)} placeholder={localize('com_access_role_name_placeholder')} autoFocus - className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-disabled)" + className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-muted)" />
@@ -144,7 +144,7 @@ export function CreateRoleDialog({ open, onClose }: t.CreateRoleDialogProps) { value={description} onChange={(e) => setDescription(e.target.value)} placeholder={localize('com_access_role_desc_placeholder')} - className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-disabled)" + className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-muted)" />
diff --git a/src/components/access/EditGroupDialog.tsx b/src/components/access/EditGroupDialog.tsx index dc44e7eb..88f59e41 100644 --- a/src/components/access/EditGroupDialog.tsx +++ b/src/components/access/EditGroupDialog.tsx @@ -10,16 +10,10 @@ import { updateGroupFn, MEMBERS_PAGE_SIZE, } from '@/server'; -import { - Avatar, - LoadingState, - Pagination, - SelectedMemberList, - TrashButton, - UserSearchInline, -} from '@/components/shared'; +import { SelectedMemberList, UserSearchInline } from '@/components/shared'; import { useLocalize, useReturnFocus, useRefreshScopes } from '@/hooks'; import { cn, notifySuccess, notifyError } from '@/utils'; +import { MemberList } from './MemberList'; type EditGroupTab = 'details' | 'members'; @@ -175,7 +169,7 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog disabled={!canManage} readOnly={!canManage} autoFocus - className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-disabled) disabled:cursor-not-allowed disabled:opacity-50" + className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-muted) disabled:cursor-not-allowed disabled:opacity-50" />
@@ -193,7 +187,7 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog placeholder={localize('com_access_group_desc_placeholder')} disabled={!canManage} readOnly={!canManage} - className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-disabled) disabled:cursor-not-allowed disabled:opacity-50" + className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-muted) disabled:cursor-not-allowed disabled:opacity-50" />
@@ -249,6 +243,7 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog fetching={membersQuery.isFetching} removalIds={removalIds} onRemove={stageRemoval} + onUndoRemove={unstageRemoval} canManage={canManage} total={total} currentPage={page} @@ -286,105 +281,3 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog ); } - -interface MemberListProps { - members: AdminMember[]; - loading: boolean; - error: boolean; - fetching: boolean; - removalIds: Set; - onRemove: (member: AdminMember) => void; - canManage: boolean; - total: number; - currentPage: number; - totalPages: number; - onPageChange: (page: number) => void; -} - -function MemberList({ - members, - loading, - error, - fetching, - removalIds, - onRemove, - canManage, - total, - currentPage, - totalPages, - onPageChange, -}: MemberListProps) { - const localize = useLocalize(); - - if (loading) { - return ( - - ); - } - - if (error) { - return ( -
- {localize('com_error_load_members')} -
- ); - } - - if (members.length === 0 && total === 0) { - return ( -
- {localize('com_access_no_members')} -
- ); - } - - return ( -
- - {localize('com_access_member_count', { count: total })} - -
- {members.map((member, i) => { - const staged = removalIds.has(member.userId); - return ( -
-
- -
- - {member.name} - - {member.email} -
-
- {canManage && !staged && ( - onRemove(member)} - ariaLabel={`${localize('com_access_remove_member')} ${member.name}`} - /> - )} -
- ); - })} -
- -
- ); -} diff --git a/src/components/access/EditRoleDialog.tsx b/src/components/access/EditRoleDialog.tsx index 2573485f..59842012 100644 --- a/src/components/access/EditRoleDialog.tsx +++ b/src/components/access/EditRoleDialog.tsx @@ -15,17 +15,15 @@ import { MEMBERS_PAGE_SIZE, } from '@/server'; import { - Avatar, LoadingState, - Pagination, DecorativeIcon, SelectedMemberList, - TrashButton, UserSearchInline, } from '@/components/shared'; import { useLocalize, useReturnFocus, useRefreshScopes, useConfigFeatures } from '@/hooks'; import { RolePermissionsPanel } from './RolePermissionsPanel'; import { cn, notifySuccess, notifyError } from '@/utils'; +import { MemberList } from './MemberList'; type EditRoleTab = 'details' | 'permissions' | 'members'; @@ -238,7 +236,7 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro aria-describedby={ isRenaming && hasProfile ? 'edit-role-rename-note' : undefined } - className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-disabled) disabled:cursor-not-allowed disabled:opacity-50" + className="rounded-lg border border-(--cui-color-stroke-default) bg-(--cui-color-background-default) px-3 py-2 text-sm text-(--cui-color-text-default) placeholder:text-(--cui-color-text-muted) disabled:cursor-not-allowed disabled:opacity-50" /> {isRenaming && hasProfile && ( @@ -355,6 +353,7 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro fetching={membersQuery.isFetching} removalIds={removalIds} onRemove={stageRemoval} + onUndoRemove={unstageRemoval} canManage={canManage} total={total} currentPage={page} @@ -423,105 +422,3 @@ function RenameProfileNote({ id, name, moves }: t.RenameProfileNoteProps) {

); } - -interface MemberListProps { - members: AdminMember[]; - loading: boolean; - error: boolean; - fetching: boolean; - removalIds: Set; - onRemove: (member: AdminMember) => void; - canManage: boolean; - total: number; - currentPage: number; - totalPages: number; - onPageChange: (page: number) => void; -} - -function MemberList({ - members, - loading, - error, - fetching, - removalIds, - onRemove, - canManage, - total, - currentPage, - totalPages, - onPageChange, -}: MemberListProps) { - const localize = useLocalize(); - - if (loading) { - return ( - - ); - } - - if (error) { - return ( -
- {localize('com_error_load_members')} -
- ); - } - - if (members.length === 0 && total === 0) { - return ( -
- {localize('com_access_no_members')} -
- ); - } - - return ( -
- - {localize('com_access_member_count', { count: total })} - -
- {members.map((member, i) => { - const staged = removalIds.has(member.userId); - return ( -
-
- -
- - {member.name} - - {member.email} -
-
- {canManage && !staged && ( - onRemove(member)} - ariaLabel={`${localize('com_access_remove_member')} ${member.name}`} - /> - )} -
- ); - })} -
- -
- ); -} diff --git a/src/components/access/GroupsTab.tsx b/src/components/access/GroupsTab.tsx index 7a987c23..f4cd6169 100644 --- a/src/components/access/GroupsTab.tsx +++ b/src/components/access/GroupsTab.tsx @@ -1,5 +1,5 @@ import { Button } from '@clickhouse/click-ui'; -import { useState, useEffect, useRef } from 'react'; +import { useState, useEffect, useRef, useCallback } from 'react'; import { useQuery, useMutation, useQueryClient, keepPreviousData } from '@tanstack/react-query'; import type { AdminGroup } from '@librechat/data-schemas'; import type * as t from '@/types'; @@ -8,10 +8,11 @@ import { SearchInput, EmptyState, Pagination, + ScreenReaderAnnouncer, TrashButton, } from '@/components/shared'; import { deleteGroupFn, groupsQueryOptions, GROUPS_PAGE_SIZE, MAX_SEARCH_LENGTH } from '@/server'; -import { useLocalize, useCapabilities, useRefreshScopes } from '@/hooks'; +import { useLocalize, useAnnouncement, useCapabilities, useRefreshScopes } from '@/hooks'; import { cn, notifySuccess, notifyError } from '@/utils'; import { EditGroupDialog } from './EditGroupDialog'; import { SystemCapabilities } from '@/constants'; @@ -29,6 +30,10 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { const [debouncedSearch, setDebouncedSearch] = useState(''); const [page, setPage] = useState(1); const debounceRef = useRef>(undefined); + const { message: announcement, announce } = useAnnouncement(); + const announcedSearch = useRef(debouncedSearch); + const searchRef = useRef(null); + const searchField = useCallback(() => searchRef.current, []); useEffect(() => { return () => clearTimeout(debounceRef.current); @@ -53,6 +58,13 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { const total = data?.total ?? 0; const totalPages = Math.ceil(total / GROUPS_PAGE_SIZE); + /** Announces the match count once each new search has loaded; the first load stays quiet. */ + useEffect(() => { + if (isFetching || isError || announcedSearch.current === debouncedSearch) return; + announcedSearch.current = debouncedSearch; + announce(localize('com_a11y_groups_found', { count: total })); + }, [debouncedSearch, isFetching, isError, total, announce, localize]); + const deleteMutation = useMutation({ mutationFn: (group: AdminGroup) => deleteGroupFn({ data: { id: group.id } }), onSuccess: (_data, group) => { @@ -75,7 +87,7 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { if (isError && !data) { return ( -
+
{localize('com_error_load_groups')}
); @@ -85,6 +97,7 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) {
setDeleteTarget(null)} + returnFocusTo={searchField} /> + +
); } diff --git a/src/components/access/MemberList.tsx b/src/components/access/MemberList.tsx new file mode 100644 index 00000000..6d83e1d2 --- /dev/null +++ b/src/components/access/MemberList.tsx @@ -0,0 +1,138 @@ +import { useRef } from 'react'; +import { Badge, Button } from '@clickhouse/click-ui'; +import type * as t from '@/types'; +import { Avatar, LoadingState, Pagination, TrashButton } from '@/components/shared'; +import { useLocalize } from '@/hooks'; +import { cn } from '@/utils'; + +/** + * The members of a role or group in its Edit dialog. A member staged for + * removal stays at full contrast, struck through and tagged "Will be removed", + * and its Remove button turns into Undo in place. Focus follows that swap, + * so it never falls out of the dialog. + */ +export function MemberList({ + members, + loading, + error, + fetching, + removalIds, + onRemove, + onUndoRemove, + canManage, + total, + currentPage, + totalPages, + onPageChange, +}: t.MemberListProps) { + const localize = useLocalize(); + const focusUserId = useRef(null); + + const focusIfSwapped = (userId: string) => (button: HTMLButtonElement | null) => { + if (!button || focusUserId.current !== userId) return; + focusUserId.current = null; + button.focus(); + }; + + if (loading) { + return ( + + ); + } + + if (error) { + return ( +
+ {localize('com_error_load_members')} +
+ ); + } + + if (members.length === 0 && total === 0) { + return ( +
+ {localize('com_access_no_members')} +
+ ); + } + + return ( +
+ + {localize('com_access_member_count', { count: total })} + +
+ {members.map((member, i) => { + const staged = removalIds.has(member.userId); + return ( +
+
+ +
+ + + {member.name} + + {staged && ( + + )} + + + {member.email} + +
+
+ {canManage && staged && ( +
+ ); + })} +
+ +
+ ); +} diff --git a/src/components/access/RolePermissionsPanel.tsx b/src/components/access/RolePermissionsPanel.tsx index 41325255..4be5eb75 100644 --- a/src/components/access/RolePermissionsPanel.tsx +++ b/src/components/access/RolePermissionsPanel.tsx @@ -143,7 +143,7 @@ export function RolePermissionsPanel({
6 - ? 'grid w-fit grid-cols-[auto_auto_auto] gap-x-4 gap-y-1.5' + ? 'grid min-w-0 grid-cols-1 gap-x-4 gap-y-1.5 sm:grid-cols-2 md:grid-cols-3' : 'flex flex-wrap gap-x-6 gap-y-1.5' } > diff --git a/src/components/access/RolesTab.tsx b/src/components/access/RolesTab.tsx index c267103b..f6fa8142 100644 --- a/src/components/access/RolesTab.tsx +++ b/src/components/access/RolesTab.tsx @@ -1,16 +1,17 @@ -import { useMemo, useState } from 'react'; import { Button } from '@clickhouse/click-ui'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import type * as t from '@/types'; import { LoadingState, Pagination, + ScreenReaderAnnouncer, SearchInput, EmptyState, TrashButton, } from '@/components/shared'; +import { useLocalize, useAnnouncement, useCapabilities, useRefreshScopes } from '@/hooks'; import { deleteRoleFn, allRolesQueryOptions, ROLES_PAGE_SIZE } from '@/server'; -import { useLocalize, useCapabilities, useRefreshScopes } from '@/hooks'; import { notifySuccess, notifyError } from '@/utils'; import { EditRoleDialog } from './EditRoleDialog'; import { SystemCapabilities } from '@/constants'; @@ -18,6 +19,10 @@ import { ConfirmDialog } from './ConfirmDialog'; export function RolesTab({ onCreateRole }: t.RolesTabProps) { const localize = useLocalize(); + const { message: announcement, announce } = useAnnouncement(); + const announcedSearch = useRef(''); + const searchRef = useRef(null); + const searchField = useCallback(() => searchRef.current, []); const queryClient = useQueryClient(); const refreshScopes = useRefreshScopes(); const { hasCapability } = useCapabilities(); @@ -37,6 +42,13 @@ export function RolesTab({ onCreateRole }: t.RolesTabProps) { ); }, [allRoles, search]); + /** Announces the match count after each change to the search; the first render stays quiet. */ + useEffect(() => { + if (announcedSearch.current === search) return; + announcedSearch.current = search; + announce(localize('com_a11y_roles_found', { count: filtered.length })); + }, [search, filtered.length, announce, localize]); + const totalPages = Math.ceil(filtered.length / ROLES_PAGE_SIZE); const paged = useMemo( () => filtered.slice((page - 1) * ROLES_PAGE_SIZE, page * ROLES_PAGE_SIZE), @@ -76,6 +88,7 @@ export function RolesTab({ onCreateRole }: t.RolesTabProps) {
setDeleteTarget(null)} + returnFocusTo={searchField} /> + +
); } diff --git a/src/components/access/__tests__/ConfirmDialog.test.tsx b/src/components/access/__tests__/ConfirmDialog.test.tsx new file mode 100644 index 00000000..eb3b8bb7 --- /dev/null +++ b/src/components/access/__tests__/ConfirmDialog.test.tsx @@ -0,0 +1,173 @@ +import { useState } from 'react'; +import { describe, expect, it, vi } from 'vitest'; +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import type * as t from '@/types'; +import { + DIALOG_CLOSERS, + expectFocusBack, + findFocusedDialog, + pressEscape, + renderWithOpener, +} from '@/test/focus'; +import { ConfirmDialog } from '../ConfirmDialog'; +import '@/locales/i18n'; + +const MESSAGE = 'Are you sure you want to delete "Support"?'; + +const PROPS: t.ConfirmDialogProps = { + open: true, + title: 'Delete role', + description: MESSAGE, + confirmLabel: 'Delete', + saving: false, + onConfirm: () => {}, + onCancel: () => {}, +}; + +/** How the delete that Confirm starts goes. */ +type DeleteOutcome = 'success' | 'failure' | 'saving'; + +/** + * A row whose Delete opens the dialog, as on the Roles tab. Confirm runs the + * delete: it succeeds and closes the dialog, fails and leaves it open, or is + * still saving. The row goes once `removed` (the list's refetch, which lands + * after the dialog has closed). + */ +function DeletableRow({ removed, outcome }: { removed: boolean; outcome: DeleteOutcome }) { + const [open, setOpen] = useState(false); + const [saving, setSaving] = useState(false); + const [search, setSearch] = useState(null); + const confirm = () => { + if (outcome === 'success') setOpen(false); + if (outcome === 'saving') setSaving(true); + }; + return ( + <> + + {!removed && ( + + )} + setOpen(false)} + returnFocusTo={() => search} + /> + + ); +} + +/** Opens the dialog from the row's Delete and presses its Delete. */ +async function confirmDelete(): Promise { + const opener = screen.getByRole('button', { name: 'Delete Support' }); + opener.focus(); + fireEvent.click(opener); + const dialog = await findFocusedDialog(); + fireEvent.click(within(dialog).getByRole('button', { name: 'Delete' })); + return opener; +} + +const searchBox = () => screen.getByRole('textbox', { name: 'Search roles' }); + +describe('ConfirmDialog', () => { + it('is described by its message and opens on Cancel', async () => { + render(); + const dialog = await findFocusedDialog(); + expect(dialog).toHaveAccessibleName('Delete role'); + expect(dialog).toHaveAccessibleDescription(MESSAGE); + expect(within(dialog).getByRole('button', { name: 'Cancel' })).toHaveFocus(); + }); + + it('stays open while the action saves, busy but still focusable, until the caller closes it', async () => { + const onConfirm = vi.fn(); + const onCancel = vi.fn(); + const { rerender } = render( + , + ); + const dialog = await findFocusedDialog(); + const confirm = within(dialog).getByRole('button', { name: 'Delete' }); + confirm.focus(); + fireEvent.click(confirm); + expect(onConfirm).toHaveBeenCalledTimes(1); + expect(onCancel).not.toHaveBeenCalled(); + + rerender(); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(confirm).toHaveAttribute('aria-busy', 'true'); + expect(confirm).toHaveAttribute('aria-disabled', 'true'); + expect(confirm).toBeEnabled(); + expect(confirm).toHaveFocus(); + fireEvent.click(confirm); + expect(onConfirm).toHaveBeenCalledTimes(1); + }); + + it('shows an error as an alert below its message', async () => { + render(); + const dialog = await findFocusedDialog(); + expect(within(dialog).getByRole('alert')).toHaveTextContent('Role is in use'); + expect(dialog).toHaveAccessibleDescription(MESSAGE); + }); + + it.each([ + ...DIALOG_CLOSERS, + [ + 'Cancel', + (dialog: HTMLElement) => + fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' })), + ] as const, + ])('gives focus back to the Delete button that opened it on %s', async (_, close) => { + const opener = renderWithOpener((open, onClose) => ( + + )); + opener.focus(); + fireEvent.click(opener); + close(await findFocusedDialog()); + await expectFocusBack(opener); + }); + + it('gives focus to the fallback after a confirmed delete, before the refetch removes its opener', async () => { + const { rerender } = render(); + await confirmDelete(); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + await waitFor(() => expect(document.body).not.toHaveFocus()); + + rerender(); + expect(screen.queryByRole('button', { name: 'Delete Support' })).toBeNull(); + expect(searchBox()).toHaveFocus(); + }); + + it('gives focus to the fallback when the confirmed delete removed its opener first', async () => { + const { rerender } = render(); + await confirmDelete(); + rerender(); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + await waitFor(() => expect(searchBox()).toHaveFocus()); + }); + + it('gives focus to the fallback when Escape closes it while the delete still saves', async () => { + render(); + await confirmDelete(); + const dialog = screen.getByRole('dialog'); + await waitFor(() => + expect(within(dialog).getByRole('button', { name: 'Delete' })).toHaveAttribute( + 'aria-busy', + 'true', + ), + ); + pressEscape(dialog); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + await waitFor(() => expect(searchBox()).toHaveFocus()); + }); + + it('gives focus back to its opener on Cancel after a confirmed delete failed', async () => { + render(); + const opener = await confirmDelete(); + const dialog = screen.getByRole('dialog'); + fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' })); + await expectFocusBack(opener); + }); +}); diff --git a/src/components/access/__tests__/GroupsTab.test.tsx b/src/components/access/__tests__/GroupsTab.test.tsx index d49ca30c..8194ce66 100644 --- a/src/components/access/__tests__/GroupsTab.test.tsx +++ b/src/components/access/__tests__/GroupsTab.test.tsx @@ -30,11 +30,15 @@ vi.mock('@/server/utils/api', () => ({ vi.mock('@/server', () => vi.importActual('@/server/groups')); vi.mock('@/hooks', async () => ({ - useLocalize: () => (key: string) => key, + useLocalize: () => (key: string, options?: Record) => + options ? `${key}(${Object.values(options).join(',')})` : key, useCapabilities: () => ({ hasCapability: () => true }), useRefreshScopes: () => vi.fn(), useReturnFocus: (await vi.importActual('@/hooks/dialogs')) .useReturnFocus, + useAnnouncement: ( + await vi.importActual('@/hooks/useAnnouncement') + ).useAnnouncement, })); /** The `search` of every groups list request sent so far. */ @@ -62,3 +66,19 @@ describe('GroupsTab search', () => { expect(clamped).toBeUndefined(); }); }); + +describe('GroupsTab search announcement', () => { + it('announces the match count once a search has loaded, and not on the first load', async () => { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + const field = await screen.findByPlaceholderText('com_access_search_groups'); + const status = screen.getByRole('status'); + expect(status).toBeEmptyDOMElement(); + fireEvent.change(field, { target: { value: 'eng' } }); + await waitFor(() => expect(status).toHaveTextContent('com_a11y_groups_found(1)')); + }); +}); diff --git a/src/components/access/__tests__/MemberList.test.tsx b/src/components/access/__tests__/MemberList.test.tsx new file mode 100644 index 00000000..8df7fc73 --- /dev/null +++ b/src/components/access/__tests__/MemberList.test.tsx @@ -0,0 +1,86 @@ +import { useState } from 'react'; +import { describe, expect, it } from 'vitest'; +import { fireEvent, render, screen } from '@testing-library/react'; +import type { AdminMember } from '@librechat/data-schemas'; +import type * as t from '@/types'; +import { MemberList } from '../MemberList'; +import '@/locales/i18n'; + +const MEMBERS: AdminMember[] = [ + { userId: 'u1', name: 'Alice Chen', email: 'alice@example.com' }, + { userId: 'u2', name: 'Bob Martinez', email: 'bob@example.com' }, +]; + +const BASE: t.MemberListProps = { + members: MEMBERS, + loading: false, + error: false, + fetching: false, + removalIds: new Set(), + onRemove: () => {}, + onUndoRemove: () => {}, + canManage: true, + total: MEMBERS.length, + currentPage: 1, + totalPages: 1, + onPageChange: () => {}, +}; + +/** The list as an Edit dialog drives it: removals are staged in the dialog's state. */ +function StagedMemberList() { + const [removalIds, setRemovalIds] = useState>(new Set()); + return ( + setRemovalIds((prev) => new Set(prev).add(member.userId))} + onUndoRemove={(userId) => + setRemovalIds((prev) => new Set([...prev].filter((id) => id !== userId))) + } + /> + ); +} + +describe('MemberList', () => { + it('counts members in the singular and the plural', () => { + const { rerender } = render(); + expect(screen.getByText('2 members')).toBeInTheDocument(); + rerender(); + expect(screen.getByText('1 member')).toBeInTheDocument(); + }); + + it('marks a staged member "Will be removed" at full contrast and swaps Remove for a focused Undo', () => { + render(); + fireEvent.click(screen.getByRole('button', { name: 'Remove Alice Chen' })); + + const undo = screen.getByRole('button', { name: 'Undo removing Alice Chen' }); + expect(undo).toHaveTextContent('Undo'); + expect(undo).toHaveFocus(); + expect(screen.queryByRole('button', { name: 'Remove Alice Chen' })).toBeNull(); + expect(screen.getByText('Will be removed')).toBeVisible(); + expect(screen.getByText('Alice Chen')).toHaveClass('line-through'); + expect(screen.getByText('Alice Chen').closest('.opacity-40')).toBeNull(); + + fireEvent.click(undo); + expect(screen.getByRole('button', { name: 'Remove Alice Chen' })).toHaveFocus(); + expect(screen.queryByText('Will be removed')).toBeNull(); + }); + + it('leaves the other rows alone', () => { + render(); + fireEvent.click(screen.getByRole('button', { name: 'Remove Alice Chen' })); + expect(screen.getByRole('button', { name: 'Remove Bob Martinez' })).toBeEnabled(); + expect(screen.getByText('Bob Martinez')).not.toHaveClass('line-through'); + }); + + it('reports a failed load as an alert', () => { + render(); + expect(screen.getByRole('alert')).toHaveTextContent(/members/i); + }); + + it('offers no Remove or Undo without the manage capability', () => { + render(); + expect(screen.queryByRole('button')).toBeNull(); + expect(screen.getByText('Will be removed')).toBeVisible(); + }); +}); diff --git a/src/components/access/__tests__/RolesTab.test.tsx b/src/components/access/__tests__/RolesTab.test.tsx new file mode 100644 index 00000000..dbb4c832 --- /dev/null +++ b/src/components/access/__tests__/RolesTab.test.tsx @@ -0,0 +1,63 @@ +import { describe, expect, it, vi } from 'vitest'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import type * as t from '@/types'; +import { RolesTab } from '../RolesTab'; +import '@/locales/i18n'; + +const role = (name: string, description: string, isSystemRole = false): t.Role => ({ + id: name, + name, + description, + isSystemRole, + isActive: true, + userCount: 0, + permissions: {} as t.RolePermissions, +}); + +const ROLES = [ + role('ADMIN', '', true), + role('Billing', 'Invoices and usage reports'), + role('Support', 'Answers user tickets'), +]; + +vi.mock('@/hooks', async (importOriginal) => ({ + ...(await importOriginal()), + useCapabilities: () => ({ hasCapability: () => true }), + useRefreshScopes: () => vi.fn(), +})); + +vi.mock('@/server', () => ({ + ROLES_PAGE_SIZE: 50, + allRolesQueryOptions: { queryKey: ['roles'], queryFn: async () => ROLES }, + deleteRoleFn: vi.fn(), +})); + +vi.mock('../EditRoleDialog', () => ({ EditRoleDialog: () => null })); + +function renderTab() { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); +} + +describe('RolesTab search', () => { + it('announces how many roles match each search', async () => { + renderTab(); + const field = await screen.findByPlaceholderText('Search roles…'); + const status = () => + screen.getAllByRole('status').find((el) => el.textContent?.includes('found')); + + fireEvent.change(field, { target: { value: 'support' } }); + await waitFor(() => expect(status()).toHaveTextContent('1 role found')); + + fireEvent.change(field, { target: { value: 'us' } }); + await waitFor(() => expect(status()).toHaveTextContent('2 roles found')); + + fireEvent.change(field, { target: { value: 'zzz' } }); + await waitFor(() => expect(status()).toHaveTextContent('0 roles found')); + }); +}); diff --git a/src/components/grants/AuditLogDetailDrawer.tsx b/src/components/grants/AuditLogDetailDrawer.tsx index 81251b28..6af9a9e4 100644 --- a/src/components/grants/AuditLogDetailDrawer.tsx +++ b/src/components/grants/AuditLogDetailDrawer.tsx @@ -1,5 +1,5 @@ import * as Dialog from '@radix-ui/react-dialog'; -import { Badge, Button, Icon } from '@clickhouse/click-ui'; +import { Badge, Button } from '@clickhouse/click-ui'; import { useCallback, useEffect, useRef, useState } from 'react'; import type { ReactElement } from 'react'; import type * as t from '@/types'; @@ -17,42 +17,13 @@ import { CloseButton, DecorativeIcon, LoadingState } from '@/components/shared'; import { useLocalize, useReturnFocus } from '@/hooks'; import { cn } from '@/utils'; -interface AuditLogDetailDrawerProps { - entry: t.AuditLogEntryWithDiff | null; - open: boolean; - onClose: () => void; - /** Resolves to `true` when the clipboard write succeeded so the drawer only - * flips to its "Copied!" affordance after a real success. */ - onCopyPermalink: (entryId: string) => Promise; - /** Invoked when any inline `CopyableMono` (timestamp, actor/target/entry IDs, - * capability) fails to write to the clipboard. The caller is responsible for - * surfacing the failure — typically via the same screen-reader announcement - * used for the permalink-copy failure path so the drawer's copy controls are - * not silent on clipboard error. */ - onCopyFailed?: () => void; - /** Render a "no entry found" message instead of the detail body when the - * deep-linked id couldn't be located (e.g. the entry was purged). */ - notFound?: boolean; - /** Render a loading shell while the single-entry deep-link query is in flight - * for an entry that is not on the current page. Without this, the drawer - * would either flicker shut during the fetch or never appear on cold load. */ - loading?: boolean; - /** Render a load-error message + close affordance when the single-entry - * fetch failed for reasons other than not-found (network failure, 5xx). The - * caller is responsible for distinguishing this from `notFound` (which is - * the 404 case where the request itself succeeded but the entry is gone). */ - loadError?: boolean; -} - -function CopyableMono({ - value, - ariaLabel, - onCopyFailed, -}: { - value: string; - ariaLabel: string; - onCopyFailed?: () => void; -}): ReactElement { +/** + * A value that copies itself. Its name starts with the value it shows and then + * says what pressing it does ("…, copy actor ID"); a copy is announced through + * `onCopied`, since only the button's hidden icon changes. + */ +function CopyableMono({ value, field, onCopied, onCopyFailed }: t.CopyableMonoProps): ReactElement { + const localize = useLocalize(); const [copied, setCopied] = useState(false); const timerRef = useRef | undefined>(undefined); useEffect(() => { @@ -73,16 +44,16 @@ function CopyableMono({ return; } setCopied(true); + onCopied?.(field); if (timerRef.current) clearTimeout(timerRef.current); timerRef.current = setTimeout(() => setCopied(false), 1500); - }, [value, onCopyFailed]); + }, [value, field, onCopied, onCopyFailed]); return (
@@ -438,7 +412,8 @@ export function AuditLogDetailDrawer({
@@ -452,7 +427,7 @@ export function AuditLogDetailDrawer({ state="neutral" text={ - + {localize(targetConfig.labelKey)} } @@ -463,7 +438,8 @@ export function AuditLogDetailDrawer({
@@ -477,7 +453,8 @@ export function AuditLogDetailDrawer({
@@ -489,7 +466,8 @@ export function AuditLogDetailDrawer({ {subject.label} @@ -498,7 +476,8 @@ export function AuditLogDetailDrawer({ diff --git a/src/components/grants/AuditLogTab.tsx b/src/components/grants/AuditLogTab.tsx index 48f5c5d9..2b0dc7b5 100644 --- a/src/components/grants/AuditLogTab.tsx +++ b/src/components/grants/AuditLogTab.tsx @@ -14,18 +14,19 @@ import { localDayBoundaryIso, } from './auditLogUtils'; import { - AUDIT_LOG_PAGE_SIZE, - auditLogEntryQueryOptions, - auditLogQueryOptions, - exportAuditLogServerFn, -} from '@/server'; -import { + BusyButton, EmptyState, LoadingState, Pagination, ScreenReaderAnnouncer, SearchInput, } from '@/components/shared'; +import { + AUDIT_LOG_PAGE_SIZE, + auditLogEntryQueryOptions, + auditLogQueryOptions, + exportAuditLogServerFn, +} from '@/server'; import { useAnnouncement, useDebouncedFilter, useLocalize } from '@/hooks'; import { AuditLogDetailDrawer } from './AuditLogDetailDrawer'; import { AUDIT_ACTIONS, isAuditEntryId } from '@/constants'; @@ -41,6 +42,7 @@ const TARGET_TYPE_OPTIONS: readonly t.AuditTargetType[] = [ /** Radix `Select.Item` cannot use `value=""` (Radix reserves empty string for * "no selection"). Use a non-empty sentinel and translate to `''` in state. */ const TARGET_TYPE_ALL = '__all__'; +const DATE_FROM_ID = 'audit-date-from'; function downloadBlob(blob: Blob): void { const url = URL.createObjectURL(blob); @@ -67,6 +69,10 @@ export function AuditLogTab() { const [currentPage, setCurrentPage] = useState(1); const { message: announcement, announce } = useAnnouncement(); + const searchRef = useRef(null); + const rowButtons = useRef>(new Map()); + const lastEntryId = useRef(entryId); + const focusDateFrom = useRef(false); const resetToFirstPage = useCallback(() => setCurrentPage(1), []); const searchFilter = useDebouncedFilter('', resetToFirstPage); @@ -241,6 +247,26 @@ export function AuditLogTab() { !entryOnPage && (!isAuditEntryId(entryId) || (entryFetch.isSuccess && entryFetch.data?.entry === null)); + useEffect(() => { + if (entryId) lastEntryId.current = entryId; + }, [entryId]); + + /** + * Where focus goes when the drawer closes and nothing opened it (a permalink): + * the entry's row when it is on this page, else the search field. + */ + const drawerFallback = useCallback(() => { + const id = lastEntryId.current; + return (id ? rowButtons.current.get(id) : undefined) ?? searchRef.current; + }, []); + + /** Clear dates removes itself, so focus moves to the remounted From picker. */ + useEffect(() => { + if (!focusDateFrom.current) return; + focusDateFrom.current = false; + document.getElementById(DATE_FROM_ID)?.focus(); + }, [dateResetNonce]); + const openEntry = useCallback( (id: string) => { void navigate({ search: (prev: Record) => ({ ...prev, entryId: id }) }); @@ -275,6 +301,9 @@ export function AuditLogTab() { ); try { await navigator.clipboard.writeText(url); + announce( + localize('com_audit_detail_copied_field', { field: localize('com_audit_field_link') }), + ); return true; } catch { announce(localize('com_a11y_copy_failed')); @@ -334,24 +363,25 @@ export function AuditLogTab() { return (
-
+
{AUDIT_ACTIONS.map((act) => { const selected = actionFilter.includes(act); @@ -374,7 +404,7 @@ export function AuditLogTab() {
{ setDateFrom(''); setDateTo(''); + focusDateFrom.current = true; setDateResetNonce((n) => n + 1); resetToFirstPage(); }} @@ -410,14 +441,14 @@ export function AuditLogTab() { )}
-
-
@@ -517,6 +548,10 @@ export function AuditLogTab() { entry={entry} isLast={i === pageEntries.length - 1} onOpen={() => openEntry(entry.id)} + buttonRef={(button) => { + if (button) rowButtons.current.set(entry.id, button); + else rowButtons.current.delete(entry.id); + }} /> ))} {!showLoading && !isError && pageEntries.length === 0 && ( @@ -580,7 +615,9 @@ export function AuditLogTab() { */ loadError={!!entryId && !entryOnPage && entryFetch.isError} onClose={closeEntry} + returnFocusTo={drawerFallback} onCopyPermalink={handleCopyPermalink} + onCopied={(field) => announce(localize('com_audit_detail_copied_field', { field }))} onCopyFailed={() => announce(localize('com_a11y_copy_failed'))} />
diff --git a/src/components/grants/AuditLogTableRow.tsx b/src/components/grants/AuditLogTableRow.tsx index fad89b69..17b0ed80 100644 --- a/src/components/grants/AuditLogTableRow.tsx +++ b/src/components/grants/AuditLogTableRow.tsx @@ -13,7 +13,7 @@ import { useLocalize } from '@/hooks'; import { cn } from '@/utils'; /** One audit entry; its target's name is the button that opens the entry's drawer. */ -export function AuditLogTableRow({ entry, isLast, onOpen }: t.AuditLogTableRowProps) { +export function AuditLogTableRow({ entry, isLast, onOpen, buttonRef }: t.AuditLogTableRowProps) { const localize = useLocalize(); const targetConfig = auditTargetConfig(entry.target.type); const subject = auditSubject(entry, localize); @@ -40,6 +40,7 @@ export function AuditLogTableRow({ entry, isLast, onOpen }: t.AuditLogTableRowPr } /> - - - ), -})); - -const TRANSLATIONS: Record = { - com_a11y_pagination: 'Pagination', - com_a11y_previous_page: 'Previous page', - com_a11y_next_page: 'Next page', - com_a11y_page_n: 'Page {{page}}', -}; - -vi.mock('@/hooks/useLocalize', () => ({ - default: () => (key: string, opts?: Record) => { - let str = TRANSLATIONS[key] ?? key; - if (opts) for (const [k, v] of Object.entries(opts)) str = str.replace(`{{${k}}}`, v); - return str; - }, - useLocalize: () => (key: string, opts?: Record) => { - let str = TRANSLATIONS[key] ?? key; - if (opts) for (const [k, v] of Object.entries(opts)) str = str.replace(`{{${k}}}`, v); - return str; - }, -})); +/** A pager that turns pages the way its callers do: the page lives in their state. */ +function Pager({ initialPage, totalPages }: { initialPage: number; totalPages: number }) { + const [page, setPage] = useState(initialPage); + return ; +} describe('getPageNumbers', () => { it('returns all pages when total <= 7', () => { @@ -81,12 +52,65 @@ describe('Pagination', () => { expect(container.innerHTML).toBe(''); }); - it('renders CUI Pagination when totalPages > 1', () => { - const { getByTestId } = render( - {}} />, + it('names its controls in words, not by their icons or the page number', () => { + render( {}} />); + const nav = screen.getByRole('navigation', { name: 'Pagination' }); + expect(nav).toContainElement(screen.getByRole('button', { name: 'Previous page' })); + expect(nav).toContainElement(screen.getByRole('button', { name: 'Next page' })); + expect(screen.getByRole('spinbutton', { name: 'Page 2 of 5' })).toHaveValue(2); + expect(nav).toHaveTextContent('of 5'); + expect(screen.queryByRole('button', { name: /chevron/ })).toBeNull(); + }); + + it('turns pages with Previous, Next and the page field', () => { + const onPageChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: 'Previous page' })); + fireEvent.click(screen.getByRole('button', { name: 'Next page' })); + const field = screen.getByRole('spinbutton', { name: 'Page 2 of 5' }); + fireEvent.change(field, { target: { value: '4' } }); + fireEvent.change(field, { target: { value: '9' } }); + fireEvent.change(field, { target: { value: '0' } }); + expect(onPageChange.mock.calls).toEqual([[1], [3], [4]]); + }); + + it('disables Previous on the first page and Next on the last', () => { + const { rerender } = render( + {}} />, ); - const nav = getByTestId('cui-pagination'); - expect(nav).toHaveAttribute('data-current', '2'); - expect(nav).toHaveAttribute('data-total', '5'); + expect(screen.getByRole('button', { name: 'Previous page' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Next page' })).toBeEnabled(); + rerender( {}} />); + expect(screen.getByRole('button', { name: 'Previous page' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Next page' })).toBeDisabled(); + }); + + it('keeps focus on an enabled control when Next reaches the last page', () => { + render(); + const next = screen.getByRole('button', { name: 'Next page' }); + next.focus(); + fireEvent.click(next); + expect(screen.getByRole('spinbutton')).toHaveValue(3); + expect(next).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Previous page' })).toHaveFocus(); + }); + + it('keeps focus on an enabled control when Previous reaches the first page', () => { + render(); + const previous = screen.getByRole('button', { name: 'Previous page' }); + previous.focus(); + fireEvent.click(previous); + expect(screen.getByRole('spinbutton')).toHaveValue(1); + expect(previous).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Next page' })).toHaveFocus(); + }); + + it('leaves focus on Next while there are pages after the new one', () => { + render(); + const next = screen.getByRole('button', { name: 'Next page' }); + next.focus(); + fireEvent.click(next); + expect(next).toBeEnabled(); + expect(next).toHaveFocus(); }); }); diff --git a/src/components/shared/Pagination.tsx b/src/components/shared/Pagination.tsx index 09c2bd31..4ef215a0 100644 --- a/src/components/shared/Pagination.tsx +++ b/src/components/shared/Pagination.tsx @@ -1,5 +1,7 @@ -import { Pagination as CUIPagination } from '@clickhouse/click-ui'; +import { useLayoutEffect, useRef } from 'react'; +import { IconButton, NumberField } from '@clickhouse/click-ui'; import type * as t from '@/types'; +import { useLocalize } from '@/hooks'; export function getPageNumbers(current: number, total: number): (number | 'ellipsis')[] { if (total <= 7) { @@ -15,9 +17,79 @@ export function getPageNumbers(current: number, total: number): (number | 'ellip return pages; } +/** + * Previous, page number and Next, as click-ui's Pagination lays them out, with + * localized names (click-ui names its buttons by their icons and the field by + * its value). Pressing Previous on the second page or Next on the one before + * last disables that button, which would drop focus on ``, so focus + * moves to the other button. + */ export function Pagination({ currentPage, totalPages, onPageChange }: t.PaginationProps) { + const localize = useLocalize(); + const previousRef = useRef(null); + const nextRef = useRef(null); + const pressedRef = useRef(null); + const atFirst = currentPage <= 1; + const atLast = currentPage >= totalPages; + + useLayoutEffect(() => { + const pressed = pressedRef.current; + pressedRef.current = null; + if (!pressed?.disabled) return; + const other = pressed === previousRef.current ? nextRef.current : previousRef.current; + other?.focus(); + }, [currentPage, totalPages]); + if (totalPages <= 1) return null; + + const goTo = (page: number, pressed: HTMLButtonElement | null) => { + pressedRef.current = pressed; + onPageChange(page); + }; + + const handlePageInput = (value: string) => { + const page = parseInt(value, 10); + if (Number.isNaN(page) || page < 1 || page > totalPages) return; + onPageChange(page); + }; + return ( - + ); } diff --git a/src/components/shared/SearchInput.tsx b/src/components/shared/SearchInput.tsx index 1b133faa..d8640cfa 100644 --- a/src/components/shared/SearchInput.tsx +++ b/src/components/shared/SearchInput.tsx @@ -7,10 +7,12 @@ export function SearchInput({ placeholder, className, ariaLabel, + inputRef, }: t.SearchInputProps) { return (
) => { event.preventDefault(); onClick(); @@ -16,6 +22,7 @@ export function TrashButton({ onClick, ariaLabel, size = 'sm', disabled }: t.Tra return (