xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(mobile): keep source-control layout steady while Create PR eligibility loads (#11467) 

* fix(mobile): keep source-control layout steady while Create PR eligibility loads

The Create PR entry unmounted until the first hostedReview.getCreationEligibility
answer arrived, so on a cold open the changed-files list painted first and then
shifted down 54pt (createPrBlock marginTop 12 + createPrButton minHeight 42)
when the button appeared — while the user was already tapping (#8411).

- buildMobileCreatePrAction: cold loading now reserves the row with a disabled
  placeholder instead of unmounting it.
- useMobileHostedReviewEligibility: a fetch-imminent idle frame renders as an
  in-flight load, so the reservation is present on the first painted frame.
- New per-worktree+branch memory of the last resolved eligibility seeds cold
  loads, so branches whose answer is hidden (existing review, unsupported
  provider) do not get a placeholder that collapses on every reopen.

Fixes #8411

* fix(mobile): harden source-control layout reservation

* fix(mobile): keep review status row footprint fixed

* fix(mobile): derive eligibility state from keyed snapshots
This commit is contained in:
Brennan Benson 2026-07-31 15:35:56 -07:00 committed by GitHub
parent 5e258a9447
commit c6d2180417
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 525 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
mobile/src/source-control/MobileSourceControlCreatePrEntry.tsx
View File

@ -13,6 +13,7 @@ export function MobileSourceControlCreatePrEntry({ action }: Props) {
return null
}
const enabled = !action.disabled
const copy = action.hint ?? action.label
return (
<View style={styles.createPrBlock}>
<Pressable
@ -36,15 +37,17 @@ export function MobileSourceControlCreatePrEntry({ action }: Props) {
strokeWidth={2.2}
/>
)}
<Text style={[styles.createPrButtonText, !enabled && styles.createPrButtonTextDisabled]}>
{action.label}
<Text
style={[
styles.createPrButtonText,
!enabled && styles.createPrButtonTextDisabled,
action.hint && styles.createPrButtonHint
]}
numberOfLines={2}
>
{copy}
</Text>
</Pressable>
{action.hint ? (
<Text style={styles.createPrHint} numberOfLines={2}>
{action.hint}
</Text>
) : null}
</View>
)
}

109
mobile/src/source-control/mobile-create-pr-action.test.ts
View File

