Conversation
There was a problem hiding this comment.
Pull request overview
This pull request aims to fix accessibility issues and update to React 19. The PR adds comprehensive accessibility improvements across numerous UI components and introduces a testing infrastructure with Jest and jest-axe for automated accessibility testing.
Changes:
- Added Jest testing infrastructure with accessibility testing using jest-axe
- Added aria-label attributes to interactive elements (buttons, inputs, sliders, progress bars)
- Added alt text to Avatar and Image components throughout the codebase
- Added test files for all components with accessibility validation
Reviewed changes
Copilot reviewed 195 out of 197 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated dependencies for React 19, added testing libraries (Jest, Testing Library, jest-axe) |
| jest.config.js, jest.setup.js | Jest configuration and setup for testing environment |
| test-utils/* | Custom render utilities and axe wrapper for accessibility testing |
| lib/*/**.tsx | Accessibility improvements: aria-labels, alt attributes, and semantic markup |
| lib/*/**.test.tsx | New test files with accessibility validation for all components |
| lib/*/**.module.css | Styling updates including new CSS variable usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {items.map((item) => ( | ||
| <Tabs.Panel value={item.key as string} key={item.key} /> | ||
| ))} |
There was a problem hiding this comment.
The items array contains React elements (Tabs.Tab components), not objects with a 'key' property. Attempting to access item.key will fail. The Tabs.Panel components should use the same tab value strings from the tabs array instead. This code will cause a runtime error.
| <Table.Th w={40} aria-label="Select all rows"> | ||
| <Checkbox | ||
| onChange={toggleAll} | ||
| checked={selection.length === data.length} | ||
| indeterminate={selection.length > 0 && selection.length !== data.length} | ||
| aria-label="Select all rows" |
There was a problem hiding this comment.
The aria-label "Select all rows" is specified twice - once on the Table.Th element and once on the Checkbox itself. This creates redundant ARIA labels. Remove the aria-label from the Table.Th element as the Checkbox already has the appropriate label.
| <Checkbox | ||
| checked={value} | ||
| onChange={() => {}} | ||
| tabIndex={-1} | ||
| onChange={(event) => onChange(event.currentTarget.checked)} | ||
| size="md" | ||
| mr="xl" | ||
| styles={{ input: { cursor: 'pointer' } }} | ||
| aria-hidden | ||
| /> |
There was a problem hiding this comment.
The Checkbox is missing an accessible label. While the component is wrapped in a label element, the Checkbox itself should have a label prop (e.g., label="@mantine/core") or an aria-label to be properly accessible to screen readers.
test-utils/render.tsx
Outdated
| export async function renderWithAct(ui: React.ReactElement) { | ||
| return render(ui); | ||
| } |
There was a problem hiding this comment.
The renderWithAct function is declared as async but doesn't perform any asynchronous operations. With React 19, the act() wrapper is no longer needed in most cases as updates are handled automatically. This function should either remove the async keyword or properly handle async rendering if that's the intent.
1 participant
No description provided.