fix(mobile): mount host before opening tasks (#11853)
This commit is contained in:
Brennan Benson
GitHub
parent
ed00ab0f34
commit
4c03cdff72
|
|
@ -49,6 +49,7 @@ import {
|
|||
normalizeVisibleTaskProviders,
|
||||
type TaskProvider
|
||||
} from '../src/tasks/mobile-task-providers'
|
||||
import { useOpenMobileTasks } from '../src/tasks/use-open-mobile-tasks'
|
||||
import { useResponsiveLayout } from '../src/layout/responsive-layout'
|
||||
import { createMobileSessionHref } from '../src/session/mobile-session-route'
|
||||
|
||||
|
|
@ -286,6 +287,7 @@ function repoColor(name: string): string {
|
|||
|
||||
export default function HomeScreen() {
|
||||
const router = useRouter()
|
||||
const openMobileTasks = useOpenMobileTasks()
|
||||
const insets = useSafeAreaInsets()
|
||||
// Why: cap/center content on wide/tablet canvases so cards don't stretch edge-to-edge on iPad.
|
||||
const { isWideLayout, contentMaxWidth } = useResponsiveLayout()
|
||||
|
|
@ -601,10 +603,9 @@ export default function HomeScreen() {
|
|||
if (!primaryConnectedHost) {
|
||||
return
|
||||
}
|
||||
const suffix = provider ? `?taskSource=${provider}` : ''
|
||||
router.push(`/h/${primaryConnectedHost.id}/tasks${suffix}`)
|
||||
openMobileTasks(primaryConnectedHost.id, provider)
|
||||
},
|
||||
[primaryConnectedHost, router]
|
||||
[openMobileTasks, primaryConnectedHost]
|
||||
)
|
||||
const renderTaskHomeCard = () => (
|
||||
<Pressable
|
||||
|
|
|
|||
|
|
@ -0,0 +1,206 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
coordinateMobileTasksNavigation,
|
||||
mobileTasksHostRoute,
|
||||
navigateToMobileTasks,
|
||||
type MobileTasksNavigationState
|
||||
} from './mobile-task-navigation'
|
||||
|
||||
function navigationHarness(initialState: MobileTasksNavigationState) {
|
||||
let stateListener = () => {}
|
||||
let state = initialState
|
||||
const unsubscribeState = vi.fn()
|
||||
const navigation = {
|
||||
addListener: vi.fn((_event: 'state', listener: () => void) => {
|
||||
stateListener = listener
|
||||
return unsubscribeState
|
||||
}),
|
||||
dispatch: vi.fn(),
|
||||
getState: () => state
|
||||
}
|
||||
return {
|
||||
navigation,
|
||||
setState(nextState: MobileTasksNavigationState) {
|
||||
state = nextState
|
||||
stateListener()
|
||||
},
|
||||
unsubscribeState
|
||||
}
|
||||
}
|
||||
|
||||
describe('mobile task navigation', () => {
|
||||
it('waits for the expected host commit before navigating to Tasks', () => {
|
||||
const harness = navigationHarness({ index: 0, routes: [{ name: 'index' }] })
|
||||
const push = vi.fn()
|
||||
|
||||
navigateToMobileTasks(harness.navigation, { push }, 'host/1')
|
||||
|
||||
expect(harness.navigation.addListener.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
push.mock.invocationCallOrder[0]!
|
||||
)
|
||||
expect(push).toHaveBeenCalledWith(mobileTasksHostRoute('host/1'))
|
||||
expect(harness.navigation.dispatch).not.toHaveBeenCalled()
|
||||
|
||||
harness.setState({
|
||||
index: 1,
|
||||
routes: [{ name: 'index' }, { name: 'h', params: { hostId: 'host/1' } }]
|
||||
})
|
||||
expect(harness.navigation.dispatch).not.toHaveBeenCalled()
|
||||
|
||||
harness.setState({
|
||||
index: 1,
|
||||
routes: [
|
||||
{ name: 'index' },
|
||||
{
|
||||
name: 'h',
|
||||
state: {
|
||||
key: '/h',
|
||||
index: 0,
|
||||
routes: [{ key: 'host-index', name: '[hostId]/index', params: { hostId: 'host/1' } }]
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
expect(harness.unsubscribeState).toHaveBeenCalledOnce()
|
||||
expect(harness.unsubscribeState.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
harness.navigation.dispatch.mock.invocationCallOrder[0]!
|
||||
)
|
||||
expect(harness.navigation.dispatch).toHaveBeenCalledWith({
|
||||
type: 'REPLACE',
|
||||
target: '/h',
|
||||
source: 'host-index',
|
||||
payload: { name: '[hostId]/tasks', params: { hostId: 'host/1' } }
|
||||
})
|
||||
})
|
||||
|
||||
it('ignores unrelated state events and preserves the provider', () => {
|
||||
const harness = navigationHarness({ index: 0, routes: [{ name: 'index' }] })
|
||||
|
||||
navigateToMobileTasks(harness.navigation, { push: vi.fn() }, 'host-1', 'linear')
|
||||
harness.setState({ index: 1, routes: [{ name: 'index' }, { name: 'settings' }] })
|
||||
expect(harness.navigation.dispatch).not.toHaveBeenCalled()
|
||||
harness.setState({ index: 0, routes: [{ name: 'h', params: { hostId: 'host-2' } }] })
|
||||
expect(harness.navigation.dispatch).not.toHaveBeenCalled()
|
||||
harness.setState({
|
||||
index: 0,
|
||||
routes: [
|
||||
{
|
||||
name: 'h',
|
||||
state: {
|
||||
key: '/h',
|
||||
index: 0,
|
||||
routes: [{ key: 'host-index', name: '[hostId]/index', params: { hostId: 'host-1' } }]
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
expect(harness.navigation.dispatch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
payload: {
|
||||
name: '[hostId]/tasks',
|
||||
params: { hostId: 'host-1', taskSource: 'linear' }
|
||||
}
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('cleanup prevents a stale navigation from replacing', () => {
|
||||
const harness = navigationHarness({ index: 0, routes: [{ name: 'index' }] })
|
||||
const controller = navigateToMobileTasks(harness.navigation, { push: vi.fn() }, 'host-1')
|
||||
|
||||
controller.cancel()
|
||||
harness.setState({ index: 0, routes: [{ name: 'h', params: { hostId: 'host-1' } }] })
|
||||
|
||||
expect(harness.unsubscribeState).toHaveBeenCalledOnce()
|
||||
expect(harness.navigation.dispatch).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reuses a pending host push and applies the latest provider', () => {
|
||||
const harness = navigationHarness({ index: 0, routes: [{ name: 'index' }] })
|
||||
const push = vi.fn()
|
||||
|
||||
const first = coordinateMobileTasksNavigation(
|
||||
null,
|
||||
harness.navigation,
|
||||
{ push },
|
||||
'host-1',
|
||||
'github'
|
||||
)
|
||||
const second = coordinateMobileTasksNavigation(
|
||||
first,
|
||||
harness.navigation,
|
||||
{ push },
|
||||
'host-1',
|
||||
'linear'
|
||||
)
|
||||
harness.setState({
|
||||
index: 0,
|
||||
routes: [
|
||||
{
|
||||
name: 'h',
|
||||
state: {
|
||||
key: '/h',
|
||||
index: 0,
|
||||
routes: [{ key: 'host-index', name: '[hostId]/index', params: { hostId: 'host-1' } }]
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
expect(second).toBe(first)
|
||||
expect(push).toHaveBeenCalledOnce()
|
||||
expect(harness.navigation.dispatch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
payload: {
|
||||
name: '[hostId]/tasks',
|
||||
params: { hostId: 'host-1', taskSource: 'linear' }
|
||||
}
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps waiting through host setup and cleans up when navigation leaves', () => {
|
||||
const harness = navigationHarness({ index: 0, routes: [{ name: 'index' }] })
|
||||
|
||||
navigateToMobileTasks(harness.navigation, { push: vi.fn() }, 'host-1')
|
||||
harness.setState({ index: 0, routes: [{ name: 'h', params: { hostId: 'host-1' } }] })
|
||||
expect(harness.unsubscribeState).not.toHaveBeenCalled()
|
||||
harness.setState({ index: 0, routes: [{ name: 'index' }] })
|
||||
harness.setState({
|
||||
index: 0,
|
||||
routes: [
|
||||
{
|
||||
name: 'h',
|
||||
state: {
|
||||
key: '/h',
|
||||
index: 0,
|
||||
routes: [{ key: 'host-index', name: '[hostId]/index', params: { hostId: 'host-1' } }]
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
expect(harness.unsubscribeState).toHaveBeenCalledOnce()
|
||||
expect(harness.navigation.dispatch).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('unsubscribes when mounting the host throws synchronously', () => {
|
||||
const harness = navigationHarness({ index: 0, routes: [{ name: 'index' }] })
|
||||
const error = new Error('navigation failed')
|
||||
|
||||
expect(() =>
|
||||
navigateToMobileTasks(
|
||||
harness.navigation,
|
||||
{
|
||||
push: () => {
|
||||
throw error
|
||||
}
|
||||
},
|
||||
'host-1'
|
||||
)
|
||||
).toThrow(error)
|
||||
expect(harness.unsubscribeState).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,154 @@
|
|||
import type { TaskProvider } from './mobile-task-providers'
|
||||
|
||||
export type MobileTasksHostRoute = `/h/${string}`
|
||||
|
||||
export type MobileTasksNavigationState = Readonly<{
|
||||
key?: string
|
||||
index: number
|
||||
routes: readonly MobileTasksNavigationRoute[]
|
||||
}>
|
||||
|
||||
export type MobileTasksNavigationRoute = Readonly<{
|
||||
key?: string
|
||||
name: string
|
||||
params?: Readonly<{ hostId?: unknown }>
|
||||
state?: MobileTasksNavigationState
|
||||
}>
|
||||
|
||||
export type MobileTasksRootNavigation = {
|
||||
addListener: (event: 'state', listener: () => void) => () => void
|
||||
dispatch: (action: MobileTasksHostReplaceAction) => void
|
||||
getState: () => MobileTasksNavigationState
|
||||
}
|
||||
|
||||
export type MobileTasksHostReplaceAction = Readonly<{
|
||||
type: 'REPLACE'
|
||||
target: string
|
||||
source: string
|
||||
payload: Readonly<{
|
||||
name: '[hostId]/tasks'
|
||||
params: Readonly<{ hostId: string; taskSource?: TaskProvider }>
|
||||
}>
|
||||
}>
|
||||
|
||||
export type MobileTasksRouter = {
|
||||
push: (route: MobileTasksHostRoute) => void
|
||||
}
|
||||
|
||||
export type MobileTasksNavigationController = Readonly<{
|
||||
cancel: () => void
|
||||
isActive: () => boolean
|
||||
selectProvider: (provider?: TaskProvider) => void
|
||||
}>
|
||||
|
||||
export type PendingMobileTasksNavigation = Readonly<{
|
||||
hostId: string
|
||||
controller: MobileTasksNavigationController
|
||||
}>
|
||||
|
||||
export function mobileTasksHostRoute(hostId: string): MobileTasksHostRoute {
|
||||
return `/h/${encodeURIComponent(hostId)}`
|
||||
}
|
||||
|
||||
function mountedHostStack(
|
||||
state: MobileTasksNavigationState,
|
||||
expectedHostId: string
|
||||
): { key: string; routeKey: string } | null {
|
||||
const hostContainer = state.routes[state.index]
|
||||
const hostState = hostContainer?.state
|
||||
const hostRoute = hostState?.routes[hostState.index]
|
||||
if (
|
||||
hostContainer?.name !== 'h' ||
|
||||
!hostState?.key ||
|
||||
hostRoute?.name !== '[hostId]/index' ||
|
||||
!hostRoute.key ||
|
||||
hostRoute.params?.hostId !== expectedHostId
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return { key: hostState.key, routeKey: hostRoute.key }
|
||||
}
|
||||
|
||||
export function navigateToMobileTasks(
|
||||
navigation: MobileTasksRootNavigation,
|
||||
router: MobileTasksRouter,
|
||||
hostId: string,
|
||||
provider?: TaskProvider
|
||||
): MobileTasksNavigationController {
|
||||
let active = true
|
||||
let hostRouteSeen = false
|
||||
let selectedProvider = provider
|
||||
let unsubscribeState = () => {}
|
||||
const dispose = () => {
|
||||
if (!active) {
|
||||
return
|
||||
}
|
||||
active = false
|
||||
unsubscribeState()
|
||||
}
|
||||
|
||||
// Why: cold Expo deep links resolve to index; target Tasks only after its HostStack exists.
|
||||
const onState = () => {
|
||||
if (!active) {
|
||||
return
|
||||
}
|
||||
const state = navigation.getState()
|
||||
const currentRoute = state.routes[state.index]
|
||||
if (currentRoute?.name === 'h') {
|
||||
hostRouteSeen = true
|
||||
} else if (hostRouteSeen) {
|
||||
dispose()
|
||||
return
|
||||
}
|
||||
const hostStack = mountedHostStack(state, hostId)
|
||||
if (!hostStack) {
|
||||
return
|
||||
}
|
||||
dispose()
|
||||
const action: MobileTasksHostReplaceAction = {
|
||||
type: 'REPLACE',
|
||||
target: hostStack.key,
|
||||
source: hostStack.routeKey,
|
||||
payload: {
|
||||
name: '[hostId]/tasks',
|
||||
params: selectedProvider ? { hostId, taskSource: selectedProvider } : { hostId }
|
||||
}
|
||||
}
|
||||
navigation.dispatch(action)
|
||||
}
|
||||
|
||||
try {
|
||||
unsubscribeState = navigation.addListener('state', onState)
|
||||
router.push(mobileTasksHostRoute(hostId))
|
||||
} catch (error) {
|
||||
dispose()
|
||||
throw error
|
||||
}
|
||||
return {
|
||||
cancel: dispose,
|
||||
isActive: () => active,
|
||||
selectProvider: (nextProvider) => {
|
||||
if (active) {
|
||||
selectedProvider = nextProvider
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function coordinateMobileTasksNavigation(
|
||||
current: PendingMobileTasksNavigation | null,
|
||||
navigation: MobileTasksRootNavigation,
|
||||
router: MobileTasksRouter,
|
||||
hostId: string,
|
||||
provider?: TaskProvider
|
||||
): PendingMobileTasksNavigation {
|
||||
if (current?.hostId === hostId && current.controller.isActive()) {
|
||||
current.controller.selectProvider(provider)
|
||||
return current
|
||||
}
|
||||
current?.controller.cancel()
|
||||
return {
|
||||
hostId,
|
||||
controller: navigateToMobileTasks(navigation, router, hostId, provider)
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
import { useCallback, useEffect, useRef } from 'react'
|
||||
import { useNavigation, useRouter } from 'expo-router'
|
||||
import {
|
||||
coordinateMobileTasksNavigation,
|
||||
type MobileTasksRootNavigation,
|
||||
type PendingMobileTasksNavigation
|
||||
} from './mobile-task-navigation'
|
||||
import type { TaskProvider } from './mobile-task-providers'
|
||||
|
||||
export function useOpenMobileTasks(): (hostId: string, provider?: TaskProvider) => void {
|
||||
const navigation = useNavigation<MobileTasksRootNavigation>()
|
||||
const router = useRouter()
|
||||
const pendingRef = useRef<PendingMobileTasksNavigation | null>(null)
|
||||
|
||||
useEffect(
|
||||
() => () => {
|
||||
pendingRef.current?.controller.cancel()
|
||||
pendingRef.current = null
|
||||
},
|
||||
[]
|
||||
)
|
||||
|
||||
return useCallback(
|
||||
(hostId, provider) => {
|
||||
pendingRef.current = coordinateMobileTasksNavigation(
|
||||
pendingRef.current,
|
||||
navigation,
|
||||
router,
|
||||
hostId,
|
||||
provider
|
||||
)
|
||||
},
|
||||
[navigation, router]
|
||||
)
|
||||
}
|
||||
Loading…
Reference in New Issue