mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(mobile): defer tasks until host stack commits
This commit is contained in:
@@ -65,6 +65,7 @@ import { BottomDrawer } from '../../../src/components/BottomDrawer'
|
||||
import { useHostProtocolGates } from '../../../src/components/HostProtocolGate'
|
||||
import { AuthFailedBanner } from '../../../src/components/AuthFailedBanner'
|
||||
import { MobileSearchField } from '../../../src/components/MobileSearchField'
|
||||
import { WorkspaceDetailPlaceholder } from '../../../src/components/WorkspaceDetailPlaceholder'
|
||||
import { getCachedWorktrees, setCachedWorktrees } from '../../../src/cache/worktree-cache'
|
||||
import { setCachedRepos } from '../../../src/cache/repo-cache'
|
||||
import { colors, radii, spacing, typography } from '../../../src/theme/mobile-theme'
|
||||
@@ -101,7 +102,6 @@ import {
|
||||
import type { RepoSummary } from '../../../src/worktree/host-worktree-rpc-types'
|
||||
import type { WorkspaceStatusDefinition } from '../../../../src/shared/types'
|
||||
import { DEFAULT_MOBILE_WORKSPACE_STATUSES } from '../../../src/worktree/mobile-workspace-statuses'
|
||||
import { MobileTaskHostRoute } from '../../../src/tasks/MobileTaskHostRoute'
|
||||
|
||||
function isErrorVerdict(v: ConnectionVerdict): boolean {
|
||||
return v.kind === 'warning' || v.kind === 'unreachable' || v.kind === 'auth-failed'
|
||||
@@ -1396,7 +1396,11 @@ export function HostScreen({
|
||||
|
||||
// On wide layouts the sidebar hosts the list, so this route is just the empty detail pane.
|
||||
export default function HostWorktreeRoute() {
|
||||
return <MobileTaskHostRoute hostScreen={HostScreen} />
|
||||
const { isWideLayout } = useResponsiveLayout()
|
||||
if (isWideLayout) {
|
||||
return <WorkspaceDetailPlaceholder />
|
||||
}
|
||||
return <HostScreen />
|
||||
}
|
||||
|
||||
function ListSeparator() {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { View, StyleSheet, PanResponder } from 'react-native'
|
||||
import { Stack, useGlobalSearchParams, usePathname } from 'expo-router'
|
||||
import { Stack, useGlobalSearchParams, usePathname, useRouter } from 'expo-router'
|
||||
import { colors } from '../../src/theme/mobile-theme'
|
||||
import { useResponsiveLayout } from '../../src/layout/responsive-layout'
|
||||
import {
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
} from '../../src/storage/preferences'
|
||||
import { HostProtocolGate } from '../../src/components/HostProtocolGate'
|
||||
import { HostScreen } from './[hostId]/index'
|
||||
import { useMobileTaskHostNavigation } from '../../src/tasks/use-mobile-task-host-navigation'
|
||||
|
||||
// Keep at least this much room for the detail pane when resizing the sidebar.
|
||||
const MIN_DETAIL_WIDTH = 320
|
||||
@@ -63,6 +64,8 @@ export default function HostGroupLayout() {
|
||||
const { isWideLayout, width: windowWidth } = useResponsiveLayout()
|
||||
const { hostId, action } = useGlobalSearchParams<{ hostId?: string; action?: string }>()
|
||||
const pathname = usePathname()
|
||||
const router = useRouter()
|
||||
useMobileTaskHostNavigation(router, hostId)
|
||||
const [sidebarOpen, setSidebarOpen] = useState(true)
|
||||
const [sidebarWidth, setSidebarWidth] = useState(HOST_SIDEBAR_DEFAULT_WIDTH)
|
||||
|
||||
|
||||
@@ -1,99 +0,0 @@
|
||||
import { createElement } from 'react'
|
||||
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { MobileTaskHostRoute } from './MobileTaskHostRoute'
|
||||
import {
|
||||
clearPendingMobileTasksNavigation,
|
||||
getPendingMobileTasksNavigation,
|
||||
navigateToMobileTasks
|
||||
} from './mobile-task-navigation'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
params: {} as { hostId?: string },
|
||||
isWideLayout: false
|
||||
}))
|
||||
|
||||
vi.mock('expo-router', () => ({
|
||||
Redirect: 'Redirect',
|
||||
useLocalSearchParams: () => mocks.params
|
||||
}))
|
||||
|
||||
vi.mock('../layout/responsive-layout', () => ({
|
||||
useResponsiveLayout: () => ({ isWideLayout: mocks.isWideLayout })
|
||||
}))
|
||||
|
||||
vi.mock('../components/WorkspaceDetailPlaceholder', () => ({
|
||||
WorkspaceDetailPlaceholder: 'WorkspaceDetailPlaceholder'
|
||||
}))
|
||||
|
||||
function HostScreen() {
|
||||
return createElement('HostScreen')
|
||||
}
|
||||
|
||||
describe('MobileTaskHostRoute', () => {
|
||||
let renderer: ReactTestRenderer | null = null
|
||||
|
||||
beforeEach(() => {
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
act(() => renderer?.unmount())
|
||||
renderer = null
|
||||
mocks.params = {}
|
||||
mocks.isWideLayout = false
|
||||
const pending = getPendingMobileTasksNavigation()
|
||||
if (pending) {
|
||||
clearPendingMobileTasksNavigation(pending)
|
||||
}
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
async function renderRoute(): Promise<ReactTestRenderer> {
|
||||
const consoleError = vi.spyOn(console, 'error').mockImplementation((...args) => {
|
||||
if (typeof args[0] !== 'string' || !args[0].includes('react-test-renderer is deprecated')) {
|
||||
throw new Error(String(args[0]))
|
||||
}
|
||||
})
|
||||
try {
|
||||
await act(async () => {
|
||||
renderer = create(createElement(MobileTaskHostRoute, { hostScreen: HostScreen }))
|
||||
})
|
||||
} finally {
|
||||
consoleError.mockRestore()
|
||||
}
|
||||
return renderer!
|
||||
}
|
||||
|
||||
it('redirects from the cold host shell even when Expo drops its route params', async () => {
|
||||
navigateToMobileTasks({ push: vi.fn() }, 'host/1', 'github')
|
||||
const pending = getPendingMobileTasksNavigation()
|
||||
|
||||
const route = await renderRoute()
|
||||
|
||||
expect(route.root.findByType('Redirect').props.href).toBe('/h/host%2F1/tasks?taskSource=github')
|
||||
expect(getPendingMobileTasksNavigation()).toBe(pending)
|
||||
|
||||
act(() => route.unmount())
|
||||
renderer = null
|
||||
expect(getPendingMobileTasksNavigation()).toBeNull()
|
||||
})
|
||||
|
||||
it('clears a stale intent instead of redirecting a different mounted host', async () => {
|
||||
navigateToMobileTasks({ push: vi.fn() }, 'host-1', 'linear')
|
||||
mocks.params = { hostId: 'host-2' }
|
||||
|
||||
const route = await renderRoute()
|
||||
|
||||
expect(route.root.findByType('HostScreen')).toBeDefined()
|
||||
expect(getPendingMobileTasksNavigation()).toBeNull()
|
||||
})
|
||||
|
||||
it('preserves the wide-layout placeholder without a pending intent', async () => {
|
||||
mocks.isWideLayout = true
|
||||
|
||||
const route = await renderRoute()
|
||||
|
||||
expect(route.root.findByType('WorkspaceDetailPlaceholder')).toBeDefined()
|
||||
})
|
||||
})
|
||||
@@ -1,33 +0,0 @@
|
||||
import { useEffect, type ComponentType } from 'react'
|
||||
import { Redirect, useLocalSearchParams } from 'expo-router'
|
||||
import { useResponsiveLayout } from '../layout/responsive-layout'
|
||||
import { WorkspaceDetailPlaceholder } from '../components/WorkspaceDetailPlaceholder'
|
||||
import {
|
||||
clearPendingMobileTasksNavigation,
|
||||
getPendingMobileTasksNavigation,
|
||||
mobileTasksRouteForMountedHost
|
||||
} from './mobile-task-navigation'
|
||||
|
||||
export function MobileTaskHostRoute({ hostScreen: HostScreen }: { hostScreen: ComponentType }) {
|
||||
const params = useLocalSearchParams<{ hostId?: string }>()
|
||||
const { isWideLayout } = useResponsiveLayout()
|
||||
const intent = getPendingMobileTasksNavigation()
|
||||
const tasksRoute = mobileTasksRouteForMountedHost(params.hostId, intent)
|
||||
|
||||
// Why: unmount proves the redirect committed; identity cleanup cannot erase a newer tap.
|
||||
useEffect(() => {
|
||||
if (!intent) {
|
||||
return
|
||||
}
|
||||
if (!tasksRoute) {
|
||||
clearPendingMobileTasksNavigation(intent)
|
||||
return
|
||||
}
|
||||
return () => clearPendingMobileTasksNavigation(intent)
|
||||
}, [intent, tasksRoute])
|
||||
|
||||
if (tasksRoute) {
|
||||
return <Redirect href={tasksRoute} />
|
||||
}
|
||||
return isWideLayout ? <WorkspaceDetailPlaceholder /> : <HostScreen />
|
||||
}
|
||||
@@ -0,0 +1,114 @@
|
||||
import { createElement, type ReactNode, useEffect } from 'react'
|
||||
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
clearPendingMobileTasksNavigation,
|
||||
getPendingMobileTasksNavigation,
|
||||
navigateToMobileTasks,
|
||||
type MobileTasksRoute
|
||||
} from './mobile-task-navigation'
|
||||
import { useMobileTaskHostNavigation } from './use-mobile-task-host-navigation'
|
||||
|
||||
function HostStackContent({ onCommit }: { onCommit: () => void }) {
|
||||
useEffect(onCommit, [onCommit])
|
||||
return createElement('HostScreen')
|
||||
}
|
||||
|
||||
function HostLayoutHarness({
|
||||
children,
|
||||
hostId,
|
||||
replace
|
||||
}: {
|
||||
children: ReactNode
|
||||
hostId?: string
|
||||
replace: (route: MobileTasksRoute) => void
|
||||
}) {
|
||||
useMobileTaskHostNavigation({ replace }, hostId)
|
||||
return children
|
||||
}
|
||||
|
||||
describe('useMobileTaskHostNavigation', () => {
|
||||
let renderer: ReactTestRenderer | null = null
|
||||
|
||||
beforeEach(() => {
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
act(() => renderer?.unmount())
|
||||
renderer = null
|
||||
const pending = getPendingMobileTasksNavigation()
|
||||
if (pending) {
|
||||
clearPendingMobileTasksNavigation(pending)
|
||||
}
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
async function renderLayout(args: {
|
||||
hostId?: string
|
||||
onCommit: () => void
|
||||
replace: (route: MobileTasksRoute) => void
|
||||
}): Promise<void> {
|
||||
const consoleError = vi.spyOn(console, 'error').mockImplementation((...errorArgs) => {
|
||||
if (
|
||||
typeof errorArgs[0] !== 'string' ||
|
||||
!errorArgs[0].includes('react-test-renderer is deprecated')
|
||||
) {
|
||||
throw new Error(String(errorArgs[0]))
|
||||
}
|
||||
})
|
||||
try {
|
||||
await act(async () => {
|
||||
renderer = create(
|
||||
createElement(
|
||||
HostLayoutHarness,
|
||||
{ hostId: args.hostId, replace: args.replace },
|
||||
createElement(HostStackContent, { onCommit: args.onCommit })
|
||||
)
|
||||
)
|
||||
})
|
||||
} finally {
|
||||
consoleError.mockRestore()
|
||||
}
|
||||
}
|
||||
|
||||
it('waits for host content to commit before replacing a cold route with lost params', async () => {
|
||||
let hostCommitted = false
|
||||
navigateToMobileTasks({ push: vi.fn() }, 'host/1', 'github')
|
||||
const replace = vi.fn((route: MobileTasksRoute) => {
|
||||
expect(hostCommitted).toBe(true)
|
||||
expect(route).toBe('/h/host%2F1/tasks?taskSource=github')
|
||||
})
|
||||
|
||||
await renderLayout({
|
||||
onCommit: () => {
|
||||
hostCommitted = true
|
||||
},
|
||||
replace
|
||||
})
|
||||
|
||||
expect(replace).toHaveBeenCalledOnce()
|
||||
expect(getPendingMobileTasksNavigation()).toBeNull()
|
||||
})
|
||||
|
||||
it('clears a mismatched host intent without replacing', async () => {
|
||||
navigateToMobileTasks({ push: vi.fn() }, 'host-1', 'linear')
|
||||
const replace = vi.fn()
|
||||
|
||||
await renderLayout({ hostId: 'host-2', onCommit: vi.fn(), replace })
|
||||
|
||||
expect(replace).not.toHaveBeenCalled()
|
||||
expect(getPendingMobileTasksNavigation()).toBeNull()
|
||||
})
|
||||
|
||||
it('consumes only the latest intent from repeated taps', async () => {
|
||||
navigateToMobileTasks({ push: vi.fn() }, 'host-1', 'github')
|
||||
navigateToMobileTasks({ push: vi.fn() }, 'host-2', 'gitlab')
|
||||
const replace = vi.fn()
|
||||
|
||||
await renderLayout({ hostId: 'host-2', onCommit: vi.fn(), replace })
|
||||
|
||||
expect(replace).toHaveBeenCalledWith('/h/host-2/tasks?taskSource=gitlab')
|
||||
expect(getPendingMobileTasksNavigation()).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,32 @@
|
||||
import { useEffect } from 'react'
|
||||
import {
|
||||
clearPendingMobileTasksNavigation,
|
||||
getPendingMobileTasksNavigation,
|
||||
mobileTasksRouteForMountedHost,
|
||||
type MobileTasksRoute
|
||||
} from './mobile-task-navigation'
|
||||
|
||||
type MobileTasksReplaceRouter = {
|
||||
replace: (route: MobileTasksRoute) => void
|
||||
}
|
||||
|
||||
export function useMobileTaskHostNavigation(
|
||||
router: MobileTasksReplaceRouter,
|
||||
hostId: string | undefined
|
||||
): void {
|
||||
// Why: this host-layout effect runs after HostStack's children commit, when nested replace is valid.
|
||||
useEffect(() => {
|
||||
const intent = getPendingMobileTasksNavigation()
|
||||
if (!intent) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
const route = mobileTasksRouteForMountedHost(hostId, intent)
|
||||
if (route) {
|
||||
router.replace(route)
|
||||
}
|
||||
} finally {
|
||||
clearPendingMobileTasksNavigation(intent)
|
||||
}
|
||||
}, [hostId, router])
|
||||
}
|
||||
Reference in New Issue
Block a user