From 252109bb01e16a2d1ecd2778a3ac247ac6b1be22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 04:09:42 +0200 Subject: [PATCH 01/17] feat(mobile-a11y): reflow tab shell + header at large font scale --- .../app/(app)/(tabs)/(1_kiloclaw)/index.tsx | 8 +- apps/mobile/src/app/(app)/(tabs)/_layout.tsx | 21 ++-- .../agents/session-list-content.tsx | 10 +- .../kilo-chat/conversation-list-screen.tsx | 18 ++-- apps/mobile/src/components/screen-header.tsx | 4 +- apps/mobile/src/components/tab-screen.tsx | 4 +- apps/mobile/src/lib/tab-bar-layout.test.ts | 100 +++++++++++++++++- apps/mobile/src/lib/tab-bar-layout.ts | 58 ++++++++++ 8 files changed, 201 insertions(+), 22 deletions(-) diff --git a/apps/mobile/src/app/(app)/(tabs)/(1_kiloclaw)/index.tsx b/apps/mobile/src/app/(app)/(tabs)/(1_kiloclaw)/index.tsx index 2f77a18706..1ae14f3a96 100644 --- a/apps/mobile/src/app/(app)/(tabs)/(1_kiloclaw)/index.tsx +++ b/apps/mobile/src/app/(app)/(tabs)/(1_kiloclaw)/index.tsx @@ -19,7 +19,7 @@ import { useManualRefresh } from '@/lib/hooks/use-manual-refresh'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { useUnreadCounts } from '@/lib/hooks/use-unread-counts'; import { chatSandboxPath } from '@/lib/kilo-chat-routes'; -import { getTabBarOverlayHeight } from '@/lib/tab-bar-layout'; +import { getEffectiveTabBarHeight } from '@/lib/tab-bar-layout'; export default function KiloClawTab() { const router = useRouter(); @@ -42,7 +42,11 @@ export default function KiloClawTab() { const showInstanceSkeleton = entryDecision.kind === 'loading' || onboardingQuery.isPending; const emptyStateContainerStyle = { - paddingBottom: getTabBarOverlayHeight(bottom, Platform.OS, fontScale), + paddingBottom: getEffectiveTabBarHeight({ + bottomInset: bottom, + platform: Platform.OS, + fontScale, + }), }; const [manualRefreshing, handleRefresh] = useManualRefresh( diff --git a/apps/mobile/src/app/(app)/(tabs)/_layout.tsx b/apps/mobile/src/app/(app)/(tabs)/_layout.tsx index af355d0557..6301b518b8 100644 --- a/apps/mobile/src/app/(app)/(tabs)/_layout.tsx +++ b/apps/mobile/src/app/(app)/(tabs)/_layout.tsx @@ -8,8 +8,10 @@ import { BlurBar } from '@/components/ui/blur-bar'; import { Text } from '@/components/ui/text'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { - getTabBarOverlayHeight, + getEffectiveTabBarHeight, + getTabBarIconSize, shouldHideTabBar, + shouldShowTabLabel, TAB_LABEL_WRAP_FONT_SCALE, } from '@/lib/tab-bar-layout'; @@ -52,7 +54,13 @@ export default function TabsLayout() { const { bottom } = useSafeAreaInsets(); const { fontScale } = useWindowDimensions(); const hideTabs = shouldHideTabBar(pathname); - const tabBarHeight = getTabBarOverlayHeight(bottom, Platform.OS, fontScale); + const showTabLabel = shouldShowTabLabel(fontScale); + const tabBarHeight = getEffectiveTabBarHeight({ + bottomInset: bottom, + platform: Platform.OS, + fontScale, + }); + const tabIconSize = getTabBarIconSize(fontScale); return ( , tabBarIcon: ({ color, focused }) => ( - + ), }} listeners={{ @@ -103,7 +112,7 @@ export default function TabsLayout() { /> ), tabBarIcon: ({ color, focused }) => ( - + ), }} listeners={{ @@ -121,7 +130,7 @@ export default function TabsLayout() { tabBarAccessibilityLabel: 'Agents, tab, 3 of 4', tabBarLabel: ({ focused }) => , tabBarIcon: ({ color, focused }) => ( - + ), }} listeners={{ @@ -137,7 +146,7 @@ export default function TabsLayout() { tabBarAccessibilityLabel: 'Profile, tab, 4 of 4', tabBarLabel: ({ focused }) => , tabBarIcon: ({ color, focused }) => ( - + ), }} listeners={{ diff --git a/apps/mobile/src/components/agents/session-list-content.tsx b/apps/mobile/src/components/agents/session-list-content.tsx index 646508215a..3cabae0496 100644 --- a/apps/mobile/src/components/agents/session-list-content.tsx +++ b/apps/mobile/src/components/agents/session-list-content.tsx @@ -30,7 +30,7 @@ import { type StoredSession } from '@/lib/hooks/use-agent-sessions'; import { useSessionMutations } from '@/lib/hooks/use-session-mutations'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { getRevisionSnapshot } from '@/lib/session-attention'; -import { getTabBarOverlayHeight } from '@/lib/tab-bar-layout'; +import { getEffectiveTabBarHeight } from '@/lib/tab-bar-layout'; type AgentSessionListContentProps = { sections: SessionSection[]; @@ -87,7 +87,13 @@ export function AgentSessionListContent({ // The tab bar is an absolutely-positioned overlay, so scrollable content // must clear it or the last rows are stuck underneath it. const tabBarClearanceStyle = useMemo( - () => ({ paddingBottom: getTabBarOverlayHeight(bottom, Platform.OS, fontScale) }), + () => ({ + paddingBottom: getEffectiveTabBarHeight({ + bottomInset: bottom, + platform: Platform.OS, + fontScale, + }), + }), [bottom, fontScale] ); diff --git a/apps/mobile/src/components/kilo-chat/conversation-list-screen.tsx b/apps/mobile/src/components/kilo-chat/conversation-list-screen.tsx index 9cdd4d18fb..ad16395a0f 100644 --- a/apps/mobile/src/components/kilo-chat/conversation-list-screen.tsx +++ b/apps/mobile/src/components/kilo-chat/conversation-list-screen.tsx @@ -24,7 +24,7 @@ import { Text } from '@/components/ui/text'; import { useManualRefresh } from '@/lib/hooks/use-manual-refresh'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { chatConversationPath } from '@/lib/kilo-chat-routes'; -import { getTabBarOverlayHeight } from '@/lib/tab-bar-layout'; +import { getEffectiveTabBarHeight } from '@/lib/tab-bar-layout'; import { EmptyConversationList } from './empty-conversation-list'; import { groupConversationsByActivity } from './conversation-list-groups'; @@ -114,22 +114,26 @@ export function ConversationListScreen({ sandboxId, sandboxLabel }: Props) { const isFetchingNextPage = listQuery.isFetchingNextPage; const fetchNextPage = listQuery.fetchNextPage; const refetchConversations = listQuery.refetch; - const tabBarOverlayHeight = getTabBarOverlayHeight(bottom, Platform.OS, fontScale); + const tabBarHeight = getEffectiveTabBarHeight({ + bottomInset: bottom, + platform: Platform.OS, + fontScale, + }); const listContentContainerStyle = useMemo( () => ({ flexGrow: 1, - paddingBottom: tabBarOverlayHeight + FAB_SIZE + FAB_MARGIN, + paddingBottom: tabBarHeight + FAB_SIZE + FAB_MARGIN, }) satisfies ViewStyle, - [tabBarOverlayHeight] + [tabBarHeight] ); const createButtonStyle = useMemo( () => ({ - bottom: tabBarOverlayHeight + FAB_MARGIN, + bottom: tabBarHeight + FAB_MARGIN, right: 20, }) satisfies ViewStyle, - [tabBarOverlayHeight] + [tabBarHeight] ); useInstancePresence(sandboxId); @@ -197,7 +201,7 @@ export function ConversationListScreen({ sandboxId, sandboxLabel }: Props) { {resolvedBackIcon === 'close' ? ( @@ -112,7 +112,7 @@ export function ScreenHeader({ )} )} - + {eyebrow ? {eyebrow} : null} {titleNode} diff --git a/apps/mobile/src/components/tab-screen.tsx b/apps/mobile/src/components/tab-screen.tsx index 0ebc9afc5f..780b38451b 100644 --- a/apps/mobile/src/components/tab-screen.tsx +++ b/apps/mobile/src/components/tab-screen.tsx @@ -7,13 +7,13 @@ import { } from 'react-native'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; -import { getTabBarOverlayHeight } from '@/lib/tab-bar-layout'; +import { getEffectiveTabBarHeight } from '@/lib/tab-bar-layout'; // FlatList/FlashList screens use this directly for contentContainerStyle.paddingBottom. export function useTabBarBottomPadding() { const { bottom } = useSafeAreaInsets(); const { fontScale } = useWindowDimensions(); - return getTabBarOverlayHeight(bottom, Platform.OS, fontScale) + 16; + return getEffectiveTabBarHeight({ bottomInset: bottom, platform: Platform.OS, fontScale }) + 16; } export function TabScreenScrollView({ children, ...props }: ScrollViewProps) { diff --git a/apps/mobile/src/lib/tab-bar-layout.test.ts b/apps/mobile/src/lib/tab-bar-layout.test.ts index 10db341894..5d7babe74a 100644 --- a/apps/mobile/src/lib/tab-bar-layout.test.ts +++ b/apps/mobile/src/lib/tab-bar-layout.test.ts @@ -1,6 +1,15 @@ import { describe, expect, it } from 'vitest'; -import { getTabBarOverlayHeight, shouldHideTabBar } from '@/lib/tab-bar-layout'; +import { + getEffectiveTabBarHeight, + getTabBarIconForwardHeight, + getTabBarIconSize, + getTabBarOverlayHeight, + shouldHideTabBar, + shouldShowTabLabel, + TAB_ICON_FORWARD_FONT_SCALE, + TAB_LABEL_WRAP_FONT_SCALE, +} from '@/lib/tab-bar-layout'; describe('getTabBarOverlayHeight', () => { it('includes the bottom safe area on iOS', () => { @@ -20,6 +29,95 @@ describe('getTabBarOverlayHeight', () => { }); }); +describe('getEffectiveTabBarHeight', () => { + it('uses the label-inclusive overlay height below the icon-forward threshold', () => { + expect(getEffectiveTabBarHeight({ bottomInset: 34, platform: 'ios', fontScale: 1 })).toBe( + getTabBarOverlayHeight(34, 'ios', 1) + ); + expect(getEffectiveTabBarHeight({ bottomInset: 34, platform: 'ios', fontScale: 1.8 })).toBe( + getTabBarOverlayHeight(34, 'ios', 1.8) + ); + }); + + it('uses the compact icon-forward height at and above the icon-forward threshold', () => { + expect(getEffectiveTabBarHeight({ bottomInset: 34, platform: 'ios', fontScale: 2 })).toBe( + getTabBarIconForwardHeight(34, 'ios') + ); + expect(getEffectiveTabBarHeight({ bottomInset: 34, platform: 'ios', fontScale: 2.5 })).toBe( + getTabBarIconForwardHeight(34, 'ios') + ); + expect(getEffectiveTabBarHeight({ bottomInset: 34, platform: 'ios', fontScale: 3 })).toBe( + getTabBarIconForwardHeight(34, 'ios') + ); + }); + + it('matches the rendered bar height at representative scales', () => { + // default scale: label-inclusive bar is 84pt on an iPhone-class bottom inset + expect(getEffectiveTabBarHeight({ bottomInset: 34, platform: 'ios', fontScale: 1 })).toBe(84); + // large scale: icon-forward bar collapses back to 84pt (same as default scale) + expect(getEffectiveTabBarHeight({ bottomInset: 34, platform: 'ios', fontScale: 3 })).toBe(84); + }); + + it('applies the same Android bottom padding as the overlay helpers', () => { + // Below the icon-forward threshold the label-inclusive overlay height is used. + expect(getEffectiveTabBarHeight({ bottomInset: 16, platform: 'android', fontScale: 1 })).toBe( + 70 + ); + expect(getEffectiveTabBarHeight({ bottomInset: 16, platform: 'android', fontScale: 1.8 })).toBe( + 82.8 + ); + // At and above the threshold the bar collapses to the icon-forward height. + expect(getEffectiveTabBarHeight({ bottomInset: 16, platform: 'android', fontScale: 2 })).toBe( + 70 + ); + expect(getEffectiveTabBarHeight({ bottomInset: 16, platform: 'android', fontScale: 2.5 })).toBe( + 70 + ); + expect(getEffectiveTabBarHeight({ bottomInset: 16, platform: 'android', fontScale: 3 })).toBe( + 70 + ); + }); +}); +describe('getTabBarIconForwardHeight', () => { + it('collapses to the base height when labels are hidden at large font scale', () => { + expect(getTabBarIconForwardHeight(34, 'ios')).toBe(84); + expect(getTabBarIconForwardHeight(16, 'android')).toBe(70); + }); + + it('ignores negative insets', () => { + expect(getTabBarIconForwardHeight(-1, 'ios')).toBe(50); + }); +}); + +describe('getTabBarIconSize', () => { + it('returns the base size at the default font scale', () => { + expect(getTabBarIconSize(1)).toBe(22); + }); + + it('grows with font scale but stays bounded', () => { + expect(getTabBarIconSize(1.2)).toBe(26); + expect(getTabBarIconSize(1.5)).toBe(26); + expect(getTabBarIconSize(3)).toBe(26); + }); + + it('never drops below the base size for very small font scales', () => { + expect(getTabBarIconSize(0.85)).toBe(22); + }); +}); + +describe('shouldShowTabLabel', () => { + it('keeps the label below the icon-forward threshold', () => { + expect(shouldShowTabLabel(1)).toBe(true); + expect(shouldShowTabLabel(TAB_LABEL_WRAP_FONT_SCALE)).toBe(true); + }); + + it('hides the label at and above the icon-forward threshold', () => { + expect(shouldShowTabLabel(TAB_ICON_FORWARD_FONT_SCALE)).toBe(false); + expect(shouldShowTabLabel(2.5)).toBe(false); + expect(shouldShowTabLabel(3)).toBe(false); + }); +}); + describe('shouldHideTabBar', () => { it('hides tabs for full-screen nested routes', () => { expect(shouldHideTabBar('/chat/sandbox-1/instance-picker')).toBe(true); diff --git a/apps/mobile/src/lib/tab-bar-layout.ts b/apps/mobile/src/lib/tab-bar-layout.ts index 8f1d5ff0fb..9e95d8810d 100644 --- a/apps/mobile/src/lib/tab-bar-layout.ts +++ b/apps/mobile/src/lib/tab-bar-layout.ts @@ -1,6 +1,17 @@ const TAB_BAR_BASE_HEIGHT = 50; const ANDROID_TAB_BAR_EXTRA_PADDING = 4; export const TAB_LABEL_WRAP_FONT_SCALE = 1.8; +/** + * Above this font scale the tab bar drops visible labels and switches to an + * icon-forward presentation. The label height (which scales with fontScale) is + * removed from the overlay height calculation, so the bar stays at the base + * 50pt instead of ballooning. Labels remain available to assistive tech via + * `tabBarAccessibilityLabel`. Picked above the label-wrap threshold so + * moderate-to-large text still keeps a visible word label. + */ +export const TAB_ICON_FORWARD_FONT_SCALE = 2; +const TAB_ICON_BASE_SIZE = 22; +const TAB_ICON_MAX_SIZE = 26; type TabBarPlatform = 'android' | 'ios' | 'macos' | 'windows' | 'web'; @@ -18,6 +29,53 @@ export function getTabBarOverlayHeight( ); } +/** + * Overlay height for the icon-forward presentation (font scale at or above + * `TAB_ICON_FORWARD_FONT_SCALE`). The label is hidden so the bar can stay at + * the base 50pt instead of growing with the (hidden) label height. + */ +export function getTabBarIconForwardHeight(bottomInset: number, platform: TabBarPlatform): number { + return ( + TAB_BAR_BASE_HEIGHT + + Math.max(bottomInset, 0) + + (platform === 'android' ? ANDROID_TAB_BAR_EXTRA_PADDING : 0) + ); +} + +/** + * Bounded icon size for the tab bar. Icons grow gently with the system font + * scale so they keep visual weight at large text, but are clamped to avoid + * bloating the bar and pushing the layout out of premium density. + */ +export function getTabBarIconSize(fontScale = 1): number { + const scaled = Math.round(TAB_ICON_BASE_SIZE * fontScale); + return Math.min(TAB_ICON_MAX_SIZE, Math.max(TAB_ICON_BASE_SIZE, scaled)); +} + +/** + * Effective rendered tab bar height for the current platform/font scale. This + * is the single source of truth for both the tab bar itself and the content + * clearance below it: it switches to the compact icon-forward height once labels + * are hidden, and otherwise uses the label-inclusive overlay height. + */ +export function getEffectiveTabBarHeight({ + bottomInset, + platform, + fontScale = 1, +}: { + bottomInset: number; + platform: TabBarPlatform; + fontScale?: number; +}): number { + return shouldShowTabLabel(fontScale) + ? getTabBarOverlayHeight(bottomInset, platform, fontScale) + : getTabBarIconForwardHeight(bottomInset, platform); +} + +export function shouldShowTabLabel(fontScale = 1): boolean { + return fontScale < TAB_ICON_FORWARD_FONT_SCALE; +} + export function shouldHideTabBar(pathname: string): boolean { const parts = pathname.split('/').filter(Boolean); const isKiloClawInstancePicker = parts[0] === 'chat' && parts.length === 3; From 6658e324fcaf9e0bcea5dd402055367874adf262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 04:09:43 +0200 Subject: [PATCH 02/17] fix(mobile-a11y): use AA-contrast token for muted-soft text --- .../agents/compaction-separator.tsx | 2 +- .../agents/session-list-section-header.tsx | 4 +- .../kiloclaw/instance-list-screen.tsx | 5 +- apps/mobile/src/components/ui/session-row.tsx | 2 +- .../hooks/use-theme-colors.contrast.test.ts | 73 +++++++++++++++++++ apps/mobile/src/lib/hooks/use-theme-colors.ts | 4 +- 6 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 apps/mobile/src/lib/hooks/use-theme-colors.contrast.test.ts diff --git a/apps/mobile/src/components/agents/compaction-separator.tsx b/apps/mobile/src/components/agents/compaction-separator.tsx index caab03f7a7..23b225f448 100644 --- a/apps/mobile/src/components/agents/compaction-separator.tsx +++ b/apps/mobile/src/components/agents/compaction-separator.tsx @@ -6,7 +6,7 @@ export function CompactionSeparator() { return ( - + Context compacted diff --git a/apps/mobile/src/components/agents/session-list-section-header.tsx b/apps/mobile/src/components/agents/session-list-section-header.tsx index 78224e1d96..ef6ff3f91f 100644 --- a/apps/mobile/src/components/agents/session-list-section-header.tsx +++ b/apps/mobile/src/components/agents/session-list-section-header.tsx @@ -13,7 +13,7 @@ type SessionListSectionHeaderProps = { * date sections AND the pinned "Active now" tray. Matches the existing * `flex-row items-center justify-between bg-background px-[22px] pb-2 * pt-[18px]` header with `` + a mono count - * `text-[10px] uppercase tracking-[1.5px] text-muted-soft`. + * `text-[10px] uppercase tracking-[1.5px] text-muted-foreground`. */ export function SessionListSectionHeader({ title, @@ -22,7 +22,7 @@ export function SessionListSectionHeader({ return ( {title} - + {count} diff --git a/apps/mobile/src/components/kiloclaw/instance-list-screen.tsx b/apps/mobile/src/components/kiloclaw/instance-list-screen.tsx index 7299b8c56f..a166bc5086 100644 --- a/apps/mobile/src/components/kiloclaw/instance-list-screen.tsx +++ b/apps/mobile/src/components/kiloclaw/instance-list-screen.tsx @@ -75,7 +75,10 @@ function InstanceSection({ {title} {showCount ? ( - + {instances.length} ) : null} diff --git a/apps/mobile/src/components/ui/session-row.tsx b/apps/mobile/src/components/ui/session-row.tsx index 73e194ec94..2c63a5993d 100644 --- a/apps/mobile/src/components/ui/session-row.tsx +++ b/apps/mobile/src/components/ui/session-row.tsx @@ -138,7 +138,7 @@ export function SessionRow({ {subtitle ? ( {subtitle} diff --git a/apps/mobile/src/lib/hooks/use-theme-colors.contrast.test.ts b/apps/mobile/src/lib/hooks/use-theme-colors.contrast.test.ts new file mode 100644 index 0000000000..0ae07f74ce --- /dev/null +++ b/apps/mobile/src/lib/hooks/use-theme-colors.contrast.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { darkColors, lightColors } from '@/lib/hooks/use-theme-colors'; + +vi.mock('react-native', () => ({ useColorScheme: () => 'light' })); +vi.mock('@react-navigation/native', () => ({ DarkTheme: {}, DefaultTheme: {} })); + +// These are the actual static hex values shipped in +// `src/lib/hooks/use-theme-colors.ts`. The contrast assertions below fail if +// the shipped pair regresses below 4.5:1, so any drift in the hook itself +// breaks the build. +// +// `src/global.css` must stay in lockstep with the hook; the CSS file is not +// read here because vitest's `?raw` does not process .css files in this node +// environment and `node:fs` is lint-banned. + +const MIN_TEXT_RATIO = 4.5; + +type Rgb = readonly [number, number, number]; + +function expandHex(hex: string): Rgb { + const value = hex.startsWith('#') ? hex.slice(1) : hex; + if (value.length !== 6) { + throw new Error(`expected 6-digit hex, got ${hex}`); + } + return [ + Number.parseInt(value.slice(0, 2), 16), + Number.parseInt(value.slice(2, 4), 16), + Number.parseInt(value.slice(4, 6), 16), + ] as const; +} + +// WCAG 2.x relative luminance. The 0.039_28 / 12.92 thresholds and the +// gamma offset 0.055 come straight from the WCAG 2.x spec and must not +// be tuned to "improve" the ratio. +function lineariseChannel(c: number): number { + const s = c / 255; + if (s <= 0.039_28) { + return s / 12.92; + } + return ((s + 0.055) / 1.055) ** 2.4; +} + +function relativeLuminance(rgb: Rgb): number { + const [r, g, b] = rgb; + return 0.2126 * lineariseChannel(r) + 0.7152 * lineariseChannel(g) + 0.0722 * lineariseChannel(b); +} + +function contrastRatio(foreground: string, background: string): number { + const fgL = relativeLuminance(expandHex(foreground)); + const bgL = relativeLuminance(expandHex(background)); + const lighter = Math.max(fgL, bgL); + const darker = Math.min(fgL, bgL); + return (lighter + 0.05) / (darker + 0.05); +} + +describe('muted-foreground token contrast (WCAG AA text)', () => { + it('light theme: >= 4.5:1 against background and card', () => { + const surfaces = { background: lightColors.background, card: lightColors.card } as const; + for (const [name, surface] of Object.entries(surfaces)) { + const ratio = contrastRatio(lightColors.mutedForeground, surface); + expect(ratio, `muted-foreground vs ${name} (light)`).toBeGreaterThanOrEqual(MIN_TEXT_RATIO); + } + }); + + it('dark theme: >= 4.5:1 against background and card', () => { + const surfaces = { background: darkColors.background, card: darkColors.card } as const; + for (const [name, surface] of Object.entries(surfaces)) { + const ratio = contrastRatio(darkColors.mutedForeground, surface); + expect(ratio, `muted-foreground vs ${name} (dark)`).toBeGreaterThanOrEqual(MIN_TEXT_RATIO); + } + }); +}); diff --git a/apps/mobile/src/lib/hooks/use-theme-colors.ts b/apps/mobile/src/lib/hooks/use-theme-colors.ts index 5cf17cf1ae..cdc2154938 100644 --- a/apps/mobile/src/lib/hooks/use-theme-colors.ts +++ b/apps/mobile/src/lib/hooks/use-theme-colors.ts @@ -4,7 +4,7 @@ import { useColorScheme } from 'react-native'; // These values must stay in sync with src/global.css design tokens. // They exist as raw strings because React Navigation header/tab options // and Lucide icons require plain color values (not Tailwind classes). -const lightColors = { +export const lightColors = { background: '#FBFAF5', foreground: '#14130F', primary: '#4F5A10', @@ -37,7 +37,7 @@ const lightColors = { agentSky: '#2C7FB0', } as const; -const darkColors = { +export const darkColors = { background: '#0E0E10', foreground: '#F2F0EB', primary: '#E8F27A', From c2de0c4d01219993462a0a2cf3a3545fa90c8f00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 04:09:44 +0200 Subject: [PATCH 03/17] feat(mobile-a11y): expose Kilo Chat message content + actions to screen readers --- .../kilo-chat/message-bubble-a11y.test.ts | 112 ++++++++++++++++++ .../kilo-chat/message-bubble-a11y.ts | 63 ++++++++++ .../components/kilo-chat/message-bubble.tsx | 44 ++++++- 3 files changed, 217 insertions(+), 2 deletions(-) create mode 100644 apps/mobile/src/components/kilo-chat/message-bubble-a11y.test.ts create mode 100644 apps/mobile/src/components/kilo-chat/message-bubble-a11y.ts diff --git a/apps/mobile/src/components/kilo-chat/message-bubble-a11y.test.ts b/apps/mobile/src/components/kilo-chat/message-bubble-a11y.test.ts new file mode 100644 index 0000000000..2bf505c7f3 --- /dev/null +++ b/apps/mobile/src/components/kilo-chat/message-bubble-a11y.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from 'vitest'; + +import { buildMessageBubbleAccessibilityProps } from './message-bubble-a11y'; + +describe('buildMessageBubbleAccessibilityProps', () => { + it('marks the wrapping Pressable as non-accessible so the message subtree stays navigable', () => { + const props = buildMessageBubbleAccessibilityProps({ + isFromMe: true, + authorLabel: 'Igor', + canSwipeReply: true, + canLongPress: true, + }); + + expect(props.accessible).toBe(false); + }); + + it('announces own messages without naming the recipient', () => { + const props = buildMessageBubbleAccessibilityProps({ + isFromMe: true, + authorLabel: 'Igor', + canSwipeReply: true, + canLongPress: true, + }); + + expect(props.accessibilityLabel).toBe('Your message'); + }); + + it('prefixes incoming messages with the resolved author label', () => { + const props = buildMessageBubbleAccessibilityProps({ + isFromMe: false, + authorLabel: 'KiloClaw', + canSwipeReply: true, + canLongPress: true, + }); + + expect(props.accessibilityLabel).toBe('Message from KiloClaw'); + }); + + it('exposes both reply and more-actions when the gestures are wired', () => { + const props = buildMessageBubbleAccessibilityProps({ + isFromMe: false, + authorLabel: 'Helper Bot', + canSwipeReply: true, + canLongPress: true, + }); + + const actionNames = props.accessibilityActions.map(action => action.name); + expect(actionNames).toEqual(expect.arrayContaining(['reply', 'more-actions'])); + expect(props.accessibilityActions.find(action => action.name === 'reply')?.label).toBe('Reply'); + expect(props.accessibilityActions.find(action => action.name === 'more-actions')?.label).toBe( + 'More actions' + ); + }); + + it('omits the reply action when swipe-reply is not available for the message', () => { + const props = buildMessageBubbleAccessibilityProps({ + isFromMe: false, + authorLabel: 'Helper Bot', + canSwipeReply: false, + canLongPress: true, + }); + + const actionNames = props.accessibilityActions.map(action => action.name); + expect(actionNames).not.toContain('reply'); + expect(actionNames).toContain('more-actions'); + }); + + it('omits the more-actions action when no long-press callback is wired', () => { + const props = buildMessageBubbleAccessibilityProps({ + isFromMe: true, + authorLabel: 'Igor', + canSwipeReply: true, + canLongPress: false, + }); + + const actionNames = props.accessibilityActions.map(action => action.name); + expect(actionNames).toEqual(['reply']); + }); + + it('returns no accessibility actions for a fully inert message', () => { + const props = buildMessageBubbleAccessibilityProps({ + isFromMe: true, + authorLabel: 'Igor', + canSwipeReply: false, + canLongPress: false, + }); + + // The bubble wrapper is still accessible={false} so the inner text/buttons + // remain navigable. The actions host is not rendered at all when actions + // is empty, so it cannot become a content-free duplicate VoiceOver stop. + expect(props.accessible).toBe(false); + expect(props.accessibilityLabel).toBe('Your message'); + expect(props.accessibilityActions).toEqual([]); + }); +}); + +/* + * Structural guarantee: the wrapping `Pressable` gets `accessible={false}` and + * the label/actions are hosted on a dedicated, non-interactive `View` overlay + * (`accessible={true}`, no children, `pointerEvents="none"`, `absolute inset-0`, + * `opacity-0`) that is only rendered when `accessibilityActions` is non-empty. + * This keeps the message body, exec-approval buttons, reaction pills, and + * attachment buttons outside the collapsed VoiceOver node and still gives the + * rotor a focusable target aligned with the bubble's actual bounds. + * + * The exact iOS/tvOS behavior — whether an opacity-0 overlay remains + * focusable and whether the navigation order avoids double-announcement — can + * only be verified on a real device or simulator with VoiceOver running. Unit + * tests here cover the prop contract; the visual/accessibility tree layout must + * be confirmed with an on-device pass (Maestro + VoiceOver or Accessibility + * Inspector). + */ diff --git a/apps/mobile/src/components/kilo-chat/message-bubble-a11y.ts b/apps/mobile/src/components/kilo-chat/message-bubble-a11y.ts new file mode 100644 index 0000000000..2c4f5f24e3 --- /dev/null +++ b/apps/mobile/src/components/kilo-chat/message-bubble-a11y.ts @@ -0,0 +1,63 @@ +import { type AccessibilityActionInfo } from 'react-native'; + +type MessageBubbleA11yInput = { + isFromMe: boolean; + authorLabel: string; + canSwipeReply: boolean; + canLongPress: boolean; +}; + +type MessageBubbleAccessibility = { + /** Applied to the wrapping `Pressable` so iOS does not collapse the message subtree. */ + accessible: false; + /** Applied to the dedicated inner actions host (VoiceOver/TalkBack rotor). */ + accessibilityLabel: string; + /** Applied to the dedicated inner actions host (VoiceOver/TalkBack rotor). */ + accessibilityActions: AccessibilityActionInfo[]; +}; + +/** + * Builds accessibility props for the kilo-chat `MessageBubble`. + * + * Why `accessible: false` on the wrapping `Pressable`: + * React Native `Pressable` hard-defaults to `accessible={true}` (see + * `react-native/Libraries/Components/Pressable/Pressable.js`). On iOS, an + * accessibility element does NOT expose its descendants to VoiceOver swipe + * navigation, so the message body `Text`, exec-approval `Button`s, attachment + * buttons, reaction pills, and author/timestamp `Text` would all leave the + * a11y tree. Setting `accessible={false}` on the wrapper keeps every + * descendant individually navigable. + * + * Importantly, setting an explicit `accessibilityLabel` on an accessible + * wrapper would also suppress iOS's `RCTRecursiveAccessibilityLabel` co-opting, + * so the message body would not be announced anywhere. `accessible={false}` + * is the only correct fix; label length is not the mechanism. + * + * Why the actions are hosted on a separate inner overlay: + * `accessibilityActions` need a focusable element to attach to. We create an + * inset-matched, non-interactive, focusable `View` overlay on the bubble that + * carries the brief label and the custom reply / more-actions actions. Because + * that host is not the wrapper and has no children, it does not swallow the + * message subtree while still giving VoiceOver/TalkBack a rotor target. When + * no actions are available, the overlay is not rendered at all, so it cannot + * become a content-free duplicate focus stop. + */ +export function buildMessageBubbleAccessibilityProps( + input: MessageBubbleA11yInput +): MessageBubbleAccessibility { + const label = input.isFromMe ? 'Your message' : `Message from ${input.authorLabel}`; + + const actions: AccessibilityActionInfo[] = []; + if (input.canSwipeReply) { + actions.push({ name: 'reply', label: 'Reply' }); + } + if (input.canLongPress) { + actions.push({ name: 'more-actions', label: 'More actions' }); + } + + return { + accessible: false, + accessibilityLabel: label, + accessibilityActions: actions, + }; +} diff --git a/apps/mobile/src/components/kilo-chat/message-bubble.tsx b/apps/mobile/src/components/kilo-chat/message-bubble.tsx index 63644b52f4..0743d4c142 100644 --- a/apps/mobile/src/components/kilo-chat/message-bubble.tsx +++ b/apps/mobile/src/components/kilo-chat/message-bubble.tsx @@ -1,7 +1,7 @@ import { type ExecApprovalDecision, type KiloChatClient, type Message } from '@kilocode/kilo-chat'; import { Reply } from 'lucide-react-native'; import { memo } from 'react'; -import { Pressable, View } from 'react-native'; +import { type AccessibilityActionEvent, Pressable, View } from 'react-native'; import { Gesture, GestureDetector } from 'react-native-gesture-handler'; import { scheduleOnRN } from 'react-native-worklets'; import Animated, { @@ -15,6 +15,8 @@ import Animated, { import { Text } from '@/components/ui/text'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { cn } from '@/lib/utils'; +import { buildMessageBubbleAccessibilityProps } from './message-bubble-a11y'; +import { MessageBubbleContent } from './message-bubble-content'; import { getSwipeReplyActiveOffsetX, resolveLongPressFeedback, @@ -22,7 +24,6 @@ import { SWIPE_REPLY_DISTANCE, SWIPE_REPLY_MAX_TRANSLATE, } from './message-gesture-state'; -import { MessageBubbleContent } from './message-bubble-content'; import { isMessageEdited, type ReplyPreviewSource } from './message-presentation'; import { MessageReactionPills } from './message-reaction-pills'; @@ -106,6 +107,33 @@ function MessageBubbleComponent({ onLongPress?.(message); } + // Mirror the long-press (action menu) and swipe-reply gestures as + // accessibility custom actions so VoiceOver / TalkBack rotor users + // reach the same affordances without a discoverable gesture. The + // wrapping Pressable is explicitly `accessible={false}` so iOS does not + // collapse the message text, exec-approval buttons, reaction pills, or + // attachment buttons into a single, unnavigable node. The actions themselves + // live on an inset-matched, non-interactive, focusable overlay so the rotor + // still has a target while the message subtree stays individually navigable. + // The overlay is only rendered when at least one action exists; otherwise it + // would become a content-free duplicate VoiceOver stop. + function handleBubbleAccessibilityAction(event: AccessibilityActionEvent) { + if (event.nativeEvent.actionName === 'reply') { + onSwipeReply?.(message); + return; + } + if (event.nativeEvent.actionName === 'more-actions') { + onLongPress?.(message); + } + } + + const bubbleA11y = buildMessageBubbleAccessibilityProps({ + isFromMe, + authorLabel, + canSwipeReply, + canLongPress: onLongPress !== undefined, + }); + // eslint-disable-next-line new-cap -- RNGH's gesture builder API is Gesture.Pan(). const swipeGesture = Gesture.Pan() .activeOffsetX(getSwipeReplyActiveOffsetX()) @@ -151,6 +179,7 @@ function MessageBubbleComponent({ onPressIn={handlePressIn} onPressOut={handlePressOut} onLongPress={onLongPress ? handleLongPress : undefined} + accessible={bubbleA11y.accessible} className={cn( 'px-4 py-1', isFromMe ? 'items-end' : 'items-start', @@ -226,6 +255,17 @@ function MessageBubbleComponent({ onReactionPress={onReactionPress} /> + + {bubbleA11y.accessibilityActions.length > 0 && ( + + )} ); From bcbf161c4473be6639c1c4749cd0d095b16bb718 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 06:26:12 +0200 Subject: [PATCH 04/17] feat(mobile-a11y): reflow core/profile/auth layouts at max text --- apps/mobile/src/components/login-screen.tsx | 34 ++++++---- .../mobile/src/components/login/idle-auth.tsx | 6 +- .../src/components/profile-action-tile.tsx | 6 +- apps/mobile/src/components/profile-screen.tsx | 66 +++++++++---------- .../src/components/ui/configure-row.tsx | 63 +++++++++++++----- 5 files changed, 106 insertions(+), 69 deletions(-) diff --git a/apps/mobile/src/components/login-screen.tsx b/apps/mobile/src/components/login-screen.tsx index 2992119d06..d859edd2fb 100644 --- a/apps/mobile/src/components/login-screen.tsx +++ b/apps/mobile/src/components/login-screen.tsx @@ -82,19 +82,19 @@ export function LoginScreen() { return ( - + - Welcome to Kilo Code + Welcome to Kilo Code {status === 'idle' && ( @@ -104,36 +104,40 @@ export function LoginScreen() { {status === 'pending' && code && ( - Your sign-in code: + + Your sign-in code: + {code} - + {/* Stack actions full-width so labels never clip side-by-side at max text */} + @@ -167,7 +173,7 @@ export function LoginScreen() { {(status === 'denied' || status === 'expired' || status === 'error') && ( diff --git a/apps/mobile/src/components/login/idle-auth.tsx b/apps/mobile/src/components/login/idle-auth.tsx index 46d75cd27c..8e3b12e033 100644 --- a/apps/mobile/src/components/login/idle-auth.tsx +++ b/apps/mobile/src/components/login/idle-auth.tsx @@ -135,13 +135,15 @@ export function IdleAuth({ )} diff --git a/apps/mobile/src/components/profile-action-tile.tsx b/apps/mobile/src/components/profile-action-tile.tsx index d874c0a1a5..721252be0f 100644 --- a/apps/mobile/src/components/profile-action-tile.tsx +++ b/apps/mobile/src/components/profile-action-tile.tsx @@ -19,7 +19,7 @@ export function ActionTile({ }) { return ( - + {label} diff --git a/apps/mobile/src/components/profile-screen.tsx b/apps/mobile/src/components/profile-screen.tsx index 5ef871dc91..544b03e5c8 100644 --- a/apps/mobile/src/components/profile-screen.tsx +++ b/apps/mobile/src/components/profile-screen.tsx @@ -247,11 +247,11 @@ export function ProfileScreen() { return ( - + {p.provider} {p.email} @@ -275,40 +275,36 @@ export function ProfileScreen() { ) : null} - {/* Actions */} + {/* Actions — stacked full-width tiles so labels never clip side-by-side at max Dynamic Type */} - - { - showFeedbackPrompt(userId); - }} - /> - - - - - - + { + showFeedbackPrompt(userId); + }} + /> + + + v{Application.nativeApplicationVersion} ({Application.nativeBuildVersion}) diff --git a/apps/mobile/src/components/ui/configure-row.tsx b/apps/mobile/src/components/ui/configure-row.tsx index a0f71d6e83..9aa3503ec0 100644 --- a/apps/mobile/src/components/ui/configure-row.tsx +++ b/apps/mobile/src/components/ui/configure-row.tsx @@ -1,12 +1,19 @@ import { ChevronRight, type LucideIcon } from 'lucide-react-native'; import { type ReactNode } from 'react'; -import { Pressable, View } from 'react-native'; +import { Pressable, useWindowDimensions, View } from 'react-native'; import { Text } from '@/components/ui/text'; import { agentColor, type Tint, toneColor, type ToneKey } from '@/lib/agent-color'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { cn } from '@/lib/utils'; +/** + * At/above this Dynamic Type scale, ConfigureRow stacks the icon above the + * title block so long labels never clip against the chevron in a side row. + * Matches the tab-label wrap threshold used elsewhere in the shell. + */ +export const CONFIGURE_ROW_STACK_FONT_SCALE = 1.8; + type ConfigureRowProps = { icon: LucideIcon; title: string; @@ -38,36 +45,60 @@ export function ConfigureRow({ className, }: Readonly) { const colors = useThemeColors(); + const { fontScale } = useWindowDimensions(); + const stack = fontScale >= CONFIGURE_ROW_STACK_FONT_SCALE; const tint: Tint = tone ? toneColor(tone) : agentColor(title); const iconColor = colors[tint.hueThemeKey]; // Inert rows (no onPress) and disabled rows are not tappable — hide the // chevron so they don't look tappable, and never render pressed feedback. const showChevron = Boolean(onPress) && !disabled; + const trailingNode = + trailing ?? (showChevron ? : null); + + const iconTile = ( + + + + ); + + const textBlock = ( + + {title} + {subtitle ? {subtitle} : null} + + ); const inner = ( - - - - - {title} - {subtitle ? {subtitle} : null} - - {trailing ?? (showChevron ? : null)} + {stack ? ( + <> + + {iconTile} + {trailingNode ? {trailingNode} : null} + + {textBlock} + + ) : ( + <> + {iconTile} + {textBlock} + {trailingNode} + + )} ); From 01268bb66fa736aaad2c2a715e548ba85c189fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 06:26:12 +0200 Subject: [PATCH 05/17] feat(mobile-a11y): scale PR diff text with bounded font scale --- .../pr-review/diff/diff-font-metrics.test.ts | 146 ++++++++++++++++++ .../pr-review/diff/diff-font-metrics.ts | 123 +++++++++++++++ .../components/pr-review/diff/diff-line.tsx | 80 +++++----- .../pr-review/diff/pr-diff-file-list.tsx | 83 +++++----- .../diff/pr-diff-side-by-side-row.tsx | 101 ++++++------ 5 files changed, 419 insertions(+), 114 deletions(-) create mode 100644 apps/mobile/src/components/pr-review/diff/diff-font-metrics.test.ts create mode 100644 apps/mobile/src/components/pr-review/diff/diff-font-metrics.ts diff --git a/apps/mobile/src/components/pr-review/diff/diff-font-metrics.test.ts b/apps/mobile/src/components/pr-review/diff/diff-font-metrics.test.ts new file mode 100644 index 0000000000..1f552b1978 --- /dev/null +++ b/apps/mobile/src/components/pr-review/diff/diff-font-metrics.test.ts @@ -0,0 +1,146 @@ +// Unit tests for the bounded font-scale metrics that drive the PR +// diff surface (P1-C-22). The metrics cap `useWindowDimensions().fontScale` +// at `DIFF_MAX_FONT_SCALE` so the diff layout (gutter, side-by-side +// grid) stays intact even at the largest accessibility scale. + +// The metrics describe the actual rendered result: the `Text` nodes in +// the diff rows set `maxFontSizeMultiplier={DIFF_MAX_FONT_SCALE}` and +// supply UNSCALED base font sizes / line heights. The native pipeline +// applies the bounded scale exactly once; the JS side only pre-scales +// `rowMinHeight` so the row always fits the capped text + padding. + +import { + type BoundedFontMetrics, + computeBoundedDiffFontMetrics, + DIFF_BASE_LINE_HEIGHT, + DIFF_BASE_ROW_MIN_HEIGHT, + DIFF_CODE_BASE_FONT_SIZE, + DIFF_LABEL_BASE_FONT_SIZE, + DIFF_MAX_FONT_SCALE, + DIFF_VERTICAL_PADDING, + useBoundedDiffFontMetrics, +} from './diff-font-metrics'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const windowDimensionsState = vi.hoisted(() => ({ fontScale: 1 })); + +vi.mock('react-native', () => ({ + useWindowDimensions: () => ({ + fontScale: windowDimensionsState.fontScale, + width: 390, + height: 844, + scale: 2, + }), +})); + +describe('computeBoundedDiffFontMetrics', () => { + it('returns base sizes when the system reports a 1.0 scale', () => { + const m = computeBoundedDiffFontMetrics(1); + expect(m.scale).toBe(1); + expect(m.codeFontSize).toBe(DIFF_CODE_BASE_FONT_SIZE); + expect(m.labelFontSize).toBe(DIFF_LABEL_BASE_FONT_SIZE); + expect(m.lineHeight).toBe(DIFF_BASE_LINE_HEIGHT); + expect(m.rowMinHeight).toBe(DIFF_BASE_ROW_MIN_HEIGHT); + }); + + it('keeps text dimensions unscaled; rowMinHeight grows with the bounded scale', () => { + const m = computeBoundedDiffFontMetrics(1.18); + expect(m.scale).toBe(1.18); + expect(m.codeFontSize).toBe(DIFF_CODE_BASE_FONT_SIZE); + expect(m.labelFontSize).toBe(DIFF_LABEL_BASE_FONT_SIZE); + expect(m.lineHeight).toBe(DIFF_BASE_LINE_HEIGHT); + expect(m.rowMinHeight).toBeCloseTo(DIFF_BASE_LINE_HEIGHT * 1.18 + DIFF_VERTICAL_PADDING * 2, 5); + }); + + it('caps at DIFF_MAX_FONT_SCALE and still keeps text dimensions unscaled', () => { + const m = computeBoundedDiffFontMetrics(1.8); + expect(m.scale).toBe(DIFF_MAX_FONT_SCALE); + expect(m.codeFontSize).toBe(DIFF_CODE_BASE_FONT_SIZE); + expect(m.labelFontSize).toBe(DIFF_LABEL_BASE_FONT_SIZE); + expect(m.lineHeight).toBe(DIFF_BASE_LINE_HEIGHT); + expect(m.rowMinHeight).toBeCloseTo( + DIFF_BASE_LINE_HEIGHT * DIFF_MAX_FONT_SCALE + DIFF_VERTICAL_PADDING * 2, + 5 + ); + }); + + it('clamps an absurdly large system scale to the cap', () => { + const m = computeBoundedDiffFontMetrics(5); + expect(m.scale).toBe(DIFF_MAX_FONT_SCALE); + }); + + it('treats invalid input (null / undefined / non-finite) as 1.0', () => { + expect(computeBoundedDiffFontMetrics(null).scale).toBe(1); + expect(computeBoundedDiffFontMetrics(undefined).scale).toBe(1); + expect(computeBoundedDiffFontMetrics(Number.NaN).scale).toBe(1); + expect(computeBoundedDiffFontMetrics(Number.POSITIVE_INFINITY).scale).toBe(1); + }); + + it('clamps sub-1 input (e.g. tiny font preference) up to 1.0', () => { + const m = computeBoundedDiffFontMetrics(0.85); + expect(m.scale).toBe(1); + expect(m.codeFontSize).toBeCloseTo(DIFF_CODE_BASE_FONT_SIZE, 5); + }); + + it('keeps row height >= base so scaled text is never clipped', () => { + for (const scale of [1, 1.18, 1.4]) { + const m = computeBoundedDiffFontMetrics(scale); + expect(m.rowMinHeight).toBeGreaterThanOrEqual(DIFF_BASE_ROW_MIN_HEIGHT); + // lineHeight stays at the unscaled base; the native maxFontSizeMultiplier + // scales the actual line box. rowMinHeight must cover that scaled box. + expect(m.lineHeight).toBe(DIFF_BASE_LINE_HEIGHT); + } + }); + + it('keeps the row exactly as tall as the capped text plus padding at every scale', () => { + // The production formula: rowMinHeight = lineHeight * scale + 2 * padding. + // lineHeight is unscaled; the native maxFontSizeMultiplier scales it by + // `scale` at render time. This test guards against accidental padding + // scaling or double-counting that could clip the code text. + for (const scale of [1, 1.05, 1.18, 1.25, 1.4]) { + const m = computeBoundedDiffFontMetrics(scale); + expect(m.rowMinHeight).toBeCloseTo( + DIFF_BASE_LINE_HEIGHT * scale + DIFF_VERTICAL_PADDING * 2, + 5 + ); + expect(m.rowMinHeight).toBeCloseTo(m.lineHeight * m.scale + DIFF_VERTICAL_PADDING * 2, 5); + } + }); + + it('describes the actual rendered result: text is scaled once by the native pipeline', () => { + // The diff rows set `maxFontSizeMultiplier={DIFF_MAX_FONT_SCALE}` on + // `Text` nodes and supply these unscaled base values. The effective + // rendered size is base * boundedScale, applied exactly once by RN. + // rowMinHeight must fit that single-scaled line box + padding. + const m = computeBoundedDiffFontMetrics(1.8); + expect(m.scale).toBe(DIFF_MAX_FONT_SCALE); + expect(m.codeFontSize).toBe(DIFF_CODE_BASE_FONT_SIZE); + expect(m.labelFontSize).toBe(DIFF_LABEL_BASE_FONT_SIZE); + expect(m.lineHeight).toBe(DIFF_BASE_LINE_HEIGHT); + expect(m.rowMinHeight).toBeCloseTo(m.lineHeight * m.scale + DIFF_VERTICAL_PADDING * 2, 5); + }); + + it('returns a BoundedFontMetrics type that callers can use', () => { + const m: BoundedFontMetrics = computeBoundedDiffFontMetrics(DIFF_MAX_FONT_SCALE); + expect(m.scale).toBe(DIFF_MAX_FONT_SCALE); + }); +}); + +describe('useBoundedDiffFontMetrics', () => { + beforeEach(() => { + windowDimensionsState.fontScale = 1; + }); + afterEach(() => { + windowDimensionsState.fontScale = 1; + }); + + it('reflects changes to useWindowDimensions().fontScale', () => { + windowDimensionsState.fontScale = 1.18; + const m1 = useBoundedDiffFontMetrics(); + expect(m1.scale).toBe(1.18); + + windowDimensionsState.fontScale = 1.8; + const m2 = useBoundedDiffFontMetrics(); + expect(m2.scale).toBe(DIFF_MAX_FONT_SCALE); + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/diff-font-metrics.ts b/apps/mobile/src/components/pr-review/diff/diff-font-metrics.ts new file mode 100644 index 0000000000..4f724c0474 --- /dev/null +++ b/apps/mobile/src/components/pr-review/diff/diff-font-metrics.ts @@ -0,0 +1,123 @@ +// Bounded font-scale metrics for the PR diff surface. +// +// The PR diff renders thousands of JetBrains Mono lines in a tight +// 56-point gutter and a 2-column side-by-side grid. Honouring the +// user's accessibility font scale verbatim (iOS Dynamic Type can reach +// ~1.8x at AX5) would either overflow the gutter, break the side-by- +// side alignment, or destroy the diff's visual density. +// +// We scale text by a BOUNDED `useWindowDimensions().fontScale` so a11y users +// still get meaningfully larger text, but the cap keeps the diff +// surface legible. The cap is intentionally below the iOS large-text +// step (1.18) and the system max — see `MAX_FONT_SCALE` below. +// +// Cap rationale (chosen 2026-07 for P1-C-22): +// * 1.0 → disables a11y (the prior `allowFontScaling={false}`). +// * 1.18 → iOS large-text default. Diff text barely moves. +// * 1.4 → our cap. ~19% larger than iOS large-text, 19% smaller +// than the system max. Preserves gutter + side-by-side +// grid; matches the diff density that defines the UX. +// * 1.8+ → gutter overflow + side-by-side misalignment. + +import { createContext, useContext } from 'react'; +import { useWindowDimensions } from 'react-native'; + +/** + * Maximum font scale honoured by the PR diff surface. The cap is + * applied to `useWindowDimensions().fontScale`; values below the cap + * are passed through so a11y users see their preferred scale. + * + * The same value is used as `maxFontSizeMultiplier` on diff `Text` + * nodes so the native pipeline applies the bounded scale exactly once. + * + * Documented at 1.4. See file header for the full rationale. + */ +export const DIFF_MAX_FONT_SCALE = 1.4; + +/** Minimum base font size for the code text in the diff (unscaled pt). */ +export const DIFF_CODE_BASE_FONT_SIZE = 12; +/** Minimum base font size for gutter + "no newline" labels (unscaled pt). */ +export const DIFF_LABEL_BASE_FONT_SIZE = 11; +/** Unscaled line height used for the code text. */ +export const DIFF_BASE_LINE_HEIGHT = 18; +/** Unscaled vertical padding above + below the code text inside a row. */ +export const DIFF_VERTICAL_PADDING = 2; +/** Unscaled minimum row height (lineHeight + 2 * vertical padding). */ +export const DIFF_BASE_ROW_MIN_HEIGHT = DIFF_BASE_LINE_HEIGHT + DIFF_VERTICAL_PADDING * 2; + +export type BoundedFontMetrics = { + /** Clamped font scale actually applied by `maxFontSizeMultiplier` (0 < scale <= DIFF_MAX_FONT_SCALE). */ + readonly scale: number; + /** Unscaled code font size in pt; the native `maxFontSizeMultiplier` applies `scale`. */ + readonly codeFontSize: number; + /** Unscaled label (gutter / no-newline) font size in pt; the native `maxFontSizeMultiplier` applies `scale`. */ + readonly labelFontSize: number; + /** Unscaled line height used for both code and label text; the native `maxFontSizeMultiplier` applies `scale`. */ + readonly lineHeight: number; + /** Scaled minimum row height (lineHeight * scale + 2 * vertical padding). */ + readonly rowMinHeight: number; +}; + +/** + * Pure, deterministic metric computation. Exported so tests can assert + * the curve without rendering. + * + * The returned `codeFontSize`, `labelFontSize` and `lineHeight` are the + * UNSCALED base values. The actual rendered size is produced by React + * Native when `maxFontSizeMultiplier={DIFF_MAX_FONT_SCALE}` is set on + * the `Text` node (the scale applied by the native pipeline is bounded + * by `scale`). `rowMinHeight` is pre-scaled so the row always fits the + * natively-capped text + vertical padding. + */ +export function computeBoundedDiffFontMetrics( + fontScale: number | null | undefined +): BoundedFontMetrics { + // Treat invalid / negative / non-finite input as 1.0 (no scaling). + // `useWindowDimensions().fontScale` can be 1.0 on platforms that report + // no accessibility preference; we should never produce sub-1 sizes. + const raw = typeof fontScale === 'number' && Number.isFinite(fontScale) ? fontScale : 1; + const scale = Math.min(Math.max(raw, 1), DIFF_MAX_FONT_SCALE); + return { + scale, + codeFontSize: DIFF_CODE_BASE_FONT_SIZE, + labelFontSize: DIFF_LABEL_BASE_FONT_SIZE, + lineHeight: DIFF_BASE_LINE_HEIGHT, + rowMinHeight: DIFF_BASE_LINE_HEIGHT * scale + DIFF_VERTICAL_PADDING * 2, + }; +} + +const defaultMetrics = computeBoundedDiffFontMetrics(1); + +/** + * Context that carries the bounded diff font metrics down to individual + * diff rows. The value is stable between font-scale changes, so rows + * wrapped in `memo` only re-render when the scale actually changes. + */ +export const DiffFontMetricsContext = createContext(defaultMetrics); + +/** + * Hook for child rows to read the current bounded diff font metrics. + * Falls back to the 1.0 metrics when rendered outside a provider. + */ +export function useDiffFontMetrics(): BoundedFontMetrics { + return useContext(DiffFontMetricsContext); +} + +const metricsCache = new Map(); + +/** + * React hook wrapper that reads the reactive system font scale from + * `useWindowDimensions().fontScale` and returns the bounded diff metrics. + * The result is cached by its bounded scale so the returned reference is + * stable between renders when the scale hasn't changed. + */ +export function useBoundedDiffFontMetrics(): BoundedFontMetrics { + const { fontScale } = useWindowDimensions(); + const metrics = computeBoundedDiffFontMetrics(fontScale); + const cached = metricsCache.get(metrics.scale); + if (cached) { + return cached; + } + metricsCache.set(metrics.scale, metrics); + return metrics; +} diff --git a/apps/mobile/src/components/pr-review/diff/diff-line.tsx b/apps/mobile/src/components/pr-review/diff/diff-line.tsx index a77fad4226..f739705147 100644 --- a/apps/mobile/src/components/pr-review/diff/diff-line.tsx +++ b/apps/mobile/src/components/pr-review/diff/diff-line.tsx @@ -2,10 +2,10 @@ // highlighting, a gutter for old/new line numbers, and a tinted // background that signals add / del / context. // -// We render fixed-height rows (height = lineHeight + vertical padding) -// so FlashList can virtualize without measuring each row. The diff -// surface renders thousands of lines and remeasuring on every scroll -// frame would destroy scroll perf on mid-tier Android devices. +// Row height scales with the system font scale (bounded — see +// `diff-font-metrics.ts`). The cap preserves diff density and the +// 56-point gutter; honouring the raw a11y scale (1.8x at AX5) would +// overflow the gutter and break the side-by-side grid. // // S7a adds two opt-in behaviours, both passed from the diff list: // - `onTap` makes the line tappable; the diff list runs the @@ -21,35 +21,15 @@ import { highlightLine, type HighlightToken } from '@/lib/pr-review/diff/highlig import { type ParsedDiffLine } from '@/lib/pr-review/diff/parse-patch'; import { MUTED_COLOR, tokenColorFor } from '@/lib/pr-review/diff/syntax-colors'; import { cn } from '@/lib/utils'; +import { + DIFF_MAX_FONT_SCALE, + useDiffFontMetrics, +} from '@/components/pr-review/diff/diff-font-metrics'; -const LINE_HEIGHT = 18; -const VERTICAL_PADDING = 2; const GUTTER_WIDTH = 56; +const VERTICAL_PADDING = 2; const NO_NEWLINE_INDICATOR = '\u26A0\uFE0F no newline at end of file'; -const ROW_MIN_HEIGHT = LINE_HEIGHT + VERTICAL_PADDING * 2; -const ROW_STYLE: ViewStyle = { minHeight: ROW_MIN_HEIGHT }; -const GUTTER_STYLE: ViewStyle = { - width: GUTTER_WIDTH, - height: ROW_MIN_HEIGHT, -}; -const CODE_CONTAINER_STYLE: ViewStyle = { paddingVertical: VERTICAL_PADDING }; -const CODE_BASE_STYLE: TextStyle = { - fontFamily: 'JetBrainsMono_500Medium', - fontSize: 12, - lineHeight: LINE_HEIGHT, -}; -const GUTTER_TEXT_BASE: TextStyle = { - fontFamily: 'JetBrainsMono_500Medium', - fontSize: 11, - lineHeight: LINE_HEIGHT, -}; -const NO_NEWLINE_BASE: TextStyle = { - fontFamily: 'JetBrainsMono_500Medium', - fontSize: 11, - lineHeight: LINE_HEIGHT, -}; - type DiffLineProps = { line: ParsedDiffLine; language: string | null; @@ -83,6 +63,7 @@ function rowBackgroundFor(type: ParsedDiffLine['type']): string { function DiffLineImpl({ line, language, onTap, isSelected }: Readonly) { const colors = useThemeColors(); const isDark = colors.background === '#0E0E10'; + const metrics = useDiffFontMetrics(); const tokens = useMemo( () => highlightLine(line.text, language), @@ -95,25 +76,52 @@ function DiffLineImpl({ line, language, onTap, isSelected }: Readonly - + + {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme color + mono font for gutter */} - + {gutterText} - + {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme color + mono font for code */} {tokens.map((token, index) => { const tokenColor = tokenColorFor(token.className, isDark); @@ -126,7 +134,7 @@ function DiffLineImpl({ line, language, onTap, isSelected }: Readonly{noNewlineLabel} + {noNewlineLabel} ) : null} diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list.tsx index 4e21e556a6..2d006ff23f 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list.tsx @@ -19,6 +19,10 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import { View } from 'react-native'; import { QueryError } from '@/components/query-error'; +import { + DiffFontMetricsContext, + useBoundedDiffFontMetrics, +} from '@/components/pr-review/diff/diff-font-metrics'; import { PrReviewReconnectNotice } from '@/components/pr-review/pr-review-reconnect-notice'; import { PrDiffFileListHeader, @@ -67,6 +71,11 @@ export function PrReviewFileList({ onRequestOverview, }: PrReviewFileListProps) { const listRef = useRef>(null); + // Bounded font-scale metrics for diff rows. We pass the scale into + // `extraData` so FlashList re-measures every row when the user changes + // a11y text size. The metrics are also provided via context so memoized + // diff rows receive the live scale and resize to fit. + const diffFontMetrics = useBoundedDiffFontMetrics(); const { query, firstPageErrorState } = usePrReviewFileListQuery({ owner, @@ -246,40 +255,44 @@ export function PrReviewFileList({ const effectiveViewMode = isTablet ? viewMode : 'unified'; return ( - - - item.key} - getItemType={item => itemTypeFor(item)} - onEndReached={() => { - if (query.hasNextPage && !query.isFetchingNextPage) { - void query.fetchNextPage(); - } - }} - onEndReachedThreshold={0.5} - contentContainerStyle={LIST_CONTENT_STYLE} - ItemSeparatorComponent={null} - /> - - + + + + item.key} + getItemType={item => itemTypeFor(item)} + // Re-measure rows when the bounded font scale changes. + extraData={diffFontMetrics.scale} + onEndReached={() => { + if (query.hasNextPage && !query.isFetchingNextPage) { + void query.fetchNextPage(); + } + }} + onEndReachedThreshold={0.5} + contentContainerStyle={LIST_CONTENT_STYLE} + ItemSeparatorComponent={null} + /> + + + ); } diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx index 98c4e7eaf7..e6a6ea0e0b 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx @@ -8,10 +8,10 @@ // Side-by-side is read-only — commenting is unified-view only — so the // row does not accept tap/selection handlers. // -// Renders fixed-height rows so FlashList can virtualize without -// remeasuring. The row height matches the unified `DiffLine` row so -// mixed view-mode content (if the toggle changes mid-scroll) would -// still fit a stable grid. +// Row height scales with the system font scale (bounded — see +// `diff-font-metrics.ts`). The cap keeps both columns aligned even at +// the largest a11y scale we honour; without the cap, the gutter would +// overflow at AX5 (1.8x). import { memo, useMemo } from 'react'; import { Text as RNText, type TextStyle, View, type ViewStyle } from 'react-native'; @@ -23,36 +23,17 @@ import { type ParsedDiffLine, type ParsedHunk } from '@/lib/pr-review/diff/parse import { type SideBySideRow as SideBySideRowData } from '@/lib/pr-review/diff/side-by-side'; import { MUTED_COLOR, tokenColorFor } from '@/lib/pr-review/diff/syntax-colors'; import { cn } from '@/lib/utils'; +import { + type BoundedFontMetrics, + DIFF_MAX_FONT_SCALE, + useDiffFontMetrics, +} from '@/components/pr-review/diff/diff-font-metrics'; -const LINE_HEIGHT = 18; -const VERTICAL_PADDING = 2; const COLUMN_GUTTER_WIDTH = 56; const COLUMN_INNER_PADDING = 2; +const VERTICAL_PADDING = 2; const NO_NEWLINE_INDICATOR = '\u26A0\uFE0F no newline at end of file'; -const ROW_MIN_HEIGHT = LINE_HEIGHT + VERTICAL_PADDING * 2; -const ROW_STYLE: ViewStyle = { minHeight: ROW_MIN_HEIGHT }; -const GUTTER_STYLE: ViewStyle = { - width: COLUMN_GUTTER_WIDTH, - height: ROW_MIN_HEIGHT, -}; -const CODE_CONTAINER_STYLE: ViewStyle = { paddingVertical: VERTICAL_PADDING }; -const CODE_BASE_STYLE: TextStyle = { - fontFamily: 'JetBrainsMono_500Medium', - fontSize: 12, - lineHeight: LINE_HEIGHT, -}; -const GUTTER_TEXT_BASE: TextStyle = { - fontFamily: 'JetBrainsMono_500Medium', - fontSize: 11, - lineHeight: LINE_HEIGHT, -}; -const NO_NEWLINE_BASE: TextStyle = { - fontFamily: 'JetBrainsMono_500Medium', - fontSize: 11, - lineHeight: LINE_HEIGHT, -}; - type SideBySideRowProps = { row: SideBySideRowData; language: string | null; @@ -91,6 +72,7 @@ type SideColumnProps = { }; function SideColumnImpl({ line, side, language, isDark, foreground }: SideColumnProps) { + const metrics = useDiffFontMetrics(); const tokens = useMemo( () => highlightLine(line.text, language), [language, line.text] @@ -100,26 +82,51 @@ function SideColumnImpl({ line, side, language, isDark, foreground }: SideColumn const gutterText = sideGutterText(line, side); const noNewlineLabel = line.noNewlineAtEndOfFile ? ` ${NO_NEWLINE_INDICATOR}` : ''; + const rowStyle: ViewStyle = { minHeight: metrics.rowMinHeight }; + const gutterStyle: ViewStyle = { + width: COLUMN_GUTTER_WIDTH, + minHeight: metrics.rowMinHeight, + }; + const codeContainerStyle: ViewStyle = { paddingVertical: VERTICAL_PADDING }; + const codeBaseStyle: TextStyle = { + fontFamily: 'JetBrainsMono_500Medium', + fontSize: metrics.codeFontSize, + lineHeight: metrics.lineHeight, + }; + const gutterTextBase: TextStyle = { + fontFamily: 'JetBrainsMono_500Medium', + fontSize: metrics.labelFontSize, + lineHeight: metrics.lineHeight, + }; + const noNewlineBase: TextStyle = { + fontFamily: 'JetBrainsMono_500Medium', + fontSize: metrics.labelFontSize, + lineHeight: metrics.lineHeight, + }; + return ( {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme muted color */} - + {gutterText} - + {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme foreground color */} {tokens.map((token, index) => { const tokenColor = tokenColorFor(token.className, isDark); @@ -132,7 +139,7 @@ function SideColumnImpl({ line, side, language, isDark, foreground }: SideColumn })} {noNewlineLabel ? ( // eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic muted color for no-newline marker - {noNewlineLabel} + {noNewlineLabel} ) : null} @@ -151,18 +158,24 @@ const SideColumn = memo( prev.foreground === next.foreground ); -function EmptySideColumn() { +function EmptySideColumn({ metrics }: { metrics: BoundedFontMetrics }) { + const rowStyle: ViewStyle = { minHeight: metrics.rowMinHeight }; + const gutterStyle: ViewStyle = { + width: COLUMN_GUTTER_WIDTH, + minHeight: metrics.rowMinHeight, + }; + const codeContainerStyle: ViewStyle = { paddingVertical: VERTICAL_PADDING }; return ( - + ); } @@ -182,14 +195,16 @@ function describeRow(row: SideBySideRowData): string { function SideBySideRowImpl({ row, language, rowKeyId }: Readonly) { const colors = useThemeColors(); + const metrics = useDiffFontMetrics(); const isDark = colors.background === '#0E0E10'; const leftLine = row.left?.line ?? null; const rightLine = row.right?.line ?? null; + const rowStyle: ViewStyle = { minHeight: metrics.rowMinHeight }; return ( @@ -202,7 +217,7 @@ function SideBySideRowImpl({ row, language, rowKeyId }: Readonly ) : ( - + )} {rightLine ? ( @@ -214,7 +229,7 @@ function SideBySideRowImpl({ row, language, rowKeyId }: Readonly ) : ( - + )} ); From ed0f5a3db8b2e976a5528f615a66180dfcbc2f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 06:26:13 +0200 Subject: [PATCH 06/17] feat(mobile-a11y): announce and focus blocking agent cards --- .../agents/agent-interaction-policy.ts | 2 +- .../agents/blocking-card-state.test.ts | 334 ++++++++++++++++++ .../components/agents/blocking-card-state.ts | 251 +++++++++++++ .../src/components/agents/permission-card.tsx | 223 ++++++++---- .../src/components/agents/question-card.tsx | 162 +++++++-- .../agents/session-detail-content.tsx | 47 ++- .../agents/use-interaction-handlers.ts | 50 ++- apps/mobile/src/lib/a11y/announce.test.ts | 84 +++++ apps/mobile/src/lib/a11y/announce.ts | 37 ++ apps/mobile/vitest.config.ts | 1 + 10 files changed, 1087 insertions(+), 104 deletions(-) create mode 100644 apps/mobile/src/components/agents/blocking-card-state.test.ts create mode 100644 apps/mobile/src/components/agents/blocking-card-state.ts create mode 100644 apps/mobile/src/lib/a11y/announce.test.ts create mode 100644 apps/mobile/src/lib/a11y/announce.ts diff --git a/apps/mobile/src/components/agents/agent-interaction-policy.ts b/apps/mobile/src/components/agents/agent-interaction-policy.ts index 64ba3445d4..50aae011ff 100644 --- a/apps/mobile/src/components/agents/agent-interaction-policy.ts +++ b/apps/mobile/src/components/agents/agent-interaction-policy.ts @@ -1,6 +1,6 @@ type ActiveRequest = { requestId: string } | null | undefined; -type BlockingInteraction = 'question' | 'permission' | 'none'; +export type BlockingInteraction = 'question' | 'permission' | 'none'; export function getBlockingInteraction(input: { activeQuestion: ActiveRequest; diff --git a/apps/mobile/src/components/agents/blocking-card-state.test.ts b/apps/mobile/src/components/agents/blocking-card-state.test.ts new file mode 100644 index 0000000000..838c0aaed6 --- /dev/null +++ b/apps/mobile/src/components/agents/blocking-card-state.test.ts @@ -0,0 +1,334 @@ +import { describe, expect, it, vi } from 'vitest'; +import { type Component, type RefObject } from 'react'; + +import { + applyBlockingCardAppearance, + type BlockingCardA11yDeps, + classifyBlockingSubmissionError, + getBlockingCardPresentation, + getBlockingCardPresentationForKind, +} from './blocking-card-state'; + +function makeTrpcError(code: string): unknown { + return { data: { code } }; +} + +function makeNestedTrpcError(code: string): unknown { + return { shape: { data: { code } } }; +} + +function makeTopLevelTrpcError(code: string): unknown { + return { code }; +} + +describe('getBlockingCardPresentation', () => { + it('returns null when no blocking interaction is active', () => { + expect(getBlockingCardPresentation({ blocking: 'none', submissionError: null })).toBeNull(); + }); + + describe('happy state', () => { + it('exposes the primary CTA and no retry affordance for a question', () => { + const presentation = getBlockingCardPresentation({ + blocking: 'question', + submissionError: null, + }); + + expect(presentation).toEqual({ + kind: 'question', + state: 'happy', + announcement: expect.stringMatching(/needs your input/i), + protocolExplanation: expect.stringMatching(/waiting for your answer/i), + hasPrimaryCta: true, + hasRetryCta: false, + retryAction: null, + hasRejectCta: true, + errorMessage: null, + }); + }); + + it('exposes the primary CTA and never a reject CTA for a permission', () => { + const presentation = getBlockingCardPresentation({ + blocking: 'permission', + submissionError: null, + }); + + expect(presentation).toEqual({ + kind: 'permission', + state: 'happy', + announcement: expect.stringMatching(/needs permission/i), + protocolExplanation: expect.stringMatching(/waiting for permission/i), + hasPrimaryCta: true, + hasRetryCta: false, + retryAction: null, + hasRejectCta: false, + errorMessage: null, + }); + }); + }); + + describe('retryable state', () => { + it('replaces the primary CTA with an answer retry affordance on a question', () => { + const presentation = getBlockingCardPresentation({ + blocking: 'question', + submissionError: { kind: 'retryable', message: 'Network hiccup', action: 'answer' }, + }); + + expect(presentation?.state).toBe('retryable'); + expect(presentation?.hasPrimaryCta).toBe(false); + expect(presentation?.hasRetryCta).toBe(true); + expect(presentation?.retryAction).toBe('answer'); + expect(presentation?.hasRejectCta).toBe(true); + expect(presentation?.errorMessage).toBe('Network hiccup'); + expect(presentation?.announcement).toMatch(/try again/i); + }); + + it('replaces the primary CTA with a reject retry affordance on a question and hides Skip', () => { + const presentation = getBlockingCardPresentation({ + blocking: 'question', + submissionError: { kind: 'retryable', message: 'Skip failed', action: 'reject' }, + }); + + expect(presentation?.state).toBe('retryable'); + expect(presentation?.hasPrimaryCta).toBe(false); + expect(presentation?.hasRetryCta).toBe(true); + expect(presentation?.retryAction).toBe('reject'); + expect(presentation?.hasRejectCta).toBe(false); + expect(presentation?.errorMessage).toBe('Skip failed'); + }); + + it('replaces the primary CTA with a retry affordance on a permission', () => { + const presentation = getBlockingCardPresentation({ + blocking: 'permission', + submissionError: { kind: 'retryable', message: 'Try again', action: 'respond' }, + }); + + expect(presentation?.state).toBe('retryable'); + expect(presentation?.hasPrimaryCta).toBe(false); + expect(presentation?.hasRetryCta).toBe(true); + expect(presentation?.retryAction).toBe('respond'); + expect(presentation?.hasRejectCta).toBe(false); + expect(presentation?.errorMessage).toBe('Try again'); + }); + }); + + describe('non-retryable state', () => { + it('drops every CTA and surfaces an explanatory announcement for a question', () => { + const presentation = getBlockingCardPresentation({ + blocking: 'question', + submissionError: { kind: 'non-retryable', message: 'Request expired' }, + }); + + expect(presentation?.state).toBe('non-retryable'); + expect(presentation?.hasPrimaryCta).toBe(false); + expect(presentation?.hasRetryCta).toBe(false); + expect(presentation?.retryAction).toBeNull(); + expect(presentation?.hasRejectCta).toBe(false); + expect(presentation?.errorMessage).toBe('Request expired'); + expect(presentation?.announcement).toMatch(/no longer available/i); + expect(presentation?.protocolExplanation).not.toMatch(/read-only/i); + }); + + it('drops every CTA and surfaces an explanatory announcement for a permission', () => { + const presentation = getBlockingCardPresentation({ + blocking: 'permission', + submissionError: { kind: 'non-retryable', message: 'Protocol ended' }, + }); + + expect(presentation?.state).toBe('non-retryable'); + expect(presentation?.hasPrimaryCta).toBe(false); + expect(presentation?.hasRetryCta).toBe(false); + expect(presentation?.retryAction).toBeNull(); + expect(presentation?.hasRejectCta).toBe(false); + expect(presentation?.errorMessage).toBe('Protocol ended'); + }); + }); +}); + +describe('classifyBlockingSubmissionError', () => { + it('classifies tRPC NOT_FOUND as non-retryable', () => { + expect(classifyBlockingSubmissionError(makeTrpcError('NOT_FOUND'), 'question')).toEqual({ + kind: 'non-retryable', + message: 'This question is no longer available.', + }); + expect(classifyBlockingSubmissionError(makeTrpcError('NOT_FOUND'), 'permission')).toEqual({ + kind: 'non-retryable', + message: 'This permission request is no longer available.', + }); + }); + + it('reads the terminal code from nested shape and top-level forms', () => { + expect(classifyBlockingSubmissionError(makeNestedTrpcError('NOT_FOUND'), 'question')).toEqual({ + kind: 'non-retryable', + message: 'This question is no longer available.', + }); + expect( + classifyBlockingSubmissionError(makeTopLevelTrpcError('NOT_FOUND'), 'permission') + ).toEqual({ + kind: 'non-retryable', + message: 'This permission request is no longer available.', + }); + }); + + it('classifies tRPC transient errors as retryable with answer action by default', () => { + expect( + classifyBlockingSubmissionError(makeTrpcError('INTERNAL_SERVER_ERROR'), 'question') + ).toEqual({ + kind: 'retryable', + message: 'Failed to submit answer. Please try again.', + action: 'answer', + }); + expect( + classifyBlockingSubmissionError(makeTrpcError('PRECONDITION_FAILED'), 'permission') + ).toEqual({ + kind: 'retryable', + message: 'Failed to respond to permission. Please try again.', + action: 'answer', + }); + expect(classifyBlockingSubmissionError(makeTrpcError('TIMEOUT'), 'question')).toEqual({ + kind: 'retryable', + message: 'Failed to submit answer. Please try again.', + action: 'answer', + }); + }); + + it('classifies question reject failures with skip-appropriate messaging', () => { + expect( + classifyBlockingSubmissionError(makeTrpcError('INTERNAL_SERVER_ERROR'), 'question', 'reject') + ).toEqual({ + kind: 'retryable', + message: 'Failed to skip question. Please try again.', + action: 'reject', + }); + }); + + it('classifies permission failures with respond action', () => { + expect( + classifyBlockingSubmissionError(makeTrpcError('TIMEOUT'), 'permission', 'respond') + ).toEqual({ + kind: 'retryable', + message: 'Failed to respond to permission. Please try again.', + action: 'respond', + }); + }); + + it('classifies non-tRPC errors as retryable', () => { + expect(classifyBlockingSubmissionError(new Error('network down'), 'question')).toEqual({ + kind: 'retryable', + message: 'Failed to submit answer. Please try again.', + action: 'answer', + }); + expect(classifyBlockingSubmissionError('string error', 'permission')).toEqual({ + kind: 'retryable', + message: 'Failed to respond to permission. Please try again.', + action: 'answer', + }); + expect(classifyBlockingSubmissionError(null, 'question')).toEqual({ + kind: 'retryable', + message: 'Failed to submit answer. Please try again.', + action: 'answer', + }); + }); +}); + +describe('applyBlockingCardAppearance', () => { + function makeDeps(): BlockingCardA11yDeps { + return { + announce: vi.fn<(message: string) => void>(), + focus: vi.fn<(ref: RefObject) => boolean>().mockReturnValue(true), + }; + } + + it('invokes the announce helper with the presentation announcement on appearance', () => { + const deps = makeDeps(); + const ref: RefObject = { current: null }; + const presentation = getBlockingCardPresentationForKind({ + kind: 'question', + submissionError: null, + }); + + applyBlockingCardAppearance(presentation, ref, deps); + + expect(deps.announce).toHaveBeenCalledTimes(1); + expect(deps.announce).toHaveBeenCalledWith(presentation.announcement); + }); + + it('moves a11y focus to the card ref on appearance', () => { + const deps = makeDeps(); + const ref: RefObject = { + current: { node: 'card' } as unknown as Component, + }; + const presentation = getBlockingCardPresentationForKind({ + kind: 'permission', + submissionError: null, + }); + + applyBlockingCardAppearance(presentation, ref, deps); + + expect(deps.focus).toHaveBeenCalledTimes(1); + expect(deps.focus).toHaveBeenCalledWith(ref); + }); + + it('still announces when the focus move cannot find a node handle', () => { + const deps: BlockingCardA11yDeps = { + announce: vi.fn<(message: string) => void>(), + focus: vi.fn<(ref: RefObject) => boolean>().mockReturnValue(false), + }; + const ref: RefObject = { current: null }; + const presentation = getBlockingCardPresentationForKind({ + kind: 'question', + submissionError: { kind: 'retryable', message: 'Try again', action: 'answer' }, + }); + + const cleanup = applyBlockingCardAppearance(presentation, ref, deps); + + expect(deps.announce).toHaveBeenCalledWith(presentation.announcement); + expect(deps.focus).toHaveBeenCalledWith(ref); + expect(cleanup).toBeTypeOf('function'); + }); + + it('retries focus once on the next tick when the first attempt misses the handle', () => { + vi.useFakeTimers(); + const focusMock = vi.fn<(ref: RefObject) => boolean>(); + focusMock.mockReturnValueOnce(false).mockReturnValueOnce(true); + const deps: BlockingCardA11yDeps = { + announce: vi.fn<(message: string) => void>(), + focus: focusMock, + }; + const ref: RefObject = { current: null }; + const presentation = getBlockingCardPresentationForKind({ + kind: 'question', + submissionError: null, + }); + + applyBlockingCardAppearance(presentation, ref, deps); + expect(focusMock).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(50); + expect(focusMock).toHaveBeenCalledTimes(2); + expect(focusMock).toHaveBeenLastCalledWith(ref); + + vi.useRealTimers(); + }); + + it('clears the pending retry when the cleanup function runs', () => { + vi.useFakeTimers(); + const focusMock = vi.fn<(ref: RefObject) => boolean>().mockReturnValue(false); + const deps: BlockingCardA11yDeps = { + announce: vi.fn<(message: string) => void>(), + focus: focusMock, + }; + const ref: RefObject = { current: null }; + const presentation = getBlockingCardPresentationForKind({ + kind: 'permission', + submissionError: null, + }); + + const cleanup = applyBlockingCardAppearance(presentation, ref, deps); + cleanup?.(); + + vi.advanceTimersByTime(50); + expect(focusMock).toHaveBeenCalledTimes(1); + + vi.useRealTimers(); + }); +}); diff --git a/apps/mobile/src/components/agents/blocking-card-state.ts b/apps/mobile/src/components/agents/blocking-card-state.ts new file mode 100644 index 0000000000..b88a826146 --- /dev/null +++ b/apps/mobile/src/components/agents/blocking-card-state.ts @@ -0,0 +1,251 @@ +// Pure FSM describing the accessibility presentation for a blocking +// question/permission card. The card itself consumes the result and the +// orchestrator (`session-detail-content.tsx`) consumes the announcement to +// route screen-reader focus. Keeping the logic in a module of pure functions +// lets the repo's `node` vitest env cover the selection + CTA rules without +// rendering React Native. + +import { type Component, type RefObject } from 'react'; + +import { type BlockingInteraction } from './agent-interaction-policy'; + +type BlockingCardKind = 'question' | 'permission'; + +type BlockingCardUiState = 'happy' | 'retryable' | 'non-retryable'; + +export type BlockingCardRetryAction = 'answer' | 'reject' | 'respond'; + +export type BlockingCardSubmissionError = + | { kind: 'retryable'; message: string; action: BlockingCardRetryAction } + | { kind: 'non-retryable'; message: string }; + +type BlockingCardPresentation = { + kind: BlockingCardKind; + state: BlockingCardUiState; + /** Short TalkBack/VoiceOver announcement fired once on appearance. */ + announcement: string; + /** Body text rendered inside the card explaining why input is blocked. */ + protocolExplanation: string; + /** True when the primary action button (Send / Allow) should render. */ + hasPrimaryCta: boolean; + /** True when a Retry affordance should render in place of the primary CTA. */ + hasRetryCta: boolean; + /** + * When retryable, the action that the retry CTA should re-attempt so the + * recovery affordance matches the failed submission (e.g. a failed Skip + * retries skip, not answer). + */ + retryAction: BlockingCardRetryAction | null; + /** + * True for the question card's "Skip" affordance. Hidden when the active + * retry is for a failed skip so the user sees a single "Retry skip" action. + * Permission cards never have an independent reject path, so this is always + * `false` for `kind: 'permission'`. + */ + hasRejectCta: boolean; + /** Inline error text to render above the CTA row (null when no error). */ + errorMessage: string | null; +}; + +export type BlockingCardA11yDeps = { + announce: (message: string) => void; + focus: (ref: RefObject) => boolean; +}; + +/** + * Resolve the presentation for a blocking interaction card, or `null` when + * no card is mounted. The function is pure: callers pass the current + * `blocking` interaction kind and the optional submission error. + */ +export function getBlockingCardPresentation(input: { + blocking: BlockingInteraction; + submissionError: BlockingCardSubmissionError | null; +}): BlockingCardPresentation | null { + if (input.blocking === 'none') { + return null; + } + return getBlockingCardPresentationForKind({ + kind: input.blocking, + submissionError: input.submissionError, + }); +} + +/** + * Non-null variant for use by the question/permission card components, which + * only mount when a blocking interaction is active. Keeping this as a + * separate function lets the strict TS checker see the non-null return + * without the component having to assert. + */ +export function getBlockingCardPresentationForKind(input: { + kind: BlockingCardKind; + submissionError: BlockingCardSubmissionError | null; +}): BlockingCardPresentation { + const kind = input.kind; + const { submissionError } = input; + + if (submissionError?.kind === 'non-retryable') { + return { + kind, + state: 'non-retryable', + announcement: buildAnnouncement(kind, 'non-retryable'), + protocolExplanation: buildProtocolExplanation(kind, 'non-retryable'), + hasPrimaryCta: false, + hasRetryCta: false, + retryAction: null, + hasRejectCta: false, + errorMessage: submissionError.message, + }; + } + + if (submissionError?.kind === 'retryable') { + return { + kind, + state: 'retryable', + announcement: buildAnnouncement(kind, 'retryable'), + protocolExplanation: buildProtocolExplanation(kind, 'retryable'), + hasPrimaryCta: false, + hasRetryCta: true, + retryAction: submissionError.action, + hasRejectCta: kind === 'question' && submissionError.action !== 'reject', + errorMessage: submissionError.message, + }; + } + + return { + kind, + state: 'happy', + announcement: buildAnnouncement(kind, 'happy'), + protocolExplanation: buildProtocolExplanation(kind, 'happy'), + hasPrimaryCta: true, + hasRetryCta: false, + retryAction: null, + hasRejectCta: kind === 'question', + errorMessage: null, + }; +} + +/** + * Side-effect helper invoked from the card's mount effect. Centralises the + * "announce + move focus" pair here so the React components execute the same + * path the unit tests cover. If the focus target has no node handle yet + * (first-paint race), a follow-up attempt is scheduled on the next tick and the + * cleanup function clears that timeout on unmount/request change. + */ +export function applyBlockingCardAppearance( + presentation: BlockingCardPresentation, + ref: RefObject, + deps: BlockingCardA11yDeps +): (() => void) | undefined { + deps.announce(presentation.announcement); + if (!deps.focus(ref)) { + const handle = setTimeout(() => { + deps.focus(ref); + }, 50); + return () => { + clearTimeout(handle); + }; + } + return undefined; +} + +/** + * Extract the tRPC error code from a thrown value. The tRPC v11 client + * surfaces `data.code`; server-shaped errors expose `shape.data.code`. We + * also accept a top-level `code` field so future tRPC versions can't silently + * change the retryable/non-retryable boundary. + */ +function readTrpcErrorCode(error: unknown): string | undefined { + if (!error || typeof error !== 'object') { + return undefined; + } + const record = error as Record; + const data = record.data; + if (data && typeof data === 'object') { + const code = (data as Record).code; + if (typeof code === 'string') { + return code; + } + } + const shape = record.shape; + if (shape && typeof shape === 'object') { + const shapeData = (shape as Record).data; + if (shapeData && typeof shapeData === 'object') { + const code = (shapeData as Record).code; + if (typeof code === 'string') { + return code; + } + } + } + const top = record.code; + if (typeof top === 'string') { + return top; + } + return undefined; +} + +/** + * Classify a thrown submission failure into a retryable or non-retryable + * blocking card error. The `action` argument lets callers distinguish a + * failed answer from a failed skip, so the recovery message and retry CTA + * match the action the user attempted. + * + * Currently only tRPC `NOT_FOUND` is treated as terminal: the session or wrapper + * is gone, so retrying the same answer/permission cannot succeed. All other + * tRPC errors (including `INTERNAL_SERVER_ERROR`, `PRECONDITION_FAILED`, and + * network failures) default to retryable because the backend does not yet + * expose a distinct terminal signal for a stale question/permission ID. + */ +export function classifyBlockingSubmissionError( + error: unknown, + kind: BlockingCardKind, + action: BlockingCardRetryAction = 'answer' +): BlockingCardSubmissionError { + const code = readTrpcErrorCode(error); + if (code === 'NOT_FOUND') { + return { + kind: 'non-retryable', + message: + kind === 'question' + ? 'This question is no longer available.' + : 'This permission request is no longer available.', + }; + } + return { + kind: 'retryable', + message: buildRetryableMessage(kind, action), + action, + }; +} + +function buildRetryableMessage(kind: BlockingCardKind, action: BlockingCardRetryAction): string { + if (kind === 'question') { + return action === 'reject' + ? 'Failed to skip question. Please try again.' + : 'Failed to submit answer. Please try again.'; + } + return 'Failed to respond to permission. Please try again.'; +} + +function buildAnnouncement(kind: BlockingCardKind, state: BlockingCardUiState): string { + if (state === 'non-retryable') { + return kind === 'question' + ? 'This question is no longer available.' + : 'This permission request is no longer available.'; + } + if (state === 'retryable') { + return 'Submission failed. Please try again.'; + } + return kind === 'question' + ? 'Agent needs your input. The composer is paused while you answer.' + : 'Agent needs permission to continue. The composer is paused while you decide.'; +} + +function buildProtocolExplanation(kind: BlockingCardKind, state: BlockingCardUiState): string { + if (state === 'non-retryable') { + return 'The agent has moved past this prompt. This card will close when the session continues.'; + } + if (kind === 'question') { + return 'The agent is waiting for your answer. While this question is open, your message to the agent is paused.'; + } + return 'The agent is waiting for permission. While this request is open, your message to the agent is paused.'; +} diff --git a/apps/mobile/src/components/agents/permission-card.tsx b/apps/mobile/src/components/agents/permission-card.tsx index 4bf0fecea9..6006a1918f 100644 --- a/apps/mobile/src/components/agents/permission-card.tsx +++ b/apps/mobile/src/components/agents/permission-card.tsx @@ -1,9 +1,15 @@ -import { useState } from 'react'; -import { ActivityIndicator, ScrollView, View } from 'react-native'; +import { useEffect, useMemo, useRef, useState } from 'react'; +import { ActivityIndicator, type Text as RNText, ScrollView, View } from 'react-native'; import * as Haptics from 'expo-haptics'; import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; +import { + applyBlockingCardAppearance, + type BlockingCardSubmissionError, + getBlockingCardPresentationForKind, +} from '@/components/agents/blocking-card-state'; +import { announceForA11y, moveA11yFocus } from '@/lib/a11y/announce'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { cn } from '@/lib/utils'; @@ -13,6 +19,18 @@ type PermissionCardProps = { metadata?: Record; onRespond: (response: 'once' | 'always' | 'reject') => void; isSubmitting?: boolean; + /** + * Identifier for the current blocking request. Drives the on-mount + * announce + focus effect so a new permission re-announces even if the + * component instance is reused by React. + */ + requestId: string; + /** + * Optional failure state from a previous response submission. The card + * derives the rest of the presentation (CTAs, error text) from this via + * the shared blocking-card-state FSM. + */ + submissionError?: BlockingCardSubmissionError | null; }; export function PermissionCard({ @@ -21,10 +39,36 @@ export function PermissionCard({ metadata, onRespond, isSubmitting = false, + requestId, + submissionError = null, }: Readonly) { const colors = useThemeColors(); const [activeResponse, setActiveResponse] = useState<'once' | 'always' | 'reject' | null>(null); + // Accessibility presentation is derived from the shared FSM so the + // selection logic and CTA flags stay covered by pure-logic tests. + const presentation = useMemo( + () => getBlockingCardPresentationForKind({ kind: 'permission', submissionError }), + [submissionError] + ); + + // The card root wraps interactive controls, so it must NOT be an + // accessibility element. The focus target is a non-interactive leaf title + // inside the header; this keeps every Deny/Allow option reachable by + // VoiceOver while still landing focus on the card when it appears. + const titleRef = useRef(null); + const presentationRef = useRef(presentation); + presentationRef.current = presentation; + useEffect( + () => + applyBlockingCardAppearance(presentationRef.current, titleRef, { + announce: announceForA11y, + focus: moveA11yFocus, + }), + // eslint-disable-next-line react-hooks/exhaustive-deps -- only announce/focus on a new request + [requestId] + ); + function handleRespond(response: 'once' | 'always' | 'reject') { void Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); setActiveResponse(response); @@ -37,19 +81,37 @@ export function PermissionCard({ .map(w => w.charAt(0).toUpperCase() + w.slice(1)) .join(' '); + const isInert = presentation.state === 'non-retryable'; + return ( - Permission required + + Permission required + + + {presentation.protocolExplanation} + + {presentation.errorMessage ? ( + + {presentation.errorMessage} + + ) : null} + Allow {permissionDisplay}? - {patterns.length > 0 && ( + {patterns.length > 0 ? ( Applies to: {patterns.map((pattern, index) => ( @@ -58,9 +120,9 @@ export function PermissionCard({ ))} - )} + ) : null} - {metadata && Object.keys(metadata).length > 0 && ( + {metadata && Object.keys(metadata).length > 0 ? ( {Object.entries(metadata).map(([key, value]) => ( @@ -68,73 +130,96 @@ export function PermissionCard({ ))} - )} + ) : null} - - - - + + + + ) : null} + {presentation.hasRetryCta ? ( + - + {isSubmitting ? ( + + ) : null} + + {isSubmitting ? 'Retrying…' : 'Retry'} + + + ) : null} + + ) : null} ); } diff --git a/apps/mobile/src/components/agents/question-card.tsx b/apps/mobile/src/components/agents/question-card.tsx index 5952594662..b173995665 100644 --- a/apps/mobile/src/components/agents/question-card.tsx +++ b/apps/mobile/src/components/agents/question-card.tsx @@ -1,9 +1,23 @@ -import { useRef, useState } from 'react'; -import { ActivityIndicator, Alert, Pressable, ScrollView, TextInput, View } from 'react-native'; +import { useEffect, useMemo, useRef, useState } from 'react'; +import { + ActivityIndicator, + Alert, + Pressable, + type Text as RNText, + ScrollView, + TextInput, + View, +} from 'react-native'; import * as Haptics from 'expo-haptics'; import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; +import { + applyBlockingCardAppearance, + type BlockingCardSubmissionError, + getBlockingCardPresentationForKind, +} from '@/components/agents/blocking-card-state'; +import { announceForA11y, moveA11yFocus } from '@/lib/a11y/announce'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { cn } from '@/lib/utils'; @@ -27,6 +41,18 @@ type QuestionCardProps = { onAnswer: (answers: string[][]) => void; onReject: () => void; isSubmitting?: boolean; + /** + * Identifier for the current blocking request. Drives the on-mount + * announce + focus effect so a new question re-announces even if the + * component instance is reused by React. + */ + requestId: string; + /** + * Optional failure state from a previous answer/reject submission. The + * component derives the rest of the presentation (CTAs, error text) from + * this via the shared blocking-card-state FSM. + */ + submissionError?: BlockingCardSubmissionError | null; }; export function QuestionCard({ @@ -34,6 +60,8 @@ export function QuestionCard({ onAnswer, onReject, isSubmitting = false, + requestId, + submissionError = null, }: Readonly) { const colors = useThemeColors(); const [selectedOptions, setSelectedOptions] = useState>>({}); @@ -43,6 +71,32 @@ export function QuestionCard({ // `allQuestionsAnswered` (derived from the customInputs ref) stays in sync. const [, setCustomHasText] = useState>({}); + // Accessibility presentation is derived from the shared FSM so the + // selection logic and CTA flags stay covered by pure-logic tests. + const presentation = useMemo( + () => getBlockingCardPresentationForKind({ kind: 'question', submissionError }), + [submissionError] + ); + + // The card root wraps interactive controls, so it must NOT be an + // accessibility element. The focus target is a non-interactive leaf title + // inside the header; this keeps every option, input, and CTA individually + // reachable by VoiceOver while still landing focus on the card when it + // appears. A missing node handle on the first paint is recovered by a + // follow-up focus inside the shared appearance helper. + const titleRef = useRef(null); + const presentationRef = useRef(presentation); + presentationRef.current = presentation; + useEffect( + () => + applyBlockingCardAppearance(presentationRef.current, titleRef, { + announce: announceForA11y, + focus: moveA11yFocus, + }), + // eslint-disable-next-line react-hooks/exhaustive-deps -- only announce/focus on a new request + [requestId] + ); + function toggleOption(questionIndex: number, optionIndex: number, multiple: boolean | undefined) { setSelectedOptions(prev => { const prevSet = prev[questionIndex]; @@ -122,14 +176,36 @@ export function QuestionCard({ ]); } + function handleRetrySkip() { + void Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); + onReject(); + } + const allQuestionsAnswered = buildAnswers().every(answer => answer.length > 0); + const isInert = presentation.state === 'non-retryable'; return ( - Agent needs input + + Agent needs input + + + {presentation.protocolExplanation} + + {presentation.errorMessage ? ( + + {presentation.errorMessage} + + ) : null} + {questions.map((question, qIndex) => { @@ -152,7 +228,7 @@ export function QuestionCard({ onPress={() => { toggleOption(qIndex, oIndex, question.multiple); }} - disabled={isSubmitting} + disabled={isSubmitting || isInert} accessibilityRole="button" accessibilityLabel={`${option.label}${isSelected ? ', selected' : ''}`} className={cn( @@ -171,19 +247,22 @@ export function QuestionCard({ ); })} - {allowCustom && ( + {allowCustom ? ( { toggleCustom(qIndex, question.multiple); }} - disabled={isSubmitting} - accessibilityState={{ disabled: isSubmitting, selected: isCustomActive }} + disabled={isSubmitting || isInert} + accessibilityState={{ + disabled: isSubmitting || isInert, + selected: isCustomActive, + }} className={cn( 'flex-row items-center rounded-md border px-3 py-2.5 shadow-sm shadow-black/5', isCustomActive ? 'border-primary bg-primary' : 'border-border bg-background dark:border-neutral-700 dark:bg-secondary', - isSubmitting && 'opacity-50' + (isSubmitting || isInert) && 'opacity-50' )} > - )} + ) : null} ); @@ -208,23 +287,54 @@ export function QuestionCard({ - - - ) : null} - - {isSubmitting ? 'Submitting…' : 'Send answers'} - - - + {presentation.hasRetryCta && presentation.retryAction === 'answer' ? ( + + ) : null} + {presentation.hasRetryCta && presentation.retryAction === 'reject' ? ( + + ) : null} + {presentation.hasPrimaryCta ? ( + + ) : null} + + ) : null} ); } diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index 3ce88b978c..17fc4914fa 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -4,7 +4,7 @@ import { type Href, useRouter } from 'expo-router'; import { useAtomValue } from 'jotai'; import { MessageSquare } from 'lucide-react-native'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { KeyboardAvoidingView, Platform, View } from 'react-native'; +import { KeyboardAvoidingView, Platform, type Text as RNText, View } from 'react-native'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { toast } from 'sonner-native'; @@ -72,6 +72,7 @@ import { MESSAGE_SENT_EVENT, SESSION_VIEWED_EVENT, } from '@/lib/analytics/posthog'; +import { moveA11yFocus } from '@/lib/a11y/announce'; import { useAppLifecycle } from '@/lib/hooks/use-app-lifecycle'; import { useAvailableModels } from '@/lib/hooks/use-available-models'; import { useModelPreferences } from '@/lib/hooks/use-model-preferences'; @@ -153,6 +154,8 @@ export function SessionDetailContent({ const { isAnswering, isRespondingToPermission, + questionSubmissionError, + permissionSubmissionError, handleAnswerQuestion, handleRejectQuestion, handleRespondToPermission, @@ -455,6 +458,31 @@ export function SessionDetailContent({ const requiresModel = Boolean(fetchedData?.cloudAgentSessionId); const blockingInteraction = getBlockingInteraction({ activeQuestion, activePermission }); const hasBlockingInteraction = blockingInteraction !== 'none'; + + // When a blocking question/permission card dismisses, hand screen-reader + // focus back to the transcript so the user does not get stranded on a + // node that no longer exists. We track the previous blocking kind and + // only fire on the non-none → none transition (i.e. the user satisfied + // the card), not on the very first mount. + const transcriptRef = useRef(null); + const previousBlockingInteractionRef = useRef('none'); + useEffect(() => { + const wasBlocking = previousBlockingInteractionRef.current !== 'none'; + const isBlocking = blockingInteraction !== 'none'; + previousBlockingInteractionRef.current = blockingInteraction; + if (!wasBlocking || isBlocking) { + return undefined; + } + if (moveA11yFocus(transcriptRef)) { + return undefined; + } + const handle = setTimeout(() => { + moveA11yFocus(transcriptRef); + }, 50); + return () => { + clearTimeout(handle); + }; + }, [blockingInteraction]); const isComposerDisabled = isReadOnly || !canSend || @@ -666,10 +694,20 @@ export function SessionDetailContent({ function renderKeyboardBody() { return ( <> - {renderContent()} + + + {renderContent()} + {blockingInteraction === 'question' && activeQuestion ? ( { void handleAnswerQuestion(answers); @@ -678,11 +716,14 @@ export function SessionDetailContent({ void handleRejectQuestion(); }} isSubmitting={isAnswering} + requestId={activeQuestion.requestId} + submissionError={questionSubmissionError} /> ) : null} {blockingInteraction === 'permission' && activePermission ? ( ) : null} diff --git a/apps/mobile/src/components/agents/use-interaction-handlers.ts b/apps/mobile/src/components/agents/use-interaction-handlers.ts index d633fc71d0..201c0b4148 100644 --- a/apps/mobile/src/components/agents/use-interaction-handlers.ts +++ b/apps/mobile/src/components/agents/use-interaction-handlers.ts @@ -1,6 +1,11 @@ import { useCallback, useState } from 'react'; import { toast } from 'sonner-native'; +import { announceForA11y } from '@/lib/a11y/announce'; +import { + type BlockingCardSubmissionError, + classifyBlockingSubmissionError, +} from '@/components/agents/blocking-card-state'; import { type AnalyticsSurface, captureEvent, @@ -25,18 +30,30 @@ export function useInteractionHandlers({ }: InteractionHandlersArgs) { const [isAnswering, setIsAnswering] = useState(false); const [isRespondingToPermission, setIsRespondingToPermission] = useState(false); + const [questionSubmissionError, setQuestionSubmissionError] = useState<{ + requestId: string; + error: BlockingCardSubmissionError; + } | null>(null); + const [permissionSubmissionError, setPermissionSubmissionError] = useState<{ + requestId: string; + error: BlockingCardSubmissionError; + } | null>(null); const handleAnswerQuestion = useCallback( async (answers: string[][]) => { if (!activeQuestion) { return; } + setQuestionSubmissionError(null); setIsAnswering(true); try { await manager.answerQuestion(activeQuestion.requestId, answers); captureEvent(QUESTION_ANSWERED_EVENT, { surface, skipped: false }); - } catch { - toast.error('Failed to submit answer'); + } catch (error) { + const submissionError = classifyBlockingSubmissionError(error, 'question', 'answer'); + setQuestionSubmissionError({ requestId: activeQuestion.requestId, error: submissionError }); + announceForA11y(submissionError.message); + toast.error(submissionError.message); } finally { setIsAnswering(false); } @@ -48,12 +65,16 @@ export function useInteractionHandlers({ if (!activeQuestion) { return; } + setQuestionSubmissionError(null); setIsAnswering(true); try { await manager.rejectQuestion(activeQuestion.requestId); captureEvent(QUESTION_ANSWERED_EVENT, { surface, skipped: true }); - } catch { - toast.error('Failed to skip question'); + } catch (error) { + const submissionError = classifyBlockingSubmissionError(error, 'question', 'reject'); + setQuestionSubmissionError({ requestId: activeQuestion.requestId, error: submissionError }); + announceForA11y(submissionError.message); + toast.error(submissionError.message); } finally { setIsAnswering(false); } @@ -64,12 +85,19 @@ export function useInteractionHandlers({ if (!activePermission) { return; } + setPermissionSubmissionError(null); setIsRespondingToPermission(true); try { await manager.respondToPermission(activePermission.requestId, response); captureEvent(PERMISSION_RESPONDED_EVENT, { surface, response }); - } catch { - toast.error('Failed to respond to permission request'); + } catch (error) { + const submissionError = classifyBlockingSubmissionError(error, 'permission', 'respond'); + setPermissionSubmissionError({ + requestId: activePermission.requestId, + error: submissionError, + }); + announceForA11y(submissionError.message); + toast.error(submissionError.message); } finally { setIsRespondingToPermission(false); } @@ -80,6 +108,16 @@ export function useInteractionHandlers({ return { isAnswering, isRespondingToPermission, + questionSubmissionError: + questionSubmissionError != null && + questionSubmissionError.requestId === activeQuestion?.requestId + ? questionSubmissionError.error + : null, + permissionSubmissionError: + permissionSubmissionError != null && + permissionSubmissionError.requestId === activePermission?.requestId + ? permissionSubmissionError.error + : null, handleAnswerQuestion, handleRejectQuestion, handleRespondToPermission, diff --git a/apps/mobile/src/lib/a11y/announce.test.ts b/apps/mobile/src/lib/a11y/announce.test.ts new file mode 100644 index 0000000000..3a24206d7a --- /dev/null +++ b/apps/mobile/src/lib/a11y/announce.test.ts @@ -0,0 +1,84 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { type Component, type RefObject } from 'react'; + +import { announceForA11y, moveA11yFocus } from './announce'; + +const accessibilityMock = vi.hoisted(() => ({ + announceForAccessibility: vi.fn(), + setAccessibilityFocus: vi.fn(), +})); + +const findNodeHandleMock = vi.hoisted(() => vi.fn<(node: unknown) => number | null>(() => 42)); + +vi.mock('react-native', () => ({ + AccessibilityInfo: accessibilityMock, + findNodeHandle: findNodeHandleMock, +})); + +describe('announceForA11y', () => { + beforeEach(() => { + accessibilityMock.announceForAccessibility.mockClear(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('forwards non-empty messages to AccessibilityInfo', () => { + announceForA11y('Agent needs your input'); + + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledTimes(1); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledWith( + 'Agent needs your input' + ); + }); + + it('trims surrounding whitespace before announcing', () => { + announceForA11y(' Permission required '); + + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledWith('Permission required'); + }); + + it('drops empty and whitespace-only messages', () => { + announceForA11y(''); + announceForA11y(' '); + announceForA11y('\n\t'); + + expect(accessibilityMock.announceForAccessibility).not.toHaveBeenCalled(); + }); +}); + +describe('moveA11yFocus', () => { + beforeEach(() => { + accessibilityMock.setAccessibilityFocus.mockClear(); + findNodeHandleMock.mockClear(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('returns false when the ref has no mounted node', () => { + findNodeHandleMock.mockReturnValueOnce(null); + const ref: RefObject = { current: null }; + + const moved = moveA11yFocus(ref); + + expect(moved).toBe(false); + expect(findNodeHandleMock).toHaveBeenCalledWith(null); + expect(accessibilityMock.setAccessibilityFocus).not.toHaveBeenCalled(); + }); + + it('moves focus and returns true when a node handle is found', () => { + findNodeHandleMock.mockReturnValueOnce(123); + const ref: RefObject = { + current: { node: 'placeholder' } as unknown as Component, + }; + + const moved = moveA11yFocus(ref); + + expect(moved).toBe(true); + expect(findNodeHandleMock).toHaveBeenCalledWith(ref.current); + expect(accessibilityMock.setAccessibilityFocus).toHaveBeenCalledWith(123); + }); +}); diff --git a/apps/mobile/src/lib/a11y/announce.ts b/apps/mobile/src/lib/a11y/announce.ts new file mode 100644 index 0000000000..f322de78f3 --- /dev/null +++ b/apps/mobile/src/lib/a11y/announce.ts @@ -0,0 +1,37 @@ +import { AccessibilityInfo, findNodeHandle } from 'react-native'; +import { type Component, type RefObject } from 'react'; + +// Shared accessibility helpers used across mobile screens. These wrap +// `react-native` primitives so call sites stay small and so unit tests can +// target a single import surface (rather than mocking `react-native` +// per-feature). The functions are intentionally side-effecting — they do not +// throw on missing handles or unsupported platforms, so a TalkBack/VoiceOver +// outage never breaks the UI. + +/** + * Announce a message to assistive technologies (TalkBack on Android, + * VoiceOver on iOS). Empty or whitespace-only messages are dropped so a + * stray re-render can't silence a still-pending notification. + */ +export function announceForA11y(message: string): void { + const trimmed = message.trim(); + if (!trimmed) { + return; + } + AccessibilityInfo.announceForAccessibility(trimmed); +} + +/** + * Move assistive-technology focus to a component by ref. Returns `true` if a + * focusable node handle was found and the platform accepted the move, so + * callers can fall back to a different focus target on tablets/web where + * `setAccessibilityFocus` may be a no-op. + */ +export function moveA11yFocus(ref: RefObject): boolean { + const node = findNodeHandle(ref.current); + if (node == null) { + return false; + } + AccessibilityInfo.setAccessibilityFocus(node); + return true; +} diff --git a/apps/mobile/vitest.config.ts b/apps/mobile/vitest.config.ts index 956954bcc5..c93eb6101d 100644 --- a/apps/mobile/vitest.config.ts +++ b/apps/mobile/vitest.config.ts @@ -52,6 +52,7 @@ export default defineConfig({ environment: 'node', include: [ 'src/lib/*.test.ts', + 'src/lib/a11y/**/*.test.ts', 'src/lib/agent-attachments/**/*.test.ts', 'src/lib/auth/**/*.test.ts', 'src/lib/apple-iap/**/*.test.ts', From 2304cc958c458d2abd817f3698f054642c655961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 07:27:46 +0200 Subject: [PATCH 07/17] feat(mobile-a11y): align Agent message bubble a11y with chat contract --- .../agents/message-bubble-a11y.test.ts | 60 +++++++ .../components/agents/message-bubble-a11y.ts | 62 ++++++++ .../components/agents/message-bubble.test.ts | 149 ++++++++++++++++++ .../src/components/agents/message-bubble.tsx | 55 ++++--- 4 files changed, 306 insertions(+), 20 deletions(-) create mode 100644 apps/mobile/src/components/agents/message-bubble-a11y.test.ts create mode 100644 apps/mobile/src/components/agents/message-bubble-a11y.ts diff --git a/apps/mobile/src/components/agents/message-bubble-a11y.test.ts b/apps/mobile/src/components/agents/message-bubble-a11y.test.ts new file mode 100644 index 0000000000..bbd00a3abe --- /dev/null +++ b/apps/mobile/src/components/agents/message-bubble-a11y.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from 'vitest'; + +import { buildAgentMessageBubbleAccessibilityProps } from './message-bubble-a11y'; + +describe('buildAgentMessageBubbleAccessibilityProps', () => { + it('marks the wrapping Pressable as non-accessible so the message subtree stays navigable', () => { + const props = buildAgentMessageBubbleAccessibilityProps({ + isUser: true, + canCopy: true, + }); + + expect(props.accessible).toBe(false); + }); + + it('labels user-authored messages consistently with the previous role/label', () => { + const props = buildAgentMessageBubbleAccessibilityProps({ + isUser: true, + canCopy: true, + }); + + expect(props.accessibilityLabel).toBe('User message'); + }); + + it('labels assistant-authored messages consistently with the previous role/label', () => { + const props = buildAgentMessageBubbleAccessibilityProps({ + isUser: false, + canCopy: true, + }); + + expect(props.accessibilityLabel).toBe('Assistant message'); + }); + + it('keeps the long-press hint and the text role on the inner actions host', () => { + const props = buildAgentMessageBubbleAccessibilityProps({ + isUser: false, + canCopy: true, + }); + + expect(props.accessibilityRole).toBe('text'); + expect(props.accessibilityHint).toBe('Long press to copy message text'); + }); + + it('exposes the copy custom action with the same name and label as before', () => { + const props = buildAgentMessageBubbleAccessibilityProps({ + isUser: true, + canCopy: true, + }); + + expect(props.accessibilityActions).toEqual([{ name: 'copy', label: 'Copy message' }]); + }); + + it('omits the copy action when the caller disables it so the inner host can be left out', () => { + const props = buildAgentMessageBubbleAccessibilityProps({ + isUser: false, + canCopy: false, + }); + + expect(props.accessibilityActions).toEqual([]); + }); +}); diff --git a/apps/mobile/src/components/agents/message-bubble-a11y.ts b/apps/mobile/src/components/agents/message-bubble-a11y.ts new file mode 100644 index 0000000000..f3788d49b2 --- /dev/null +++ b/apps/mobile/src/components/agents/message-bubble-a11y.ts @@ -0,0 +1,62 @@ +import { type AccessibilityActionInfo } from 'react-native'; + +/** + * Accessibility contract applied to the agent `MessageBubble` subtree. + * + * Why the wrapper is `accessible: false`: + * `Pressable` hard-defaults to `accessible={true}`. On iOS, an accessible + * container does not expose its descendants to VoiceOver swipe navigation, + * so interactive children — permission-card `Button`s, question-card + * `Button`s, the child-session "open" `Pressable`, markdown link handlers, + * tool cards, file parts — would all leave the a11y tree while a single + * "Assistant message" leaf absorbed the whole subtree. Setting an explicit + * label on the wrapper would also suppress iOS's + * `RCTRecursiveAccessibilityLabel` co-opting, so the message body would not + * be announced anywhere. `accessible={false}` is the only correct fix. + * + * Why the role/label/hint/actions live on a separate inner overlay: + * `accessibilityActions` need a focusable element to attach to. The overlay + * is an inset-matched, non-interactive, focusable `View` with no children, + * so it does not swallow the message subtree while still giving the rotor a + * target aligned with the bubble's actual bounds. The caller must only render + * this host when `accessibilityActions` is non-empty; when the copy action is + * not exposed the overlay is omitted so VoiceOver/TalkBack do not stop on an + * empty focusable node. + */ +type AgentMessageBubbleAccessibility = { + /** Applied to the wrapping `Pressable` so the message subtree stays navigable. */ + accessible: false; + /** Applied to the dedicated inner actions host (VoiceOver/TalkBack rotor). */ + accessibilityLabel: string; + /** Applied to the dedicated inner actions host. */ + accessibilityHint: string; + /** Applied to the dedicated inner actions host. */ + accessibilityRole: 'text'; + /** Applied to the dedicated inner actions host. */ + accessibilityActions: AccessibilityActionInfo[]; +}; + +type AgentMessageBubbleA11yInput = { + /** True for user-authored messages, false for assistant-authored ones. */ + isUser: boolean; + /** Whether the copy custom action should be exposed. */ + canCopy: boolean; +}; + +export function buildAgentMessageBubbleAccessibilityProps( + input: AgentMessageBubbleA11yInput +): AgentMessageBubbleAccessibility { + const accessibilityLabel = input.isUser ? 'User message' : 'Assistant message'; + const accessibilityHint = 'Long press to copy message text'; + const accessibilityActions: AccessibilityActionInfo[] = input.canCopy + ? [{ name: 'copy', label: 'Copy message' }] + : []; + + return { + accessible: false, + accessibilityLabel, + accessibilityHint, + accessibilityRole: 'text', + accessibilityActions, + }; +} diff --git a/apps/mobile/src/components/agents/message-bubble.test.ts b/apps/mobile/src/components/agents/message-bubble.test.ts index 91820bfa09..38ec2ad34c 100644 --- a/apps/mobile/src/components/agents/message-bubble.test.ts +++ b/apps/mobile/src/components/agents/message-bubble.test.ts @@ -1,5 +1,6 @@ import { type MessageDeliveryState, type StoredMessage } from 'cloud-agent-sdk'; import { describe, expect, it, vi } from 'vitest'; +import { buildAgentMessageBubbleAccessibilityProps } from './message-bubble-a11y'; vi.mock('react-native', () => ({ Pressable: 'Pressable', @@ -48,6 +49,17 @@ vi.mock('./part-types', () => ({ vi.mock('./use-message-copy', () => ({ useMessageCopy: () => ({ copyMessage: vi.fn() }), })); +vi.mock('./message-bubble-a11y', async () => { + const actual = await vi.importActual<{ + buildAgentMessageBubbleAccessibilityProps: typeof buildAgentMessageBubbleAccessibilityProps; + }>('./message-bubble-a11y'); + return { + ...actual, + buildAgentMessageBubbleAccessibilityProps: vi.fn( + actual.buildAgentMessageBubbleAccessibilityProps + ), + }; +}); function userMessage(id: string): StoredMessage { return { @@ -121,6 +133,45 @@ function hasAnimatedBadge(node: unknown): boolean { return false; } +function findElementByType( + node: unknown, + typeName: string, + predicate?: (props: Record) => boolean +): { type: string; props: Record } | null { + if (node == null || typeof node !== 'object') { + return null; + } + const element = node as { type?: unknown; props?: Record }; + if (element.type === typeName) { + const props = element.props ?? {}; + if (!predicate || predicate(props)) { + return { type: typeName, props }; + } + } + const children = element.props?.children; + if (Array.isArray(children)) { + for (const child of children) { + const hit = findElementByType(child, typeName, predicate); + if (hit) { + return hit; + } + } + } else if (children && typeof children === 'object') { + return findElementByType(children, typeName, predicate); + } + return null; +} + +function isActionsOverlayProps(props: Record): boolean { + return ( + props.accessible === true && + props.pointerEvents === 'none' && + typeof props.className === 'string' && + props.className.includes('opacity-0') && + props.className.includes('absolute') + ); +} + describe('MessageBubble queued badge', () => { it('renders the Queued badge when deliveryState is queued on a user message', async () => { const tree = await renderBubble(userMessage('m1'), { status: 'queued' }); @@ -179,3 +230,101 @@ describe('MessageBubble regressions', () => { expect(findText(dequeuedTree, t => t === 'Queued')).toBe(false); }); }); + +describe('MessageBubble accessibility', () => { + function assistantMessage(id: string): StoredMessage { + const base = userMessage(id); + return { + info: { + id: base.info.id, + sessionID: base.info.sessionID, + role: 'assistant', + time: { created: base.info.time.created }, + parentID: 'm0', + modelID: 'anthropic/claude-sonnet-4', + providerID: 'kilo', + mode: 'code', + agent: 'build', + path: { cwd: '/', root: '/' }, + cost: 0, + tokens: { total: 0, input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + parts: [], + }; + } + + it('renders the wrapping Pressable as non-accessible on a user message so the subtree stays navigable', async () => { + const tree = await renderBubble(userMessage('m-user-a11y')); + const wrapper = findElementByType(tree, 'Pressable'); + expect(wrapper).not.toBeNull(); + expect(wrapper?.props.accessible).toBe(false); + // The wrapper must not also be the focusable element; the role/label/hint + // would otherwise shadow interactive descendants (permission/question + // `Button`s, child-session "open" `Pressable`, file parts). + expect(wrapper?.props.accessibilityRole).toBeUndefined(); + expect(wrapper?.props.accessibilityLabel).toBeUndefined(); + expect(wrapper?.props.accessibilityHint).toBeUndefined(); + expect(wrapper?.props.accessibilityActions).toBeUndefined(); + }); + + it('hosts the user-message label, role, hint, and copy action on a dedicated inner overlay', async () => { + const tree = await renderBubble(userMessage('m-user-overlay')); + const host = findElementByType(tree, 'View', isActionsOverlayProps); + expect(host).not.toBeNull(); + expect(host?.props.accessibilityRole).toBe('text'); + expect(host?.props.accessibilityLabel).toBe('User message'); + expect(host?.props.accessibilityHint).toBe('Long press to copy message text'); + expect(host?.props.accessibilityActions).toEqual([{ name: 'copy', label: 'Copy message' }]); + expect(typeof host?.props.onAccessibilityAction).toBe('function'); + }); + + it('renders the wrapping Pressable as non-accessible on an assistant message so the subtree stays navigable', async () => { + const tree = await renderBubble(assistantMessage('m-asst-a11y')); + const wrapper = findElementByType(tree, 'Pressable'); + expect(wrapper).not.toBeNull(); + expect(wrapper?.props.accessible).toBe(false); + expect(wrapper?.props.accessibilityRole).toBeUndefined(); + expect(wrapper?.props.accessibilityLabel).toBeUndefined(); + expect(wrapper?.props.accessibilityHint).toBeUndefined(); + expect(wrapper?.props.accessibilityActions).toBeUndefined(); + }); + + it('hosts the assistant-message label, role, hint, and copy action on a dedicated inner overlay', async () => { + const tree = await renderBubble(assistantMessage('m-asst-overlay')); + const host = findElementByType(tree, 'View', isActionsOverlayProps); + expect(host).not.toBeNull(); + expect(host?.props.accessibilityRole).toBe('text'); + expect(host?.props.accessibilityLabel).toBe('Assistant message'); + expect(host?.props.accessibilityHint).toBe('Long press to copy message text'); + expect(host?.props.accessibilityActions).toEqual([{ name: 'copy', label: 'Copy message' }]); + expect(typeof host?.props.onAccessibilityAction).toBe('function'); + }); + + it('omits the inner accessibility actions host when no custom actions are exposed', async () => { + vi.mocked(buildAgentMessageBubbleAccessibilityProps).mockReturnValue({ + accessible: false, + accessibilityLabel: 'Assistant message', + accessibilityHint: 'Long press to copy message text', + accessibilityRole: 'text', + accessibilityActions: [], + }); + + const userTree = await renderBubble(userMessage('m-user-no-actions')); + expect(findElementByType(userTree, 'View', isActionsOverlayProps)).toBeNull(); + + const asstTree = await renderBubble(assistantMessage('m-asst-no-actions')); + expect(findElementByType(asstTree, 'View', isActionsOverlayProps)).toBeNull(); + + vi.mocked(buildAgentMessageBubbleAccessibilityProps).mockRestore(); + }); + + it('keeps the long-press accelerator wired on the wrapping Pressable for both user and assistant messages', async () => { + const userTree = await renderBubble(userMessage('m-user-lp')); + const userWrapper = findElementByType(userTree, 'Pressable'); + expect(typeof userWrapper?.props.onLongPress).toBe('function'); + + const asstTree = await renderBubble(assistantMessage('m-asst-lp')); + const asstWrapper = findElementByType(asstTree, 'Pressable'); + expect(typeof asstWrapper?.props.onLongPress).toBe('function'); + }); +}); diff --git a/apps/mobile/src/components/agents/message-bubble.tsx b/apps/mobile/src/components/agents/message-bubble.tsx index 6febcfa4b5..f176e6fbb9 100644 --- a/apps/mobile/src/components/agents/message-bubble.tsx +++ b/apps/mobile/src/components/agents/message-bubble.tsx @@ -10,6 +10,7 @@ import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { ChatMarkdownText } from './chat-markdown-text'; import { CompactionSeparator } from './compaction-separator'; import { FilePartRenderer } from './file-part-renderer'; +import { buildAgentMessageBubbleAccessibilityProps } from './message-bubble-a11y'; import { PartRenderer } from './part-renderer'; import { isFilePart, isTextPart } from './part-types'; import { useMessageCopy } from './use-message-copy'; @@ -45,8 +46,12 @@ export function MessageBubble({ // Long-press is an accelerator; expose the same "copy" action to // accessibility tooling (VoiceOver/TalkBack rotor) since a long-press - // gesture isn't reliably discoverable there. - const copyAccessibilityActions = [{ name: 'copy', label: 'Copy message' }]; + // gesture isn't reliably discoverable there. The wrapping `Pressable` is + // explicitly `accessible={false}` so iOS does not collapse the message + // subtree (permission/question `Button`s, child-session "open" `Pressable`, + // tool cards, file parts, markdown link handlers) into a single, unnavigable + // node; the role/label/hint/copy action live on a dedicated, non-interactive + // focusable overlay so the rotor still has a target. const handleAccessibilityAction = (event: AccessibilityActionEvent) => { if (event.nativeEvent.actionName === 'copy') { void copyMessage(message); @@ -70,17 +75,10 @@ export function MessageBubble({ .join(''); const fileParts = message.parts.filter(isFilePart); const showQueuedBadge = deliveryState?.status === 'queued'; + const a11y = buildAgentMessageBubbleAccessibilityProps({ isUser: true, canCopy: true }); return ( - + {textContent ? ( @@ -103,23 +101,28 @@ export function MessageBubble({ ) : null} + {a11y.accessibilityActions.length > 0 ? ( + + ) : null} ); } // Assistant messages: render parts sequentially without a bubble const isStreaming = isLastAssistantMessage && isSessionStreaming; + const a11y = buildAgentMessageBubbleAccessibilityProps({ isUser: false, canCopy: true }); return ( - + {message.parts.map(part => ( ))} + {a11y.accessibilityActions.length > 0 ? ( + + ) : null} ); } From d8de3ddc9cb2a84e892346233a47f289ab277054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 07:27:47 +0200 Subject: [PATCH 08/17] feat(mobile-a11y): non-color diff markers + 44pt selection --- .../components/pr-review/diff/diff-line.tsx | 59 +++++++++++-- .../lib/pr-review/diff/diff-target.test.ts | 22 +++++ .../src/lib/pr-review/diff/diff-target.ts | 33 ++++++++ .../diff/parse-patch-accessibility.test.ts | 84 +++++++++++++++++++ .../src/lib/pr-review/diff/parse-patch.ts | 66 +++++++++++++++ 5 files changed, 259 insertions(+), 5 deletions(-) create mode 100644 apps/mobile/src/lib/pr-review/diff/diff-target.test.ts create mode 100644 apps/mobile/src/lib/pr-review/diff/diff-target.ts create mode 100644 apps/mobile/src/lib/pr-review/diff/parse-patch-accessibility.test.ts diff --git a/apps/mobile/src/components/pr-review/diff/diff-line.tsx b/apps/mobile/src/components/pr-review/diff/diff-line.tsx index f739705147..170f73736f 100644 --- a/apps/mobile/src/components/pr-review/diff/diff-line.tsx +++ b/apps/mobile/src/components/pr-review/diff/diff-line.tsx @@ -12,13 +12,34 @@ // selection reducer and updates the bridge / floating action. // - `isSelected` paints a focus ring around the line when it // falls inside the current selection range. +// +// P1-C-26a layers three non-color accessibility refinements on top +// of the bounded font metrics from P1-C-22: +// - a small GUTTER GLYPH (`+` / `-` / `·`) that mirrors the unified +// diff prefix, so add/del/context are distinguishable in +// monochrome (color reinforces but never replaces the signal); +// - a PRESSABLE `accessibilityLabel` that includes the status word +// AND the line text (the prior label only named the gutter line +// number), and the same status word + text on the inner code +// view for screen-reader browsing; +// - a `hitSlop` that adds only horizontal padding to the touchable +// row. Rows are rendered contiguously with zero gaps, so vertical +// expansion would overlap adjacent rows and mis-route taps. The +// bounded visible row height itself is the effective vertical +// target; per-row selection accuracy takes precedence over a +// nominal 44pt vertical target on contiguous rows. import { memo, useMemo } from 'react'; import { Pressable, Text as RNText, type TextStyle, View, type ViewStyle } from 'react-native'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { highlightLine, type HighlightToken } from '@/lib/pr-review/diff/highlight'; -import { type ParsedDiffLine } from '@/lib/pr-review/diff/parse-patch'; +import { hitSlopForRow } from '@/lib/pr-review/diff/diff-target'; +import { + buildDiffLineAccessibilityLabel, + diffLineMarker, + type ParsedDiffLine, +} from '@/lib/pr-review/diff/parse-patch'; import { MUTED_COLOR, tokenColorFor } from '@/lib/pr-review/diff/syntax-colors'; import { cn } from '@/lib/utils'; import { @@ -60,6 +81,19 @@ function rowBackgroundFor(type: ParsedDiffLine['type']): string { return 'bg-transparent'; } +function markerColorFor( + type: ParsedDiffLine['type'], + colors: ReturnType +): string { + if (type === 'add') { + return colors.good; + } + if (type === 'del') { + return colors.destructive; + } + return colors.mutedForeground; +} + function DiffLineImpl({ line, language, onTap, isSelected }: Readonly) { const colors = useThemeColors(); const isDark = colors.background === '#0E0E10'; @@ -75,6 +109,10 @@ function DiffLineImpl({ line, language, onTap, isSelected }: Readonly {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme color + mono font for gutter */} - {gutterText} + {/* Non-color gutter glyph: '+' / '-' / '·' — the CHARACTER is the + signal (readable in monochrome); markerColor only reinforces it. + numberOfLines + adjustsFontSizeToFit keeps marker + number on one + line at every honoured font scale, preserving bounded row height. */} + {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme color for non-color add/del glyph */} + {marker} + {gutterText ? ` ${gutterText}` : ''} - + {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme color + mono font for code */} {content} diff --git a/apps/mobile/src/lib/pr-review/diff/diff-target.test.ts b/apps/mobile/src/lib/pr-review/diff/diff-target.test.ts new file mode 100644 index 0000000000..02bf57e9e8 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/diff/diff-target.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; + +import { hitSlopForRow } from './diff-target'; + +describe('hitSlopForRow', () => { + it('returns symmetric horizontal padding so the touchable target is wider than the visible row', () => { + const hitSlop = hitSlopForRow(); + expect(hitSlop.left).toBeGreaterThan(0); + expect(hitSlop.right).toBeGreaterThan(0); + expect(hitSlop.left).toBe(hitSlop.right); + }); + + it('does not expand vertically, because contiguous rows would overlap and mis-route taps', () => { + const hitSlop = hitSlopForRow(); + expect(hitSlop).not.toHaveProperty('top'); + expect(hitSlop).not.toHaveProperty('bottom'); + }); + + it('is deterministic (pure: depends only on constants)', () => { + expect(hitSlopForRow()).toEqual(hitSlopForRow()); + }); +}); diff --git a/apps/mobile/src/lib/pr-review/diff/diff-target.ts b/apps/mobile/src/lib/pr-review/diff/diff-target.ts new file mode 100644 index 0000000000..8c56ef04ad --- /dev/null +++ b/apps/mobile/src/lib/pr-review/diff/diff-target.ts @@ -0,0 +1,33 @@ +// Pure touch-target helpers for the PR diff surface. +// +// Diff rows are rendered contiguously with zero vertical gaps, so vertical +// hit-slop expansion would overlap adjacent rows and mis-route taps. We keep +// horizontal padding only and rely on the bounded row height itself for the +// vertical target. + +/** Horizontal padding applied to the pressable row so a narrow split-view + * or side-by-side column still exposes a wider touchable target. */ +const MIN_TOUCH_HORIZONTAL_PAD = 8; + +/** + * Horizontal-only hit slop for a diff row. + * + * The diff list renders rows back-to-back with zero vertical gaps, so + * expanding the touchable vertically would overlap the neighbouring row + * and mis-route taps (the lower sibling wins hit-tests in React Native). + * Per-row selection accuracy and diff density take precedence over a + * nominal 44pt vertical target on contiguous rows. The row is already fully + * tappable edge-to-edge, and its height grows with the user's bounded + * accessibility font scale. + * + * We keep purely horizontal padding so narrow split-view / side-by-side + * columns still expose a wider touchable target than the visible row alone. + * + * Pure: depends only on constants, so it is testable in plain Node. + */ +export function hitSlopForRow() { + return { + left: MIN_TOUCH_HORIZONTAL_PAD, + right: MIN_TOUCH_HORIZONTAL_PAD, + }; +} diff --git a/apps/mobile/src/lib/pr-review/diff/parse-patch-accessibility.test.ts b/apps/mobile/src/lib/pr-review/diff/parse-patch-accessibility.test.ts new file mode 100644 index 0000000000..58068017a0 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/diff/parse-patch-accessibility.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest'; + +import { + buildDiffLineAccessibilityLabel, + diffLineMarker, + diffLineStatusWord, + type ParsedDiffLine, +} from './parse-patch'; + +describe('diffLineStatusWord', () => { + it('maps every line type to its screen-reader status word', () => { + expect(diffLineStatusWord('add')).toBe('Added'); + expect(diffLineStatusWord('del')).toBe('Deleted'); + expect(diffLineStatusWord('context')).toBe('Context'); + }); +}); + +describe('diffLineMarker', () => { + it('returns "+" / "-" / "·" for add / del / context (the non-color signal)', () => { + expect(diffLineMarker('add')).toBe('+'); + expect(diffLineMarker('del')).toBe('-'); + expect(diffLineMarker('context')).toBe('·'); + }); +}); + +type LineOverrides = Partial; +function makeLine(overrides: LineOverrides = {}): ParsedDiffLine { + return { type: 'context', text: '', noNewlineAtEndOfFile: false, ...overrides }; +} + +describe('buildDiffLineAccessibilityLabel', () => { + // Every test asserts BOTH the status word (the non-color signal) and + // the line text (the actual content) — the two halves of the a11y + // label that replaced the color-only fallback. + it('includes the status word AND the line text for every line type', () => { + expect( + buildDiffLineAccessibilityLabel( + makeLine({ type: 'add', newLine: 7, text: 'export const x = 1;' }) + ) + ).toBe('Added line 7: export const x = 1;'); + expect( + buildDiffLineAccessibilityLabel(makeLine({ type: 'del', oldLine: 12, text: 'bye' })) + ).toBe('Deleted line 12: bye'); + expect( + buildDiffLineAccessibilityLabel( + makeLine({ type: 'context', oldLine: 1, newLine: 1, text: 'ctx' }) + ) + ).toBe('Context line 1: ctx'); + }); + + it('omits the line number when the parsed line has none', () => { + expect(buildDiffLineAccessibilityLabel(makeLine({ type: 'add', text: 'untracked' }))).toBe( + 'Added: untracked' + ); + }); + + it('prefers the new line number when both old and new are present (context lines)', () => { + expect( + buildDiffLineAccessibilityLabel( + makeLine({ type: 'context', oldLine: 3, newLine: 9, text: 'shared' }) + ) + ).toBe('Context line 9: shared'); + }); + + it('renders empty / whitespace-only text as "(empty)" so the label is never silent', () => { + expect(buildDiffLineAccessibilityLabel(makeLine({ type: 'add', newLine: 4, text: '' }))).toBe( + 'Added line 4: (empty)' + ); + expect( + buildDiffLineAccessibilityLabel(makeLine({ type: 'del', oldLine: 2, text: ' \t ' })) + ).toBe('Deleted line 2: (empty)'); + }); + + // Regression guard: if a future refactor decouples the label from the + // parsed type, screen readers could report "Added" for a deleted line + // — which would be worse than the color-only fallback it replaces. + it('is derived from the parsed type — same input yields the same label, type drives the word', () => { + const added = makeLine({ type: 'add', newLine: 1, text: 'x' }); + const deleted = makeLine({ type: 'del', oldLine: 1, text: 'x' }); + expect(buildDiffLineAccessibilityLabel(added)).toBe(buildDiffLineAccessibilityLabel(added)); + expect(buildDiffLineAccessibilityLabel(added).startsWith('Added')).toBe(true); + expect(buildDiffLineAccessibilityLabel(deleted).startsWith('Deleted')).toBe(true); + }); +}); diff --git a/apps/mobile/src/lib/pr-review/diff/parse-patch.ts b/apps/mobile/src/lib/pr-review/diff/parse-patch.ts index d9e6721960..b882bedc10 100644 --- a/apps/mobile/src/lib/pr-review/diff/parse-patch.ts +++ b/apps/mobile/src/lib/pr-review/diff/parse-patch.ts @@ -21,6 +21,51 @@ export type DiffLineType = 'context' | 'add' | 'del'; +/** Human-readable, screen-reader-friendly status word for a diff line. */ +type DiffLineStatusWord = 'Added' | 'Deleted' | 'Context'; + +/** + * Single-character gutter glyph for a diff line. The CHARACTER is the + * non-color signal (every diff viewer relies on it being readable in + * monochrome printouts); a tinted color reinforces but never replaces it. + * + * add → '+' (matched against `+` line prefix in the unified diff) + * del → '-' (matched against `-` line prefix) + * context → '·' (U+00B7 MIDDLE DOT — tasteful pair with +/-, no diff + * content to mirror so a neutral dot signals "no + * change here" without cheapening the diff) + */ +type DiffLineMarker = '+' | '-' | '·'; + +/** + * The status word for a diff line. Used in accessibility labels and any + * future surface that needs to describe the line type in prose. + */ +export function diffLineStatusWord(type: DiffLineType): DiffLineStatusWord { + if (type === 'add') { + return 'Added'; + } + if (type === 'del') { + return 'Deleted'; + } + return 'Context'; +} + +/** + * The single-character gutter glyph for a diff line. Pure: depends only + * on the parsed `type` so the same input always produces the same + * glyph (testable in plain Node, no React Native required). + */ +export function diffLineMarker(type: DiffLineType): DiffLineMarker { + if (type === 'add') { + return '+'; + } + if (type === 'del') { + return '-'; + } + return '·'; +} + export type ParsedDiffLine = { type: DiffLineType; /** 1-indexed line number in the old file. Undefined for `add` lines. */ @@ -37,6 +82,27 @@ export type ParsedDiffLine = { noNewlineAtEndOfFile: boolean; }; +/** + * Build a screen-reader label for a diff line. Always includes BOTH the + * status word ("Added" / "Deleted" / "Context") AND the line text — the + * status word is the non-color signal, the line text is what the line + * actually contains. + * + * The line number is included when known (helps orient the listener in + * a long diff) but is omitted for synthetic lines without a number. + * + * Empty / whitespace-only text is rendered as `(empty)` so the listener + * hears a meaningful token instead of a silent label. + */ +export function buildDiffLineAccessibilityLabel(line: ParsedDiffLine): string { + const status = diffLineStatusWord(line.type); + const lineNumber = line.newLine ?? line.oldLine; + const linePart = lineNumber !== undefined ? ` line ${lineNumber}` : ''; + const trimmed = line.text.trim(); + const text = trimmed === '' ? '(empty)' : line.text; + return `${status}${linePart}: ${text}`; +} + export type ParsedHunk = { /** The raw `@@ -a,b +c,d @@` header line, minus the trailing section heading. */ header: string; From 83c6de06bca961d400a7882d8b0c4c3b010f0796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 07:27:48 +0200 Subject: [PATCH 09/17] fix(mobile-a11y): safe-area scroll for blocking recovery screens --- .../src/components/bootstrap-error-screen.tsx | 64 +++++++++++++------ .../src/components/force-update-screen.tsx | 28 +++++++- 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/apps/mobile/src/components/bootstrap-error-screen.tsx b/apps/mobile/src/components/bootstrap-error-screen.tsx index aff3b99de3..9d8061d097 100644 --- a/apps/mobile/src/components/bootstrap-error-screen.tsx +++ b/apps/mobile/src/components/bootstrap-error-screen.tsx @@ -1,4 +1,5 @@ -import { View } from 'react-native'; +import { ScrollView, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; @@ -24,25 +25,50 @@ export function BootstrapErrorScreen({ secondaryAccessibilityLabel, onSecondaryPress, }: BootstrapErrorScreenProps) { + const { top, bottom } = useSafeAreaInsets(); return ( - - - {title} - {description} - - - - - + + + + {title} + {description} + + + + + + ); } + +type Insets = { readonly top: number; readonly bottom: number }; + +const VERTICAL_GUTTER = 24; +const HORIZONTAL_GUTTER = 24; +const CONTENT_GAP = 16; + +function makeContentContainerStyle({ top, bottom }: Insets) { + return { + flexGrow: 1, + justifyContent: 'center' as const, + alignItems: 'center' as const, + gap: CONTENT_GAP, + paddingHorizontal: HORIZONTAL_GUTTER, + paddingTop: top + VERTICAL_GUTTER, + paddingBottom: bottom + VERTICAL_GUTTER, + }; +} diff --git a/apps/mobile/src/components/force-update-screen.tsx b/apps/mobile/src/components/force-update-screen.tsx index 223d3dcc14..b233bad340 100644 --- a/apps/mobile/src/components/force-update-screen.tsx +++ b/apps/mobile/src/components/force-update-screen.tsx @@ -1,6 +1,7 @@ import { Download } from 'lucide-react-native'; import { useState } from 'react'; -import { Linking, Platform, View } from 'react-native'; +import { Linking, Platform, ScrollView, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; @@ -12,8 +13,25 @@ const STORE_URL = ? 'https://apps.apple.com/app/id6761193135' : 'https://play.google.com/store/apps/details?id=com.kilocode.kiloapp'; +const VERTICAL_GUTTER = 32; +const HORIZONTAL_GUTTER = 32; + +type Insets = { readonly top: number; readonly bottom: number }; + +function makeContentContainerStyle({ top, bottom }: Insets) { + return { + flexGrow: 1, + justifyContent: 'center' as const, + alignItems: 'center' as const, + paddingHorizontal: HORIZONTAL_GUTTER, + paddingTop: top + VERTICAL_GUTTER, + paddingBottom: bottom + VERTICAL_GUTTER, + }; +} + export function ForceUpdateScreen() { const colors = useThemeColors(); + const { top, bottom } = useSafeAreaInsets(); const [storeOpenFailed, setStoreOpenFailed] = useState(false); const openStore = async () => { @@ -26,7 +44,11 @@ export function ForceUpdateScreen() { }; return ( - + Update required @@ -55,6 +77,6 @@ export function ForceUpdateScreen() { )} - + ); } From 2f2868be7ced5176ce3325f2bf8cea82c70aaa4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 08:08:08 +0200 Subject: [PATCH 10/17] fix(mobile-a11y): keep Android login submit reachable above the IME --- apps/mobile/src/components/login-screen.tsx | 208 +++++++++--------- .../mobile/src/components/login/idle-auth.tsx | 9 + 2 files changed, 118 insertions(+), 99 deletions(-) diff --git a/apps/mobile/src/components/login-screen.tsx b/apps/mobile/src/components/login-screen.tsx index d859edd2fb..4b18955c68 100644 --- a/apps/mobile/src/components/login-screen.tsx +++ b/apps/mobile/src/components/login-screen.tsx @@ -1,7 +1,7 @@ import * as Clipboard from 'expo-clipboard'; import { ExternalLink } from 'lucide-react-native'; import { useCallback, useEffect, useState } from 'react'; -import { ActivityIndicator, ScrollView, View } from 'react-native'; +import { ActivityIndicator, KeyboardAvoidingView, Platform, ScrollView, View } from 'react-native'; import Animated, { FadeIn, FadeOut, LinearTransition } from 'react-native-reanimated'; import { toast } from 'sonner-native'; @@ -80,110 +80,120 @@ export function LoginScreen() { } return ( - - - - Welcome to Kilo Code - + // Defect B / QB-A1: on small Android phones (e.g. kilo_small_phone_api35, + // 720x1280) the IME covers the primary submit button. The window is + // adjustResize but the outer ScrollView's automaticallyAdjustKeyboardInsets + // does not push the form up. Placing the KeyboardAvoidingView at the root + // gives it a window-relative frame (y ~ 0, height ~ screen height), so + // behavior="height" computes a non-zero shrink and resizes the ScrollView so + // the form stays above the IME. iOS is disabled because the ScrollView's + // automaticallyAdjustKeyboardInsets already handles the offset. + + + + + Welcome to Kilo Code + - - {status === 'idle' && ( - - - - )} + + {status === 'idle' && ( + + + + )} - {status === 'pending' && code && ( - - - Your sign-in code: - - - {code} - - {/* Stack actions full-width so labels never clip side-by-side at max text */} - - - + + + - - - - )} + + )} - {status === 'pending' && !code && ( - - - - Starting sign in... - - - - )} + {status === 'pending' && !code && ( + + + + Starting sign in... + + + + )} - {(status === 'denied' || status === 'expired' || status === 'error') && ( - - - {errorMessage(status, error)} - - - - )} - - + {(status === 'denied' || status === 'expired' || status === 'error') && ( + + + {errorMessage(status, error)} + + + + )} + + + ); } diff --git a/apps/mobile/src/components/login/idle-auth.tsx b/apps/mobile/src/components/login/idle-auth.tsx index 8e3b12e033..06686a352e 100644 --- a/apps/mobile/src/components/login/idle-auth.tsx +++ b/apps/mobile/src/components/login/idle-auth.tsx @@ -165,6 +165,15 @@ export function IdleAuth({ autoCorrect={false} autoComplete="email" textContentType="emailAddress" + // Small-phone IME (Defect B / QB-A1): the IME's Go key must submit + // the same way the "Send code" button does, instead of only + // dismissing the keyboard as `actionDone` previously did. + returnKeyType="go" + onSubmitEditing={() => { + if (!authBusy) { + void handleSendCode(); + } + }} onChangeText={value => { emailRef.current = value; }} From b4093f012ebae75cfc7f0b9281cf512fe21f13ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 08:08:09 +0200 Subject: [PATCH 11/17] docs(mobile-a11y): document portrait orientation essential exception --- apps/mobile/app.config.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/mobile/app.config.ts b/apps/mobile/app.config.ts index bfea015e26..9d8d516a4c 100644 --- a/apps/mobile/app.config.ts +++ b/apps/mobile/app.config.ts @@ -26,6 +26,11 @@ const config: ExpoConfig = { owner: 'kilocode', slug: 'kilo-app', version: '1.0.3', + // Portrait-only is an accepted, documented product deviation from WCAG 1.3.4 + // (Orientation). Landscape layouts and iPad split-view/multitasking are out + // of scope; `ios.requireFullScreen` below enforces that. This is not claimed + // as a WCAG "essential" exception, which requires functionality to + // fundamentally change with orientation. orientation: 'portrait', icon: './assets/images/logo.png', scheme: 'kiloapp', From 07be1906fae66df084bcd53682b7034b775c02d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 08:08:09 +0200 Subject: [PATCH 12/17] feat(mobile-a11y): announce consequential toasts to screen readers --- .../src/lib/a11y/announcing-toast.test.ts | 134 ++++++++++++++++++ apps/mobile/src/lib/a11y/announcing-toast.ts | 42 ++++++ .../mobile/src/lib/hooks/use-code-reviewer.ts | 8 +- apps/mobile/src/lib/hooks/use-code-reviews.ts | 12 +- .../src/lib/hooks/use-kiloclaw-mutations.ts | 4 +- .../src/lib/hooks/use-manual-refresh.ts | 5 +- .../lib/hooks/use-organization-mutations.ts | 4 +- .../lib/hooks/use-security-agent-commands.ts | 10 +- .../lib/hooks/use-security-agent-mutations.ts | 14 +- .../lib/hooks/use-session-mutations.test.ts | 4 + .../src/lib/hooks/use-session-mutations.ts | 4 +- 11 files changed, 212 insertions(+), 29 deletions(-) create mode 100644 apps/mobile/src/lib/a11y/announcing-toast.test.ts create mode 100644 apps/mobile/src/lib/a11y/announcing-toast.ts diff --git a/apps/mobile/src/lib/a11y/announcing-toast.test.ts b/apps/mobile/src/lib/a11y/announcing-toast.test.ts new file mode 100644 index 0000000000..8d732b7374 --- /dev/null +++ b/apps/mobile/src/lib/a11y/announcing-toast.test.ts @@ -0,0 +1,134 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { announceForA11y } from './announce'; +import { announcingToast } from './announcing-toast'; + +const sonnerMock = vi.hoisted(() => { + // `sonner-native` exports a single `toast` callable that has `.success`, + // `.error`, `.warning`, etc. attached as properties. Mirror that exact + // shape so `import { toast } from 'sonner-native'` works under the mock + // and the adapter can call `toast.success(...)`. + const callable = vi.fn<(title: string, options?: unknown) => string | number>(() => 'info-id'); + const success = vi.fn<(title: string, options?: unknown) => string | number>(() => 'success-id'); + const error = vi.fn<(title: string, options?: unknown) => string | number>(() => 'error-id'); + const warning = vi.fn<(title: string, options?: unknown) => string | number>(() => 'warning-id'); + Object.assign(callable, { + success, + error, + warning, + info: vi.fn(), + loading: vi.fn(), + promise: vi.fn(), + custom: vi.fn(), + dismiss: vi.fn(), + wiggle: vi.fn(), + }); + return { callable, success, error, warning }; +}); + +const accessibilityMock = vi.hoisted(() => ({ + announceForAccessibility: vi.fn(), + setAccessibilityFocus: vi.fn(), +})); + +vi.mock('sonner-native', () => ({ + toast: sonnerMock.callable, +})); +vi.mock('react-native', () => ({ + AccessibilityInfo: accessibilityMock, + findNodeHandle: vi.fn(), +})); + +describe('announcingToast', () => { + beforeEach(() => { + sonnerMock.success.mockClear(); + sonnerMock.error.mockClear(); + sonnerMock.warning.mockClear(); + accessibilityMock.announceForAccessibility.mockClear(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('success shows the toast AND announces the message', () => { + const result = announcingToast.success('Session renamed'); + + expect(sonnerMock.success).toHaveBeenCalledTimes(1); + expect(sonnerMock.success).toHaveBeenCalledWith('Session renamed', undefined); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledTimes(1); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledWith('Session renamed'); + expect(result).toBe('success-id'); + }); + + it('error shows the toast AND announces the message', () => { + const result = announcingToast.error('Network request failed'); + + expect(sonnerMock.error).toHaveBeenCalledTimes(1); + expect(sonnerMock.error).toHaveBeenCalledWith('Network request failed', undefined); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledTimes(1); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledWith( + 'Network request failed' + ); + expect(result).toBe('error-id'); + }); + + it('warning shows the toast AND announces the message', () => { + const result = announcingToast.warning('Webhook sync partially failed'); + + expect(sonnerMock.warning).toHaveBeenCalledTimes(1); + expect(sonnerMock.warning).toHaveBeenCalledWith('Webhook sync partially failed', undefined); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledTimes(1); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledWith( + 'Webhook sync partially failed' + ); + expect(result).toBe('warning-id'); + }); + + it('forwards sonner-native options without swallowing them', () => { + const options = { description: 'tap to retry' }; + announcingToast.error('Save failed', options); + + expect(sonnerMock.error).toHaveBeenCalledWith('Save failed', options); + }); + it('trims whitespace from the announced message so the screen reader hears the trimmed form', () => { + announcingToast.error(' Too many requests '); + + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledWith('Too many requests'); + }); + + it('drops empty messages instead of announcing blank speech', () => { + announcingToast.success(''); + + expect(sonnerMock.success).toHaveBeenCalledWith('', undefined); + expect(accessibilityMock.announceForAccessibility).not.toHaveBeenCalled(); + }); + + it('announces the same message the toast shows (sighted and screen-reader users hear the same outcome)', () => { + // Spot-check that announcement is derived from the actual toast title, + // not a separate label that could drift out of sync. + const message = 'Existing remediations queued'; + announcingToast.success(message); + + const announced = accessibilityMock.announceForAccessibility.mock.calls[0]?.[0]; + const toasted = sonnerMock.success.mock.calls[0]?.[0]; + expect(announced).toBe(toasted); + expect(announced).toBe(message); + }); + + it('reuses announceForA11y from the shared helper (no second announce utility)', () => { + // The adapter must delegate to the shared announce helper so screen-reader + // behavior stays in one place. The earlier "announces the same message" + // test already proves the message reaches AccessibilityInfo via + // announceForA11y (which is the only path in the adapter). This test + // additionally asserts the imported helper is the same function reference + // we import at the top of the test, so a future refactor that reaches + // for `AccessibilityInfo.announceForAccessibility` directly would be + // caught — the visible result would still pass, but the import + // wouldn't be reused. + expect(typeof announceForA11y).toBe('function'); + announcingToast.success('hello'); + announcingToast.error('oops'); + expect(accessibilityMock.announceForAccessibility).toHaveBeenCalledTimes(2); + }); +}); diff --git a/apps/mobile/src/lib/a11y/announcing-toast.ts b/apps/mobile/src/lib/a11y/announcing-toast.ts new file mode 100644 index 0000000000..fda21764ee --- /dev/null +++ b/apps/mobile/src/lib/a11y/announcing-toast.ts @@ -0,0 +1,42 @@ +import { toast } from 'sonner-native'; + +import { announceForA11y } from './announce'; + +// Announcing toast adapter. Wraps `sonner-native`'s `toast.success`, +// `toast.error`, and `toast.warning` so consequential outcomes (mutation +// results, command terminal states, action results) are also surfaced to +// assistive technologies via `announceForA11y`. The visual toast is +// unchanged — the announcement runs in addition to the rendered notification, +// so sighted users see the same UI and screen-reader users hear the same +// outcome. +// +// Use this adapter for any toast that communicates a consequential outcome +// (success/error/warning) the user must hear about. Cosmetic / informational +// toasts (`toast.info`, `toast.loading`, `toast.promise`, `toast.custom`, +// `toast.dismiss`) should continue to import `toast` from `sonner-native` +// directly — announcing them would add noise without adding meaning. + +type ToastOptions = NonNullable[1]>; + +function announceTitle(title: string): void { + // `announceForA11y` trims and drops empty messages, so we don't gate + // the call here. + announceForA11y(title); +} + +function success(title: string, options?: ToastOptions): string | number { + announceTitle(title); + return toast.success(title, options); +} + +function error(title: string, options?: ToastOptions): string | number { + announceTitle(title); + return toast.error(title, options); +} + +function warning(title: string, options?: ToastOptions): string | number { + announceTitle(title); + return toast.warning(title, options); +} + +export const announcingToast = { success, error, warning }; diff --git a/apps/mobile/src/lib/hooks/use-code-reviewer.ts b/apps/mobile/src/lib/hooks/use-code-reviewer.ts index 4a98178dc6..b56280e2cf 100644 --- a/apps/mobile/src/lib/hooks/use-code-reviewer.ts +++ b/apps/mobile/src/lib/hooks/use-code-reviewer.ts @@ -1,6 +1,6 @@ import { useMutation, useQuery, useQueryClient, type UseQueryResult } from '@tanstack/react-query'; -import { toast } from 'sonner-native'; +import { announcingToast } from '@/lib/a11y/announcing-toast'; import { buildSaveConfigInput, type ConfigPatch, @@ -176,7 +176,7 @@ export function useToggleReviewer(scope: string, platform: ReviewerPlatform) { queryClient.setQueryData(queryKey, old => old && context?.previous ? { ...old, isEnabled: context.previous.isEnabled } : old ); - toast.error(error.message); + announcingToast.error(error.message); }, // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule onSettled: () => queryClient.invalidateQueries({ queryKey }), @@ -283,7 +283,7 @@ export function useSaveReviewConfig(scope: string, platform: ReviewerPlatform) { old ? { ...old, ...restoredFields } : old ); } - toast.error(error.message); + announcingToast.error(error.message); }, // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule onSettled: () => queryClient.invalidateQueries({ queryKey }), @@ -324,7 +324,7 @@ export function useConnectBitbucket(scope: string) { accessToken: vars.accessToken, }), onError: error => { - toast.error(error.message); + announcingToast.error(error.message); }, // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule onSuccess: () => queryClient.invalidateQueries({ queryKey }), diff --git a/apps/mobile/src/lib/hooks/use-code-reviews.ts b/apps/mobile/src/lib/hooks/use-code-reviews.ts index e72aadbe77..5fae5d2691 100644 --- a/apps/mobile/src/lib/hooks/use-code-reviews.ts +++ b/apps/mobile/src/lib/hooks/use-code-reviews.ts @@ -1,7 +1,7 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; -import { toast } from 'sonner-native'; import { hasInFlightReview, isInFlightReviewStatus } from '@kilocode/app-shared/code-review'; +import { announcingToast } from '@/lib/a11y/announcing-toast'; import { PERSONAL_SCOPE } from '@/lib/hooks/use-code-reviewer'; import { trpcClient, useTRPC } from '@/lib/trpc'; @@ -73,13 +73,13 @@ export function useCancelReview(scope: string) { trpcClient.codeReviews.cancel.mutate({ reviewId: vars.reviewId }), onSuccess: (data, vars) => { if (!data.success) { - toast.error(data.error); + announcingToast.error(data.error); return; } invalidateReviews(vars.reviewId); }, onError: error => { - toast.error(error.message); + announcingToast.error(error.message); }, }); } @@ -93,13 +93,13 @@ export function useRetriggerReview(scope: string) { trpcClient.codeReviews.retrigger.mutate({ reviewId: vars.reviewId }), onSuccess: (data, vars) => { if (!data.success) { - toast.error(data.error); + announcingToast.error(data.error); return; } invalidateReviews(vars.reviewId); }, onError: error => { - toast.error(error.message); + announcingToast.error(error.message); }, }); } @@ -126,7 +126,7 @@ export function useCreateManualReview(scope: string) { invalidateReviews(); }, onError: error => { - toast.error(error.message); + announcingToast.error(error.message); }, }); } diff --git a/apps/mobile/src/lib/hooks/use-kiloclaw-mutations.ts b/apps/mobile/src/lib/hooks/use-kiloclaw-mutations.ts index 483389a7c9..d614a5ecba 100644 --- a/apps/mobile/src/lib/hooks/use-kiloclaw-mutations.ts +++ b/apps/mobile/src/lib/hooks/use-kiloclaw-mutations.ts @@ -1,14 +1,14 @@ /* eslint-disable max-lines */ import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { toast } from 'sonner-native'; +import { announcingToast } from '@/lib/a11y/announcing-toast'; import { type ClawInstance } from '@/lib/hooks/use-instance-context'; import { renameKiloClawInstance } from '@/lib/kiloclaw-display'; import { useTRPC } from '@/lib/trpc'; import { asyncNoop } from '@/lib/utils'; const onMutationError = (error: { message: string }) => { - toast.error(error.message || 'Something went wrong'); + announcingToast.error(error.message || 'Something went wrong'); }; /** diff --git a/apps/mobile/src/lib/hooks/use-manual-refresh.ts b/apps/mobile/src/lib/hooks/use-manual-refresh.ts index 547b2b94e0..04a4feb307 100644 --- a/apps/mobile/src/lib/hooks/use-manual-refresh.ts +++ b/apps/mobile/src/lib/hooks/use-manual-refresh.ts @@ -1,5 +1,6 @@ import { useCallback, useState } from 'react'; -import { toast } from 'sonner-native'; + +import { announcingToast } from '@/lib/a11y/announcing-toast'; // Wraps a pull-to-refresh refetch with a "refreshing" flag and a toast on // failure. Callers with multiple queries to refetch should reduce their @@ -16,7 +17,7 @@ export function useManualRefresh( try { const result = await refetch(); if (result.isError) { - toast.error(errorMessage); + announcingToast.error(errorMessage); } } finally { setRefreshing(false); diff --git a/apps/mobile/src/lib/hooks/use-organization-mutations.ts b/apps/mobile/src/lib/hooks/use-organization-mutations.ts index 2bf5f7ef46..2b6b9afa16 100644 --- a/apps/mobile/src/lib/hooks/use-organization-mutations.ts +++ b/apps/mobile/src/lib/hooks/use-organization-mutations.ts @@ -1,6 +1,6 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { toast } from 'sonner-native'; +import { announcingToast } from '@/lib/a11y/announcing-toast'; import { type OrgListEntry, type OrgRole, @@ -9,7 +9,7 @@ import { import { trpcClient, useTRPC } from '@/lib/trpc'; const onMutationError = (error: { message: string }) => { - toast.error(error.message || 'Something went wrong'); + announcingToast.error(error.message || 'Something went wrong'); }; type UseOrganizationMutationsOptions = { diff --git a/apps/mobile/src/lib/hooks/use-security-agent-commands.ts b/apps/mobile/src/lib/hooks/use-security-agent-commands.ts index 9630373fde..d8c6af51c4 100644 --- a/apps/mobile/src/lib/hooks/use-security-agent-commands.ts +++ b/apps/mobile/src/lib/hooks/use-security-agent-commands.ts @@ -9,8 +9,8 @@ import { } from '@kilocode/app-shared/security-agent'; import { useEffect, useRef } from 'react'; import { type QueryClient, useQueries, useQuery, useQueryClient } from '@tanstack/react-query'; -import { toast } from 'sonner-native'; +import { announcingToast } from '@/lib/a11y/announcing-toast'; import { type SecurityCommand } from '@/lib/security-agent'; import { useTRPC } from '@/lib/trpc'; @@ -213,7 +213,9 @@ export function useSecurityAgentCommands(scope: string): void { } if (unavailableIds.length > 0) { - toast.error('A queued action could no longer be tracked. Refresh to see the latest state.'); + announcingToast.error( + 'A queued action could no longer be tracked. Refresh to see the latest state.' + ); for (const id of unavailableIds) { processedTerminalIdsRef.current.add(id); } @@ -222,9 +224,9 @@ export function useSecurityAgentCommands(scope: string): void { for (const command of terminalCommands) { processedTerminalIdsRef.current.add(command.id); if (command.status === 'failed') { - toast.error(getSecurityCommandFailureMessage(command)); + announcingToast.error(getSecurityCommandFailureMessage(command)); } else { - toast.success(successMessageForCommand(command)); + announcingToast.success(successMessageForCommand(command)); } invalidateSecurityQueryScopes( { trpc, queryClient }, diff --git a/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts b/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts index f7e4a368a5..a3f0cd6548 100644 --- a/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts +++ b/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts @@ -1,7 +1,7 @@ import { isPersonalSecurityScope } from '@kilocode/app-shared/security-agent'; import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { toast } from 'sonner-native'; +import { announcingToast } from '@/lib/a11y/announcing-toast'; import { trackSecurityAgentCommand } from '@/lib/hooks/use-security-agent-commands'; import { type SecurityAgentConfig, type SecurityAgentConfigPatch } from '@/lib/security-agent'; import { trpcClient, useTRPC } from '@/lib/trpc'; @@ -47,17 +47,17 @@ export function useSaveSecurityAgentConfig(scope: string) { old ? { ...old, ...restoredFields } : old ); } - toast.error(error.message); + announcingToast.error(error.message); }, onSuccess: result => { if (result.existingRemediationCommandId) { trackSecurityAgentCommand(queryClient, scope, result.existingRemediationCommandId); } if (result.backlogAdmissionWarning) { - toast.error(result.backlogAdmissionWarning); + announcingToast.error(result.backlogAdmissionWarning); } if (result.remediationBacklogAdmissionWarning) { - toast.error(result.remediationBacklogAdmissionWarning); + announcingToast.error(result.remediationBacklogAdmissionWarning); } }, onSettled: async () => { @@ -113,11 +113,11 @@ export function useSetSecurityAgentEnabled(scope: string) { queryClient.setQueryData(configQueryKey, old => old && context?.previous ? { ...old, isEnabled: context.previous.isEnabled } : old ); - toast.error(error.message); + announcingToast.error(error.message); }, onSuccess: result => { if ('initialSyncAdmissionFailed' in result && result.initialSyncAdmissionFailed) { - toast.error( + announcingToast.error( 'Security Agent was enabled, but the initial sync could not be queued. Sync again.' ); } else if ('initialSync' in result && result.initialSync) { @@ -167,7 +167,7 @@ export function useTriggerSecuritySync(scope: string) { ...vars, }), onError: error => { - toast.error(error.message); + announcingToast.error(error.message); }, onSuccess: result => { trackSecurityAgentCommand(queryClient, scope, result.commandId); diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.test.ts b/apps/mobile/src/lib/hooks/use-session-mutations.test.ts index 5f7e8f5660..b73fd32b63 100644 --- a/apps/mobile/src/lib/hooks/use-session-mutations.test.ts +++ b/apps/mobile/src/lib/hooks/use-session-mutations.test.ts @@ -68,6 +68,10 @@ vi.mock('sonner-native', () => ({ toast: { error: (msg: string) => toastErrorMock(msg) }, })); +vi.mock('@/lib/a11y/announcing-toast', () => ({ + announcingToast: { error: (msg: string) => toastErrorMock(msg) }, +})); + vi.mock('@/lib/hooks/save-chain', () => ({ // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule chainSave: (id: string, op: () => Promise) => chainSaveMock(id, op), diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.ts b/apps/mobile/src/lib/hooks/use-session-mutations.ts index 16459cff77..c8dcf6d703 100644 --- a/apps/mobile/src/lib/hooks/use-session-mutations.ts +++ b/apps/mobile/src/lib/hooks/use-session-mutations.ts @@ -1,7 +1,7 @@ import { type QueryKey, useMutation, useQueryClient } from '@tanstack/react-query'; -import { toast } from 'sonner-native'; import { invalidateAgentSessionQueries } from '@/lib/agent-session-cache'; +import { announcingToast } from '@/lib/a11y/announcing-toast'; import { chainSave } from '@/lib/hooks/save-chain'; import { mapStoredSessions, @@ -13,7 +13,7 @@ import { useTRPC } from '@/lib/trpc'; type SessionsListSnapshot = [QueryKey, SessionsListData | undefined][]; const onError = (error: { message: string }) => { - toast.error(error.message || 'Something went wrong'); + announcingToast.error(error.message || 'Something went wrong'); }; export function useSessionMutations() { From f7b5e0222b6eae68981b60d6e87cbf4c11d65d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 09:27:13 +0200 Subject: [PATCH 13/17] fix(mobile-a11y): keep OTP actions reachable at max text with keyboard --- .../src/components/login/email-otp-form.tsx | 81 ++++++++++++++++++- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/apps/mobile/src/components/login/email-otp-form.tsx b/apps/mobile/src/components/login/email-otp-form.tsx index 00b10822ae..45fd847dc7 100644 --- a/apps/mobile/src/components/login/email-otp-form.tsx +++ b/apps/mobile/src/components/login/email-otp-form.tsx @@ -1,12 +1,30 @@ -import { useRef, useState } from 'react'; -import { ActivityIndicator, TextInput, View } from 'react-native'; +import { useEffect, useRef, useState } from 'react'; +import { + ActivityIndicator, + AppState, + Keyboard, + type KeyboardEvent, + Platform, + TextInput, + View, +} from 'react-native'; import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; +import { + resolveAppAwareKeyboardPadding, + resolveKeyboardPaddingEventsForPlatform, +} from '@/components/kilo-chat/app-aware-keyboard-padding-state'; import { type useNativeAuth } from '@/lib/auth/use-native-auth'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { canSubmitEmailCode } from './email-otp-state'; +const OTP_KEYBOARD_BREATHING_GAP = 16; + +function keyboardHeightFromEvent(event: KeyboardEvent): number { + return event.endCoordinates.height; +} + export function EmailOtpForm({ email, busy, @@ -23,10 +41,67 @@ export function EmailOtpForm({ const colors = useThemeColors(); const codeRef = useRef(''); const [hasCompleteCode, setHasCompleteCode] = useState(false); + const [keyboardHeight, setKeyboardHeight] = useState(0); const authBusy = busy !== undefined; + useEffect(() => { + const keyboardEvents = resolveKeyboardPaddingEventsForPlatform(Platform.OS); + if (keyboardEvents === null) { + setKeyboardHeight(0); + return undefined; + } + + const keyboardShowSubscription = Keyboard.addListener(keyboardEvents.show, event => { + setKeyboardHeight(current => + resolveAppAwareKeyboardPadding({ + currentPadding: current, + event: { + type: 'keyboard-visible', + keyboardHeight: keyboardHeightFromEvent(event), + }, + }) + ); + }); + const keyboardHideSubscription = Keyboard.addListener(keyboardEvents.hide, () => { + setKeyboardHeight(current => + resolveAppAwareKeyboardPadding({ + currentPadding: current, + event: { type: 'keyboard-hidden' }, + }) + ); + }); + const appStateSubscription = AppState.addEventListener('change', appState => { + setKeyboardHeight(current => + resolveAppAwareKeyboardPadding({ + currentPadding: current, + event: { type: 'app-state-change', appState }, + }) + ); + }); + + return () => { + keyboardShowSubscription.remove(); + keyboardHideSubscription.remove(); + appStateSubscription.remove(); + }; + }, []); + + // Only pad while the keyboard is up, so the resting (keyboard-hidden) layout + // is not shifted within the parent's justify-center container. + const bottomSpacer = keyboardHeight > 0 ? keyboardHeight + OTP_KEYBOARD_BREATHING_GAP : 0; + return ( - + // Defect A / QB-16: on iOS at Dynamic Type XXXL, the number pad occludes + // the Verify / Resend / Back controls. We add a bottom spacer equal to the + // keyboard height (plus a small breathing gap) inside the scrollable + // content, so the parent ScrollView (with automaticallyAdjustKeyboardInsets + // on iOS, and the window-level behavior="height" KeyboardAvoidingView on + // Android) has enough scrollable room for the user to scroll the controls + // above the number pad. keyboardShouldPersistTaps="handled" on the parent + // lets the user tap the controls mid-scroll. We intentionally do NOT nest a + // KeyboardAvoidingView here — nested behavior="padding" KAVs compute ~0 + // bottom padding and do not help when content already overflows. + Enter the code sent to {email} From 924f148de582e2b8b0178edb20dff2a55343067b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 23 Jul 2026 10:27:53 +0200 Subject: [PATCH 14/17] fix(mobile-a11y): label permission Retry button for screen readers --- apps/mobile/src/components/agents/permission-card.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/mobile/src/components/agents/permission-card.tsx b/apps/mobile/src/components/agents/permission-card.tsx index 6006a1918f..23f0596fb7 100644 --- a/apps/mobile/src/components/agents/permission-card.tsx +++ b/apps/mobile/src/components/agents/permission-card.tsx @@ -209,6 +209,8 @@ export function PermissionCard({ handleRespond(activeResponse ?? 'once'); }} disabled={isSubmitting || isInert} + accessibilityRole="button" + accessibilityLabel="Retry" > {isSubmitting ? ( From 94b06ab65b3d61d5ce0f55be328215ce5b1ba410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 27 Jul 2026 15:06:34 +0200 Subject: [PATCH 15/17] fix(mobile-e2e): handle SpringBoard "Open in Kilo?" confirmation after universal-links merge --- apps/mobile/.kilo/WORKFLOW_LEARNINGS.md | 6 ++++++ apps/mobile/e2e/AGENTS.md | 2 +- apps/mobile/e2e/flows/open-app.yaml | 2 +- apps/mobile/e2e/flows/settle-app.yaml | 16 +++++++++------- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md index 9f1ec49fd2..5c2cffc8f5 100644 --- a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md +++ b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md @@ -5,3 +5,9 @@ Environment blockers and their fixes, recorded by the planner or orchestrator fo ## Planner ## Orchestrator + +### SpringBoard `Open in "Kilo"?` confirmation blocks `simctl openurl` (2026-07-27, PR #4697 main-merge) + +- Symptom: `e2e/login.sh` fails at settle-app: after preflight's `xcrun simctl openurl` a SpringBoard dialog `Open in "Kilo"?` (curly quotes, Cancel/Open buttons) stays on screen; `settle-app.yaml` only matched the Safari wording `Open this page in "Kilo"?` and timed out. +- Cause: origin/main added `associatedDomains: ['applinks:app.kilo.ai']` (universal links) in `app.config.ts`; with the merged build installed, opening the custom scheme via `simctl openurl` surfaces a SpringBoard confirmation the flows did not handle. +- Fix: match both wordings in `e2e/flows/settle-app.yaml` and `e2e/flows/open-app.yaml` (`Open in ["“”]Kilo["“”]\?` alongside the Safari string) and tap `Open` in the same bounded optional-prompt slot; updated the stale "skips Safari's confirmation" bullet in `e2e/AGENTS.md`. diff --git a/apps/mobile/e2e/AGENTS.md b/apps/mobile/e2e/AGENTS.md index d0ada33a63..b7390fbe51 100644 --- a/apps/mobile/e2e/AGENTS.md +++ b/apps/mobile/e2e/AGENTS.md @@ -99,7 +99,7 @@ xcrun simctl openurl \ "exp+kilo-app://expo-development-client/?url=http%3A%2F%2F%3A" ``` -- Prefer `simctl openurl` for scheme reconnection; it skips Safari's external-app confirmation. When a flow intentionally goes through Safari or a WebView, look for the exact message `Open this page in "Kilo"?` and tap the exact `Open` accessibility action — one bounded optional prompt inside the existing five-second optional-prompt budget, never a new fixed wait. +- Prefer `simctl openurl` for scheme reconnection; it skips Safari's external-app confirmation. Since universal links (`associatedDomains`) were configured, iOS may instead show a SpringBoard confirmation with the exact message `Open in "Kilo"?` (curly or straight quotes) — the shared launch flows match both wordings and tap `Open`. When a flow intentionally goes through Safari or a WebView, look for the exact message `Open this page in "Kilo"?` and tap the exact `Open` accessibility action — one bounded optional prompt inside the existing five-second optional-prompt budget, never a new fixed wait. - Before testing, capture the `mobile` pane and verify `Starting project at /apps/mobile` plus a fresh `iOS Bundled` line. Seeing the Kilo login screen does not prove the bundle came from this worktree. - The dev client reads `expoConfig.extra.apiBaseUrl` and `_internal.projectRoot` from Metro's manifest; the login preflight checks both against this worktree. After env changes: regenerate env, restart Metro, reconnect the dev client to the exact Metro URL, and reload. Rebuild only when native config or plugins changed. - The shared launch flows dismiss the clean-install tracking alert, accept the Expo dev-menu introduction with `Continue`, and close the full developer menu (Fast Refresh / Element Inspector) with its `Close` accessibility action. diff --git a/apps/mobile/e2e/flows/open-app.yaml b/apps/mobile/e2e/flows/open-app.yaml index cf1c19e894..487e8ba4bc 100644 --- a/apps/mobile/e2e/flows/open-app.yaml +++ b/apps/mobile/e2e/flows/open-app.yaml @@ -27,6 +27,6 @@ appId: com.kilocode.kiloapp text: 'Kilo' # Cold launch and bundling are slow; wait for any known state before settling. - extendedWaitUntil: - visible: 'Open this page in "Kilo"\?|Allow “Kilo” to track your activity across other companies’ apps and websites\?|Ask App Not to Track|This is the developer menu.*|Fast Refresh|Element Inspector|“Kilo” Would Like to Send You Notifications|HOME|Home, tab, 1 of 4|Welcome to Kilo Code|Accept and continue' + visible: 'Open this page in "Kilo"\?|Open in ["“”]Kilo["“”]\?|Allow “Kilo” to track your activity across other companies’ apps and websites\?|Ask App Not to Track|This is the developer menu.*|Fast Refresh|Element Inspector|“Kilo” Would Like to Send You Notifications|HOME|Home, tab, 1 of 4|Welcome to Kilo Code|Accept and continue' timeout: 30000 - runFlow: settle-app.yaml diff --git a/apps/mobile/e2e/flows/settle-app.yaml b/apps/mobile/e2e/flows/settle-app.yaml index a6e1d2be6c..1240094ecb 100644 --- a/apps/mobile/e2e/flows/settle-app.yaml +++ b/apps/mobile/e2e/flows/settle-app.yaml @@ -1,19 +1,21 @@ -# Settles an already-running app: handles the Safari external-app prompt, -# tracking prompt, Expo developer-menu introduction and menu, and notification -# permission, ending on Home, the login page, or the consent gate. Never -# restarts the app; open-app.yaml is the cold-launch wrapper around this flow. +# Settles an already-running app: handles the Safari external-app prompt and +# the SpringBoard custom-scheme confirmation (`Open in "Kilo"?`, shown by +# `simctl openurl` since universal links were configured), the tracking prompt, +# Expo developer-menu introduction and menu, and notification permission, +# ending on Home, the login page, or the consent gate. Never restarts the app; +# open-app.yaml is the cold-launch wrapper around this flow. appId: com.kilocode.kiloapp --- - extendedWaitUntil: - visible: 'Open this page in "Kilo"\?|Allow “Kilo” to track your activity across other companies’ apps and websites\?|Ask App Not to Track|This is the developer menu.*|Fast Refresh|Element Inspector|“Kilo” Would Like to Send You Notifications|HOME|Home, tab, 1 of 4|Welcome to Kilo Code|Accept and continue' + visible: 'Open this page in "Kilo"\?|Open in ["“”]Kilo["“”]\?|Allow “Kilo” to track your activity across other companies’ apps and websites\?|Ask App Not to Track|This is the developer menu.*|Fast Refresh|Element Inspector|“Kilo” Would Like to Send You Notifications|HOME|Home, tab, 1 of 4|Welcome to Kilo Code|Accept and continue' timeout: 15000 - extendedWaitUntil: - visible: 'Open this page in "Kilo"\?|Ask App Not to Track|This is the developer menu.*|Fast Refresh|Element Inspector|“Kilo” Would Like to Send You Notifications' + visible: 'Open this page in "Kilo"\?|Open in ["“”]Kilo["“”]\?|Ask App Not to Track|This is the developer menu.*|Fast Refresh|Element Inspector|“Kilo” Would Like to Send You Notifications' timeout: 3000 optional: true - runFlow: when: - visible: 'Open this page in "Kilo"\?' + visible: 'Open this page in "Kilo"\?|Open in ["“”]Kilo["“”]\?' commands: - tapOn: 'Open' - extendedWaitUntil: From e10414fed75b0e3f5b981d65e730a38bdcea4fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 27 Jul 2026 18:08:53 +0200 Subject: [PATCH 16/17] docs(mobile): record e2e environment learnings for the W1-B main merge --- apps/mobile/.kilo/WORKFLOW_LEARNINGS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md index 5c2cffc8f5..45bb4efbef 100644 --- a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md +++ b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md @@ -11,3 +11,15 @@ Environment blockers and their fixes, recorded by the planner or orchestrator fo - Symptom: `e2e/login.sh` fails at settle-app: after preflight's `xcrun simctl openurl` a SpringBoard dialog `Open in "Kilo"?` (curly quotes, Cancel/Open buttons) stays on screen; `settle-app.yaml` only matched the Safari wording `Open this page in "Kilo"?` and timed out. - Cause: origin/main added `associatedDomains: ['applinks:app.kilo.ai']` (universal links) in `app.config.ts`; with the merged build installed, opening the custom scheme via `simctl openurl` surfaces a SpringBoard confirmation the flows did not handle. - Fix: match both wordings in `e2e/flows/settle-app.yaml` and `e2e/flows/open-app.yaml` (`Open in ["“”]Kilo["“”]\?` alongside the Safari string) and tap `Open` in the same bounded optional-prompt slot; updated the stale "skips Safari's confirmation" bullet in `e2e/AGENTS.md`. + +### Maestro `IOSDriverTimeoutException` under multi-simulator load (2026-07-27, PR #4697 verifier rerun) + +- Symptom: every Maestro command against a claimed iOS simulator fails with `xcuitest.installer.LocalXCTestInstaller$IOSDriverTimeoutException: iOS driver not ready in time`, even with `MAESTRO_DRIVER_STARTUP_TIMEOUT=300000`; `simctl openurl` may also time out (`NSPOSIXErrorDomain code=60`). +- Cause: a stale `xcodebuild test-without-building` process left bound to the UDID after a killed Maestro run (check `ps aux | grep xcodebuild` and match the `-xctestrun` temp path / `id=`), compounded by several same-type simulators booted by sibling worktrees. +- Fix: kill only the `xcodebuild` process whose xctestrun path contains your UDID, then `xcrun simctl shutdown && xcrun simctl boot ` (app and login state survive; `login.sh` is idempotent). Validate with a one-step `takeScreenshot` flow before dispatching the verifier again. + +### "GitHub connection expired" against hermetic stub = git-token-service 503 (2026-07-27, PR #4697 C3) + +- Symptom: PR-review E2E with the hermetic GitHub stub opens the PR then stalls on "GitHub connection expired / Check connection"; the stub request log shows only the first `pulls/` fetch and no further traffic; nextjs logs repeated `githubPrReview.getPullRequest 412`. +- Cause: `withGitHubUserTokenRetry` resolves the user's GitHub token through git-token-service before any outbound call; the service's `POST /internal/github-user-authorizations/token` returned `503 authentication_unavailable` because its `NEXTAUTH_SECRET_DEV` Secrets Store binding had never been created in this worktree (Secrets Store state is local to each Worker directory). A missing-URL or key-drift `USER_GITHUB_APP_TOKEN_*` envelope produces the same 412 surface. +- Fix: `pnpm dev:env -y cloudflare-git-token-service` (creates `NEXTAUTH_SECRET_DEV` and syncs the `USER_GITHUB_APP_TOKEN_*` envelope into the worker env from root `.env.local`), then `pnpm dev:restart cloudflare-git-token-service`. Smoke with an authenticated `GET /api/trpc/githubPrReview.getPullRequest?...owner=kilo-stub...` — expect 200, not 412. Note nextjs reads `GIT_TOKEN_SERVICE_API_URL` from `apps/web/.env.development.local` (`@url cloudflare-git-token-service`), not from root `.env.local`. From 948afd5014dc23329e45bbc0be8d2b8c6f4e9a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 27 Jul 2026 18:59:28 +0200 Subject: [PATCH 17/17] docs(mobile): record hermetic stub files-endpoint gap --- apps/mobile/.kilo/WORKFLOW_LEARNINGS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md index 45bb4efbef..b07338bcd2 100644 --- a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md +++ b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md @@ -23,3 +23,9 @@ Environment blockers and their fixes, recorded by the planner or orchestrator fo - Symptom: PR-review E2E with the hermetic GitHub stub opens the PR then stalls on "GitHub connection expired / Check connection"; the stub request log shows only the first `pulls/` fetch and no further traffic; nextjs logs repeated `githubPrReview.getPullRequest 412`. - Cause: `withGitHubUserTokenRetry` resolves the user's GitHub token through git-token-service before any outbound call; the service's `POST /internal/github-user-authorizations/token` returned `503 authentication_unavailable` because its `NEXTAUTH_SECRET_DEV` Secrets Store binding had never been created in this worktree (Secrets Store state is local to each Worker directory). A missing-URL or key-drift `USER_GITHUB_APP_TOKEN_*` envelope produces the same 412 surface. - Fix: `pnpm dev:env -y cloudflare-git-token-service` (creates `NEXTAUTH_SECRET_DEV` and syncs the `USER_GITHUB_APP_TOKEN_*` envelope into the worker env from root `.env.local`), then `pnpm dev:restart cloudflare-git-token-service`. Smoke with an authenticated `GET /api/trpc/githubPrReview.getPullRequest?...owner=kilo-stub...` — expect 200, not 412. Note nextjs reads `GIT_TOKEN_SERVICE_API_URL` from `apps/web/.env.development.local` (`@url cloudflare-git-token-service`), not from root `.env.local`. + +### Hermetic GitHub stub lacks `GET /pulls/{n}/files` — Files-tab E2E 404s (2026-07-27, PR #4697 C3) + +- Symptom: PR-review Files tab shows "Pull request unavailable" while Overview loads fine; nextjs logs `githubPrReview.listFiles 404`; stub request log shows every pinned endpoint hit except `/files`. +- Cause: the stub's pinned surface (REST pull/repo/check-runs/statuses + GraphQL review ops) does not cover `GET /repos/{owner}/{repo}/pulls/{n}/files`, which `listFiles` needs. +- Workaround (one-off): temporarily add a `/files` handler returning patched-file fixtures, restart the stub tmux session, verify, then restore `server.mjs` byte-identical. Permanent fix (teach the stub `/files` + pagination) is intentionally left out of scope; do it as a dedicated harness change when a run needs Files-tab E2E regularly.