mirror of
https://github.com/vernu/textbee.git
synced 2026-07-30 08:57:13 -04:00
fix: repair form labelling and navigation landmarks
None of the three password fields declared autoComplete, so password managers could not tell them apart and would fill the saved password into the wrong box. They now declare current-password and new-password. The delete-account dialog, the most destructive action in the app, had a label whose htmlFor pointed at an id the textarea never had, and an email confirmation field with no label at all. Both are now wired, and the controls moved out of DialogDescription, which Radix targets with aria-describedby: opening the dialog previously read the labels and placeholders back as one long description string. Validation errors are now tied to their inputs via aria-describedby and announced with role=alert instead of appearing silently. confirmPassword required 4 characters while newPassword required 8, so a five-character confirmation reported "Passwords must match" rather than the actual length problem. Five icon-only buttons had no accessible name: copy API key, copy device ID, the API key menu, and both promo code copy buttons. Four of the five nav landmarks were unlabelled, leaving them indistinguishable in a screen reader's landmark list. Added a skip-to-content link, since keyboard users otherwise tab the whole sidebar on every page. The delete-account label guard was confirmed to fail against the previous markup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -236,7 +236,12 @@ export default function ApiKeys() {
|
||||
<div className=''>
|
||||
<DropdownMenu>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button variant='ghost' size='icon' className='h-6 w-6'>
|
||||
<Button
|
||||
variant='ghost'
|
||||
size='icon'
|
||||
className='h-6 w-6'
|
||||
aria-label='API key actions'
|
||||
>
|
||||
<MoreVertical className='h-3 w-3' />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
|
||||
@@ -117,7 +117,7 @@ export default function BlackFridayModal() {
|
||||
</div>
|
||||
<div className="flex items-center gap-2 bg-background p-2 rounded border border-dashed border-primary/50">
|
||||
<code className="flex-1 font-mono font-bold text-center text-primary text-sm">BLACKFRIDAY40</code>
|
||||
<Button size="icon" variant="ghost" className="h-8 w-8" onClick={() => copyCode('BLACKFRIDAY40')}>
|
||||
<Button size="icon" variant="ghost" className="h-8 w-8" aria-label="Copy code BLACKFRIDAY40" title="Copy code BLACKFRIDAY40" onClick={() => copyCode('BLACKFRIDAY40')}>
|
||||
{copiedCode === 'BLACKFRIDAY40' ? <Check className="h-4 w-4 text-green-500" /> : <Copy className="h-4 w-4" />}
|
||||
</Button>
|
||||
</div>
|
||||
@@ -139,7 +139,7 @@ export default function BlackFridayModal() {
|
||||
</div>
|
||||
<div className="flex items-center gap-2 bg-background p-2 rounded border border-dashed border-primary/50">
|
||||
<code className="flex-1 font-mono font-bold text-center text-primary text-sm">BLACKFRIDAY50</code>
|
||||
<Button size="icon" variant="ghost" className="h-8 w-8" onClick={() => copyCode('BLACKFRIDAY50')}>
|
||||
<Button size="icon" variant="ghost" className="h-8 w-8" aria-label="Copy code BLACKFRIDAY50" title="Copy code BLACKFRIDAY50" onClick={() => copyCode('BLACKFRIDAY50')}>
|
||||
{copiedCode === 'BLACKFRIDAY50' ? <Check className="h-4 w-4 text-green-500" /> : <Copy className="h-4 w-4" />}
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
@@ -20,9 +20,11 @@ const changePasswordSchema = z
|
||||
newPassword: z
|
||||
.string()
|
||||
.min(8, { message: 'Password must be at least 8 characters long' }),
|
||||
// Matches newPassword's floor. At 4 a five-character confirmation failed
|
||||
// with "Passwords must match" instead of the real length problem.
|
||||
confirmPassword: z
|
||||
.string()
|
||||
.min(4, { message: 'Please confirm your password' }),
|
||||
.min(1, { message: 'Please confirm your password' }),
|
||||
})
|
||||
.superRefine((data, ctx) => {
|
||||
if (data.newPassword !== data.confirmPassword) {
|
||||
@@ -84,14 +86,30 @@ export default function ChangePasswordForm() {
|
||||
>
|
||||
<div className='space-y-2'>
|
||||
<Label htmlFor='oldPassword'>Old Password</Label>
|
||||
{/* Without autoComplete, password managers cannot tell these three
|
||||
boxes apart and fill the saved password into the wrong one. */}
|
||||
<Input
|
||||
id='oldPassword'
|
||||
type='password'
|
||||
autoComplete='current-password'
|
||||
aria-invalid={
|
||||
Boolean(changePasswordForm.formState.errors.oldPassword) ||
|
||||
undefined
|
||||
}
|
||||
aria-describedby={
|
||||
changePasswordForm.formState.errors.oldPassword
|
||||
? 'oldPassword-error'
|
||||
: undefined
|
||||
}
|
||||
{...changePasswordForm.register('oldPassword')}
|
||||
placeholder='Enter your old password'
|
||||
/>
|
||||
{changePasswordForm.formState.errors.oldPassword && (
|
||||
<p className='text-sm text-destructive'>
|
||||
<p
|
||||
id='oldPassword-error'
|
||||
role='alert'
|
||||
className='text-sm text-destructive'
|
||||
>
|
||||
{changePasswordForm.formState.errors.oldPassword.message}
|
||||
</p>
|
||||
)}
|
||||
@@ -102,11 +120,25 @@ export default function ChangePasswordForm() {
|
||||
<Input
|
||||
id='newPassword'
|
||||
type='password'
|
||||
autoComplete='new-password'
|
||||
aria-invalid={
|
||||
Boolean(changePasswordForm.formState.errors.newPassword) ||
|
||||
undefined
|
||||
}
|
||||
aria-describedby={
|
||||
changePasswordForm.formState.errors.newPassword
|
||||
? 'newPassword-error'
|
||||
: undefined
|
||||
}
|
||||
{...changePasswordForm.register('newPassword')}
|
||||
placeholder='Enter your new password'
|
||||
/>
|
||||
{changePasswordForm.formState.errors.newPassword && (
|
||||
<p className='text-sm text-destructive'>
|
||||
<p
|
||||
id='newPassword-error'
|
||||
role='alert'
|
||||
className='text-sm text-destructive'
|
||||
>
|
||||
{changePasswordForm.formState.errors.newPassword.message}
|
||||
</p>
|
||||
)}
|
||||
@@ -117,11 +149,25 @@ export default function ChangePasswordForm() {
|
||||
<Input
|
||||
id='confirmPassword'
|
||||
type='password'
|
||||
autoComplete='new-password'
|
||||
aria-invalid={
|
||||
Boolean(changePasswordForm.formState.errors.confirmPassword) ||
|
||||
undefined
|
||||
}
|
||||
aria-describedby={
|
||||
changePasswordForm.formState.errors.confirmPassword
|
||||
? 'confirmPassword-error'
|
||||
: undefined
|
||||
}
|
||||
{...changePasswordForm.register('confirmPassword')}
|
||||
placeholder='Enter your confirm password'
|
||||
placeholder='Re-enter your new password'
|
||||
/>
|
||||
{changePasswordForm.formState.errors.confirmPassword && (
|
||||
<p className='text-sm text-destructive'>
|
||||
<p
|
||||
id='confirmPassword-error'
|
||||
role='alert'
|
||||
className='text-sm text-destructive'
|
||||
>
|
||||
{changePasswordForm.formState.errors.confirmPassword.message}
|
||||
</p>
|
||||
)}
|
||||
|
||||
@@ -120,60 +120,77 @@ export default function DeleteAccountForm() {
|
||||
<AlertTriangle className='h-5 w-5 text-destructive' />
|
||||
Delete Account
|
||||
</DialogTitle>
|
||||
{/* Description holds prose only. The form controls used to live
|
||||
inside it, and Radix points the dialog's aria-describedby here,
|
||||
so opening the dialog read the labels, placeholders and all
|
||||
back as one long description string. */}
|
||||
<DialogDescription className='pt-4'>
|
||||
<p className='mb-4'>
|
||||
Are you sure you want to delete your account? This action:
|
||||
</p>
|
||||
<ul className='list-disc list-inside space-y-2 mb-4'>
|
||||
<ul className='list-disc list-inside space-y-2'>
|
||||
<li>Cannot be undone</li>
|
||||
<li>Will permanently delete all your data</li>
|
||||
<li>Will cancel all active subscriptions</li>
|
||||
<li>Will remove access to all services</li>
|
||||
</ul>
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
|
||||
<div className='space-y-4'>
|
||||
<div className='space-y-2'>
|
||||
<Label htmlFor='deleteReason'>Reason for deletion</Label>
|
||||
{/* htmlFor pointed at an id this field never had, so the label
|
||||
was decorative and the textarea was announced unlabelled. */}
|
||||
<Textarea
|
||||
className='my-2'
|
||||
id='deleteReason'
|
||||
placeholder='Enter your reason for deletion'
|
||||
value={deleteReason}
|
||||
onChange={(e) => setDeleteReason(e.target.value)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
<p>Please type your email address to confirm:</p>
|
||||
|
||||
<div className='space-y-2'>
|
||||
<Label htmlFor='deleteConfirmEmail'>
|
||||
Type your email address to confirm
|
||||
</Label>
|
||||
<Input
|
||||
className='mt-2'
|
||||
id='deleteConfirmEmail'
|
||||
type='email'
|
||||
autoComplete='off'
|
||||
placeholder='Enter your email address'
|
||||
value={deleteConfirmEmail}
|
||||
onChange={(e) => setDeleteConfirmEmail(e.target.value)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
<div className='mt-4 space-y-2'>
|
||||
<div
|
||||
ref={turnstileRef}
|
||||
className='min-h-[65px] w-full flex justify-center'
|
||||
/>
|
||||
{turnstileError && (
|
||||
<p className='text-sm text-destructive'>{turnstileError}</p>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{requestAccountDeletionError && (
|
||||
<p className='text-sm text-destructive'>
|
||||
{(requestAccountDeletionError as any).response?.data
|
||||
?.message ||
|
||||
requestAccountDeletionError.message ||
|
||||
'Failed to submit account deletion request'}
|
||||
<div className='space-y-2'>
|
||||
<div
|
||||
ref={turnstileRef}
|
||||
className='min-h-[65px] w-full flex justify-center'
|
||||
/>
|
||||
{turnstileError && (
|
||||
<p role='alert' className='text-sm text-destructive'>
|
||||
{turnstileError}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{requestAccountDeletionError && (
|
||||
<p role='alert' className='text-sm text-destructive'>
|
||||
{(requestAccountDeletionError as any).response?.data?.message ||
|
||||
requestAccountDeletionError.message ||
|
||||
'Failed to submit account deletion request'}
|
||||
</p>
|
||||
)}
|
||||
|
||||
{isRequestAccountDeletionSuccess && (
|
||||
<p role='status' className='text-sm text-green-500'>
|
||||
Account deletion request submitted
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{isRequestAccountDeletionSuccess && (
|
||||
<p className='text-sm text-green-500'>
|
||||
Account deletion request submitted
|
||||
</p>
|
||||
)}
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
<DialogFooter className='gap-2 sm:gap-0'>
|
||||
<Button
|
||||
variant='outline'
|
||||
|
||||
@@ -238,6 +238,8 @@ export default function DeviceList() {
|
||||
variant='ghost'
|
||||
size='icon'
|
||||
className='h-6 w-6'
|
||||
aria-label='Copy device ID'
|
||||
title='Copy device ID'
|
||||
onClick={() => handleCopyId(device._id)}
|
||||
>
|
||||
<Copy className='h-3 w-3' />
|
||||
|
||||
@@ -132,7 +132,13 @@ const GenerateApiKey = forwardRef<GenerateApiKeyHandle, GenerateApiKeyProps>(
|
||||
<code className='relative rounded bg-muted px-[0.5rem] py-[0.3rem] font-mono text-sm flex-1'>
|
||||
{generatedApiKey?.data}
|
||||
</code>
|
||||
<Button variant='outline' size='icon' onClick={handleCopyKey}>
|
||||
<Button
|
||||
variant='outline'
|
||||
size='icon'
|
||||
aria-label='Copy API key'
|
||||
title='Copy API key'
|
||||
onClick={handleCopyKey}
|
||||
>
|
||||
<Copy className='h-4 w-4' />
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
@@ -34,6 +34,14 @@ export default function DashboardLayout({
|
||||
|
||||
return (
|
||||
<div className='min-h-[calc(100vh-3.5rem)]'>
|
||||
{/* Visible only on focus. Without it, keyboard users tab through the
|
||||
whole sidebar on every page before reaching the content. */}
|
||||
<a
|
||||
href='#main-content'
|
||||
className='sr-only focus:not-sr-only focus:fixed focus:left-4 focus:top-4 focus:z-50 focus:rounded-md focus:bg-primary focus:px-4 focus:py-2 focus:text-primary-foreground focus:shadow-lg focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2'
|
||||
>
|
||||
Skip to content
|
||||
</a>
|
||||
<CommandMenu open={searchOpen} onOpenChange={setSearchOpen} />
|
||||
|
||||
{/* Desktop sidebar, sits below the sticky app header (h-14). */}
|
||||
@@ -42,7 +50,9 @@ export default function DashboardLayout({
|
||||
<div className='mb-4'>
|
||||
<SearchTrigger onOpen={() => setSearchOpen(true)} />
|
||||
</div>
|
||||
<nav className='flex flex-col gap-1'>
|
||||
{/* Distinct labels: several nav landmarks on one page are otherwise
|
||||
indistinguishable in a screen reader's landmark list. */}
|
||||
<nav className='flex flex-col gap-1' aria-label='Main'>
|
||||
{navItems.map((item) => (
|
||||
<SidebarLink
|
||||
key={item.href}
|
||||
@@ -84,7 +94,9 @@ export default function DashboardLayout({
|
||||
<AccountDeletionAlert />
|
||||
<UpgradeToProAlert />
|
||||
</div>
|
||||
<main>{children}</main>
|
||||
<main id='main-content' tabIndex={-1}>
|
||||
{children}
|
||||
</main>
|
||||
{/* Inside the sidebar-offset column so the fixed sidebar cannot paint
|
||||
over it, and padded clear of the fixed mobile tab bar. */}
|
||||
<div className='pb-20 pt-8 md:pb-0'>
|
||||
@@ -93,7 +105,10 @@ export default function DashboardLayout({
|
||||
</div>
|
||||
|
||||
{/* Mobile bottom tab bar (max 4 items; the rest are desktop/palette only). */}
|
||||
<nav className='fixed inset-x-0 bottom-0 z-40 border-t border-border bg-card/95 backdrop-blur supports-[backdrop-filter]:bg-card/80 md:hidden'>
|
||||
<nav
|
||||
aria-label='Primary (mobile)'
|
||||
className='fixed inset-x-0 bottom-0 z-40 border-t border-border bg-card/95 backdrop-blur supports-[backdrop-filter]:bg-card/80 md:hidden'
|
||||
>
|
||||
<div className='flex h-16 items-center justify-around'>
|
||||
{mobileNavItems.map((item) => (
|
||||
<MobileNavLink
|
||||
|
||||
@@ -78,7 +78,7 @@ export default function AppHeader({ session }: { session: Session }) {
|
||||
</Button>
|
||||
</SheetTrigger>
|
||||
<SheetContent side='right' className='w-[300px] sm:w-[400px]'>
|
||||
<nav className='flex flex-col gap-4'>
|
||||
<nav aria-label='Account' className='flex flex-col gap-4'>
|
||||
<Button asChild variant='ghost' className='justify-start'>
|
||||
<Link href={Routes.login}>Log in</Link>
|
||||
</Button>
|
||||
|
||||
@@ -40,7 +40,10 @@ export default function Footer() {
|
||||
|
||||
{/* Stacked on mobile: wrapped inline links produced a ragged two-line
|
||||
block that was hard to scan and gave small tap targets. */}
|
||||
<nav className='flex w-full flex-col items-start gap-3 sm:w-auto sm:flex-row sm:flex-wrap sm:items-center sm:justify-center sm:gap-x-5 sm:gap-y-2'>
|
||||
<nav
|
||||
aria-label='Footer'
|
||||
className='flex w-full flex-col items-start gap-3 sm:w-auto sm:flex-row sm:flex-wrap sm:items-center sm:justify-center sm:gap-x-5 sm:gap-y-2'
|
||||
>
|
||||
{links.map((link) => (
|
||||
<Link
|
||||
key={link.label}
|
||||
|
||||
@@ -20,6 +20,53 @@ test.describe('account settings (mocked API, no real backend)', () => {
|
||||
await expect(page.getByRole('heading', { name: 'Pro' })).toBeVisible()
|
||||
})
|
||||
|
||||
// Password managers key off autoComplete to tell the three password boxes
|
||||
// apart. Without it they fill the saved password into the wrong field.
|
||||
test('password fields declare their autocomplete roles', async ({
|
||||
page,
|
||||
context,
|
||||
}) => {
|
||||
await authenticate(context)
|
||||
await mockApi(page)
|
||||
await page.goto('/dashboard/account/security')
|
||||
|
||||
await expect(page.getByLabel('Old Password')).toHaveAttribute(
|
||||
'autocomplete',
|
||||
'current-password'
|
||||
)
|
||||
await expect(page.getByLabel('New Password')).toHaveAttribute(
|
||||
'autocomplete',
|
||||
'new-password'
|
||||
)
|
||||
await expect(page.getByLabel('Confirm Password')).toHaveAttribute(
|
||||
'autocomplete',
|
||||
'new-password'
|
||||
)
|
||||
})
|
||||
|
||||
// The most destructive action in the app had a label pointing at an id that
|
||||
// did not exist and an email field with no label at all.
|
||||
test('the delete-account fields are reachable by their labels', async ({
|
||||
page,
|
||||
context,
|
||||
}) => {
|
||||
await authenticate(context)
|
||||
await mockApi(page)
|
||||
await page.goto('/dashboard/account/security')
|
||||
|
||||
await page.getByRole('button', { name: 'Delete Account' }).click()
|
||||
const dialog = page.getByRole('dialog')
|
||||
|
||||
await dialog.getByLabel('Reason for deletion').fill('Testing labels')
|
||||
await dialog
|
||||
.getByLabel('Type your email address to confirm')
|
||||
.fill('test@example.com')
|
||||
|
||||
await expect(dialog.getByLabel('Reason for deletion')).toHaveValue(
|
||||
'Testing labels'
|
||||
)
|
||||
})
|
||||
|
||||
test('security deep link survives refresh and separates the danger zone', async ({
|
||||
page,
|
||||
context,
|
||||
|
||||
Reference in New Issue
Block a user