diff --git a/apps/mobile/app.config.ts b/apps/mobile/app.config.ts index 72d0170cf3..a181f49589 100644 --- a/apps/mobile/app.config.ts +++ b/apps/mobile/app.config.ts @@ -66,12 +66,13 @@ const config: ExpoConfig = { owner: 'kilocode', slug: 'kilo-app', version: '1.0.10', - // 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', + // Rotation is supported on iOS and Android: `default` resolves to portrait + + // both landscapes in UISupportedInterfaceOrientations on iOS and all + // orientations in the Android manifest, satisfying WCAG 1.3.4 (Orientation) + // without claiming an "essential" exception. `ios.requireFullScreen` below + // STAYS true so iPad split-view/multitasking remains out of scope: + // full-screen rotation yes, Split View/Slide Over no. + orientation: 'default', icon: './assets/images/logo.png', scheme: 'kiloapp', userInterfaceStyle: 'automatic', @@ -262,6 +263,9 @@ const config: ExpoConfig = { }, ], './plugins/withAndroidManifestFix', + // Window background follows the app theme (values-night aware) so the + // rotation surface resize never paints a foreign blank frame. + './plugins/withAndroidRotationSurface', './plugins/withAndroidExpoModuleRepos', // Declares the app's languages on the widget extension, which expo-widgets // leaves English-only. This must be registered BEFORE 'expo-widgets': diff --git a/apps/mobile/plugins/withAndroidRotationSurface.js b/apps/mobile/plugins/withAndroidRotationSurface.js new file mode 100644 index 0000000000..7514075030 --- /dev/null +++ b/apps/mobile/plugins/withAndroidRotationSurface.js @@ -0,0 +1,80 @@ +const { + AndroidConfig, + withAndroidColors, + withAndroidColorsNight, + withAndroidStyles, +} = require('expo/config-plugins'); +const { assignColorValue } = AndroidConfig.Colors; + +/** + * Pins the Android window background to the app's own theme background while + * rotation is enabled. + * + * With `orientation: 'default'` the activity handles orientation config + * changes itself, and Android resizes the window surface across the rotation. + * Until React paints the first frame in the new orientation, the window shows + * `android:windowBackground` — the AppCompat DayNight default (foreign white + * in light mode, near-black in dark mode), which is exactly the blank frame a + * screen capture taken during the rotation records. Pointing the attribute at + * the same tokens `src/global.css` resolves (`--background`: #FBFAF5 light, + * #0E0E10 dark, via values-night) makes every such gap render the app's own + * screen color in both UI modes instead of a foreign blank. + * + * The splash theme (`Theme.App.SplashScreen`, yellow) is untouched: it only + * governs the launch frame before `postSplashScreenTheme` (AppTheme) applies. + */ + +/** Mirrors src/global.css `--background` (light). */ +const APP_BACKGROUND_LIGHT = '#FBFAF5'; +/** Mirrors src/global.css `--background` (dark, prefers-color-scheme). */ +const APP_BACKGROUND_DARK = '#0E0E10'; + +const COLOR_NAME = 'app_background'; +const THEME_NAME = 'AppTheme'; +const WINDOW_BACKGROUND_ITEM = 'android:windowBackground'; + +function setItem(theme, name, value) { + theme.item ??= []; + const existing = theme.item.find(item => item.$?.name === name); + if (existing) { + existing._ = value; + return; + } + theme.item.push({ $: { name }, _: value }); +} + +function withRotationSurfaceColors(config) { + return withAndroidColors(config, config => { + assignColorValue(config.modResults, { + name: COLOR_NAME, + value: APP_BACKGROUND_LIGHT, + }); + return config; + }); +} + +function withRotationSurfaceColorsNight(config) { + return withAndroidColorsNight(config, config => { + assignColorValue(config.modResults, { + name: COLOR_NAME, + value: APP_BACKGROUND_DARK, + }); + return config; + }); +} + +function withRotationSurfaceStyles(config) { + return withAndroidStyles(config, config => { + const themes = config.modResults.resources.style ?? []; + const appTheme = themes.find(theme => theme.$?.name === THEME_NAME); + if (appTheme) { + setItem(appTheme, WINDOW_BACKGROUND_ITEM, `@color/${COLOR_NAME}`); + } + return config; + }); +} + +const withAndroidRotationSurface = config => + withRotationSurfaceStyles(withRotationSurfaceColorsNight(withRotationSurfaceColors(config))); + +module.exports = withAndroidRotationSurface; diff --git a/apps/mobile/scripts/assert-expo-config.mjs b/apps/mobile/scripts/assert-expo-config.mjs index 6f68e09021..6c06a5d4a5 100644 --- a/apps/mobile/scripts/assert-expo-config.mjs +++ b/apps/mobile/scripts/assert-expo-config.mjs @@ -5,7 +5,7 @@ import { fileURLToPath } from 'node:url'; import { ENV_KEYS } from '../src/lib/env-keys.js'; // Contract values mirrored from app.config.ts (bundle id, package, scheme, -// associated domain, blocked permissions, and Sentry plugin). ENV_KEYS is +// orientation, associated domain, blocked permissions, and Sentry plugin). ENV_KEYS is // imported live from src/lib/env-keys.js. The script runs the full evaluated // config, so these must match the resolved build-time output, not the raw // app.config.ts source. @@ -19,6 +19,7 @@ const BLOCKED_PERMISSIONS = [ 'android.permission.READ_MEDIA_AUDIO', ]; const SENTRY_PLUGIN = '@sentry/react-native/expo'; +const ROTATION_SURFACE_PLUGIN = './plugins/withAndroidRotationSurface'; const mobileDir = join(dirname(fileURLToPath(import.meta.url)), '..'); @@ -57,6 +58,15 @@ check( check(config.android?.package === ANDROID_PACKAGE, `android.package must be "${ANDROID_PACKAGE}"`); check(config.scheme === SCHEME, `scheme must be "${SCHEME}"`); +// Rotation contract: all device orientations enabled (portrait + both +// landscapes on iOS, all orientations on Android), while iPad multitasking +// stays off — requireFullScreen keeps Split View/Slide Over out of scope. +check(config.orientation === 'default', `orientation must be "default"`); +check( + config.ios?.requireFullScreen === true, + 'ios.requireFullScreen must be true (iPad Split View/Slide Over stays out of scope)' +); + const associatedDomains = config.ios?.associatedDomains ?? []; check( associatedDomains.includes(ASSOCIATED_DOMAIN), @@ -76,6 +86,13 @@ const pluginNames = (config.plugins ?? []).map(plugin => Array.isArray(plugin) ? plugin[0] : plugin ); check(pluginNames.includes(SENTRY_PLUGIN), `plugins must include "${SENTRY_PLUGIN}"`); +// The rotation surface plugin pins the Android window background to the theme +// background; without it a rotation paints the AppCompat DayNight default +// until React's first frame lands in the new orientation. +check( + pluginNames.includes(ROTATION_SURFACE_PLUGIN), + `plugins must include "${ROTATION_SURFACE_PLUGIN}"` +); const extra = config.extra ?? {}; for (const key of Object.keys(ENV_KEYS)) { diff --git a/apps/mobile/src/app/(app)/(tabs)/_layout.tsx b/apps/mobile/src/app/(app)/(tabs)/_layout.tsx index ae9e60f1cc..a5d524f6ee 100644 --- a/apps/mobile/src/app/(app)/(tabs)/_layout.tsx +++ b/apps/mobile/src/app/(app)/(tabs)/_layout.tsx @@ -23,6 +23,7 @@ import { } from '@/lib/session-attention'; import { getEffectiveTabBarHeight, + getTabBarHorizontalInset, getTabBarIconSize, shouldHideTabBar, shouldShowTabLabel, @@ -69,7 +70,7 @@ export default function TabsLayout() { const pathname = usePathname(); const segments = useSegments(); const colors = useThemeColors(); - const { bottom } = useSafeAreaInsets(); + const { bottom, left, right } = useSafeAreaInsets(); const { fontScale } = useWindowDimensions(); const hideTabs = shouldHideTabBar(pathname); const showTabLabel = shouldShowTabLabel(fontScale); @@ -78,6 +79,7 @@ export default function TabsLayout() { platform: Platform.OS, fontScale, }); + const tabBarHorizontalInset = getTabBarHorizontalInset({ left, right }); const tabIconSize = getTabBarIconSize(fontScale); const showKiloClawTab = useKiloClawTabVisible(); const showQuickChatTab = useFeatureFlag(FEATURE_FLAG_QUICK_CHAT, false); @@ -136,6 +138,7 @@ export default function TabsLayout() { elevation: 0, height: tabBarHeight, position: 'absolute', + ...tabBarHorizontalInset, }, tabBarShowLabel: showTabLabel, }} diff --git a/apps/mobile/src/app/_layout.tsx b/apps/mobile/src/app/_layout.tsx index 53834553d1..87d0dd8501 100644 --- a/apps/mobile/src/app/_layout.tsx +++ b/apps/mobile/src/app/_layout.tsx @@ -975,9 +975,12 @@ function RootLayoutNav({ // from touch, but not from screen readers. Leave both accessibility // trees while hidden (iOS, then Android). The held error surface // forces the same presentation: it owns the screen above the wrapper. + // `bg-background` keeps the root surface opaque: while a rotation + // relayout runs, frames before React's first commit must show the + // app's own background, never the window's foreign default. accessibilityElementsHidden={hidden || showRestoreError} importantForAccessibility={hidden || showRestoreError ? 'no-hide-descendants' : 'auto'} - className={`flex-1 ${hidden || showRestoreError ? 'opacity-0' : 'opacity-100'}`} + className={`flex-1 bg-background ${hidden || showRestoreError ? 'opacity-0' : 'opacity-100'}`} pointerEvents={hidden || showRestoreError ? 'none' : 'auto'} > @@ -1009,7 +1012,10 @@ function AppContentReveal({ children }: Readonly<{ children: React.ReactNode }>) transform: [{ scale: splashContentScale.value }], })); return ( - + // bg-background keeps the scaled wrapper opaque over the window: the + // overscan frame and every relayout gap behind it render the app's own + // background, never the platform default. + {children} ); diff --git a/apps/mobile/src/components/agents/chat-composer.test.ts b/apps/mobile/src/components/agents/chat-composer.test.ts index 2294645e25..25841e658e 100644 --- a/apps/mobile/src/components/agents/chat-composer.test.ts +++ b/apps/mobile/src/components/agents/chat-composer.test.ts @@ -11,6 +11,7 @@ import { CLOUD_AGENT_PROMPT_MAX_LENGTH } from '@kilocode/cloud-agent-sdk/limits' import { type ChatComposer } from './chat-composer'; const layoutDirection = vi.hoisted(() => ({ isRTL: false })); +const safeAreaInsets = vi.hoisted(() => ({ bottom: 0, left: 0, right: 0, top: 0 })); const TEXT_DIRECTIONS = [ { direction: 'LTR', isRTL: false, style: undefined }, { direction: 'RTL', isRTL: true, style: [{ writingDirection: 'rtl' }, undefined] }, @@ -95,7 +96,7 @@ vi.mock('react-native', () => ({ })); vi.mock('react-native-safe-area-context', () => ({ - useSafeAreaInsets: () => ({ bottom: 0, left: 0, right: 0, top: 0 }), + useSafeAreaInsets: () => safeAreaInsets, })); vi.mock('react-native-gesture-handler', () => ({ @@ -223,9 +224,11 @@ vi.mock('@/components/agents/chat-composer-input-state', () => ({ }, })); -vi.mock('@/components/ui/blur-bar', () => ({ - BlurBar: () => null, -})); +// The composer's root element; located by identity, never by a __testMarker +// (findInputRowProps treats any marked function as the input row). +const MockBlurBar = () => null; + +vi.mock('@/components/ui/blur-bar', () => ({ BlurBar: MockBlurBar })); vi.mock('@/components/voice-input-control', () => ({ VoiceInputStatus: () => null, @@ -378,6 +381,27 @@ function findStripProps(node: Node): Record | null { return null; } +// The composer pads the content inside its root BlurBar with the landscape +// sensor side insets. The container is the only View in the returned tree +// carrying a style prop, so it is located by that style shape. +function findComposerInsetContainer(render: React.ReactElement): { + type: unknown; + props: Record; +} { + const container = findNode( + render, + (type, props) => + type === 'View' && + typeof props.style === 'object' && + props.style !== null && + 'paddingLeft' in props.style + ); + if (container === null) { + throw new Error('composer side-inset container not found in the BlurBar content'); + } + return container; +} + function requireInputRowOnSubmit(render: React.ReactElement): () => void { const rowProps = findInputRowProps(render); const onSubmit = rowProps?.onSubmit as (() => void) | undefined; @@ -452,6 +476,10 @@ beforeEach(() => { returnSendsPref.returnSendsMessage = false; reducedMotionOn.value = false; layoutDirection.isRTL = false; + safeAreaInsets.bottom = 0; + safeAreaInsets.left = 0; + safeAreaInsets.right = 0; + safeAreaInsets.top = 0; }); // The restore contract has one axis: whether the host resolved a draft. Both @@ -657,3 +685,30 @@ describe('ChatComposer attachment strip wiring', () => { expect(stripProps.onReorder).toBe(uploadReorderAttachmentsMock); }); }); + +describe('ChatComposer landscape side insets', () => { + it('keeps portrait geometry with zero side padding', async () => { + safeAreaInsets.left = 0; + safeAreaInsets.right = 0; + const render = await mount(makeProps({})); + + const container = findComposerInsetContainer(render); + expect(container.props.style).toEqual({ paddingLeft: 0, paddingRight: 0 }); + // The unpadded container still hosts the whole composer content. + expect(findNode(container, type => type === MockInputRow)).not.toBeNull(); + }); + + it('pads the composer content by the landscape sensor insets', async () => { + // iPhone sensor notch in landscape: a wider left inset than right. + safeAreaInsets.left = 59; + safeAreaInsets.right = 47; + const render = await mount(makeProps({})); + + const container = findComposerInsetContainer(render); + expect(container.props.style).toEqual({ paddingLeft: 59, paddingRight: 47 }); + // Toolbar, input row, and send control all clear the sensor area because + // they live inside the padded container. + expect(findNode(container, type => type === MockChatToolbar)).not.toBeNull(); + expect(findNode(container, type => type === MockInputRow)).not.toBeNull(); + }); +}); diff --git a/apps/mobile/src/components/agents/chat-composer.tsx b/apps/mobile/src/components/agents/chat-composer.tsx index 212e65f760..b167d04408 100644 --- a/apps/mobile/src/components/agents/chat-composer.tsx +++ b/apps/mobile/src/components/agents/chat-composer.tsx @@ -1127,128 +1127,136 @@ export function ChatComposer({ ) : null; + // Landscape safe area: pad the whole composer content (suggestion card, + // toolbar, attachment strip, voice row, counter, input row with the send + // control) by the sensor side insets while the BlurBar background stays + // full-bleed. The insets are 0 in portrait, so portrait geometry is + // unchanged, and the rotation applies as a style-only re-render — no + // remount, so the uncontrolled input keeps its text. return ( - {measure.measureElement} - - {suggestionRow} - - {!suggestionRow && control.showToolbar ? ( - - - - ) : null} - - {showAttachments ? ( - - ) : null} + + {measure.measureElement} - {upload.attachments.some(attachment => attachment.metadataStripFailed === true) ? ( - - ) : null} - - {slashCommandSuggestions.length > 0 && !isSending ? ( - - + + + ) : null} + + {showAttachments ? ( + - - ) : null} + ) : null} - - - + {upload.attachments.some(attachment => attachment.metadataStripFailed === true) ? ( + + ) : null} - {CLOUD_AGENT_PROMPT_MAX_LENGTH - characterCount <= COMPOSER_COUNTER_VISIBLE_REMAINING ? ( - - 0 && !isSending ? ( + - {CLOUD_AGENT_PROMPT_MAX_LENGTH - characterCount} - - - ) : null} - - - - { - void handleAddAttachment(); - }} - onChangeText={handleChangeText} - onInputBlur={() => { - inputFocusedRef.current = false; - setIsFocused(false); - }} - onInputFocus={() => { - inputFocusedRef.current = true; - setIsFocused(true); - }} - onInputLayout={handleInputLayout} - onInsertNewline={handleInsertNewline} - onSelectionChange={handleSelectionChange} - onStop={handleStop} - onSubmit={() => { - void submit(); - }} - onToggleVoice={() => { - void voiceInput.toggle(); - }} - paperclipDisabled={control.paperclipDisabled} - placeholder={placeholder} - returnSendsMessage={returnSendsMessage} - textInputStyle={textInputStyle} - voiceDisabled={control.voiceDisabled} - voiceInputAvailable={voiceInput.available} - voiceInputStatus={voiceInput.status} - /> + + + ) : null} + + + - + + {CLOUD_AGENT_PROMPT_MAX_LENGTH - characterCount <= COMPOSER_COUNTER_VISIBLE_REMAINING ? ( + + + {CLOUD_AGENT_PROMPT_MAX_LENGTH - characterCount} + + + ) : null} + + + + { + void handleAddAttachment(); + }} + onChangeText={handleChangeText} + onInputBlur={() => { + inputFocusedRef.current = false; + setIsFocused(false); + }} + onInputFocus={() => { + inputFocusedRef.current = true; + setIsFocused(true); + }} + onInputLayout={handleInputLayout} + onInsertNewline={handleInsertNewline} + onSelectionChange={handleSelectionChange} + onStop={handleStop} + onSubmit={() => { + void submit(); + }} + onToggleVoice={() => { + void voiceInput.toggle(); + }} + paperclipDisabled={control.paperclipDisabled} + placeholder={placeholder} + returnSendsMessage={returnSendsMessage} + textInputStyle={textInputStyle} + voiceDisabled={control.voiceDisabled} + voiceInputAvailable={voiceInput.available} + voiceInputStatus={voiceInput.status} + /> + + + ); } diff --git a/apps/mobile/src/components/agents/session-detail-content.test.ts b/apps/mobile/src/components/agents/session-detail-content.test.ts index 8039c57f12..28ac691ea7 100644 --- a/apps/mobile/src/components/agents/session-detail-content.test.ts +++ b/apps/mobile/src/components/agents/session-detail-content.test.ts @@ -598,6 +598,22 @@ describe('session detail status placement', () => { ); }); +describe('session detail bottom strip', () => { + it('keeps the home-indicator strip full-bleed (pure background, no side padding)', async () => { + const { renderer } = await mountDetails([]); + const strips = renderer.root.findAll(node => Object.is(node.type, 'BlurBar')); + expect(strips).toHaveLength(1); + // The spacer pads only the bottom inset: it hosts no controls, so it + // stays full-bleed in landscape while the composer content carries the + // sensor side insets. + const spacer = strips[0]?.findAll(node => Object.is(node.type, 'View'))[0]; + expect(spacer).toBeDefined(); + const spacerStyle = spacer?.props.style as { height: number } | undefined; + expect(spacerStyle).toEqual({ height: 16 }); + expect(Object.keys(spacerStyle ?? {})).toEqual(['height']); + }); +}); + describe.each([true, false])('session detail return with history=%s', hasHistory => { beforeEach(() => { if (hasHistory) { diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index f8b911b998..6578897c9d 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -1243,6 +1243,9 @@ export function SessionDetailContent({ // One condition for the composer and for the bottom BlurBar that reserves // its space: if the bar claimed the space on a condition the composer does // not share, the composer pops in and the layout jumps on every open. + // The strip is a pure full-bleed background/spacer: it hosts no controls, + // so it deliberately carries no horizontal safe-area padding — the + // composer's own content clears the landscape sensor insets. const isComposerMounted = !isReadOnly || messages.length === 0; const isComposerVisible = isComposerMounted && !hasBlockingInteraction; const isComposerDisabled = resolveSessionComposerDisabled({ diff --git a/apps/mobile/src/components/agents/session-list-content.mounted.test.tsx b/apps/mobile/src/components/agents/session-list-content.mounted.test.tsx index c50045de57..ece7561558 100644 --- a/apps/mobile/src/components/agents/session-list-content.mounted.test.tsx +++ b/apps/mobile/src/components/agents/session-list-content.mounted.test.tsx @@ -4,6 +4,7 @@ import TestRenderer, { act } from 'react-test-renderer'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { i18n } from '@/i18n'; +import { getEffectiveTabBarHeight } from '@/lib/tab-bar-layout'; import { type StoredSession } from '@/lib/hooks/use-agent-sessions'; import { AgentSessionListContent } from './session-list-content'; import { type SessionSection } from './session-list-helpers'; @@ -28,6 +29,8 @@ const controls = vi.hoisted(() => ({ scrollResets: 0, deleteSession: vi.fn(), renameSession: vi.fn(), + leftInset: 0, + rightInset: 0, })); vi.mock('@/components/centered-state', () => ({ CenteredState: 'CenteredState' })); @@ -92,7 +95,13 @@ vi.mock('react-native-reanimated', () => ({ FadeIn: { duration: () => undefined }, FadeOut: { duration: () => undefined }, })); -vi.mock('react-native-safe-area-context', () => ({ useSafeAreaInsets: () => ({ bottom: 0 }) })); +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ + bottom: 0, + left: controls.leftInset, + right: controls.rightInset, + }), +})); vi.mock('@/components/agents/session-row', () => ({ StoredSessionRow: 'StoredSessionRow' })); vi.mock('@/components/agents/session-list-section-header', () => ({ SessionListSectionHeader: 'SessionListSectionHeader', @@ -429,4 +438,29 @@ describe('AgentSessionListContent liveness', () => { expect(hosts(renderer, 'Button')).toHaveLength(0); expect(rows(renderer)).toEqual([]); }); + + it('pads the SectionList content by the landscape side insets', () => { + const sections = [{ title: 'Today', data: [session('padded')] }]; + const renderer = mount(contentProps({ sections })); + const style = () => + ( + renderer.root.find(node => isHost(node, 'SectionList')).props as { + contentContainerStyle: Record; + } + ).contentContainerStyle; + const tabClearance = getEffectiveTabBarHeight({ + bottomInset: 0, + platform: 'ios', + fontScale: 1, + }); + expect(style()).toEqual({ paddingBottom: tabClearance, paddingLeft: 0, paddingRight: 0 }); + + // Rotation pads only the sides; the tab-bar clearance is unchanged. + controls.leftInset = 47; + controls.rightInset = 59; + act(() => { + renderer.update(createElement(AgentSessionListContent, contentProps({ sections }))); + }); + expect(style()).toEqual({ paddingBottom: tabClearance, paddingLeft: 47, paddingRight: 59 }); + }); }); diff --git a/apps/mobile/src/components/agents/session-list-content.tsx b/apps/mobile/src/components/agents/session-list-content.tsx index 0703b3bfc2..75cc674d88 100644 --- a/apps/mobile/src/components/agents/session-list-content.tsx +++ b/apps/mobile/src/components/agents/session-list-content.tsx @@ -101,7 +101,7 @@ export function AgentSessionListContent({ const colors = useThemeColors(); const { t } = useTranslation(); - const { bottom } = useSafeAreaInsets(); + const { bottom, left, right } = useSafeAreaInsets(); const { fontScale } = useWindowDimensions(); const { deleteSession, renameSession } = useSessionMutations(); // The stored refetch resolves void: a pull failure surfaces through the @@ -130,7 +130,9 @@ 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. The history list - // owns no FAB, so tab-bar-only clearance is the only inset it needs. + // owns no FAB, so tab-bar-only clearance plus the landscape side insets that + // keep row text clear of the sensor housing (portrait insets are 0, keeping + // the geometry unchanged) are the only insets it needs. const tabBarOnlyClearanceStyle = useMemo( () => ({ paddingBottom: getEffectiveTabBarHeight({ @@ -138,8 +140,10 @@ export function AgentSessionListContent({ platform: Platform.OS, fontScale, }), + paddingLeft: left, + paddingRight: right, }), - [bottom, fontScale] + [bottom, fontScale, left, right] ); // Pure body decision — see `session-list-body-model.ts`. diff --git a/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx b/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx index a757f689b3..9532f032ed 100644 --- a/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx +++ b/apps/mobile/src/components/agents/session-list-screen.mounted.test.tsx @@ -19,6 +19,8 @@ const state = vi.hoisted(() => ({ focused: true, fontScale: 1, topInset: 0, + leftInset: 0, + rightInset: 0, tabBarHeight: 60, focusCallbacks: new Set<() => void>(), listeners: new Set<(state: string) => void>(), @@ -102,7 +104,12 @@ vi.mock('react-native-reanimated', () => ({ LinearTransition: 'LinearTransition', })); vi.mock('react-native-safe-area-context', () => ({ - useSafeAreaInsets: () => ({ top: state.topInset, bottom: 0 }), + useSafeAreaInsets: () => ({ + top: state.topInset, + bottom: 0, + left: state.leftInset, + right: state.rightInset, + }), })); vi.mock('expo-router', () => ({ useNavigation: () => ({ isFocused: () => state.focused }), @@ -328,6 +335,8 @@ beforeEach(() => { state.focused = true; state.fontScale = 1; state.topInset = 0; + state.leftInset = 0; + state.rightInset = 0; state.tabBarHeight = 60; state.focusCallbacks.clear(); state.destination = ''; @@ -754,6 +763,54 @@ describe('AgentSessionListScreen live presentation', () => { expect(typeof nodes('FlatList')[0]?.props.extraData).toBe('number'); }); + it('offsets the FAB by the landscape right inset and keeps its vertical position', async () => { + state.live.activeSessions = [row]; + await renderScreen(); + const fab = () => + nodes('Pressable').find(node => node.props.testID === 'agents-new-session-fab'); + expect(fab()?.props.style).toEqual({ + bottom: state.tabBarHeight + 16, + right: 20, + width: 48, + height: 48, + }); + + // Rotation must not move or resize the FAB vertically; only the side offset + // grows by the right inset. + state.rightInset = 59; + await renderScreen(); + expect(fab()?.props.style).toEqual({ + bottom: state.tabBarHeight + 16, + right: 79, + width: 48, + height: 48, + }); + }); + + it('pads the live list content by the landscape side insets', async () => { + state.live.activeSessions = [row]; + await renderScreen(); + const contentContainerStyle = () => + nodes('FlatList')[0]?.props.contentContainerStyle as Record; + expect(contentContainerStyle()).toEqual({ + paddingTop: 0, + paddingBottom: state.tabBarHeight + 64, + paddingLeft: 0, + paddingRight: 0, + }); + + // Rotation pads only the sides; the vertical geometry is unchanged. + state.leftInset = 47; + state.rightInset = 59; + await renderScreen(); + expect(contentContainerStyle()).toEqual({ + paddingTop: 0, + paddingBottom: state.tabBarHeight + 64, + paddingLeft: 47, + paddingRight: 59, + }); + }); + it('renders no history list, animated wrappers, or active-now section and keeps one history label without a plus icon', async () => { state.live.activeSessions = [row]; await renderScreen(); diff --git a/apps/mobile/src/components/agents/session-list-screen.tsx b/apps/mobile/src/components/agents/session-list-screen.tsx index 7e71dd4b85..efda9ae69c 100644 --- a/apps/mobile/src/components/agents/session-list-screen.tsx +++ b/apps/mobile/src/components/agents/session-list-screen.tsx @@ -37,7 +37,7 @@ export function AgentSessionListScreen() { const navigation = useNavigation(); const colors = useThemeColors(); const { t } = useTranslation(); - const { bottom } = useSafeAreaInsets(); + const { bottom, left, right } = useSafeAreaInsets(); const { fontScale } = useWindowDimensions(); const tabBarHeight = useMemo( @@ -172,23 +172,37 @@ export function AgentSessionListScreen() { // The tab bar is an absolutely-positioned overlay, so scrollable content // must clear it. The FAB adds its own inset when it shows so the last row - // scrolls clear of the button too. + // scrolls clear of the button too. The landscape side insets keep row text + // clear of the sensor housing; portrait insets are 0, keeping the geometry + // unchanged. const listPadding = useMemo( () => ({ paddingTop: 0, paddingBottom: tabBarHeight + (hasLiveRows ? FAB_SIZE + FAB_MARGIN : 0), + paddingLeft: left, + paddingRight: right, }), - [tabBarHeight, hasLiveRows] + [tabBarHeight, hasLiveRows, left, right] ); + // The fixed 20pt margin gains the landscape right inset so the FAB clears the + // sensor area; portrait insets are 0, keeping the geometry unchanged. const fabStyle = useMemo( () => ({ bottom: tabBarHeight + FAB_MARGIN, - right: 20, + right: 20 + right, width: FAB_SIZE, height: FAB_SIZE, }), - [tabBarHeight] + [tabBarHeight, right] + ); + + // The fixed 22px margins on the skeleton rows and the status wrapper gain + // the landscape side insets so they clear the sensor housing too; portrait + // insets are 0, keeping the geometry unchanged. + const sidePadding = useMemo( + () => ({ paddingLeft: 22 + left, paddingRight: 22 + right }), + [left, right] ); let body: ReactNode = null; @@ -196,8 +210,8 @@ export function AgentSessionListScreen() { body = ( {Array.from({ length: SKELETON_ROW_COUNT }, (_, i) => ( - - + + ))} @@ -267,7 +281,10 @@ export function AgentSessionListScreen() { onClearSearch={query.handleClearSearch} /> ) : null} - + ({ + insets: { top: 59, right: 0, bottom: 34, left: 0 }, +})); + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + TextInput: 'TextInput', + View: 'View', +})); +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => state.insets, +})); +vi.mock('@/components/ui/icons', () => ({ Search: 'Search', X: 'X' })); +vi.mock('@/components/ui/activity-indicator', () => ({ ActivityIndicator: 'ActivityIndicator' })); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ mutedForeground: '#000000', foreground: '#111111' }), +})); + +const baseProps = { + inputRef: createRef(), + hasText: false, + showSearchBusy: false, + onChangeText: () => undefined, + onClearSearch: () => undefined, +}; + +const renderers: TestRenderer.ReactTestRenderer[] = []; + +async function mount(element: ReactElement) { + await act(() => { + renderers.push(TestRenderer.create(element)); + }); + const renderer = renderers.at(-1); + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; +} + +function fieldRow(renderer: TestRenderer.ReactTestRenderer) { + const row = renderer.root + .findAll( + node => + node.type === ('View' as ElementType) && + typeof node.props.className === 'string' && + node.props.className.includes('rounded-[10px]') + ) + .at(0); + if (!row) { + throw new Error('search field row was not found'); + } + return row; +} + +describe('SessionListSearchHeader landscape sensor insets', () => { + beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + }); + afterEach(() => { + act(() => { + for (const renderer of renderers.splice(0)) { + renderer.unmount(); + } + }); + }); + + it('keeps the fixed 22px margins in portrait where the side insets are 0', async () => { + state.insets = { top: 59, right: 0, bottom: 34, left: 0 }; + const renderer = await mount(); + expect(fieldRow(renderer).props.style).toEqual({ marginLeft: 22, marginRight: 22 }); + }); + + it('gains the landscape sensor insets on both sides so the field clears the housing', async () => { + state.insets = { top: 59, right: 59, bottom: 34, left: 47 }; + const renderer = await mount(); + expect(fieldRow(renderer).props.style).toEqual({ marginLeft: 69, marginRight: 81 }); + }); + + it('updates the margins on rotation without a remount', async () => { + state.insets = { top: 59, right: 0, bottom: 34, left: 0 }; + const renderer = await mount(); + expect(fieldRow(renderer).props.style).toEqual({ marginLeft: 22, marginRight: 22 }); + state.insets = { top: 59, right: 59, bottom: 34, left: 47 }; + await act(() => { + renderer.update(); + }); + expect(fieldRow(renderer).props.style).toEqual({ marginLeft: 69, marginRight: 81 }); + }); +}); diff --git a/apps/mobile/src/components/agents/session-list-search-header.tsx b/apps/mobile/src/components/agents/session-list-search-header.tsx index 059acb8553..660ce52b3f 100644 --- a/apps/mobile/src/components/agents/session-list-search-header.tsx +++ b/apps/mobile/src/components/agents/session-list-search-header.tsx @@ -1,7 +1,8 @@ import { Search, X } from '@/components/ui/icons'; -import { type RefObject } from 'react'; +import { type RefObject, useMemo } from 'react'; import { Pressable, TextInput, View } from 'react-native'; import { ActivityIndicator } from '@/components/ui/activity-indicator'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { useTranslation } from 'react-i18next'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; @@ -31,9 +32,20 @@ export function SessionListSearchHeader({ }: Readonly) { const colors = useThemeColors(); const { t } = useTranslation(); + // The landscape side insets keep the field's rounded border and left tap + // area clear of the sensor housing; portrait insets are 0, keeping the + // fixed 22px margin unchanged. + const { left, right } = useSafeAreaInsets(); + const fieldMargins = useMemo( + () => ({ marginLeft: 22 + left, marginRight: 22 + right }), + [left, right] + ); return ( - + {/* Fixed-size slot: the spinner swaps in for the icon, so the row never reflows. */} {showSearchBusy ? ( diff --git a/apps/mobile/src/components/agents/session-message-list.mounted.test.tsx b/apps/mobile/src/components/agents/session-message-list.mounted.test.tsx index 8e2faa9367..715a223412 100644 --- a/apps/mobile/src/components/agents/session-message-list.mounted.test.tsx +++ b/apps/mobile/src/components/agents/session-message-list.mounted.test.tsx @@ -1,11 +1,16 @@ /* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer mounts the React Native tree without a device */ import { createElement } from 'react'; import TestRenderer, { act } from 'react-test-renderer'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { SessionMessageList } from './session-message-list'; const flashListProps = vi.hoisted(() => ({ current: null as Record | null })); +const controls = vi.hoisted(() => ({ + isAtBottom: true, + leftInset: 0, + rightInset: 0, +})); vi.mock('@shopify/flash-list', () => ({ FlashList: (props: Record) => { @@ -20,6 +25,14 @@ vi.mock('react-native', () => ({ Pressable: 'Pressable', View: 'View', })); +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ + top: 0, + bottom: 0, + left: controls.leftInset, + right: controls.rightInset, + }), +})); vi.mock('react-native-reanimated', () => ({ default: { View: 'AnimatedView' }, FadeIn: { duration: () => ({}) }, @@ -31,7 +44,7 @@ vi.mock('@/lib/hooks/use-theme-colors', () => ({ })); vi.mock('@/components/agents/use-session-list-auto-scroll', () => ({ useSessionListAutoScroll: () => ({ - isAtBottom: true, + isAtBottom: controls.isAtBottom, listRef: { current: null }, scrollToLatestAnimated: vi.fn(), handleContentSizeChange: vi.fn(), @@ -69,3 +82,89 @@ describe('SessionMessageList', () => { expect(flashListProps.current?.removeClippedSubviews).toBe(false); }); }); + +// `Object.is` keeps the host-string comparison off the ElementType union. +function scrollButton(renderer: TestRenderer.ReactTestRenderer) { + return renderer.root.find(node => Object.is(node.type, 'AnimatedView')); +} + +describe('SessionMessageList landscape side insets', () => { + const baseProps = { + sessionId: 'session-1', + items: ['message-1'], + keyExtractor: (item: string) => item, + hasOlderMessages: false, + isLoadingOlderMessages: false, + olderMessagesError: null, + olderMessagesOmittedItemCount: 0, + onLoadOlderMessages: () => undefined, + renderItem: () => null, + }; + + function mountList( + overrides: Partial>[0]> = {} + ): TestRenderer.ReactTestRenderer { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create( + createElement(SessionMessageList, { ...baseProps, ...overrides }) + ); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; + } + + afterEach(() => { + controls.isAtBottom = true; + controls.leftInset = 0; + controls.rightInset = 0; + }); + + it('keeps the module style reference and a 16pt control offset in portrait', () => { + controls.isAtBottom = false; + const renderer = mountList(); + const style = flashListProps.current?.contentContainerStyle; + expect(style).toEqual({ paddingVertical: 8 }); + expect(scrollButton(renderer).props.style).toEqual({ right: 16 }); + + // Unchanged inputs keep the same style reference so FlashList's portrait + // behavior (including `maintainVisibleContentPosition`) is untouched. + act(() => { + renderer.update(createElement(SessionMessageList, { ...baseProps })); + }); + expect(flashListProps.current?.contentContainerStyle).toBe(style); + + // A fresh mount shares the reference too: it is the module-level constant, + // not a per-mount allocation. + const remounted = mountList(); + expect(flashListProps.current?.contentContainerStyle).toBe(style); + expect(scrollButton(remounted).props.style).toEqual({ right: 16 }); + }); + + it('pads the transcript and offsets the control by the landscape side insets', () => { + controls.isAtBottom = false; + controls.leftInset = 47; + controls.rightInset = 59; + const renderer = mountList(); + expect(flashListProps.current?.contentContainerStyle).toEqual({ + paddingTop: 8, + paddingBottom: 8, + paddingLeft: 47, + paddingRight: 59, + }); + expect(scrollButton(renderer).props.style).toEqual({ right: 75 }); + }); + + it('carries the side paddings when a content bottom inset is provided', () => { + mountList({ contentBottomInset: 34 }); + expect(flashListProps.current?.contentContainerStyle).toEqual({ + paddingTop: 8, + paddingBottom: 42, + paddingLeft: 0, + paddingRight: 0, + }); + }); +}); diff --git a/apps/mobile/src/components/agents/session-message-list.tsx b/apps/mobile/src/components/agents/session-message-list.tsx index 40270b9f5f..f34c6cee0d 100644 --- a/apps/mobile/src/components/agents/session-message-list.tsx +++ b/apps/mobile/src/components/agents/session-message-list.tsx @@ -12,6 +12,7 @@ import { } from 'react-native'; import { useTranslation } from 'react-i18next'; import Animated, { FadeIn, FadeOut } from 'react-native-reanimated'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { useSessionListAutoScroll } from '@/components/agents/use-session-list-auto-scroll'; import { SessionPaginationHeader } from '@/components/agents/session-pagination-header'; @@ -100,6 +101,7 @@ export function SessionMessageList({ }); const colors = useThemeColors(); const { t } = useTranslation(); + const { left, right } = useSafeAreaInsets(); // Coalesce the trigger: only fire `onLoadOlderMessages` while there is // actually a cursor, we are not already loading, and we are not in a @@ -202,17 +204,25 @@ export function SessionMessageList({ olderArrivalNewestKeyRef.current = nextNewestKey; }, [items, keyExtractor]); - // When the optional `contentBottomInset` is omitted we return the - // original module-level `listContentContainerStyle` reference so the - // default-prop path is behavior-identical (no allocation, no value - // change). When provided we extend the bottom padding to clear safe - // areas such as the home indicator on curved-bottom iPhones. + // When the optional `contentBottomInset` is omitted and the landscape side + // insets are 0 (portrait) we return the original module-level + // `listContentContainerStyle` reference so the default-prop path is + // behavior-identical (no allocation, no value change). When provided we + // extend the bottom padding to clear safe areas such as the home indicator + // on curved-bottom iPhones, and the side padding keeps transcript text clear + // of the landscape sensor housing; portrait insets are 0, keeping the + // geometry unchanged. const resolvedContentContainerStyle = useMemo( () => - contentBottomInset - ? { paddingTop: 8, paddingBottom: 8 + contentBottomInset } - : listContentContainerStyle, - [contentBottomInset] + !contentBottomInset && left === 0 && right === 0 + ? listContentContainerStyle + : { + paddingTop: 8, + paddingBottom: 8 + (contentBottomInset ?? 0), + paddingLeft: left, + paddingRight: right, + }, + [contentBottomInset, left, right] ); return ( @@ -272,6 +282,10 @@ export function SessionMessageList({ exiting={FadeOut.duration(150)} pointerEvents="box-none" className="absolute bottom-4 right-4" + // The fixed 16pt (right-4) offset gains the landscape right inset so + // the control clears the sensor area; portrait insets are 0, keeping + // the geometry unchanged (16 == right-4). + style={{ right: 16 + right }} > + // bg-background: the gesture root is the first opaque surface above the + // window — a rotation relayout gap behind any screen must show the app's + // own background, never the platform window default (foreign white/black). + diff --git a/apps/mobile/src/components/image-viewer-modal.mounted.test.tsx b/apps/mobile/src/components/image-viewer-modal.mounted.test.tsx index 20090359a1..acaff9b419 100644 --- a/apps/mobile/src/components/image-viewer-modal.mounted.test.tsx +++ b/apps/mobile/src/components/image-viewer-modal.mounted.test.tsx @@ -1,11 +1,13 @@ /* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (same pattern as animated-splash-overlay.mounted.test.tsx) */ import { type ComponentProps, createElement } from 'react'; import TestRenderer, { act } from 'react-test-renderer'; -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ImageViewerModal } from './image-viewer-modal'; import { AccessibleStatus } from '@/components/ui/accessible-status'; +const safeArea = vi.hoisted(() => ({ top: 0, bottom: 0, left: 0, right: 0 })); + // A chainable gesture stub: each builder method returns the same object so the // modal's Pinch/Pan/Tap/Race/Simultaneous chains resolve without RNGH. function makeGesture(): Record { @@ -50,7 +52,7 @@ vi.mock('react-native-reanimated', () => ({ withTiming: (value: unknown) => value, })); vi.mock('react-native-safe-area-context', () => ({ - useSafeAreaInsets: () => ({ top: 0, bottom: 0 }), + useSafeAreaInsets: () => safeArea, })); vi.mock('react-native-worklets', () => ({ scheduleOnRN: vi.fn(), @@ -68,6 +70,46 @@ function findByType( return root.findAll(node => typeof node.type === 'string' && (node.type as string) === type); } +/** + * The outer header container: it keeps the border, background, and the fixed + * vertical geometry (`paddingTop`/`height`); the controls row lives in an inner + * wrapper that carries only the landscape side insets. + */ +function findHeaderContainer(root: TestRenderer.ReactTestInstance): TestRenderer.ReactTestInstance { + return root.find( + node => + typeof node.type === 'string' && + (node.type as string) === 'View' && + typeof node.props.className === 'string' && + node.props.className.includes('border-b') + ); +} + +function pressableByLabel( + node: TestRenderer.ReactTestInstance, + label: string +): TestRenderer.ReactTestInstance | undefined { + return node.find( + child => + typeof child.type === 'string' && + (child.type as string) === 'Pressable' && + child.props.accessibilityLabel === label + ); +} + +function findRowWrapper(header: TestRenderer.ReactTestInstance): TestRenderer.ReactTestInstance { + const wrapper = header.children.find( + (child): child is TestRenderer.ReactTestInstance => + typeof child !== 'string' && + typeof child.type === 'string' && + (child.type as string) === 'View' + ); + if (!wrapper) { + throw new Error('header row wrapper missing'); + } + return wrapper; +} + async function mountViewer( props: Partial> ): Promise { @@ -92,6 +134,10 @@ async function mountViewer( } describe('ImageViewerModal mounted', () => { + beforeEach(() => { + Object.assign(safeArea, { top: 0, bottom: 0, left: 0, right: 0 }); + }); + it('shows the Image unavailable fallback and keeps Share enabled on decode failure', async () => { const onShare = vi.fn<() => void>(); const renderer = await mountViewer({ onShare }); @@ -199,4 +245,81 @@ describe('ImageViewerModal mounted', () => { renderer.unmount(); }); + + it('keeps the header geometry keys and a styleless row wrapper at zero side insets', async () => { + const renderer = await mountViewer({ onShare: () => undefined }); + + // Portrait no-op: the outer container keeps its fixed vertical geometry and + // the row wrapper's style collapses to undefined (an inline 0 would + // override the wrapper's `px-4` className gutter). + const header = findHeaderContainer(renderer.root); + expect(header.props.style).toEqual({ paddingTop: 0, height: 56 }); + const wrapper = findRowWrapper(header); + expect(wrapper.props.style).toBeUndefined(); + expect(wrapper.props.className).toContain('px-4'); + expect(wrapper.props.className).toContain('flex-row'); + expect(wrapper.props.className).toContain('justify-between'); + + renderer.unmount(); + }); + + it('supports landscape so a full-screen modal is never portrait-locked', async () => { + const renderer = await mountViewer({ onShare: () => undefined }); + + // e11 viewer-landscape: RN locks a full-screen modal on iPhone to portrait + // unless the Modal lists the orientations it supports. With rotation + // enabled app-wide, an unset prop kept the viewer's content at portrait + // bounds in a landscape window and clipped the photo at the screen bottom. + const modal = findByType(renderer.root, 'Modal')[0]; + if (!modal) { + throw new Error('Modal missing'); + } + expect(modal.props.supportedOrientations).toEqual(['portrait', 'landscape']); + + renderer.unmount(); + }); + + it('fits the image inside the flex area below the header', async () => { + const renderer = await mountViewer({ onShare: () => undefined }); + + // The image fills its zoomable wrapper and is contained — never + // cover-scaled or clipped by the black area that starts below the header. + const image = findByType(renderer.root, 'Image')[0]; + if (!image) { + throw new Error('viewer Image missing'); + } + expect(image.props.contentFit).toBe('contain'); + expect(image.props.className).toContain('h-full'); + expect(image.props.className).toContain('w-full'); + + const zoomWrapper = image.parent; + const gestureArea = zoomWrapper?.parent; + const imageArea = gestureArea?.parent; + expect(imageArea?.props.className).toContain('flex-1'); + expect(imageArea?.props.className).toContain('overflow-hidden'); + + renderer.unmount(); + }); + + it('pads the header row by the landscape side insets and keeps the controls inside it', async () => { + safeArea.left = 47; + safeArea.right = 59; + const renderer = await mountViewer({ onShare: () => undefined }); + + // The side insets land on the row wrapper so they ADD to its `px-4` + // gutter (an inline padding on the container would override the class); + // the fixed vertical geometry stays on the outer container, so a rotation + // never shifts the header height. + const header = findHeaderContainer(renderer.root); + expect(header.props.style).toEqual({ paddingTop: 0, height: 56 }); + const wrapper = findRowWrapper(header); + expect(wrapper.props.style).toEqual({ paddingLeft: 47, paddingRight: 59 }); + expect(wrapper.props.className).toContain('px-4'); + const close = pressableByLabel(wrapper, 'Close photo.png'); + const share = pressableByLabel(wrapper, 'Share photo.png'); + expect(close?.parent).toBe(wrapper); + expect(share?.parent).toBe(wrapper); + + renderer.unmount(); + }); }); diff --git a/apps/mobile/src/components/image-viewer-modal.tsx b/apps/mobile/src/components/image-viewer-modal.tsx index 7a3a84fdbe..4d8d0a1cce 100644 --- a/apps/mobile/src/components/image-viewer-modal.tsx +++ b/apps/mobile/src/components/image-viewer-modal.tsx @@ -40,6 +40,21 @@ export function ImageViewerModal({ const insets = useSafeAreaInsets(); const { t } = useTranslation(); + // Landscape side safe areas (notch/Dynamic Island, Android cutouts) shift the + // header row off the sensor on this full-screen modal. They go on an inner + // wrapper so they ADD to the `px-4` gutter: an inline padding on the header + // container would beat the className (inline style wins in React Native) and + // swallow the gutter. Zero insets collapse the wrapper style to `undefined`, + // so portrait pixels are byte-identical and a rotation never moves anything + // vertically. + const sideInsetStyle = + insets.left > 0 || insets.right > 0 + ? { + ...(insets.left > 0 ? { paddingLeft: insets.left } : undefined), + ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), + } + : undefined; + const [imageError, setImageError] = useState(false); // Reset a prior decode error in render when the URL changes. A successful // renew writes a NEW signed URL; the reset must land in the same commit so @@ -140,35 +155,49 @@ export function ImageViewerModal({ return ( - - - - {onShare !== undefined ? ( - + - ) : null} + {onShare !== undefined ? ( + + + + ) : null} + {/* RNGH gestures need their own root inside an RN Modal — the app-root GestureHandlerRootView does not reach a Modal's native view hierarchy. */} diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.test.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.test.tsx new file mode 100644 index 0000000000..27f0379c33 --- /dev/null +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.test.tsx @@ -0,0 +1,126 @@ +/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (same pattern as pr-diff-file-list-loading.test.tsx) */ +import { createElement } from 'react'; +import TestRenderer, { act } from 'react-test-renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import '@/i18n'; +import { PrDiffFileListHeader } from './pr-diff-file-list-header'; + +const insets = vi.hoisted(() => ({ top: 0, bottom: 0, left: 0, right: 0 })); +const routerPush = vi.fn(); + +vi.mock('expo-router', () => ({ + useRouter: () => ({ push: routerPush }), +})); + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + View: 'View', +})); + +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => insets, +})); + +vi.mock('@/components/ui/icons', () => ({ + Columns2: () => null, + Rows3: () => null, +})); + +vi.mock('@/components/ui/radio-group', () => ({ + RadioGroup: 'RadioGroup', + radioItemA11y: () => ({}), +})); + +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); +vi.mock('@/lib/hooks/use-is-tablet', () => ({ useIsTablet: () => false })); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ + foreground: '#000000', + mutedForeground: '#6F6A61', + }), +})); + +const baseProps = { + owner: 'octocat', + repo: 'hello', + number: 7, + viewedCount: 1, + totalListed: 3, + isTruncated: false, + viewMode: 'unified' as const, + onViewModeChange: vi.fn(), +}; + +function mountHeader(): TestRenderer.ReactTestRenderer { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create(createElement(PrDiffFileListHeader, baseProps)); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; +} + +function findNavigatorPressable( + root: TestRenderer.ReactTestInstance +): TestRenderer.ReactTestInstance { + return root.findByProps({ accessibilityLabel: 'Open file navigator' }); +} + +describe('PrDiffFileListHeader side insets (landscape)', () => { + beforeEach(() => { + insets.top = 0; + insets.bottom = 0; + insets.left = 0; + insets.right = 0; + routerPush.mockClear(); + }); + + it('keeps the row styleless at zero portrait insets', () => { + const renderer = mountHeader(); + const pressable = findNavigatorPressable(renderer.root); + + // Portrait no-op: no wrapper style keys, so the `px-4` className gutter + // renders byte-identical to an inset-free header. + expect(pressable.parent?.props.style).toBeUndefined(); + }); + + it('clears the sensor housing with the landscape side insets', () => { + insets.left = 47; + insets.right = 59; + const renderer = mountHeader(); + const pressable = findNavigatorPressable(renderer.root); + + // The insets land on an inner wrapper so they ADD to the `px-4` gutter + // (an inline padding on the bordered container would beat the className + // and swallow the gutter). + expect(pressable.parent?.props.style).toEqual({ + paddingLeft: 47, + paddingRight: 59, + }); + // The full-width background and hairline stay on the outer container. + expect(pressable.parent?.parent?.props.className).toContain('bg-background'); + expect(pressable.parent?.parent?.props.className).toContain('border-b'); + }); + + it('opens the file navigator when the entry is pressed', () => { + const renderer = mountHeader(); + const pressable = findNavigatorPressable(renderer.root); + + const onPress = (pressable.props as { onPress?: () => void }).onPress; + act(() => { + onPress?.(); + }); + + expect(routerPush).toHaveBeenCalledTimes(1); + expect(routerPush).toHaveBeenCalledWith( + expect.objectContaining({ + pathname: '/(app)/pr-review/[owner]/[repo]/[number]/file-navigator', + params: { owner: 'octocat', repo: 'hello', number: 7 }, + }) + ); + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx index 6695116faa..4ac1853567 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx @@ -13,6 +13,7 @@ import { Columns2, Rows3 } from '@/components/ui/icons'; import { useCallback, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Pressable, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { RadioGroup, radioItemA11y } from '@/components/ui/radio-group'; import { Text } from '@/components/ui/text'; @@ -50,6 +51,20 @@ export function PrDiffFileListHeader({ const isTablet = useIsTablet(); const colors = useThemeColors(); const { t } = useTranslation(); + // Landscape side safe areas (notch/Dynamic Island, Android cutouts) shift the + // navigator row off the sensor, like ScreenHeader/SheetHeader. They go on an + // inner wrapper so they ADD to the `px-4` gutter: an inline padding on the + // bordered container would beat the className (inline style wins in React + // Native) and swallow the gutter. Zero insets collapse the wrapper style to + // `undefined`, so portrait pixels are byte-identical. + const insets = useSafeAreaInsets(); + const sideInsetStyle = + insets.left > 0 || insets.right > 0 + ? { + ...(insets.left > 0 ? { paddingLeft: insets.left } : undefined), + ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), + } + : undefined; const navigatorHref = useMemo( () => ({ pathname: FILE_NAVIGATOR_PATH, params: { owner, repo, number } }), @@ -61,33 +76,35 @@ export function PrDiffFileListHeader({ }, [router, navigatorHref]); return ( - - - - - {t('prReview.fileList.filesViewed', { - viewed: formatNumber(viewedCount, i18n.language), - total: formatNumber(totalListed, i18n.language), - })} - - {isTruncated ? ( - - {t('prReview.fileList.listed')} + + + + + + {t('prReview.fileList.filesViewed', { + viewed: formatNumber(viewedCount, i18n.language), + total: formatNumber(totalListed, i18n.language), + })} - ) : null} - - {isTablet ? : null} + {isTruncated ? ( + + {t('prReview.fileList.listed')} + + ) : null} + + {isTablet ? : null} + ); } diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.test.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.test.tsx index 085891226a..0172e38187 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.test.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.test.tsx @@ -52,3 +52,38 @@ describe('PrDiffFileListLoading bottom inset (plan §6)', () => { expect(style.paddingBottom).toBe(50); }); }); + +describe('PrDiffFileListLoading side insets (landscape)', () => { + beforeEach(() => { + insetsState.top = 0; + insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; + }); + + it('keeps the container style a portrait no-op with zero side insets', () => { + const renderer = mountLoading(); + + // No paddingLeft/paddingRight keys at zero, so the skeleton rows keep + // their portrait `px-4` gutter geometry exactly. + const style = loadingView(renderer).props.style as Record; + expect(style.paddingLeft).toBeUndefined(); + expect(style.paddingRight).toBeUndefined(); + expect(style.paddingBottom).toBe(32); + }); + + it('clears the sensor housing with the landscape side insets', () => { + insetsState.left = 47; + insetsState.right = 59; + const renderer = mountLoading(); + + // The insets land on the skeleton container so the rows (and their + // hairline separators) match the loaded FlashList geometry, whose + // content-container padding carries the same side insets. The + // height-feeding paddingBottom is unchanged. + const style = loadingView(renderer).props.style as Record; + expect(style.paddingLeft).toBe(47); + expect(style.paddingRight).toBe(59); + expect(style.paddingBottom).toBe(32); + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.tsx index 0505d53108..8b7bed73dd 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-loading.tsx @@ -4,6 +4,7 @@ import { useTranslation } from 'react-i18next'; import { View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { Skeleton } from '@/components/ui/skeleton'; import { useDetailScreenBottomPadding } from '@/lib/screen-insets'; @@ -13,12 +14,24 @@ const SKELETON_ROWS = [0, 1, 2, 3, 4, 5, 6, 7] as const; export function PrDiffFileListLoading() { const bottomPadding = useDetailScreenBottomPadding(); const { t } = useTranslation(); + // Landscape side safe areas (notch/Dynamic Island, Android cutouts) shift the + // skeleton rows off the sensor, matching the loaded FlashList whose content + // container carries the same side insets. Zero insets collapse the style so + // portrait geometry is byte-identical. + const insets = useSafeAreaInsets(); + const sideInsetStyle = + insets.left > 0 || insets.right > 0 + ? { + ...(insets.left > 0 ? { paddingLeft: insets.left } : undefined), + ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), + } + : undefined; return ( {SKELETON_ROWS.map(index => ( ({ top: 0, bottom: 0, left: 0, right: 0 })); @@ -153,9 +154,19 @@ function resetState(): void { listQueryState.firstPageErrorState = null; } +function flashListProps(renderer: TestRenderer.ReactTestRenderer): { + contentContainerStyle?: Record; +} { + return renderer.root.find(node => String(node.type) === 'FlashList').props as { + contentContainerStyle?: Record; + }; +} + describe('PrReviewFileList full-body states', () => { beforeEach(() => { insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; resetState(); }); @@ -212,3 +223,35 @@ describe('PrReviewFileList full-body states', () => { expect(renderer.root.findAll(node => String(node.type) === 'QueryError')).toHaveLength(0); }); }); + +describe('PrReviewFileList content container side insets (landscape)', () => { + beforeEach(() => { + insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; + resetState(); + listQueryState.files = [{ path: 'src/file.ts' }]; + }); + + it('keeps the current content container style at zero portrait insets', () => { + const renderer = mountList(); + + expect(flashListProps(renderer).contentContainerStyle).toEqual({ + paddingBottom: prDiffListBottomPadding(null), + paddingLeft: 0, + paddingRight: 0, + }); + }); + + it('adds the landscape side insets to the content container style', () => { + insetsState.left = 47; + insetsState.right = 59; + const renderer = mountList(); + + expect(flashListProps(renderer).contentContainerStyle).toEqual({ + paddingBottom: prDiffListBottomPadding(null), + paddingLeft: 47, + paddingRight: 59, + }); + }); +}); 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 db42a09621..84694be5e0 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 @@ -26,7 +26,7 @@ import { FlashList, type FlashListRef } from '@shopify/flash-list'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { View, type ViewStyle } from 'react-native'; +import { View } from 'react-native'; import { RefreshControl } from '@/components/ui/refresh-control'; import { QueryError } from '@/components/query-error'; @@ -45,7 +45,6 @@ import { useDiffRenderItem } from '@/components/pr-review/diff/pr-diff-file-list import { useDiffSelection } from '@/components/pr-review/diff/use-diff-selection'; import { EmptyFilesView, TabStateMessage } from '@/components/pr-review/diff/pr-diff-rows'; import { buildFileItems, buildPaginationItem } from '@/lib/pr-review/diff/pr-diff-list-builder'; -import { prDiffListBottomPadding } from '@/lib/pr-review/diff/pr-diff-list-bottom-padding'; import { itemTypeFor, type ListItem } from '@/lib/pr-review/diff/pr-diff-list-items'; import { stickyFileHeaderIndices } from '@/lib/pr-review/diff/sticky-file-headers'; import { usePrDiffContextLoader } from '@/lib/pr-review/diff/use-pr-diff-context-loader'; @@ -55,6 +54,7 @@ import { usePrReviewViewedFiles, } from '@/lib/pr-review/diff/pr-review-file-list-state'; import { usePrDiffListScroll } from '@/lib/pr-review/diff/use-pr-diff-list-scroll'; +import { usePrDiffListContentPadding } from '@/lib/pr-review/diff/use-pr-diff-list-content-padding'; import { clearDiffSelection } from '@/lib/pr-review/diff-selection-bridge'; import { CenteredState } from '@/components/centered-state'; import { useIsTablet } from '@/lib/hooks/use-is-tablet'; @@ -135,10 +135,7 @@ export function PrReviewFileList({ }); }, []); - const contentContainerStyle = useMemo( - () => ({ paddingBottom: prDiffListBottomPadding(barHeight) }), - [barHeight] - ); + const contentContainerStyle = usePrDiffListContentPadding(barHeight); const viewedCount = useMemo(() => { let count = 0; diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.test.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.test.tsx index 565b69700a..4976c0477b 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.test.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.test.tsx @@ -298,3 +298,49 @@ describe('PrDiffFloatingActions bottom inset (plan §6)', () => { expect(onHeightChange).toHaveBeenCalledWith(150); }); }); + +describe('PrDiffFloatingActions side insets (landscape)', () => { + beforeEach(() => { + insets.top = 0; + insets.bottom = 0; + insets.left = 0; + insets.right = 0; + }); + + function rootBarStyle(): Record { + // eslint-disable-next-line new-cap + const element = PrDiffFloatingActions(baseProps); + const root = findElement({ + node: element, + type: 'View', + prop: 'pointerEvents', + value: 'box-none', + }); + if (!root) { + throw new Error('floating action bar root not found'); + } + return (root.props as { style?: Record }).style ?? {}; + } + + it('keeps exactly the current style keys at zero portrait insets', () => { + const style = rootBarStyle(); + + // Spread only when nonzero: the `px-4` className gutter must survive + // portrait untouched (inline style wins over className). + expect(style.paddingLeft).toBeUndefined(); + expect(style.paddingRight).toBeUndefined(); + expect(style.paddingBottom).toBe(24); + }); + + it('clears the sensor housing with the landscape side insets', () => { + insets.left = 47; + insets.right = 59; + const style = rootBarStyle(); + + expect(style.paddingLeft).toBe(47); + expect(style.paddingRight).toBe(59); + // The card shrink is horizontal-only: the paddingBottom that feeds the + // measured onLayout height (and `prDiffListBottomPadding`) is unchanged. + expect(style.paddingBottom).toBe(24); + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx index a2b47ab08c..a34fe59e6f 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx @@ -58,6 +58,10 @@ export function PrDiffFloatingActions({ // The bar sits on the bottom edge, so its bottom padding must include the // Android system inset. The measured height (onLayout) therefore already // includes the inset, which `prDiffListBottomPadding` reserves for the list. + // Side insets clear the landscape sensor housing; like ScreenHeader they + // are spread only when nonzero, so the `px-4` gutter survives portrait + // (inline style wins over className), and they are horizontal-only, so the + // height-feeding bottom padding stays untouched. const insets = useSafeAreaInsets(); const showSelectionAction = viewMode === 'unified' && selection !== null; @@ -100,7 +104,11 @@ export function PrDiffFloatingActions({ }} pointerEvents="box-none" className="absolute inset-x-0 bottom-0 items-center gap-2 px-4 pt-3" - style={{ paddingBottom: 24 + insets.bottom }} + style={{ + paddingBottom: 24 + insets.bottom, + ...(insets.left > 0 ? { paddingLeft: insets.left } : undefined), + ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), + }} > {showSelectionAction ? ( diff --git a/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.test.tsx b/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.test.tsx index 7854879260..98a13ea361 100644 --- a/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.test.tsx +++ b/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.test.tsx @@ -78,6 +78,38 @@ function footerView(renderer: TestRenderer.ReactTestRenderer): TestRenderer.Reac return footer; } +describe('PrReviewDiscussionList side insets (landscape)', () => { + beforeEach(() => { + insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; + flashListProps.current = null; + }); + + it('carries explicit zero side insets at portrait', () => { + mountList(); + + expect(flashListProps.current?.contentContainerStyle).toEqual({ + paddingTop: 12, + paddingLeft: 0, + paddingRight: 0, + }); + }); + + it('clears the sensor housing with the landscape side insets', () => { + insetsState.left = 47; + insetsState.right = 59; + + mountList(); + + expect(flashListProps.current?.contentContainerStyle).toEqual({ + paddingTop: 12, + paddingLeft: 47, + paddingRight: 59, + }); + }); +}); + describe('PrReviewDiscussionList footer bottom inset (plan §6)', () => { beforeEach(() => { insetsState.bottom = 0; diff --git a/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx b/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx index c2b7eea1e2..1d780538e5 100644 --- a/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx +++ b/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx @@ -6,7 +6,8 @@ import { FlashList, type FlashListRef } from '@shopify/flash-list'; import { useQuery } from '@tanstack/react-query'; import { type RefObject, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; -import { View } from 'react-native'; +import { View, type ViewStyle } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { CommentRow } from '@/components/pr-review/discussion/comment-row'; import { DiscussionThread } from '@/components/pr-review/discussion/discussion-thread'; @@ -20,7 +21,6 @@ import { expandedForThread } from '@/lib/pr-review/discussion/thread-expansion'; import { useDetailScreenBottomPadding } from '@/lib/screen-insets'; import { useTRPC } from '@/lib/trpc'; -const DISCUSSION_LIST_CONTENT_STYLE = { paddingTop: 12 }; const noopReactionToggle = () => { // Conversation comments are read-only (A2.3): no reaction mutations. }; @@ -114,6 +114,16 @@ export function PrReviewDiscussionList({ return result; }, [listItems, hiddenLogins]); + // Landscape: side insets keep comment/thread cards clear of the sensor + // housing (rows keep their px-4 gutter, so the insets add to it); portrait + // insets are zero, so the style carries explicit zeros and nothing else + // changes. + const insets = useSafeAreaInsets(); + const contentContainerStyle = useMemo( + () => ({ paddingTop: 12, paddingLeft: insets.left, paddingRight: insets.right }), + [insets.left, insets.right] + ); + return ( ); }} - contentContainerStyle={DISCUSSION_LIST_CONTENT_STYLE} + contentContainerStyle={contentContainerStyle} keyboardShouldPersistTaps="handled" automaticallyAdjustKeyboardInsets onLayout={event => { diff --git a/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.test.tsx b/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.test.tsx index 2cc285d020..24afa842ee 100644 --- a/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.test.tsx +++ b/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.test.tsx @@ -32,7 +32,10 @@ function mountFooter(): TestRenderer.ReactTestRenderer { return renderer; } -function footerPaddingBottom(renderer: TestRenderer.ReactTestRenderer): number | undefined { +function footerStyleProp( + renderer: TestRenderer.ReactTestRenderer, + key: 'paddingBottom' | 'paddingLeft' | 'paddingRight' +): number | undefined { const views = renderer.root.findAll( node => typeof node.type === 'string' && (node.type as string) === 'View' ); @@ -40,20 +43,22 @@ function footerPaddingBottom(renderer: TestRenderer.ReactTestRenderer): number | if (!footer) { throw new Error('footer View not found'); } - return (footer.props.style as { paddingBottom?: number } | undefined)?.paddingBottom; + return (footer.props.style as Record | undefined)?.[key]; } describe('PrFormSheetFooter bottom inset (plan §6)', () => { beforeEach(() => { platformState.OS = 'ios'; insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; }); it('keeps the 16-point base padding on iOS at a nonzero inset', () => { insetsState.bottom = 34; const renderer = mountFooter(); - expect(footerPaddingBottom(renderer)).toBe(16); + expect(footerStyleProp(renderer, 'paddingBottom')).toBe(16); }); it('keeps the 16-point base padding on Android at a zero inset', () => { @@ -61,7 +66,7 @@ describe('PrFormSheetFooter bottom inset (plan §6)', () => { insetsState.bottom = 0; const renderer = mountFooter(); - expect(footerPaddingBottom(renderer)).toBe(16); + expect(footerStyleProp(renderer, 'paddingBottom')).toBe(16); }); it('adds the Android system inset to the 16-point base padding', () => { @@ -69,6 +74,45 @@ describe('PrFormSheetFooter bottom inset (plan §6)', () => { insetsState.bottom = 34; const renderer = mountFooter(); - expect(footerPaddingBottom(renderer)).toBe(50); + expect(footerStyleProp(renderer, 'paddingBottom')).toBe(50); + }); +}); + +describe('PrFormSheetFooter side insets (landscape)', () => { + beforeEach(() => { + platformState.OS = 'ios'; + insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; + }); + + it('keeps the 24-point gutter at zero portrait insets', () => { + const renderer = mountFooter(); + + expect(footerStyleProp(renderer, 'paddingLeft')).toBe(24); + expect(footerStyleProp(renderer, 'paddingRight')).toBe(24); + expect(footerStyleProp(renderer, 'paddingBottom')).toBe(16); + }); + + it('widens the gutter by the landscape side insets on iOS', () => { + insetsState.left = 59; + insetsState.right = 59; + const renderer = mountFooter(); + + expect(footerStyleProp(renderer, 'paddingLeft')).toBe(83); + expect(footerStyleProp(renderer, 'paddingRight')).toBe(83); + expect(footerStyleProp(renderer, 'paddingBottom')).toBe(16); + }); + + it('widens the gutter by the landscape side insets on Android too', () => { + platformState.OS = 'android'; + insetsState.left = 24; + insetsState.right = 24; + const renderer = mountFooter(); + + expect(footerStyleProp(renderer, 'paddingLeft')).toBe(48); + expect(footerStyleProp(renderer, 'paddingRight')).toBe(48); + // Side insets are independent of the Android bottom-inset rule. + expect(footerStyleProp(renderer, 'paddingBottom')).toBe(16); }); }); diff --git a/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.tsx b/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.tsx index 10138e03fa..15e35b7283 100644 --- a/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.tsx +++ b/apps/mobile/src/components/pr-review/pr-form-sheet-chrome.tsx @@ -66,14 +66,19 @@ export function PrFormSheetHeader(props: { title: string; eyebrow: string; onBac * Trailing ScrollView footer for formSheets. No keyboard-height padding — * parent ScrollView automaticallyAdjustKeyboardInsets owns that. On Android * the footer sits on the bottom edge, so it adds the system bottom inset. + * The px-6 gutter is widened by the side insets so the footer CTAs clear the + * landscape safe area when the sheet runs edge-to-edge (portrait insets are + * zero, so the gutter stays 24). */ export function PrFormSheetFooter(props: { children: ReactNode }) { - const { bottom } = useSafeAreaInsets(); + const { bottom, left, right } = useSafeAreaInsets(); const paddingBottom = Platform.OS === 'android' ? 16 + bottom : 16; + const paddingLeft = 24 + left; + const paddingRight = 24 + right; return ( {props.children} diff --git a/apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx b/apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx index a3bb280158..f5e6663d17 100644 --- a/apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx @@ -132,6 +132,49 @@ function expectCtaPresence(renderer: TestRenderer.ReactTestRenderer, present: bo expect(ctas.length > 0).toBe(present); } +describe('PrReviewDiscussionTab loading skeleton side insets (landscape)', () => { + beforeEach(() => { + insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; + resetState(); + }); + + function loadingWrapperStyle(): Record { + discussionState.query.isPending = true; + const views = bottomPaddedViews(mountTab()); + expect(views).toHaveLength(1); + const view = views[0]; + if (!view) { + throw new Error('expected a padded View'); + } + return view.props.style as Record; + } + + it('keeps exactly the current style keys at zero portrait insets', () => { + const style = loadingWrapperStyle(); + + // Spread only when nonzero: the `px-4` className gutter must survive + // portrait untouched (inline style wins over className). + expect(style.paddingLeft).toBeUndefined(); + expect(style.paddingRight).toBeUndefined(); + expect(style.paddingBottom).toBe(32); + }); + + it('clears the sensor housing with the landscape side insets', () => { + insetsState.left = 47; + insetsState.right = 59; + + const style = loadingWrapperStyle(); + + expect(style.paddingLeft).toBe(47); + expect(style.paddingRight).toBe(59); + // The skeleton gutter swap is horizontal-only: the paddingBottom that + // clears the system bar is unchanged. + expect(style.paddingBottom).toBe(32); + }); +}); + describe('PrReviewDiscussionTab full-body states', () => { beforeEach(() => { insetsState.bottom = 0; diff --git a/apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx b/apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx index 23940fdd93..272858eef0 100644 --- a/apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx @@ -54,6 +54,7 @@ import { type Href, useIsFocused, useRouter } from 'expo-router'; import { type ReactNode, useEffect, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Platform, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { PrReviewDiscussionList } from '@/components/pr-review/discussion/pr-review-discussion-list'; import { PrCommentCta } from '@/components/pr-review/discussion/pr-comment-cta'; @@ -141,6 +142,11 @@ export function PrReviewDiscussionTab({ // Bottom clearance for the non-list chrome (loading, empty, and every // first-page error state) so the last control clears the system bar. const bottomPadding = useDetailScreenBottomPadding(); + // Landscape: side insets keep the loading skeleton cards clear of the + // sensor housing. Spread only when nonzero: the skeleton wrapper's `px-4` + // className gutter must survive portrait untouched (inline style wins over + // className). Same treatment as the diff floating-actions bar. + const insets = useSafeAreaInsets(); // Single write path: the ref is the tap-time source of truth (render-closure // state can lag a queued update on rapid taps). @@ -305,7 +311,11 @@ export function PrReviewDiscussionTab({ 0 ? { paddingLeft: insets.left } : undefined), + ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), + }} > {Array.from({ length: SKELETON_ROW_COUNT }).map((_, index) => ( // eslint-disable-next-line react/no-array-index-key -- skeleton placeholders have no stable id diff --git a/apps/mobile/src/components/pr-review/pr-review-inbox-list.mounted.test.tsx b/apps/mobile/src/components/pr-review/pr-review-inbox-list.mounted.test.tsx new file mode 100644 index 0000000000..fb6afe03f2 --- /dev/null +++ b/apps/mobile/src/components/pr-review/pr-review-inbox-list.mounted.test.tsx @@ -0,0 +1,200 @@ +/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (same pattern as screen-header.mounted.test.tsx) */ +import { createElement } from 'react'; +import TestRenderer, { act } from 'react-test-renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { PrReviewInboxList } from './pr-review-inbox-list'; + +// Landscape side-inset coverage for the inbox list (the coverage audit found +// the FlashList without a contentContainerStyle, so inbox rows and the px-6 +// header/footer content sat under the landscape sensor housing). + +const insetsState = vi.hoisted(() => ({ top: 0, bottom: 0, left: 0, right: 0 })); +const inboxState = vi.hoisted(() => ({ + query: { + isPending: false, + isFetching: false, + hasNextPage: false, + isFetchingNextPage: false, + refetch: vi.fn(), + fetchNextPage: vi.fn(), + }, + items: [] as { + owner: string; + repo: string; + number: number; + title: string; + updatedAt: string; + isDraft: boolean; + }[], + firstPageErrorState: null as { kind: string } | null, + laterPageError: false, +})); + +vi.mock('react-native', () => ({ Pressable: 'Pressable', View: 'View' })); +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => insetsState, +})); +vi.mock('expo-router', () => ({ + useRouter: () => ({ push: vi.fn() }), +})); +vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })); +vi.mock('@shopify/flash-list', async () => { + const react = await import('react'); + // Render the list sections so rows and the empty view mount inside the + // content container, like the real FlashList would. + return { + FlashList: (props: { + data?: unknown[]; + renderItem: (args: { item: unknown }) => React.ReactElement | null; + ListHeaderComponent?: React.ReactElement; + ListEmptyComponent?: React.ReactElement; + ListFooterComponent?: React.ReactElement; + }) => { + const { + data, + renderItem, + ListHeaderComponent, + ListEmptyComponent, + ListFooterComponent, + ...rest + } = props; + return react.createElement( + 'FlashList', + rest, + ListHeaderComponent ?? null, + ...(data ?? []).map(item => renderItem({ item })), + (data ?? []).length === 0 ? (ListEmptyComponent ?? null) : null, + ListFooterComponent ?? null + ); + }, + }; +}); +vi.mock('@/components/empty-state', () => ({ EmptyState: 'EmptyState' })); +vi.mock('@/components/query-error', () => ({ QueryError: 'QueryError' })); +vi.mock('@/components/pr-review/pr-review-reconnect-notice', () => ({ + PrReviewReconnectNotice: 'PrReviewReconnectNotice', +})); +vi.mock('@/components/ui/button', () => ({ Button: 'Button' })); +vi.mock('@/components/ui/icons', () => ({ + Clock: 'Clock', + GitPullRequest: 'GitPullRequest', + Inbox: 'Inbox', +})); +vi.mock('@/components/ui/directional-icons', () => ({ + DirectionalChevronRight: 'DirectionalChevronRight', +})); +vi.mock('@/components/ui/skeleton', () => ({ Skeleton: 'Skeleton' })); +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ mutedForeground: '#6F6A61' }), +})); +vi.mock('@/lib/profile-agent-navigation', () => ({ + getPrReviewPath: (owner: string, repo: string, number: number) => `/${owner}/${repo}/${number}`, +})); +vi.mock('@/lib/pr-review/use-pr-inbox', () => ({ + usePrInbox: () => inboxState, +})); +// `@/lib/utils` initializes real i18n; the row only needs timestamp shaping. +vi.mock('@/lib/utils', () => ({ + parseTimestamp: (value: string) => new Date(value), + timeAgo: () => 'just now', +})); + +function makeItem( + overrides: Partial<(typeof inboxState.items)[number]> = {} +): (typeof inboxState.items)[number] { + return { + owner: 'octocat', + repo: 'hello-world', + number: 7, + title: 'Fix the thing', + updatedAt: '2026-09-01T12:00:00Z', + isDraft: false, + ...overrides, + }; +} + +function mountInboxList(): TestRenderer.ReactTestRenderer { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create( + createElement(PrReviewInboxList, { header: null, recents: null }) + ); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; +} + +function flashListProps(renderer: TestRenderer.ReactTestRenderer): { + contentContainerStyle?: Record; +} { + return renderer.root.find(node => String(node.type) === 'FlashList').props as { + contentContainerStyle?: Record; + }; +} + +describe('PrReviewInboxList side insets (landscape)', () => { + beforeEach(() => { + insetsState.top = 0; + insetsState.bottom = 0; + insetsState.left = 0; + insetsState.right = 0; + inboxState.query.isPending = false; + inboxState.query.isFetching = false; + inboxState.query.hasNextPage = false; + inboxState.query.isFetchingNextPage = false; + inboxState.items = [makeItem()]; + inboxState.firstPageErrorState = null; + inboxState.laterPageError = false; + }); + + it('carries zero side paddings at zero portrait insets and renders rows', () => { + const renderer = mountInboxList(); + + expect(flashListProps(renderer).contentContainerStyle).toEqual({ + paddingLeft: 0, + paddingRight: 0, + }); + // Populated view renders inside the padded container. + expect( + renderer.root.findAll( + node => + String(node.type) === 'Pressable' && + node.props.accessibilityLabel === 'octocat/hello-world#7' + ) + ).toHaveLength(1); + }); + + it('clears the sensor housing with the landscape side insets', () => { + insetsState.left = 47; + insetsState.right = 59; + const renderer = mountInboxList(); + + expect(flashListProps(renderer).contentContainerStyle).toEqual({ + paddingLeft: 47, + paddingRight: 59, + }); + expect( + renderer.root.findAll( + node => + String(node.type) === 'Pressable' && + node.props.accessibilityLabel === 'octocat/hello-world#7' + ) + ).toHaveLength(1); + }); + + it('keeps the empty-inbox view reachable inside the padded container', () => { + inboxState.items = []; + const renderer = mountInboxList(); + + expect(flashListProps(renderer).contentContainerStyle).toEqual({ + paddingLeft: 0, + paddingRight: 0, + }); + expect(renderer.root.findAll(node => String(node.type) === 'EmptyState')).toHaveLength(1); + }); +}); diff --git a/apps/mobile/src/components/pr-review/pr-review-inbox-list.tsx b/apps/mobile/src/components/pr-review/pr-review-inbox-list.tsx index 8d01d02197..c867c6eb75 100644 --- a/apps/mobile/src/components/pr-review/pr-review-inbox-list.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-inbox-list.tsx @@ -10,9 +10,10 @@ import { FlashList } from '@shopify/flash-list'; import { useRouter } from 'expo-router'; -import { type ReactNode } from 'react'; +import { type ReactNode, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; -import { Pressable, View } from 'react-native'; +import { Pressable, View, type ViewStyle } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { EmptyState } from '@/components/empty-state'; import { QueryError } from '@/components/query-error'; @@ -48,6 +49,15 @@ export function PrReviewInboxList({ header, recents }: Readonly( + () => ({ paddingLeft: insets.left, paddingRight: insets.right }), + [insets.left, insets.right] + ); + return ( diff --git a/apps/mobile/src/components/screen-header.mounted.test.tsx b/apps/mobile/src/components/screen-header.mounted.test.tsx index 4ffa24fbd5..9c9db3802e 100644 --- a/apps/mobile/src/components/screen-header.mounted.test.tsx +++ b/apps/mobile/src/components/screen-header.mounted.test.tsx @@ -12,6 +12,7 @@ const routerState = vi.hoisted(() => ({ canGoBack: vi.fn(() => true), })); const i18nManager = vi.hoisted(() => ({ isRTL: false })); +const safeArea = vi.hoisted(() => ({ top: 0, bottom: 0, left: 0, right: 0 })); vi.mock('expo-router', () => ({ useRouter: () => routerState, @@ -23,7 +24,7 @@ vi.mock('react-native', () => ({ View: 'View', })); vi.mock('react-native-safe-area-context', () => ({ - useSafeAreaInsets: () => ({ top: 0, bottom: 0 }), + useSafeAreaInsets: () => safeArea, })); vi.mock('@/components/ui/icons', () => ({ ChevronDown: 'ChevronDown', @@ -74,6 +75,29 @@ function findIcon(back: TestInstance, type: string): TestInstance { return icon; } +function findOuterContainer(root: TestInstance): TestInstance { + return root.find(node => typeof node.type === 'string' && (node.type as string) === 'View'); +} + +/** + * The header body sits in an inner wrapper that carries only the landscape side + * insets, so they add to the outer container's `px-4` gutter instead of + * overriding it. It is the only View in the tree without a className. + */ +function findSideInsetWrapper(root: TestInstance): TestInstance { + const wrappers = root.findAll( + node => + typeof node.type === 'string' && + (node.type as string) === 'View' && + node.props.className === undefined + ); + const wrapper = wrappers[0]; + if (!wrapper) { + throw new Error('side-inset wrapper not found'); + } + return wrapper; +} + function deriveTitleFontSize(className: string): number { const arbitrary = /text-\[(\d+)px\]/.exec(className); if (arbitrary) { @@ -108,6 +132,7 @@ describe('ScreenHeader mounted', () => { }); routerState.canGoBack.mockReset().mockImplementation(() => routerState.routes.length > 1); i18nManager.isRTL = false; + Object.assign(safeArea, { top: 0, bottom: 0, left: 0, right: 0 }); }); it('gives the back control a 44-point target and no hit slop', () => { @@ -355,4 +380,58 @@ describe('ScreenHeader mounted', () => { expect(renderer.root.props.style).toBeUndefined(); expect(renderer.root.props.className).toContain('pt-3'); }); + + it('pads the sheet header by the landscape side insets even when it skips the safe-area top', () => { + safeArea.left = 47; + safeArea.right = 59; + const renderer = renderHeader({ + title: 'Submit review', + onBack: () => undefined, + backIcon: 'close', + showBackButton: true, + safeAreaTop: false, + className: 'pt-3', + }); + + // No paddingTop key on the outer container (the sheet owns vertical padding + // via its className), but the side insets still apply on the inner wrapper + // so the close control and title clear the sensor area; with zero insets the + // wrapper style collapses to undefined. + expect(findOuterContainer(renderer.root).props.style).toBeUndefined(); + expect(findSideInsetWrapper(renderer.root).props.style).toEqual({ + paddingLeft: 47, + paddingRight: 59, + }); + expect(renderer.root.props.className).toContain('pt-3'); + }); + + it('pads the container by the landscape side insets while keeping the gutter and back pull', () => { + safeArea.left = 47; + safeArea.right = 59; + const renderer = renderHeader({ title: 'Sessions', headerRight: 'RIGHT' }); + + // The outer container keeps only its top inset and the `px-4` gutter class; + // the side insets land on the inner wrapper so they ADD to the gutter (an + // inline padding on the container would override the class and pull the + // back control's `-ml-4` chevron back into the sensor area). + const container = findOuterContainer(renderer.root); + expect(container.props.style).toEqual({ paddingTop: 8 }); + expect(container.props.className).toContain('px-4'); + expect(findSideInsetWrapper(renderer.root).props.style).toEqual({ + paddingLeft: 47, + paddingRight: 59, + }); + expect(findBackPressable(renderer.root).props.className).toContain('-ml-4'); + }); + + it('keeps the container style a portrait no-op with zero side insets', () => { + const renderer = renderHeader({ title: 'Sessions' }); + + // No paddingLeft/paddingRight keys at zero: an inline 0 would override the + // `px-4` className gutter and change the portrait geometry. The wrapper + // stays styleless so portrait pixels are byte-identical to an inset-free + // header. + expect(findOuterContainer(renderer.root).props.style).toEqual({ paddingTop: 8 }); + expect(findSideInsetWrapper(renderer.root).props.style).toBeUndefined(); + }); }); diff --git a/apps/mobile/src/components/screen-header.tsx b/apps/mobile/src/components/screen-header.tsx index 1099b18ae6..b97b5b6f3a 100644 --- a/apps/mobile/src/components/screen-header.tsx +++ b/apps/mobile/src/components/screen-header.tsx @@ -43,7 +43,9 @@ type ScreenHeaderProps = { backIcon?: 'back' | 'close'; /** * Apply the status-bar safe-area inset. Form sheets already clear the - * grabber; passing false leaves vertical padding to `className`. + * grabber; passing false leaves vertical padding to `className`. The + * landscape side insets apply regardless, so every caller clears the + * sensor/cutout horizontally. */ safeAreaTop?: boolean; /** Extra classes on the outer header container. Overrides the default `px-4` for screens that need a different horizontal inset. */ @@ -80,6 +82,27 @@ export function ScreenHeader({ // iOS modals are presented as cards already inset from the status bar const paddingTop = modal && Platform.OS === 'ios' ? 32 : insets.top + 8; + // `paddingTop` stays conditional on `safeAreaTop`: a form sheet owns its + // vertical padding through `className`. + const safeAreaStyle = safeAreaTop ? { paddingTop } : undefined; + + // Landscape side safe areas (notch/Dynamic Island, Android cutouts) shift the + // whole chrome off the sensor. They go on an inner wrapper so they ADD to the + // `px-4` gutter: an inline padding on the container would beat the className + // (inline style wins in React Native) and swallow the gutter, pulling the + // back control's `-ml-4` chevron back inside the sensor area. Zero insets + // collapse the wrapper style to `undefined`, so portrait geometry is + // byte-identical and a rotation never moves anything vertically. Side padding + // applies to every caller — a sheet with `safeAreaTop={false}` still runs + // edge-to-edge horizontally and must clear the cutout too. + const sideInsetStyle = + insets.left > 0 || insets.right > 0 + ? { + ...(insets.left > 0 ? { paddingLeft: insets.left } : undefined), + ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), + } + : undefined; + // When `backIcon` isn't specified, fall back to the historical behaviour // where iOS modals get a ChevronDown and everything else gets a ChevronLeft. const resolvedBackIcon = backIcon ?? (modal && Platform.OS === 'ios' ? 'close' : 'back'); @@ -157,44 +180,43 @@ export function ScreenHeader({ const separateHeading = centerTitle && (Boolean(title) || Boolean(eyebrow)); return ( - - {separateHeading && {heading}} - - - {canGoBack && ( - { - if (onBack) { - onBack(); - } else if (backFallback !== undefined && !router.canGoBack()) { - router.replace(backFallback); - } else { - router.back(); + + + {separateHeading && {heading}} + + + {canGoBack && ( + { + if (onBack) { + onBack(); + } else if (backFallback !== undefined && !router.canGoBack()) { + router.replace(backFallback); + } else { + router.back(); + } + }} + accessibilityRole="button" + accessibilityLabel={ + resolvedBackIcon === 'close' ? t('common.close') : t('common.goBack') } - }} - accessibilityRole="button" - accessibilityLabel={ - resolvedBackIcon === 'close' ? t('common.close') : t('common.goBack') - } - className={`${I18nManager.isRTL ? '-mr-4' : '-ml-4'} h-11 w-11 shrink-0 items-center justify-center active:opacity-70`} - > - {resolvedBackIcon === 'close' ? ( - - ) : ( - - )} - - )} - {!separateHeading && heading} - - {headerRight ? ( - - {headerRight} + className={`${I18nManager.isRTL ? '-mr-4' : '-ml-4'} h-11 w-11 shrink-0 items-center justify-center active:opacity-70`} + > + {resolvedBackIcon === 'close' ? ( + + ) : ( + + )} + + )} + {!separateHeading && heading} - ) : null} + {headerRight ? ( + + {headerRight} + + ) : null} + ); diff --git a/apps/mobile/src/components/sheet-header-layout.mounted.test.tsx b/apps/mobile/src/components/sheet-header-layout.mounted.test.tsx index 0aaabc8bc4..4cbb2475d5 100644 --- a/apps/mobile/src/components/sheet-header-layout.mounted.test.tsx +++ b/apps/mobile/src/components/sheet-header-layout.mounted.test.tsx @@ -7,6 +7,12 @@ import { SheetHeader } from './sheet-header'; import '@/i18n'; vi.mock('react-native', () => ({ Pressable: 'Pressable', View: 'View' })); +// SheetHeader reads the landscape side insets; this suite mounts without a +// device, so the hook gets portrait-zero insets (same pattern as +// sheet-header.mounted.test.tsx). +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 0, bottom: 0, left: 0, right: 0 }), +})); vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); vi.mock('@/components/ui/icons', () => ({ Share: 'Share' })); vi.mock('@/lib/hooks/use-theme-colors', () => ({ diff --git a/apps/mobile/src/components/sheet-header.mounted.test.tsx b/apps/mobile/src/components/sheet-header.mounted.test.tsx index 86f2ff5a9c..43b6602a90 100644 --- a/apps/mobile/src/components/sheet-header.mounted.test.tsx +++ b/apps/mobile/src/components/sheet-header.mounted.test.tsx @@ -1,12 +1,14 @@ -/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (same pattern as src/components/ui/accessible-status.mounted.test.tsx) */ +/* eslint-disable typescript-eslint/no-deprecated, max-lines -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (same pattern as src/components/ui/accessible-status.mounted.test.tsx) */ import { type ComponentProps, createElement, type ReactElement, useState } from 'react'; import TestRenderer, { act } from 'react-test-renderer'; -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { PickerSheet } from './picker-sheet'; import { SheetHeader } from './sheet-header'; import '@/i18n'; +const safeArea = vi.hoisted(() => ({ top: 0, bottom: 0, left: 0, right: 0 })); + vi.mock('react-native', () => ({ Pressable: 'Pressable', ScrollView: 'ScrollView', @@ -14,7 +16,7 @@ vi.mock('react-native', () => ({ I18nManager: { allowRTL: vi.fn(), isRTL: false, forceRTL: vi.fn() }, })); vi.mock('react-native-safe-area-context', () => ({ - useSafeAreaInsets: () => ({ bottom: 0 }), + useSafeAreaInsets: () => safeArea, })); vi.mock('@/components/empty-state', () => ({ EmptyState: 'EmptyState' })); vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); @@ -57,6 +59,36 @@ function pressablesByLabel( ); } +function findHeaderContainer(root: TestRenderer.ReactTestInstance): TestRenderer.ReactTestInstance { + return root.find( + node => + typeof node.type === 'string' && + (node.type as string) === 'View' && + node.props.collapsable === false + ); +} + +/** + * The header row sits in an inner wrapper that carries only the landscape side + * insets, so they add to the outer container's `px-4` gutter instead of + * overriding it. It is the only View in the header without a className. + */ +function findSideInsetWrapper( + root: TestRenderer.ReactTestInstance +): TestRenderer.ReactTestInstance { + const wrappers = root.findAll( + node => + typeof node.type === 'string' && + (node.type as string) === 'View' && + node.props.className === undefined + ); + const wrapper = wrappers[0]; + if (!wrapper) { + throw new Error('side-inset wrapper not found'); + } + return wrapper; +} + function HeaderWithActionFeedback({ initialTitle = 'report.pdf', doneLabel = 'Finish', @@ -82,6 +114,10 @@ function HeaderWithActionFeedback({ } describe('SheetHeader', () => { + beforeEach(() => { + Object.assign(safeArea, { top: 0, bottom: 0, left: 0, right: 0 }); + }); + it('renders a Share pressable in the leading slot when onShare is provided', async () => { const renderer = await mount({ title: 'report.pdf', @@ -319,4 +355,49 @@ describe('SheetHeader', () => { renderer.unmount(); } ); + + it('keeps the outer container classes and a styleless wrapper at zero side insets', async () => { + const renderer = await mount({ + title: 'report.pdf', + onDone: () => undefined, + onCancel: () => undefined, + }); + + // Portrait no-op: the wrapper style collapses to undefined (an inline 0 + // would override the `px-4` className gutter) and the outer container keeps + // the non-collapsible header contract and its gutter class untouched. + const container = findHeaderContainer(renderer.root); + expect(container.props.collapsable).toBe(false); + expect(container.props.className).toContain('px-4'); + expect(container.props.style).toBeUndefined(); + expect(findSideInsetWrapper(renderer.root).props.style).toBeUndefined(); + + renderer.unmount(); + }); + + it('pads the row by the landscape side insets inside the outer gutter', async () => { + safeArea.left = 47; + safeArea.right = 59; + const renderer = await mount({ + title: 'report.pdf', + onDone: () => undefined, + onCancel: () => undefined, + }); + + // The side insets land on the inner wrapper so they ADD to the `px-4` + // gutter (an inline padding on the container would override the class), + // and the non-collapsible header contract stays on the outer View. + const container = findHeaderContainer(renderer.root); + expect(container.props.collapsable).toBe(false); + expect(container.props.className).toContain('px-4'); + expect(container.props.style).toBeUndefined(); + const wrapper = findSideInsetWrapper(renderer.root); + expect(wrapper.props.style).toEqual({ paddingLeft: 47, paddingRight: 59 }); + const cancel = pressablesByLabel(renderer.root, 'Cancel')[0]; + const done = pressablesByLabel(renderer.root, 'Done')[0]; + expect(cancel?.parent?.parent).toBe(wrapper); + expect(done?.parent?.parent).toBe(wrapper); + + renderer.unmount(); + }); }); diff --git a/apps/mobile/src/components/sheet-header.tsx b/apps/mobile/src/components/sheet-header.tsx index f185812420..4594cae1a4 100644 --- a/apps/mobile/src/components/sheet-header.tsx +++ b/apps/mobile/src/components/sheet-header.tsx @@ -1,5 +1,6 @@ import { useTranslation } from 'react-i18next'; import { Pressable, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { Share } from '@/components/ui/icons'; import { Text } from '@/components/ui/text'; @@ -37,8 +38,22 @@ export function SheetHeader({ }) { const { t } = useTranslation(); const colors = useThemeColors(); + const insets = useSafeAreaInsets(); const resolvedDoneLabel = doneLabel ?? t('common.done'); const resolvedCancelLabel = cancelLabel ?? t('common.cancel'); + // Landscape side safe areas (notch/Dynamic Island, Android cutouts) shift the + // header row off the sensor on full-width sheets. They go on an inner wrapper + // so they ADD to the `px-4` gutter: an inline padding on the container would + // beat the className (inline style wins in React Native) and swallow the + // gutter. Zero insets collapse the wrapper style to `undefined`, so portrait + // pixels are byte-identical and a rotation never moves anything vertically. + const sideInsetStyle = + insets.left > 0 || insets.right > 0 + ? { + ...(insets.left > 0 ? { paddingLeft: insets.left } : undefined), + ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), + } + : undefined; // Native row direction and logical margin keep Cancel/Share leading and Done // trailing. Do not derive sides from i18n.dir() or the stale I18nManager.isRTL. return ( @@ -46,61 +61,63 @@ export function SheetHeader({ // view by finding the header at the screen content's subview index 0 — a // flattened header breaks that native pass and the list paints over it. - - {onShare !== undefined ? ( - - - - ) : null} - {onCancel ? ( + + + {onShare !== undefined ? ( + + + + ) : null} + {onCancel ? ( + + + {resolvedCancelLabel} + + + ) : null} + {/* min-w-0 lets the title shrink below its content width so it truncates + instead of pushing the trailing action out of the row. Cancel and + Done bracket the title, so center it between them; without Cancel the + title stays leading against the sheet edge. */} + + + {title} + + - {resolvedCancelLabel} + {resolvedDoneLabel} - ) : null} - {/* min-w-0 lets the title shrink below its content width so it truncates - instead of pushing the trailing action out of the row. Cancel and - Done bracket the title, so center it between them; without Cancel the - title stays leading against the sheet edge. */} - - - {title} - - - - {resolvedDoneLabel} - - ); diff --git a/apps/mobile/src/lib/pr-review/diff/pr-diff-list-bottom-padding.ts b/apps/mobile/src/lib/pr-review/diff/pr-diff-list-bottom-padding.ts index b8a91a6c45..6a98db7e95 100644 --- a/apps/mobile/src/lib/pr-review/diff/pr-diff-list-bottom-padding.ts +++ b/apps/mobile/src/lib/pr-review/diff/pr-diff-list-bottom-padding.ts @@ -15,3 +15,19 @@ export function prDiffListBottomPadding(floatingActionsHeight: number | null): n } return Math.round(floatingActionsHeight) + PR_DIFF_LIST_FOOTER_GAP; } + +/** + * Returns the content-container padding for the diff FlashList: the bottom + * padding that clears the floating action bar, plus the landscape side + * insets that keep rows clear of the sensor housing. + */ +export function prDiffListContentPadding( + floatingActionsHeight: number | null, + insets: { left: number; right: number } +) { + return { + paddingBottom: prDiffListBottomPadding(floatingActionsHeight), + paddingLeft: insets.left, + paddingRight: insets.right, + }; +} diff --git a/apps/mobile/src/lib/pr-review/diff/use-pr-diff-list-content-padding.ts b/apps/mobile/src/lib/pr-review/diff/use-pr-diff-list-content-padding.ts new file mode 100644 index 0000000000..b6a95be047 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/diff/use-pr-diff-list-content-padding.ts @@ -0,0 +1,16 @@ +import { useMemo } from 'react'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; + +import { prDiffListContentPadding } from './pr-diff-list-bottom-padding'; + +/** + * Content-container style for the diff FlashList: the bottom padding that + * clears the floating action bar plus the landscape side insets that keep + * rows clear of the sensor housing. Side insets are horizontal-only, so the + * height-feeding bottom padding is untouched; at zero portrait insets the + * style carries explicit zeros. + */ +export function usePrDiffListContentPadding(barHeight: number | null) { + const insets = useSafeAreaInsets(); + return useMemo(() => prDiffListContentPadding(barHeight, insets), [barHeight, insets]); +} diff --git a/apps/mobile/src/lib/tab-bar-layout.test.ts b/apps/mobile/src/lib/tab-bar-layout.test.ts index 590f4bf107..2b23681682 100644 --- a/apps/mobile/src/lib/tab-bar-layout.test.ts +++ b/apps/mobile/src/lib/tab-bar-layout.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { getEffectiveTabBarHeight, + getTabBarHorizontalInset, getTabBarIconForwardHeight, getTabBarIconSize, getTabBarOverlayHeight, @@ -108,6 +109,33 @@ describe('getTabBarIconSize', () => { }); }); +describe('getTabBarHorizontalInset', () => { + it('pads each side by its landscape safe-area inset', () => { + expect(getTabBarHorizontalInset({ left: 47, right: 59 })).toEqual({ + paddingLeft: 47, + paddingRight: 59, + }); + }); + + it('collapses to a no-op in portrait with zero insets', () => { + expect(getTabBarHorizontalInset({ left: 0, right: 0 })).toEqual({ + paddingLeft: 0, + paddingRight: 0, + }); + }); + + it('ignores negative insets', () => { + expect(getTabBarHorizontalInset({ left: -1, right: -1 })).toEqual({ + paddingLeft: 0, + paddingRight: 0, + }); + }); + + it('treats missing insets as zero', () => { + expect(getTabBarHorizontalInset({})).toEqual({ paddingLeft: 0, paddingRight: 0 }); + }); +}); + describe('shouldShowTabLabel', () => { it('keeps the label below the icon-forward threshold', () => { expect(shouldShowTabLabel(1)).toBe(true); diff --git a/apps/mobile/src/lib/tab-bar-layout.ts b/apps/mobile/src/lib/tab-bar-layout.ts index d2af69c130..94c445dc49 100644 --- a/apps/mobile/src/lib/tab-bar-layout.ts +++ b/apps/mobile/src/lib/tab-bar-layout.ts @@ -75,6 +75,25 @@ export function getEffectiveTabBarHeight({ : getTabBarIconForwardHeight(bottomInset, platform); } +/** + * Horizontal padding that keeps the tab bar icon row clear of the landscape + * side safe areas (notch/Dynamic Island, Android cutouts). The bar's BlurBar + * background stays full-bleed because it is absolutely positioned, so only the + * icon row is inset. Zero insets (portrait) collapse to a no-op. + */ +export function getTabBarHorizontalInset({ + left = 0, + right = 0, +}: { + left?: number; + right?: number; +}) { + return { + paddingLeft: Math.max(left, 0), + paddingRight: Math.max(right, 0), + }; +} + export function shouldShowTabLabel(fontScale = 1): boolean { return fontScale < TAB_ICON_FORWARD_FONT_SCALE; }