From 0b707f44fc07793ec2fbfcf4d5252f5cf6b8536d Mon Sep 17 00:00:00 2001 From: Anirudh Patwal <161865581+CapThunder19@users.noreply.github.com> Date: Wed, 12 Aug 2026 14:25:54 +0530 Subject: [PATCH] fix(a11y): make settings rows keyboard-accessible (#1430) --- src/components/settings/SeedPhraseDialog.tsx | 7 +- src/components/settings/SettingsItem.test.tsx | 183 ++++++++++++++++++ src/components/settings/SettingsItem.tsx | 62 ++++-- 3 files changed, 235 insertions(+), 17 deletions(-) create mode 100644 src/components/settings/SettingsItem.test.tsx diff --git a/src/components/settings/SeedPhraseDialog.tsx b/src/components/settings/SeedPhraseDialog.tsx index 146ade70..bddd616d 100644 --- a/src/components/settings/SeedPhraseDialog.tsx +++ b/src/components/settings/SeedPhraseDialog.tsx @@ -1,7 +1,6 @@ import { useState, useEffect, useMemo, type ComponentProps } from 'react' import { getseedOptions } from '@joinmarket-webui/joinmarket-ng-api-ts/@tanstack/react-query' import { useQuery, useQueryClient } from '@tanstack/react-query' -import { cx } from 'class-variance-authority' import { AlertTriangleIcon, ClockIcon } from 'lucide-react' import { useTranslation } from 'react-i18next' import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert' @@ -17,7 +16,7 @@ import { import { Label } from '@/components/ui/label' import { useApiClient } from '@/hooks/useApiClient' import { getErrorReason } from '@/lib/errorReason' -import type { WalletFileName } from '@/lib/utils' +import { cn, type WalletFileName } from '@/lib/utils' import type { Milliseconds, WithRequiredProperty } from '@/types/global' import { SeedPhraseGrid } from '../ui/jam/SeedPhraseGrid' import { Spinner } from '../ui/spinner' @@ -189,13 +188,13 @@ export const SeedPhraseDialog = ({
diff --git a/src/components/settings/SettingsItem.test.tsx b/src/components/settings/SettingsItem.test.tsx new file mode 100644 index 00000000..1eb8cd2e --- /dev/null +++ b/src/components/settings/SettingsItem.test.tsx @@ -0,0 +1,183 @@ +import { render, screen } from '@testing-library/react' +import user from '@testing-library/user-event' +import { KeyRoundIcon } from 'lucide-react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { SettingItem, SettingsLink, SettingSwitch } from './SettingsItem' + +const mocks = vi.hoisted(() => ({ + navigate: vi.fn(), + open: vi.fn(), +})) + +vi.mock('react-router-dom', () => ({ + useNavigate: () => mocks.navigate, +})) + +vi.mock('@/components/ui/switch', () => ({ + Switch: ({ + checked, + disabled, + onCheckedChange, + }: { + checked?: boolean + disabled?: boolean + onCheckedChange?: (checked: boolean) => void + }) => ( + + ), +})) + +describe('SettingItem', () => { + beforeEach(() => { + mocks.navigate.mockReset() + mocks.open.mockReset() + }) + + it('renders title and icon', () => { + render() + + expect(screen.getByText('settings.show_seed')).toBeInTheDocument() + }) + + it('exposes an actionable row as a button', () => { + render() + + expect(screen.getByRole('button', { name: 'settings.show_seed' })).toBeInTheDocument() + }) + + it('does not render a button without an action', () => { + render() + + expect(screen.queryByRole('button')).not.toBeInTheDocument() + }) + + it('is reachable by keyboard', async () => { + render() + + await user.tab() + + expect(screen.getByRole('button', { name: 'settings.show_seed' })).toHaveFocus() + }) + + it('triggers the action on Enter', async () => { + const action = vi.fn() + render() + + await user.tab() + await user.keyboard('{Enter}') + + expect(action).toHaveBeenCalledOnce() + }) + + it('triggers the action on Space', async () => { + const action = vi.fn() + render() + + await user.tab() + await user.keyboard(' ') + + expect(action).toHaveBeenCalledOnce() + }) + + it('triggers the action on click', async () => { + const action = vi.fn() + render() + + await user.click(screen.getByRole('button', { name: 'settings.show_seed' })) + + expect(action).toHaveBeenCalledOnce() + }) + + it('does not trigger the action while disabled', async () => { + const action = vi.fn() + render() + + const item = screen.getByRole('button', { name: 'settings.show_seed' }) + expect(item).toBeDisabled() + + await user.click(item) + await user.tab() + await user.keyboard('{Enter}') + + expect(action).not.toHaveBeenCalled() + }) + + it('stays a wrapper when children provide their own control', () => { + render( + + + , + ) + + // no button is nested inside another button + expect(screen.queryByRole('button', { name: 'settings.show_seed' })).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'child-control' })).toBeInTheDocument() + }) +}) + +describe('SettingsLink', () => { + beforeEach(() => { + mocks.navigate.mockReset() + }) + + it('navigates internally on Enter', async () => { + render() + + await user.tab() + await user.keyboard('{Enter}') + + expect(mocks.navigate).toHaveBeenCalledWith('/rescan') + }) + + it('opens external links in a new tab on Enter', async () => { + vi.stubGlobal('open', mocks.open) + render() + + await user.tab() + await user.keyboard('{Enter}') + + expect(mocks.open).toHaveBeenCalledWith('https://example.org', '_blank', 'noreferrer,noopener') + expect(mocks.navigate).not.toHaveBeenCalled() + }) +}) + +describe('SettingSwitch', () => { + it('is keyboard operable via the row when no toggle is displayed', async () => { + const onCheckedChange = vi.fn() + render( + , + ) + + await user.tab() + await user.keyboard('{Enter}') + + expect(onCheckedChange).toHaveBeenCalledWith(true) + }) + + it('is keyboard operable via the toggle when one is displayed', async () => { + const onCheckedChange = vi.fn() + render( + , + ) + + await user.tab() + expect(screen.getByRole('button', { name: 'switch:false' })).toHaveFocus() + + await user.keyboard('{Enter}') + expect(onCheckedChange).toHaveBeenCalledWith(true) + }) +}) diff --git a/src/components/settings/SettingsItem.tsx b/src/components/settings/SettingsItem.tsx index e92e70be..499d5fe7 100644 --- a/src/components/settings/SettingsItem.tsx +++ b/src/components/settings/SettingsItem.tsx @@ -1,9 +1,9 @@ import type { PropsWithChildren, ReactNode } from 'react' -import { cx } from 'class-variance-authority' import { ExternalLinkIcon } from 'lucide-react' import type { LucideIcon } from 'lucide-react' import { useNavigate } from 'react-router-dom' import { Switch } from '@/components/ui/switch' +import { cn } from '@/lib/utils' type SettingsItemProps = PropsWithChildren<{ icon?: LucideIcon @@ -11,6 +11,12 @@ type SettingsItemProps = PropsWithChildren<{ title: string disabled?: boolean action?: () => void | Promise + /** + * Set when {@link children} already renders its own focusable control (e.g. a `Switch`). + * The row then stays a plain wrapper instead of becoming a button, so a button is never + * nested inside a button - keyboard users operate the child control directly. + */ + hasInteractiveChild?: boolean }> export const SettingItem = ({ @@ -19,16 +25,22 @@ export const SettingItem = ({ title, action, disabled = false, + hasInteractiveChild = false, children, }: SettingsItemProps) => { + const rowClassName = cn('flex min-w-0 items-center justify-between gap-2 py-2', { + 'hover:bg-muted/50 cursor-pointer rounded-md px-2': !disabled, + 'cursor-not-allowed opacity-60': disabled, + }) + // The row bleeds into the card padding so the hover background spans the full width. + // A block `div` with `width: auto` gets this from the negative margin alone, but a + // `button` does not stretch to its parent, and once `w-full` pins the width the + // negative margin no longer widens it. Keeping the bleed on a wrapper makes `w-full` + // resolve against the widened box, so both variants end up the same size. + const bleedClassName = !disabled ? '-mx-2' : undefined + const content = ( -
void action() : undefined} - > + <>
{Icon && } @@ -39,13 +51,34 @@ export const SettingItem = ({
{children !== undefined &&
{children}
} -
+ ) - return content + // A row that is itself the control must be a real button: the browser then provides + // focus, Enter/Space activation and the correct role for free. + if (action && !hasInteractiveChild) { + return ( +
+ +
+ ) + } + + return ( +
void action() : undefined}> + {content} +
+ ) } -type SettingsLinkProps = Omit & { +type SettingsLinkProps = Omit & { to: string external?: boolean } @@ -69,14 +102,17 @@ export const SettingsLink = ({ to, external = false, ...props }: SettingsLinkPro ) } -type SettingsSwitchProps = Omit & { +type SettingsSwitchProps = Omit & { checked?: boolean onCheckedChange?: (checked: boolean) => void displayToggle?: boolean } export const SettingSwitch = ({ checked, onCheckedChange, displayToggle = true, ...props }: SettingsSwitchProps) => { return ( - onCheckedChange?.(!checked)}> + // With a visible toggle the `Switch` is the focusable control and the row stays a + // wrapper. Without one the row itself has to be the button, or the setting cannot be + // reached by keyboard at all. + onCheckedChange?.(!checked)} hasInteractiveChild={displayToggle}> {displayToggle && } )