fix(a11y): remove nested-interactive role='link' on spoon/thread lists
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
'use client';
|
||||
|
||||
import Link from 'next/link';
|
||||
import { useRouter } from 'next/navigation';
|
||||
import { SpoonStatusBadge } from '@/components/spoons/spoon-status-badge';
|
||||
import { ListSkeleton } from '@/components/ui/list-skeleton';
|
||||
import { useQuery } from 'convex/react';
|
||||
@@ -33,7 +32,6 @@ const formatDate = (value?: number) =>
|
||||
: 'Never';
|
||||
|
||||
const SpoonsPage = () => {
|
||||
const router = useRouter();
|
||||
const spoons = useQuery(api.spoons.listMineWithState, {});
|
||||
const threads = useQuery(api.threads.listMine, { limit: 100 });
|
||||
const spoonList = spoons ?? [];
|
||||
@@ -131,22 +129,12 @@ const SpoonsPage = () => {
|
||||
return (
|
||||
<TableRow
|
||||
key={spoon._id}
|
||||
role='link'
|
||||
tabIndex={0}
|
||||
className='hover:bg-muted/50 cursor-pointer'
|
||||
onClick={() => router.push(href)}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === 'Enter' || event.key === ' ') {
|
||||
event.preventDefault();
|
||||
router.push(href);
|
||||
}
|
||||
}}
|
||||
className='hover:bg-muted/50 relative cursor-pointer'
|
||||
>
|
||||
<TableCell className='pl-4'>
|
||||
<Link
|
||||
href={href}
|
||||
className='group inline-flex min-w-0 flex-col'
|
||||
onClick={(event) => event.stopPropagation()}
|
||||
className='group inline-flex min-w-0 flex-col after:absolute after:inset-0 after:content-[""]'
|
||||
>
|
||||
<span className='group-hover:text-primary font-medium transition-colors'>
|
||||
{spoon.name}
|
||||
@@ -190,10 +178,7 @@ const SpoonsPage = () => {
|
||||
<TableCell>{formatDate(spoon.lastCheckedAt)}</TableCell>
|
||||
<TableCell className='pr-4 text-right'>
|
||||
<Button size='sm' variant='outline' asChild>
|
||||
<Link
|
||||
href={href}
|
||||
onClick={(event) => event.stopPropagation()}
|
||||
>
|
||||
<Link href={href} className='relative z-10'>
|
||||
Open
|
||||
<ArrowUpRight className='size-3' />
|
||||
</Link>
|
||||
|
||||
@@ -319,21 +319,19 @@ const ThreadsPage = () => {
|
||||
visibleThreads.map((thread) => (
|
||||
<Card
|
||||
key={thread._id}
|
||||
role='link'
|
||||
tabIndex={0}
|
||||
className='hover:border-primary/50 cursor-pointer shadow-none transition-colors'
|
||||
onClick={() => router.push(threadTarget(thread))}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === 'Enter' || event.key === ' ') {
|
||||
event.preventDefault();
|
||||
router.push(threadTarget(thread));
|
||||
}
|
||||
}}
|
||||
className='hover:border-primary/50 relative cursor-pointer shadow-none transition-colors'
|
||||
>
|
||||
<CardContent className='grid gap-3 p-4 md:grid-cols-[1fr_auto] md:items-center'>
|
||||
<div className='min-w-0'>
|
||||
<div className='flex flex-wrap items-center gap-2'>
|
||||
<h2 className='truncate font-medium'>{thread.title}</h2>
|
||||
<h2 className='truncate font-medium'>
|
||||
<Link
|
||||
href={threadTarget(thread)}
|
||||
className='after:absolute after:inset-0 after:content-[""]'
|
||||
>
|
||||
{thread.title}
|
||||
</Link>
|
||||
</h2>
|
||||
{thread.spoonName ? (
|
||||
<Badge variant='outline'>{thread.spoonName}</Badge>
|
||||
) : null}
|
||||
@@ -364,15 +362,10 @@ const ThreadsPage = () => {
|
||||
{thread.latestJobWorkspaceStatus.replaceAll('_', ' ')}
|
||||
</p>
|
||||
) : null}
|
||||
<div className='mt-2 flex justify-start gap-2 md:justify-end'>
|
||||
<div className='relative z-10 mt-2 flex justify-start gap-2 md:justify-end'>
|
||||
{thread.latestAgentJobId ? (
|
||||
<Button size='sm' variant='outline' asChild>
|
||||
<Link
|
||||
href={threadTarget(thread)}
|
||||
onClick={(event) => event.stopPropagation()}
|
||||
>
|
||||
Open workspace
|
||||
</Link>
|
||||
<Link href={threadTarget(thread)}>Open workspace</Link>
|
||||
</Button>
|
||||
) : null}
|
||||
{thread.latestJobPullRequestUrl ? (
|
||||
@@ -381,7 +374,6 @@ const ThreadsPage = () => {
|
||||
href={thread.latestJobPullRequestUrl}
|
||||
target='_blank'
|
||||
rel='noreferrer'
|
||||
onClick={(event) => event.stopPropagation()}
|
||||
>
|
||||
PR
|
||||
</a>
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
import { cleanup, render, screen, within } from '@testing-library/react';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import SpoonsPage from '../../src/app/(app)/spoons/page';
|
||||
|
||||
const { mockUseQuery, mockPush } = vi.hoisted(() => ({
|
||||
mockUseQuery: vi.fn(),
|
||||
mockPush: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('convex/react', () => ({
|
||||
useConvexAuth: () => ({ isAuthenticated: false }),
|
||||
useMutation: vi.fn(),
|
||||
useQuery: mockUseQuery,
|
||||
}));
|
||||
|
||||
vi.mock('next/navigation', () => ({
|
||||
useParams: vi.fn(),
|
||||
useRouter: () => ({ push: mockPush, replace: vi.fn() }),
|
||||
useSearchParams: () => new URLSearchParams(),
|
||||
}));
|
||||
|
||||
const spoon = {
|
||||
_id: 'spoon-1',
|
||||
name: 'useSend',
|
||||
upstreamOwner: 'acme',
|
||||
upstreamRepo: 'app',
|
||||
forkOwner: 'me',
|
||||
forkRepo: 'app',
|
||||
status: 'active',
|
||||
syncStatus: undefined,
|
||||
syncCadence: 'daily',
|
||||
effectiveUpstreamAheadBy: 0,
|
||||
rawUpstreamAheadBy: 0,
|
||||
forkAheadBy: 0,
|
||||
ignoredUpstreamCount: 0,
|
||||
lastCheckedAt: undefined,
|
||||
};
|
||||
|
||||
describe('spoon list accessibility', () => {
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
mockUseQuery.mockReset();
|
||||
mockPush.mockReset();
|
||||
});
|
||||
|
||||
it('does not nest interactive elements inside a role="link" container', () => {
|
||||
mockUseQuery.mockImplementation((_query, args) => {
|
||||
// threads list passes a limit; spoons list passes {}.
|
||||
if (args && 'limit' in args) return [];
|
||||
return [spoon];
|
||||
});
|
||||
|
||||
const { container } = render(<SpoonsPage />);
|
||||
|
||||
// Invalid ARIA nested-interactive pattern must be gone.
|
||||
expect(container.querySelector('[role="link"]')).toBeNull();
|
||||
});
|
||||
|
||||
it('keeps a primary navigation link and a secondary "Open" link to the spoon detail', () => {
|
||||
mockUseQuery.mockImplementation((_query, args) => {
|
||||
if (args && 'limit' in args) return [];
|
||||
return [spoon];
|
||||
});
|
||||
|
||||
render(<SpoonsPage />);
|
||||
|
||||
const href = '/spoons/spoon-1';
|
||||
|
||||
// Primary link: the spoon name navigates to the detail route.
|
||||
const nameLink = screen.getByRole('link', { name: /useSend/ });
|
||||
expect(nameLink).toHaveAttribute('href', href);
|
||||
|
||||
// Secondary action: the "Open" link still exists and points to the detail.
|
||||
const openLink = screen.getByRole('link', { name: /open/i });
|
||||
expect(openLink).toHaveAttribute('href', href);
|
||||
|
||||
// Both real anchors point at the destination; no simulated role="link".
|
||||
const links = screen.getAllByRole('link');
|
||||
for (const link of links.filter((el) => within(el).queryByText('useSend'))) {
|
||||
expect(link).toHaveAttribute('href', href);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user