From 2ac818f4be61cd2d3c644da718cb19e171db4ba9 Mon Sep 17 00:00:00 2001 From: Jacobo Dominguez Date: Tue, 3 Mar 2026 02:05:52 -0700 Subject: [PATCH] fix: a11y for screen readers on roles and permissions tabs (#69) --- .../components/PermissionTable.test.tsx | 239 ++++++++++++++++++ .../components/PermissionTable.tsx | 107 +++++--- .../components/RoleCard/PermissionsRow.tsx | 63 +++-- .../components/RoleCard/index.tsx | 31 ++- .../components/RoleCard/messages.ts | 20 ++ src/authz-module/components/messages.ts | 16 ++ 6 files changed, 400 insertions(+), 76 deletions(-) create mode 100644 src/authz-module/components/PermissionTable.test.tsx create mode 100644 src/authz-module/components/messages.ts diff --git a/src/authz-module/components/PermissionTable.test.tsx b/src/authz-module/components/PermissionTable.test.tsx new file mode 100644 index 0000000..8d222db --- /dev/null +++ b/src/authz-module/components/PermissionTable.test.tsx @@ -0,0 +1,239 @@ +import { screen } from '@testing-library/react'; +import { Role, PermissionsResourceGrouped } from '@src/types'; +import { renderWrapper } from '@src/setupTest'; +import PermissionTable from './PermissionTable'; + +const mockRoles: Role[] = [ + { + name: 'Admin', + description: 'Administrator role', + userCount: 0, + permissions: [], + role: '', + }, + { + name: 'Editor', + description: 'Editor role', + userCount: 0, + permissions: [], + role: '', + }, + { + name: 'Viewer', + description: 'Viewer role', + userCount: 0, + permissions: [], + role: '', + }, +]; + +const mockPermissionsTable: PermissionsResourceGrouped[] = [ + { + key: 'users', + label: 'User Management', + description: 'Manage user accounts', + permissions: [ + { + key: 'users.read', + resource: 'users', + label: 'View Users', + actionKey: 'read', + roles: { + Admin: true, + Editor: true, + Viewer: true, + }, + }, + { + key: 'users.write', + resource: 'users', + label: 'Edit Users', + actionKey: 'write', + roles: { + Admin: true, + Editor: true, + Viewer: false, + }, + }, + ], + }, + { + key: 'courses', + label: 'Course Management', + description: 'Manage courses', + permissions: [ + { + key: 'courses.delete', + resource: 'courses', + label: 'Delete Courses', + actionKey: 'delete', + roles: { + Admin: true, + Editor: false, + Viewer: false, + }, + }, + ], + }, +]; + +describe('PermissionTable', () => { + it('renders within a Card component', () => { + renderWrapper(); + + expect(document.querySelector('.card')).toBeInTheDocument(); + }); + + it('renders table with correct class', () => { + renderWrapper(); + + const table = screen.getByRole('table'); + expect(table).toHaveClass('permission-table', 'w-100'); + }); + + it('renders table headers for all roles', () => { + renderWrapper(); + + mockRoles.forEach(role => { + expect(screen.getByRole('columnheader', { name: role.name })).toBeInTheDocument(); + }); + }); + + it('applies correct classes to role headers', () => { + renderWrapper(); + + mockRoles.forEach(role => { + const header = screen.getByRole('columnheader', { name: role.name }); + expect(header).toHaveClass('text-center', 'py-3'); + }); + }); + + it('renders resource group headers', () => { + renderWrapper(); + + expect(screen.getByText('User Management')).toBeInTheDocument(); + expect(screen.getByText('Course Management')).toBeInTheDocument(); + }); + + it('applies correct classes to resource group headers', () => { + const { container } = renderWrapper(); + + const resourceRows = container.querySelectorAll('.bg-info-100.text-primary'); + expect(resourceRows).toHaveLength(2); + }); + + it('renders resource group headers with correct colspan', () => { + const { container } = renderWrapper(); + + const resourceCells = container.querySelectorAll('td[colspan]'); + resourceCells.forEach(cell => { + expect(cell).toHaveAttribute('colspan', '4'); + }); + }); + + it('renders permission labels with icons', () => { + renderWrapper(); + + expect(screen.getByText('View Users')).toBeInTheDocument(); + expect(screen.getByText('Edit Users')).toBeInTheDocument(); + expect(screen.getByText('Delete Courses')).toBeInTheDocument(); + }); + + it('applies correct classes to permission label cells', () => { + const { container } = renderWrapper(); + + const labelCells = container.querySelectorAll('td.text-start.d-flex'); + labelCells.forEach(cell => { + expect(cell).toHaveClass('align-items-center', 'small', 'px-4', 'py-3'); + }); + }); + + it('renders permission row borders', () => { + const { container } = renderWrapper(); + + const borderRows = container.querySelectorAll('tr.border-top'); + expect(borderRows).toHaveLength(3); + }); + + it('renders Check icons for granted permissions', () => { + renderWrapper(); + + const grantedIcons = screen.getAllByLabelText(/Permission granted in/); + expect(grantedIcons.length).toBeGreaterThan(0); + }); + + it('renders Close icons for denied permissions', () => { + renderWrapper(); + + const deniedIcons = screen.getAllByLabelText(/Permission denied in/); + expect(deniedIcons.length).toBeGreaterThan(0); + }); + + it('applies text-danger class to denied permission icons', () => { + renderWrapper(); + + const deniedIcons = screen.getAllByLabelText(/Permission denied in/); + deniedIcons.forEach(icon => { + expect(icon).toHaveClass('text-danger'); + }); + }); + + it('applies correct classes to granted permission icons', () => { + renderWrapper(); + + const grantedIcons = screen.getAllByLabelText(/Permission granted in/); + grantedIcons.forEach(icon => { + expect(icon).toHaveClass('d-inline-block'); + expect(icon).not.toHaveClass('text-danger'); + }); + }); + + it('centers permission status cells', () => { + const { container } = renderWrapper(); + + const statusCells = container.querySelectorAll('tbody td.text-center'); + expect(statusCells.length).toBeGreaterThan(0); + }); + + it('renders correct aria-labels for granted permissions', () => { + renderWrapper(); + + mockRoles.forEach(role => { + const grantedLabel = `Permission granted in ${role.name} role`; + const icons = screen.queryAllByLabelText(grantedLabel); + expect(icons.length).toBeGreaterThan(0); + }); + }); + + it('renders correct aria-labels for denied permissions', () => { + renderWrapper(); + + const deniedLabel = 'Permission denied in Viewer role'; + expect(screen.getAllByLabelText(deniedLabel)).toHaveLength(2); + }); + + it('handles empty roles array', () => { + renderWrapper(); + + expect(screen.getByRole('table')).toBeInTheDocument(); + expect(screen.getByText('User Management')).toBeInTheDocument(); + }); + + it('handles empty permissions table', () => { + renderWrapper(); + + expect(screen.getByRole('table')).toBeInTheDocument(); + mockRoles.forEach(role => { + expect(screen.getByText(role.name)).toBeInTheDocument(); + }); + }); + + it('applies correct margin to permission icons', () => { + const { container } = renderWrapper(); + + const permissionIcons = container.querySelectorAll('td.text-start .paragon-icon'); + permissionIcons.forEach(icon => { + expect(icon).toHaveClass('d-inline-block', 'mr-2'); + }); + }); +}); diff --git a/src/authz-module/components/PermissionTable.tsx b/src/authz-module/components/PermissionTable.tsx index 26a90ef..bb590a4 100644 --- a/src/authz-module/components/PermissionTable.tsx +++ b/src/authz-module/components/PermissionTable.tsx @@ -1,52 +1,83 @@ +import { useIntl } from '@edx/frontend-platform/i18n'; import { Check, Close } from '@openedx/paragon/icons'; import { Card, Icon } from '@openedx/paragon'; import { PermissionsResourceGrouped, Role } from '@src/types'; import { actionsDictionary } from './RoleCard/constants'; import ResourceTooltip from './ResourceTooltip'; +import messages from './messages'; type PermissionTableProps = { roles: Role[]; permissionsTable: PermissionsResourceGrouped[]; }; -const PermissionTable = ({ permissionsTable, roles }: PermissionTableProps) => ( - - - - - - ))} - - - - {permissionsTable.map(resourceGroup => ( - <> - - - - {resourceGroup.permissions.map(permission => ( - - - {roles.map(role => ( - - ))} - +const PermissionTable = ({ permissionsTable, roles }: PermissionTableProps) => { + const { formatMessage } = useIntl(); + return ( + +
{role.name}
- {resourceGroup.label} - -
- - {permission.label} - - {permission.roles[role.name] ? : } -
+ + + ))} - - ))} - -
{role.name}
-
-); + + + + {permissionsTable.map(resourceGroup => ( + <> + + + {resourceGroup.label} + + + + {resourceGroup.permissions.map(permission => ( + + + + {permission.label} + + {roles.map(role => ( + + { + permission.roles[role.name] + ? ( + + ) + : ( + + ) +} + + ))} + + ))} + + ))} + + + + ); +}; export default PermissionTable; diff --git a/src/authz-module/components/RoleCard/PermissionsRow.tsx b/src/authz-module/components/RoleCard/PermissionsRow.tsx index 09f3a16..84ec499 100644 --- a/src/authz-module/components/RoleCard/PermissionsRow.tsx +++ b/src/authz-module/components/RoleCard/PermissionsRow.tsx @@ -1,41 +1,50 @@ import { ComponentType } from 'react'; +import { useIntl } from '@edx/frontend-platform/i18n'; import { Chip, Col, Row, } from '@openedx/paragon'; import { RoleResourceGroup } from '@src/types'; import { actionsDictionary, ActionKey } from './constants'; import ResourceTooltip from '../ResourceTooltip'; +import messages from './messages'; type PermissionRowProps = { resource: RoleResourceGroup; }; -const PermissionRow = ({ resource }: PermissionRowProps) => ( - - - {resource.label} - - - -
- {resource.permissions.map((action, index) => ( - <> - - {action.label} - - {(index === resource.permissions.length - 1) ? null - : (
)} - - ))} -
- -
-); +const PermissionRow = ({ resource }: PermissionRowProps) => { + const { formatMessage } = useIntl(); + return ( + + + {resource.label} + + + +
+ {resource.permissions.map((action, index) => ( + <> + + {action.label} + + {(index === resource.permissions.length - 1) ? null + : (
)} + + ))} +
+ +
+ ); +}; export default PermissionRow; diff --git a/src/authz-module/components/RoleCard/index.tsx b/src/authz-module/components/RoleCard/index.tsx index 2da8c3d..59b3ac2 100644 --- a/src/authz-module/components/RoleCard/index.tsx +++ b/src/authz-module/components/RoleCard/index.tsx @@ -18,17 +18,26 @@ interface RoleCardProps extends CardTitleProps { permissionsByResource: any[]; } -const CardTitle = ({ title, userCounter = null }: CardTitleProps) => ( -
- {title} - {userCounter !== null && ( - - - {userCounter} - - )} -
-); +const CardTitle = ({ title, userCounter = null }: CardTitleProps) => { + const { formatMessage } = useIntl(); + + return ( +
+ {title} + {userCounter !== null && ( + + + {userCounter} + + )} +
+ ); +}; const RoleCard = ({ title, objectName, description, handleDelete, permissionsByResource, userCounter, diff --git a/src/authz-module/components/RoleCard/messages.ts b/src/authz-module/components/RoleCard/messages.ts index 950a83e..78ff7a2 100644 --- a/src/authz-module/components/RoleCard/messages.ts +++ b/src/authz-module/components/RoleCard/messages.ts @@ -51,6 +51,26 @@ const messages = defineMessages({ defaultMessage: 'Delete role action', description: 'Alt description for delete button', }, + 'authz.role.card.userCounter': { + id: 'authz.role.card.userCounter', + defaultMessage: 'Number of users with this role', + description: 'Screen reader text for the user counter icon in the role card header', + }, + 'authz.role.card.permissions.ariaLabel': { + id: 'authz.role.card.permissions.ariaLabel', + defaultMessage: '{permissionName} permission is {permissionStatus}', + description: 'Aria label for permission chips in the role card', + }, + 'authz.role.card.permissions.status.denied': { + id: 'authz.role.card.permissions.status.denied', + defaultMessage: 'denied', + description: 'Label for denied status of a permission in the role card', + }, + 'authz.role.card.permissions.status.granted': { + id: 'authz.role.card.permissions.status.granted', + defaultMessage: 'granted', + description: 'Label for granted status of a permission in the role card', + }, }); export default messages; diff --git a/src/authz-module/components/messages.ts b/src/authz-module/components/messages.ts new file mode 100644 index 0000000..f36898c --- /dev/null +++ b/src/authz-module/components/messages.ts @@ -0,0 +1,16 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + 'authz.role.card.permission.for.role.status.granted': { + id: 'authz.role.card.permission.for.role.status.granted', + defaultMessage: 'Permission granted in {roleName} role', + description: 'Label for granted status of a permission in the permissions table', + }, + 'authz.role.card.permission.for.role.status.denied': { + id: 'authz.role.card.permission.for.role.status.denied', + defaultMessage: 'Permission denied in {roleName} role', + description: 'Label for denied status of a permission in the permissions table', + }, +}); + +export default messages;