@ -4,7 +4,10 @@ import type {
HostedReviewCreationEligibility,
HostedReviewProvider
} from '../../../src/shared/hosted-review'
import { buildMobileCreatePrAction } from './mobile-create-pr-action'
import {
buildMobileCreatePrAction,
type MobileCreatePrEligibilityState
} from './mobile-create-pr-action'
function eligibility(
overrides: Partial<HostedReviewCreationEligibility> = {}
@ -108,26 +111,21 @@ describe('buildMobileCreatePrAction', () => {
'unsupported_provider',
'detached_head',
null
])('hides %s', (blockedReason) => {
])('keeps a disabled status row for %s', (blockedReason) => {
const { descriptor } = action({
eligibility: eligibility({ canCreate: false, blockedReason })
})
expect(descriptor.visible).toBe(false)
expect(descriptor).toMatchObject({ visible: true, disabled: true })
expect(descriptor.hint).toBeTruthy()
})
it('hides cold loading, errors, and missing branches', () => {
const onCreatePr = vi.fn()
expect(
buildMobileCreatePrAction({
branch: 'feature',
eligibilityState: { kind: 'loading', eligibility: null },
busyAction: null,
onCreatePr
}).visible
).toBe(false)
expect(action({ eligibility: null }).descriptor.visible).toBe(false)
it('keeps an unavailable row after errors but hides missing branches', () => {
expect(action({ eligibility: null }).descriptor).toMatchObject({
visible: true,
disabled: true,
label: 'Review status unavailable'
})
expect(action({ branch: null }).descriptor.visible).toBe(false)
})
@ -146,12 +144,16 @@ describe('buildMobileCreatePrAction', () => {
expect(onCreatePr).not.toHaveBeenCalled()
})
it('hides any provider that does not support hosted-review creation', () => {
it('keeps a disabled status row for providers without review creation', () => {
const { descriptor } = action({
eligibility: eligibility({ provider: 'bitbucket' as HostedReviewProvider })
})
expect(descriptor.visible).toBe(false)
expect(descriptor).toMatchObject({
visible: true,
disabled: true,
label: 'Review creation unavailable for this provider'
})
})
it('keeps a creatable button visible but disabled while a newer eligibility loads', () => {
@ -167,3 +169,76 @@ describe('buildMobileCreatePrAction', () => {
expect(descriptor).toMatchObject({ visible: true, disabled: true, loading: false })
})
})
// Issue #8411: the Create PR entry renders directly above the Stage All row and
// the changed-files list, so appearing late shifts the list down by
// createPrBlock marginTop (12) + createPrButton height (42) = 54pt while the
// user is already reading it. The row must keep its footprint across the cold
// eligibility fetch.
describe('cold-mount layout stability (issue #8411)', () => {
// useMobileHostedReviewEligibility's real cold-mount sequence for a creatable
// branch: no prior snapshot exists, so `loading` carries eligibility: null.
const coldMount: MobileCreatePrEligibilityState[] = [
{ kind: 'loading', eligibility: null },
{ kind: 'ready', eligibility: eligibility({ canCreate: true }) }
]
it('reserves the button row while the first eligibility request is in flight', () => {
const descriptors = coldMount.map((eligibilityState) =>
buildMobileCreatePrAction({
branch: 'feature',
eligibilityState,
busyAction: null,
onCreatePr: vi.fn()
})
)
const footprint = descriptors.map(({ visible }) => visible)
// On the buggy parent this was [false, true] -- the false -> true step is the jump.
expect(footprint).toEqual([true, true])
})
it.each([
[
'existing review',
{
kind: 'ready',
eligibility: eligibility({ canCreate: false, blockedReason: 'existing_review' })
} satisfies MobileCreatePrEligibilityState
],
[
'unsupported provider',
{
kind: 'ready',
eligibility: eligibility({ provider: 'bitbucket' as HostedReviewProvider })
} satisfies MobileCreatePrEligibilityState
],
['eligibility error', { kind: 'error' } satisfies MobileCreatePrEligibilityState]
])('keeps the cold-load footprint when %s resolves', (_label, resolvedState) => {
const footprint = [coldMount[0], resolvedState].map((eligibilityState) => {
const descriptor = buildMobileCreatePrAction({
branch: 'feature',
eligibilityState,
busyAction: null,
onCreatePr: vi.fn()
})
return descriptor.visible
})
expect(footprint).toEqual([true, true])
})
it('does not offer a tappable action against unresolved eligibility', () => {
const descriptor = buildMobileCreatePrAction({
branch: 'feature',
eligibilityState: { kind: 'loading', eligibility: null },
busyAction: null,
onCreatePr: vi.fn()
})
// Reserving space must not make the placeholder actionable: there is no
// canCreate/pushFirst answer yet, so a tap has nothing correct to do.
expect(descriptor.disabled).toBe(true)
})
})

46
mobile/src/source-control/mobile-create-pr-action.ts
View File

@ -1,7 +1,4 @@
import type {
HostedReviewCreationBlockedReason,
HostedReviewCreationEligibility
} from '../../../src/shared/hosted-review'
import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review'
import { supportsHostedReviewCreation } from '../../../src/shared/hosted-review-creation-providers'
import { hostedReviewCopy } from './hosted-review-copy'
import { getMobilePrCreateBlockMessage } from './mobile-pr-create'
@ -29,13 +26,6 @@ export type BuildMobileCreatePrActionArgs = {
onCreatePr: (pushFirst: boolean) => void
}
const HIDDEN_BLOCKED_REASONS = new Set<HostedReviewCreationBlockedReason>([
'detached_head',
'existing_review',
'unsupported_provider',
null
])
const BUSY_ACTIONS = new Set(['create-pr', 'push-create-pr'])
function hiddenAction(onPress: () => void): MobileCreatePrAction {
@ -56,18 +46,42 @@ export function buildMobileCreatePrAction({
onCreatePr
}: BuildMobileCreatePrActionArgs): MobileCreatePrAction {
const noop = () => {}
if (!branch || eligibilityState.kind === 'idle' || eligibilityState.kind === 'error') {
if (!branch || eligibilityState.kind === 'idle') {
return hiddenAction(noop)
}
if (eligibilityState.kind === 'error') {
return {
visible: true,
label: 'Review status unavailable',
disabled: true,
loading: false,
pushFirst: false,
onPress: noop
}
}
const eligibility = eligibilityState.eligibility
if (!eligibility) {
return hiddenAction(noop)
return {
visible: true,
label: 'Checking review status…',
disabled: true,
loading: true,
pushFirst: false,
onPress: noop
}
}
// Why: mirror desktop's structural provider gate (supportsHostedReviewCreation)
// instead of relying on the host always emitting a hidden blockedReason for
// non-creatable providers like bitbucket.
if (!supportsHostedReviewCreation(eligibility.provider)) {
return hiddenAction(noop)
return {
visible: true,
label: 'Review creation unavailable for this provider',
disabled: true,
loading: false,
pushFirst: false,
onPress: noop
}
}
const copy = hostedReviewCopy(eligibility.provider)
const label = `Create ${copy.titleLabel}`
@ -95,10 +109,6 @@ export function buildMobileCreatePrAction({
}
}
if (HIDDEN_BLOCKED_REASONS.has(eligibility.blockedReason)) {
return hiddenAction(noop)
}
const hint =
getMobilePrCreateBlockMessage({
provider: eligibility.provider,

13
mobile/src/source-control/mobile-source-control-styles.ts
View File

@ -207,11 +207,10 @@ const baseStyles = StyleSheet.create({
fontWeight: '600'
},
createPrBlock: {
marginTop: spacing.md,
gap: spacing.xs
marginTop: spacing.md
},
createPrButton: {
minHeight: 42,
height: 42,
borderRadius: radii.button,
backgroundColor: colors.textPrimary,
alignItems: 'center',
@ -236,10 +235,12 @@ const baseStyles = StyleSheet.create({
createPrButtonTextDisabled: {
color: colors.textSecondary
},
createPrHint: {
color: colors.textMuted,
createPrButtonHint: {
fontSize: typography.metaSize,
lineHeight: 16
fontWeight: '600',
lineHeight: 16,
textAlign: 'center',
flexShrink: 1
}
})

324
mobile/src/source-control/use-mobile-hosted-review-eligibility.test.ts
View File

@ -1,10 +1,48 @@
import { describe, expect, it } from 'vitest'
import { createElement } from 'react'
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review'
import {
acceptsMobileHostedReviewEligibilityLoad,
buildMobileHostedReviewEligibilityLoadKey,
eligibilityStateAfterMobileHostedReviewError,
shouldFetchMobileHostedReviewEligibility
renderedMobileHostedReviewEligibilityState,
shouldFetchMobileHostedReviewEligibility,
useMobileHostedReviewEligibility,
type MobileHostedReviewEligibilityLoadKey,
type MobileHostedReviewEligibilityLoadSnapshot
} from './use-mobile-hosted-review-eligibility'
import type { MobileCreatePrEligibilityState } from './mobile-create-pr-action'
function eligibility(
overrides: Partial<HostedReviewCreationEligibility> = {}
): HostedReviewCreationEligibility {
return {
provider: 'github',
review: null,
canCreate: true,
blockedReason: null,
nextAction: null,
defaultBaseRef: 'main',
title: 'feature',
body: '',
...overrides
}
}
function deferred<T>(): { promise: Promise<T>; resolve: (value: T) => void } {
let resolve!: (value: T) => void
const promise = new Promise<T>((resolvePromise) => {
resolve = resolvePromise
})
return { promise, resolve }
}
function loadSnapshot(
key: MobileHostedReviewEligibilityLoadKey,
state: MobileCreatePrEligibilityState
): MobileHostedReviewEligibilityLoadSnapshot {
return { key, state }
}
describe('mobile hosted review eligibility loader core', () => {
it('does not fetch while disconnected or detached', () => {
@ -24,8 +62,9 @@ describe('mobile hosted review eligibility loader core', () => {
).toBe(false)
})
it('accepts only the latest generation for the current worktree branch identity', () => {
it('builds distinct keys for different branches', () => {
const first = buildMobileHostedReviewEligibilityLoadKey({
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature-a',
hasUpstream: true,
@ -34,6 +73,7 @@ describe('mobile hosted review eligibility loader core', () => {
hasUncommittedChanges: false
})
const second = buildMobileHostedReviewEligibilityLoadKey({
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature-b',
hasUpstream: true,
@ -42,17 +82,281 @@ describe('mobile hosted review eligibility loader core', () => {
hasUncommittedChanges: false
})
expect(first.identity).not.toBe(second.identity)
expect(first.fetch).not.toBe(second.fetch)
})
it('scopes load identity to the paired host', () => {
const input = {
worktreeId: 'repo-1::/workspace',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0,
hasUncommittedChanges: false
}
const local = buildMobileHostedReviewEligibilityLoadKey({ ...input, hostId: 'local' })
const ssh = buildMobileHostedReviewEligibilityLoadKey({ ...input, hostId: 'ssh-builder' })
expect(local.identity).not.toBe(ssh.identity)
expect(local.fetch).not.toBe(ssh.fetch)
})
it('renders a superseded same-identity snapshot as loading', () => {
const input = {
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0
}
const older = buildMobileHostedReviewEligibilityLoadKey({
...input,
hasUncommittedChanges: false
})
const newest = buildMobileHostedReviewEligibilityLoadKey({
...input,
hasUncommittedChanges: true
})
expect(older.identity).toBe(newest.identity)
expect(
acceptsMobileHostedReviewEligibilityLoad({
generation: 1,
currentGeneration: 2,
identity: first.identity,
currentIdentity: second.identity
renderedMobileHostedReviewEligibilityState({
snapshot: loadSnapshot(older, { kind: 'ready', eligibility: eligibility() }),
key: newest,
shouldFetch: true
})
).toBe(false)
).toMatchObject({ kind: 'loading', eligibility: eligibility() })
})
it('fails closed after errors', () => {
expect(eligibilityStateAfterMobileHostedReviewError()).toEqual({ kind: 'error' })
})
})
// #8411: what the hook returns is what paints. A fetch-imminent frame must not
// render as `idle` (hidden row) or the Create PR row pops in a frame later.
describe('rendered eligibility state', () => {
it('renders fetch-imminent idle as an in-flight load', () => {
const key = buildMobileHostedReviewEligibilityLoadKey({
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0,
hasUncommittedChanges: false
})
expect(
renderedMobileHostedReviewEligibilityState({
snapshot: loadSnapshot(key, { kind: 'idle' }),
key,
shouldFetch: true
})
).toEqual({ kind: 'loading', eligibility: null })
})
it('passes resolved and refetch states through untouched', () => {
const key = buildMobileHostedReviewEligibilityLoadKey({
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0,
hasUncommittedChanges: false
})
const ready = { kind: 'ready', eligibility: eligibility() } as const
const refetch = { kind: 'loading', eligibility: eligibility() } as const
const error = { kind: 'error' } as const
for (const state of [ready, refetch, error]) {
expect(
renderedMobileHostedReviewEligibilityState({
snapshot: loadSnapshot(key, state),
key,
shouldFetch: true
})
).toBe(state)
}
})
it('renders idle when a fetch is not possible, hiding stale snapshots in the same render', () => {
const key = buildMobileHostedReviewEligibilityLoadKey({
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0,
hasUncommittedChanges: false
})
expect(
renderedMobileHostedReviewEligibilityState({
snapshot: loadSnapshot(key, { kind: 'ready', eligibility: eligibility() }),
key,
shouldFetch: false
})
).toEqual({ kind: 'idle' })
})
})
describe('eligibility request ordering', () => {
let renderer: ReactTestRenderer | null = null
let renderedState: MobileCreatePrEligibilityState = { kind: 'idle' }
beforeEach(() => {
globalThis.IS_REACT_ACT_ENVIRONMENT = true
})
afterEach(() => {
act(() => renderer?.unmount())
renderer = null
})
it('does not let an older response overwrite the newest state', async () => {
type Response = { ok: true; result: HostedReviewCreationEligibility }
const requests: ReturnType<typeof deferred<Response>>[] = []
const client = {
sendRequest: vi.fn(() => {
const request = deferred<Response>()
requests.push(request)
return request.promise
})
}
const newest = eligibility({ canCreate: false, blockedReason: 'existing_review' })
const older = eligibility()
function Harness({ dirty }: { dirty: boolean }): null {
renderedState = useMobileHostedReviewEligibility({
client: client as never,
connState: 'connected',
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0,
hasUncommittedChanges: dirty
})
return null
}
await act(async () => {
renderer = create(createElement(Harness, { dirty: false }))
await Promise.resolve()
})
await act(async () => {
renderer?.update(createElement(Harness, { dirty: true }))
await Promise.resolve()
})
expect(requests).toHaveLength(2)
await act(async () => {
requests[1]!.resolve({ ok: true, result: newest })
await Promise.resolve()
})
await act(async () => {
requests[0]!.resolve({ ok: true, result: older })
await Promise.resolve()
})
expect(renderedState).toMatchObject({ kind: 'ready', eligibility: newest })
})
it('disables a ready snapshot in the render that changes its fetch key', async () => {
type Response = { ok: true; result: HostedReviewCreationEligibility }
const requests: ReturnType<typeof deferred<Response>>[] = []
const client = {
sendRequest: vi.fn(() => {
const request = deferred<Response>()
requests.push(request)
return request.promise
})
}
const ready = eligibility()
function Harness({ dirty }: { dirty: boolean }): null {
renderedState = useMobileHostedReviewEligibility({
client: client as never,
connState: 'connected',
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0,
hasUncommittedChanges: dirty
})
return null
}
await act(async () => {
renderer = create(createElement(Harness, { dirty: false }))
await Promise.resolve()
})
await act(async () => {
requests[0]!.resolve({ ok: true, result: ready })
await Promise.resolve()
})
expect(renderedState).toMatchObject({ kind: 'ready', eligibility: ready })
await act(async () => {
renderer?.update(createElement(Harness, { dirty: true }))
await Promise.resolve()
})
expect(renderedState).toMatchObject({ kind: 'loading', eligibility: ready })
})
it('invalidates a request when its hook instance unmounts', async () => {
type Response = { ok: true; result: HostedReviewCreationEligibility }
const requests: ReturnType<typeof deferred<Response>>[] = []
const client = {
sendRequest: vi.fn(() => {
const request = deferred<Response>()
requests.push(request)
return request.promise
})
}
const newest = eligibility({ canCreate: false, blockedReason: 'existing_review' })
function Harness(): null {
renderedState = useMobileHostedReviewEligibility({
client: client as never,
connState: 'connected',
hostId: 'host-1',
worktreeId: 'wt-1',
branch: 'feature',
hasUpstream: true,
ahead: 0,
behind: 0,
hasUncommittedChanges: false
})
return null
}
await act(async () => {
renderer = create(createElement(Harness))
await Promise.resolve()
})
act(() => renderer?.unmount())
renderer = null
await act(async () => {
renderer = create(createElement(Harness))
await Promise.resolve()
})
expect(requests).toHaveLength(2)
await act(async () => {
requests[1]!.resolve({ ok: true, result: newest })
await Promise.resolve()
})
await act(async () => {
requests[0]!.resolve({ ok: true, result: eligibility() })
await Promise.resolve()
})
expect(renderedState).toMatchObject({ kind: 'ready', eligibility: newest })
})
})

114
mobile/src/source-control/use-mobile-hosted-review-eligibility.ts
View File

@ -1,4 +1,4 @@
import { useEffect, useRef, useState } from 'react'
import { useEffect, useState } from 'react'
import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review'
import type { RpcClient } from '../transport/rpc-client'
import type { ConnectionState } from '../transport/types'
@ -11,6 +11,7 @@ import type { MobileCreatePrEligibilityState } from './mobile-create-pr-action'
export type MobileHostedReviewEligibilityLoaderInput = {
client: RpcClient | null
connState: ConnectionState
hostId: string
worktreeId: string
branch: string | null | undefined
hasUpstream: boolean | undefined
@ -24,13 +25,19 @@ export type MobileHostedReviewEligibilityLoadKey = {
fetch: string
}
export type MobileHostedReviewEligibilityLoadSnapshot = {
key: MobileHostedReviewEligibilityLoadKey
state: MobileCreatePrEligibilityState
}
export function buildMobileHostedReviewEligibilityLoadKey(
input: Omit<MobileHostedReviewEligibilityLoaderInput, 'client' | 'connState'>
): MobileHostedReviewEligibilityLoadKey {
const branch = input.branch ?? ''
return {
identity: `${input.worktreeId}\0${branch}`,
identity: `${input.hostId}\0${input.worktreeId}\0${branch}`,
fetch: [
input.hostId,
input.worktreeId,
branch,
String(input.hasUpstream ?? ''),
@ -47,25 +54,42 @@ export function shouldFetchMobileHostedReviewEligibility(
return input.connState === 'connected' && input.client !== null && !!input.branch
}
export function acceptsMobileHostedReviewEligibilityLoad(args: {
generation: number
currentGeneration: number
identity: string
currentIdentity: string
}): boolean {
return args.generation === args.currentGeneration && args.identity === args.currentIdentity
}
export function eligibilityStateAfterMobileHostedReviewError(): MobileCreatePrEligibilityState {
return { kind: 'error' }
}
export function renderedMobileHostedReviewEligibilityState(args: {
snapshot: MobileHostedReviewEligibilityLoadSnapshot
key: MobileHostedReviewEligibilityLoadKey
shouldFetch: boolean
}): MobileCreatePrEligibilityState {
if (!args.shouldFetch) {
return { kind: 'idle' }
}
const { snapshot, key } = args
if (snapshot.key.identity !== key.identity) {
return { kind: 'loading', eligibility: null }
}
const { state } = snapshot
if (snapshot.key.fetch !== key.fetch) {
return {
kind: 'loading',
eligibility: state.kind === 'ready' || state.kind === 'loading' ? state.eligibility : null
}
}
if (state.kind === 'idle') {
return { kind: 'loading', eligibility: null }
}
return state
}
export function useMobileHostedReviewEligibility(
input: MobileHostedReviewEligibilityLoaderInput
): MobileCreatePrEligibilityState {
const {
client,
connState,
hostId,
worktreeId,
branch,
hasUpstream,
@ -74,11 +98,8 @@ export function useMobileHostedReviewEligibility(
hasUncommittedChanges
} = input
const shouldFetch = shouldFetchMobileHostedReviewEligibility({ client, connState, branch })
const [state, setState] = useState<MobileCreatePrEligibilityState>({ kind: 'idle' })
const generationRef = useRef(0)
const currentIdentityRef = useRef('')
const lastResetIdentityRef = useRef('')
const key = buildMobileHostedReviewEligibilityLoadKey({
hostId,
worktreeId,
branch,
hasUpstream,
@ -86,38 +107,35 @@ export function useMobileHostedReviewEligibility(
behind,
hasUncommittedChanges
})
if (lastResetIdentityRef.current !== key.identity) {
lastResetIdentityRef.current = key.identity
setState({ kind: 'idle' })
}
currentIdentityRef.current = key.identity
const [snapshot, setSnapshot] = useState<MobileHostedReviewEligibilityLoadSnapshot>({
key: { identity: '', fetch: '' },
state: { kind: 'idle' }
})
useEffect(() => {
const generation = generationRef.current + 1
generationRef.current = generation
const isCurrent = () =>
acceptsMobileHostedReviewEligibilityLoad({
generation,
currentGeneration: generationRef.current,
identity: key.identity,
currentIdentity: currentIdentityRef.current
})
let active = true
if (!shouldFetch) {
if (isCurrent()) {
setState({ kind: 'idle' })
setSnapshot({ key, state: { kind: 'idle' } })
return () => {
active = false
}
return
}
if (!client || !branch) {
return
return () => {
active = false
}
}
setState((prev) => ({
kind: 'loading',
eligibility: prev.kind === 'ready' ? prev.eligibility : null
}))
setSnapshot((previous) => {
const previousState = previous.state
const eligibility =
previous.key.identity === key.identity &&
(previousState.kind === 'ready' || previousState.kind === 'loading')
? previousState.eligibility
: null
return { key, state: { kind: 'loading', eligibility } }
})
const requestInput: MobileHostedReviewEligibilityInput = {
branch,
hasUncommittedChanges,
@ -127,20 +145,23 @@ export function useMobileHostedReviewEligibility(
}
void fetchMobileHostedReviewEligibility(client, worktreeId, requestInput)
.then((eligibility: HostedReviewCreationEligibility | null) => {
if (!isCurrent()) {
if (!active) {
return
}
if (!eligibility) {
setState({ kind: 'error' })
setSnapshot({ key, state: eligibilityStateAfterMobileHostedReviewError() })
return
}
setState({ kind: 'ready', eligibility })
setSnapshot({ key, state: { kind: 'ready', eligibility } })
})
.catch(() => {
if (isCurrent()) {
setState(eligibilityStateAfterMobileHostedReviewError())
if (active) {
setSnapshot({ key, state: eligibilityStateAfterMobileHostedReviewError() })
}
})
return () => {
active = false
}
}, [
ahead,
behind,
@ -149,6 +170,7 @@ export function useMobileHostedReviewEligibility(
connState,
hasUncommittedChanges,
hasUpstream,
hostId,
key.fetch,
key.identity,
shouldFetch,
@ -159,5 +181,9 @@ export function useMobileHostedReviewEligibility(
// snapshot in the same render, before the effect posts `idle` — otherwise the
// Create PR button could stay enabled for one paint after the worktree is
// no longer fetchable.
return shouldFetch ? state : { kind: 'idle' }
return renderedMobileHostedReviewEligibilityState({
snapshot,
key,
shouldFetch
})
}

3
mobile/src/source-control/use-mobile-source-control-create-pr-action.ts
View File

@ -6,6 +6,7 @@ import type { MobileGitStatusResult } from './mobile-git-status'
type Params = {
client: Parameters<typeof useMobileHostedReviewEligibility>[0]['client']
connState: Parameters<typeof useMobileHostedReviewEligibility>[0]['connState']
hostId: string
worktreeId: string
status: MobileGitStatusResult | null
hasUncommittedChanges: boolean
@ -16,6 +17,7 @@ type Params = {
export function useMobileSourceControlCreatePrAction({
client,
connState,
hostId,
worktreeId,
status,
hasUncommittedChanges,
@ -26,6 +28,7 @@ export function useMobileSourceControlCreatePrAction({
const eligibilityState = useMobileHostedReviewEligibility({
client,
connState,
hostId,
worktreeId,
branch: status?.branch,
hasUpstream: upstream?.hasUpstream,

1
mobile/src/source-control/use-mobile-source-control-state.ts
View File

@ -209,6 +209,7 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara
const createPrAction = useMobileSourceControlCreatePrAction({
client,
connState,
hostId,
worktreeId,
status,
hasUncommittedChanges: entries.length > 0,