diff --git a/apps/mobile/src/components/agents/session-filter-button.mounted.test.tsx b/apps/mobile/src/components/agents/session-filter-button.mounted.test.tsx new file mode 100644 index 0000000000..f70cda514f --- /dev/null +++ b/apps/mobile/src/components/agents/session-filter-button.mounted.test.tsx @@ -0,0 +1,74 @@ +// The agents "Filter sessions" control is one of the icon-only controls the +// accessibility explorer found below 28dp: it rendered the bare 20dp sliders +// icon, so its accessibility node was the icon. This pins the box that replaced +// it, and that the count badge still hangs off the icon. + +import { createElement } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { describe, expect, it, vi } from 'vitest'; + +import { expectReliableTapTarget } from '@/test/touch-target.test-helpers'; + +import '@/i18n'; +import { SessionFilterButton } from './session-filter-button'; + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + View: 'View', +})); + +vi.mock('@/components/ui/icons', () => ({ + SlidersHorizontal: 'SlidersHorizontal', +})); + +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); + +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ foreground: '#000000', mutedForeground: '#666666' }), +})); + +async function renderButton(activeCount: number): Promise { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + await act(async () => { + ref.current = TestRenderer.create( + createElement(SessionFilterButton, { + activeCount, + onPress: vi.fn<() => void>(), + testID: 'agents-open-filters', + }) + ); + await Promise.resolve(); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; +} + +function findFilterButton(root: TestRenderer.ReactTestInstance): TestRenderer.ReactTestInstance { + return root.find( + node => String(node.type) === 'Pressable' && node.props.testID === 'agents-open-filters' + ); +} + +describe('SessionFilterButton mounted', () => { + it('gives the filter control a box at least 28dp on a side and a 44pt tap target', async () => { + const renderer = await renderButton(0); + + const button = findFilterButton(renderer.root); + expectReliableTapTarget(button.props); + expect(button.props.accessibilityLabel).toBe('Filter sessions'); + }); + + it('keeps the badge and the spoken count when filters are applied', async () => { + const renderer = await renderButton(2); + + const button = findFilterButton(renderer.root); + expectReliableTapTarget(button.props); + expect(button.props.accessibilityLabel).toBe('Filter sessions, 2'); + expect( + renderer.root.findAll(node => node.props.testID === 'session-filter-badge') + ).toHaveLength(1); + }); +}); diff --git a/apps/mobile/src/components/agents/session-filter-button.tsx b/apps/mobile/src/components/agents/session-filter-button.tsx index a2fdba02de..d087edaffe 100644 --- a/apps/mobile/src/components/agents/session-filter-button.tsx +++ b/apps/mobile/src/components/agents/session-filter-button.tsx @@ -1,8 +1,9 @@ import { SlidersHorizontal } from '@/components/ui/icons'; -import { Pressable, View } from 'react-native'; +import { View } from 'react-native'; import { useTranslation } from 'react-i18next'; import { filterButtonAccessibilityLabel } from '@/components/agents/session-filter-button-label'; +import { IconButton } from '@/components/ui/icon-button'; import { Text } from '@/components/ui/text'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; @@ -28,11 +29,8 @@ export function SessionFilterButton({ const isActive = activeCount > 0; return ( - - - {isActive ? ( - // Overlaps the icon's top-right corner; `pointer-events-none` keeps the - // whole 44pt target on the Pressable underneath. - - + + {isActive ? ( + // Overlaps the icon's top-right corner; `pointer-events-none` keeps the + // whole touch target on the Pressable underneath. + - {activeCount} - - - ) : null} - + + {activeCount} + + + ) : null} + + ); } diff --git a/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx b/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx index eb937d18ac..461223b3af 100644 --- a/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx +++ b/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx @@ -322,6 +322,20 @@ function filterButtonProps() { } return button.props as { accessibilityLabel?: string; accessibilityValue?: unknown }; } +/** + * Nearest ancestor whose className holds `token`. Each header control renders + * through its own component, so a control's parent chain depth is not fixed. + */ +function ancestorWithClassName( + node: TestRenderer.ReactTestInstance | undefined, + token: string +): TestRenderer.ReactTestInstance | null { + let current = node?.parent ?? null; + while (current && !String(current.props.className).includes(token)) { + current = current.parent; + } + return current; +} function applyFilters(projectFilter: string[], platformFilter: string[]) { act(() => { headerAction('agents-open-filters').props.onPress(); @@ -1062,7 +1076,7 @@ describe('AgentSessionListScreen header and admission', () => { const filters = nodes('Pressable').find(node => node.props.testID === 'agents-open-filters'); expect(history?.parent?.props.className).toContain('items-center'); expect(history?.parent?.props.className).toContain('min-h-11'); - expect(filters?.parent?.parent).toBe(history?.parent); + expect(ancestorWithClassName(filters, 'min-h-11')).toBe(history?.parent); const updating = nodes('Text').find(node => node.children.includes('Updating')); expect(updating).toBeUndefined(); state.live.isFetching = true; diff --git a/apps/mobile/src/components/organization/hub-screen.mounted.test.tsx b/apps/mobile/src/components/organization/hub-screen.mounted.test.tsx new file mode 100644 index 0000000000..852d4ccdff --- /dev/null +++ b/apps/mobile/src/components/organization/hub-screen.mounted.test.tsx @@ -0,0 +1,149 @@ +// The org hub's "Rename organization" pencil is one of the icon-only controls +// the accessibility explorer found below 28dp: it rendered the bare 16dp icon, +// so its accessibility node was the icon. Both routes that show the hub +// (organization/index and the organization/[org-id] deep link) render this same +// control, so this suite covers both. + +import { createElement, type ReactNode } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { describe, expect, it, vi } from 'vitest'; + +import { expectReliableTapTarget } from '@/test/touch-target.test-helpers'; + +import '@/i18n'; +import { OrganizationHubScreen } from './hub-screen'; + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + View: 'View', +})); + +vi.mock('react-native-reanimated', () => ({ + default: { View: 'AnimatedView' }, + FadeIn: { duration: () => ({}) }, +})); + +vi.mock('expo-router', () => ({ + useRouter: () => ({ push: vi.fn(), replace: vi.fn() }), +})); + +vi.mock('expo-haptics', () => ({ + notificationAsync: vi.fn(), + NotificationFeedbackType: { Success: 'success' }, +})); + +vi.mock('@/components/ui/icons', () => ({ + Bell: 'Bell', + FileText: 'FileText', + Pencil: 'Pencil', + Receipt: 'Receipt', + Users: 'Users', +})); + +vi.mock('@/components/ui/directional-icons', () => ({ + DirectionalChevronRight: 'DirectionalChevronRight', +})); + +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); + +vi.mock('@/components/ui/configure-row', () => ({ ConfigureRow: 'ConfigureRow' })); + +vi.mock('@/components/ui/kv-row', () => ({ KvRow: 'KvRow' })); + +vi.mock('@/components/tab-screen', () => ({ + TabScreenScrollView: (props: { children?: ReactNode }) => + createElement('TabScreenScrollView', null, props.children), +})); + +vi.mock('@/components/screen-header', () => ({ + ScreenHeader: (props: { title?: string }) => createElement('ScreenHeader', null, props.title), +})); + +vi.mock('@/components/rename-modal', () => ({ RenameModal: 'RenameModal' })); + +vi.mock('@/components/add-credits-row', () => ({ AddCreditsRow: 'AddCreditsRow' })); + +vi.mock('@/components/kilo-pass/kilo-pass-icon', () => ({ KiloPassIcon: 'KiloPassIcon' })); + +vi.mock('@/components/organization/organization-boundary', () => ({ + OrganizationBoundary: 'OrganizationBoundary', +})); + +vi.mock('@/components/organization/org-usage-stats', () => ({ OrgUsageStats: 'OrgUsageStats' })); + +vi.mock('@/components/organization/org-kilo-pass-row-state', () => ({ + getOrgKiloPassRowState: () => null, +})); + +vi.mock('@/lib/config', () => ({ WEB_BASE_URL: 'https://app.kilo.ai' })); + +vi.mock('@/lib/external-link', () => ({ openExternalUrl: vi.fn() })); + +vi.mock('@/lib/hooks/use-organization-mutations', () => ({ + useOrganizationMutations: () => ({ rename: { mutateAsync: vi.fn() } }), +})); + +vi.mock('@/lib/hooks/use-organization-queries', () => ({ + isMoneyRole: () => true, + useOrgBoundary: () => ({ + organizationId: 'org-1', + role: 'owner', + org: { + organizationName: 'Acme', + balance: 0, + requireSeats: false, + seatCount: { used: 1, total: 1 }, + }, + isResolving: false, + }), + useOrgWithMembers: () => ({ + data: { parent_organization_id: null, settings: { minimum_balance: null }, members: [] }, + }), + useOrgKiloPassSummary: () => ({ data: undefined, isError: false, refetch: vi.fn() }), +})); + +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ mutedForeground: '#666666', foreground: '#000000' }), +})); + +vi.mock('@/lib/organization-context', () => ({ + useOrganization: () => ({ setOrganizationId: vi.fn() }), +})); + +async function renderHub(): Promise { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + await act(async () => { + ref.current = TestRenderer.create(createElement(OrganizationHubScreen)); + await Promise.resolve(); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; +} + +function findRenameControl(root: TestRenderer.ReactTestInstance): TestRenderer.ReactTestInstance { + return root.find( + node => + String(node.type) === 'Pressable' && node.props.accessibilityLabel === 'Rename organization' + ); +} + +describe('OrganizationHubScreen rename control', () => { + it('gives the rename control a box at least 28dp on a side and a 44pt tap target', async () => { + const renderer = await renderHub(); + + expectReliableTapTarget(findRenameControl(renderer.root).props); + }); + + it('still opens the rename modal from the control', async () => { + const renderer = await renderHub(); + + act(() => { + (findRenameControl(renderer.root).props.onPress as () => void)(); + }); + + expect(renderer.root.findAll(node => String(node.type) === 'RenameModal')).toHaveLength(1); + }); +}); diff --git a/apps/mobile/src/components/organization/hub-screen.tsx b/apps/mobile/src/components/organization/hub-screen.tsx index a9d8ab19b6..cc31be7050 100644 --- a/apps/mobile/src/components/organization/hub-screen.tsx +++ b/apps/mobile/src/components/organization/hub-screen.tsx @@ -19,6 +19,7 @@ import { OrgUsageStats } from '@/components/organization/org-usage-stats'; import { RenameModal } from '@/components/rename-modal'; import { ScreenHeader } from '@/components/screen-header'; import { ConfigureRow } from '@/components/ui/configure-row'; +import { IconButton } from '@/components/ui/icon-button'; import { KvRow } from '@/components/ui/kv-row'; import { Text } from '@/components/ui/text'; import { TabScreenScrollView } from '@/components/tab-screen'; @@ -111,17 +112,14 @@ export function OrganizationHubScreen({ organizationIdOverride }: OrganizationHu {org.organizationName} {showMoney && ( - { setRenameVisible(true); }} - hitSlop={12} - accessibilityRole="button" accessibilityLabel={t('organization.hub.renameTitle')} - className="active:opacity-70" > - + )} {showMoney && ( diff --git a/apps/mobile/src/components/organization/members-screen.mounted.test.tsx b/apps/mobile/src/components/organization/members-screen.mounted.test.tsx index 2b3cd3116c..40860508c5 100644 --- a/apps/mobile/src/components/organization/members-screen.mounted.test.tsx +++ b/apps/mobile/src/components/organization/members-screen.mounted.test.tsx @@ -14,6 +14,7 @@ import { import { beforeEach, describe, expect, it, vi } from 'vitest'; import { renderWithProviders } from '@/test/render-with-providers'; +import { expectReliableTapTarget } from '@/test/touch-target.test-helpers'; import '@/i18n'; import { OrganizationMembersScreen } from './members-screen'; @@ -96,7 +97,8 @@ vi.mock('@/components/query-error', () => ({ })); vi.mock('@/components/screen-header', () => ({ - ScreenHeader: () => null, + ScreenHeader: (props: { headerRight?: ReactNode }) => + createElement('ScreenHeader', null, props.headerRight), })); vi.mock('@/components/ui/button', () => ({ @@ -213,3 +215,19 @@ describe('OrganizationMembersScreen empty-state precedence', () => { expect(texts).not.toContain('EMPTY_STATE:No members yet'); }); }); + +describe('OrganizationMembersScreen invite control', () => { + // The explorer found the header's "Invite member" control at its bare 22dp + // icon size, below the 28dp minimum. This pins the box that replaced it. + it('sizes the invite control for a reliable tap target', async () => { + const { renderer, unmount } = await renderWithProviders( + createElement(OrganizationMembersScreen) + ); + const button = renderer.root.find( + node => String(node.type) === 'Pressable' && node.props.accessibilityLabel === 'Invite member' + ); + + expectReliableTapTarget(button.props); + unmount(); + }); +}); diff --git a/apps/mobile/src/components/organization/members-screen.tsx b/apps/mobile/src/components/organization/members-screen.tsx index 86682c1743..25ac732fe6 100644 --- a/apps/mobile/src/components/organization/members-screen.tsx +++ b/apps/mobile/src/components/organization/members-screen.tsx @@ -3,7 +3,7 @@ import { type Href, useRouter } from 'expo-router'; import { UserPlus, Users } from '@/components/ui/icons'; import { type ReactNode, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; -import { Pressable, View, type ViewStyle } from 'react-native'; +import { View, type ViewStyle } from 'react-native'; import { EmptyState } from '@/components/empty-state'; import { InvitedMemberRow } from '@/components/organization/invited-member-row'; @@ -12,6 +12,7 @@ import { OrganizationBoundary } from '@/components/organization/organization-bou import { QueryError } from '@/components/query-error'; import { ScreenHeader } from '@/components/screen-header'; import { Button } from '@/components/ui/button'; +import { IconButton } from '@/components/ui/icon-button'; import { Skeleton } from '@/components/ui/skeleton'; import { Text } from '@/components/ui/text'; import { useTabBarBottomPadding } from '@/components/tab-screen'; @@ -201,17 +202,14 @@ export function OrganizationMembersScreen() { title={t('organization.members.title')} headerRight={ canInvite ? ( - { router.push('/(app)/(tabs)/(3_profile)/organization/invite-member' as Href); }} - hitSlop={12} - accessibilityRole="button" accessibilityLabel={t('organization.inviteMember.title')} - className="active:opacity-70" > - + ) : undefined } /> diff --git a/apps/mobile/src/components/ui/icon-button.mounted.test.tsx b/apps/mobile/src/components/ui/icon-button.mounted.test.tsx new file mode 100644 index 0000000000..786abbf1e9 --- /dev/null +++ b/apps/mobile/src/components/ui/icon-button.mounted.test.tsx @@ -0,0 +1,80 @@ +// An icon-only control's accessibility node is its own box — Android reports the +// view rect, not `hitSlop` — so the explorer check fails any control under 28dp +// on a side. IconButton is the single place that box and the tap target are +// defined; these tests pin both so a caller cannot regress them. + +import { createElement } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { describe, expect, it, vi } from 'vitest'; + +import { expectReliableTapTarget } from '@/test/touch-target.test-helpers'; + +import { IconButton } from './icon-button'; + +vi.mock('react-native', () => ({ Pressable: 'Pressable' })); + +type RenderProps = { + accessibilityLabel?: string; + className?: string; + hitSlop?: object; + onPress?: () => void; + testID?: string; +}; + +async function renderButton(props: RenderProps): Promise { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + await act(async () => { + ref.current = TestRenderer.create(createElement(IconButton, props, createElement('IconMock'))); + await Promise.resolve(); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; +} + +function findButton(root: TestRenderer.ReactTestInstance): TestRenderer.ReactTestInstance { + return root.find(node => String(node.type) === 'Pressable'); +} + +describe('IconButton mounted', () => { + it('renders the caller label, testID and role', async () => { + const renderer = await renderButton({ + accessibilityLabel: 'Filter sessions', + testID: 'agents-open-filters', + }); + + const button = findButton(renderer.root); + expect(button.props.accessibilityRole).toBe('button'); + expect(button.props.accessibilityLabel).toBe('Filter sessions'); + expect(button.props.testID).toBe('agents-open-filters'); + }); + + it('keeps a box at least 28dp on a side and clears the 44pt tap-target minimum', async () => { + const renderer = await renderButton({ accessibilityLabel: 'Filter sessions' }); + + expectReliableTapTarget(findButton(renderer.root).props); + }); + + it('keeps the box when a caller adds its own classes', async () => { + const renderer = await renderButton({ + accessibilityLabel: 'Filter sessions', + className: 'mt-2', + }); + + const button = findButton(renderer.root); + expectReliableTapTarget(button.props); + expect(button.props.className).toContain('mt-2'); + }); + + it('invokes onPress when pressed', async () => { + const onPress = vi.fn(() => undefined); + const renderer = await renderButton({ accessibilityLabel: 'Filter sessions', onPress }); + + const button = findButton(renderer.root); + (button.props.onPress as () => void)(); + + expect(onPress).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/mobile/src/components/ui/icon-button.tsx b/apps/mobile/src/components/ui/icon-button.tsx new file mode 100644 index 0000000000..1963ed36f8 --- /dev/null +++ b/apps/mobile/src/components/ui/icon-button.tsx @@ -0,0 +1,38 @@ +import { Pressable } from 'react-native'; + +import { cn } from '@/lib/utils'; + +/** + * Icon-only control with a layout box big enough to be tapped reliably. + * + * A control's accessibility node is its layout bounds — Android reports the + * view rect and `hitSlop` is not part of it — so an icon rendered at its own + * 16–22pt size reports a node under the 28dp minimum the accessibility check + * enforces. Render the icon centered inside this 32pt box instead of passing + * the icon's size through to the Pressable. + * + * `hitSlop` then lifts the effective target to 48pt on every side, past the + * 44pt minimum DESIGN.md requires of compact controls on touch surfaces. + */ +type IconButtonProps = Omit, 'children'> & { + children?: React.ReactNode; +}; + +export function IconButton({ + className, + hitSlop = { top: 8, bottom: 8, left: 8, right: 8 }, + accessibilityRole = 'button', + children, + ...props +}: Readonly) { + return ( + + {children} + + ); +} diff --git a/apps/mobile/src/test/touch-target.test-helpers.ts b/apps/mobile/src/test/touch-target.test-helpers.ts new file mode 100644 index 0000000000..6a564ec4be --- /dev/null +++ b/apps/mobile/src/test/touch-target.test-helpers.ts @@ -0,0 +1,37 @@ +import { expect } from 'vitest'; + +/** A control below this on a side reports an accessibility node too small to tap reliably. */ +const MIN_CONTROL_BOX = 28; + +/** DESIGN.md: "preserve at least a 44px target even when the visual control is compact". */ +const MIN_TOUCH_TARGET = 44; + +type ControlProps = { className?: unknown; hitSlop?: unknown }; + +type HitSlop = { top: number; bottom: number; left: number; right: number }; + +/** Parses the `h-[px] w-[px]` box out of a rendered control's className. */ +function controlBoxSize(className: unknown): number { + const match = /h-\[(\d+)px\] w-\[(\d+)px\]/.exec(String(className)); + const size = match?.[1]; + if (size == null) { + throw new Error(`control has no px box in className: ${String(className)}`); + } + return Number(size); +} + +/** + * Asserts a rendered control clears both tap-target minimums: its own box the + * 28dp accessibility minimum, and its box plus `hitSlop` the 44pt touch target. + */ +export function expectReliableTapTarget({ className, hitSlop }: ControlProps): number { + const box = controlBoxSize(className); + const slop = hitSlop as HitSlop | undefined; + if (slop == null) { + throw new Error('control has no hitSlop'); + } + expect(box).toBeGreaterThanOrEqual(MIN_CONTROL_BOX); + expect(box + slop.left + slop.right).toBeGreaterThanOrEqual(MIN_TOUCH_TARGET); + expect(box + slop.top + slop.bottom).toBeGreaterThanOrEqual(MIN_TOUCH_TARGET); + return box; +}