diff --git a/mobile/src/components/pr-sidebar/MobilePrTabSheets.tsx b/mobile/src/components/pr-sidebar/MobilePrTabSheets.tsx index 5ed5ede7271..57c5d472bc8 100644 --- a/mobile/src/components/pr-sidebar/MobilePrTabSheets.tsx +++ b/mobile/src/components/pr-sidebar/MobilePrTabSheets.tsx @@ -1,4 +1,4 @@ -import { useEffect, useLayoutEffect, useRef, useState } from 'react' +import { useState } from 'react' import { Text, View } from 'react-native' import { GitMerge } from 'lucide-react-native' import { useMobileColorScheme } from '../../theme/use-mobile-color-scheme' @@ -12,7 +12,7 @@ import { SheetPrimaryButton } from '../SheetPrimaryButton' import { ListGroup, ListRow } from '../list/ListGroup' import { ConfirmModal } from '../ConfirmModal' import { presentPrMerge } from './pr-actions-state' -import { PrCreateSheet, type PrCreateKept } from './PrCreateSheet' +import { PrCreateSheet } from './PrCreateSheet' import { PrReviewersSheet } from './PrReviewersSheet' import { livePrDetails, @@ -22,17 +22,14 @@ import { } from './use-mobile-pr-tab' import { prSheetStyles as sheetStyles } from './pr-tab-styles' import { FieldTextInput } from '../composer-input/FieldTextInput' - -type PrEditKept = { key: string; base: string; text: string } +import { draftTextHash, workspaceDraftRowKey } from '../../drafts/durable-draft-rows' +import { useDurableDraftField } from '../../drafts/durable-draft-fields' +import { useWorkspaceExecutionHost } from '../../navigation/workspace-execution-host' // Sheets for the PR tab's decisions; one Modal swaps between them so iOS never stacks presentations. export function MobilePrTabSheets({ tab }: { tab: MobilePrTab }) { useMobileColorScheme() const t = useMobileTranslation() - // Each sheet unmounts on Cancel; typed text waits here for the next open. - const [createKept, setCreateKept] = useState(null) - const [titleKept, setTitleKept] = useState(null) - const [bodyKept, setBodyKept] = useState(null) return ( <> case 'editTitle': - return ( - - ) + return case 'editDescription': - return ( - - ) + return case 'createPr': - return + return } }} @@ -182,55 +163,56 @@ function PrMergeSheet({ tab, request }: { tab: MobilePrTab; request: PrTabSheetR function PrEditTextSheet({ tab, request, - field, - kept, - onKeep + field }: { tab: MobilePrTab request: PrTabSheetRequest field: 'title' | 'body' - kept: PrEditKept | null - onKeep: (kept: PrEditKept | null) => void }) { useMobileColorScheme() const t = useMobileTranslation() + const executionHost = useWorkspaceExecutionHost() const data = readyPrData(tab) const details = livePrDetails(data?.details ?? null) const current = field === 'title' ? (details?.item.title ?? data?.pr.title ?? '') : (details?.body ?? '') - const keepKey = request.scope.resource + const resource = request.scope.resource + // An unsaved edit waits through Cancel and the app being killed, with the PR text it was typed against. + const keptKey = resource + ? workspaceDraftRowKey( + 'prEdit', + { hostId: tab.hostId, executionHost, worktreeId: tab.worktreeId }, + `${resource}:${field}` + ) + : null + const [keptAgainst, setKeptAgainst] = useDurableDraftField(keptKey, 'against') + const [keptText, setKeptText] = useDurableDraftField(keptKey, 'text') // An edit typed against text the PR still has comes back; once the PR text changes it would revert someone's edit. const [base] = useState(current) - const [draft, setDraft] = useState(() => - kept && kept.key === keepKey && kept.base === current ? kept.text : current - ) - // Closing keeps a changed, unsaved draft for the next open, and drops this PR's kept one otherwise. - const savedRef = useRef(false) - const closeRef = useRef({ keepKey, base, draft, kept, onKeep }) - useLayoutEffect(() => { - closeRef.current = { keepKey, base, draft, kept, onKeep } - }) - useEffect( - () => () => { - const close = closeRef.current - if (!close.keepKey) { - return - } - if (!savedRef.current && close.draft !== close.base) { - close.onKeep({ key: close.keepKey, base: close.base, text: close.draft }) - } else if (close.kept?.key === close.keepKey) { - close.onKeep(null) - } - }, - [] - ) + // A fingerprint, never empty, so an emptied PR description still marks an edit (an empty field is no field). + const [against] = useState(() => draftTextHash(base)) + const draft = keptAgainst === against ? keptText : base + const setDraft = (text: string) => { + const edited = text !== base + setKeptText(edited ? text : '') + setKeptAgainst(edited ? against : '') + } const { textAction } = tab const save = async () => { if (!tab.ownsSheet(request)) { return } - if (await textAction.save(field, draft, current)) { - savedRef.current = true + const sent = draft + if (await textAction.save(field, sent, current)) { + // Only the saved edit; one typed since in a reopened sheet stays. + let saved = false + setKeptText((text) => { + saved = text === sent + return saved ? '' : text + }) + if (saved) { + setKeptAgainst('') + } tab.closeSheetIf(request) } } diff --git a/mobile/src/components/pr-sidebar/PRCommentCard.tsx b/mobile/src/components/pr-sidebar/PRCommentCard.tsx index 006c8565720..eea6227f480 100644 --- a/mobile/src/components/pr-sidebar/PRCommentCard.tsx +++ b/mobile/src/components/pr-sidebar/PRCommentCard.tsx @@ -29,6 +29,8 @@ export type PRCommentCardActions = { isResolveBusy: (threadId: string) => boolean isEditBusy: (commentId: number) => boolean isDeleteBusy: (commentId: number) => boolean + // Where a reply to a comment waits until it is posted. + draftKey: (replyTo: number) => string | null // Repo slug for the slug-addressed edit/delete RPCs; gates the affordances when absent. prRepo: PRCommentRepoSlug | null } @@ -296,6 +298,7 @@ export const PRCommentCard = memo(function PRCommentCard({ placeholder={t('pullRequest.commentCard.replyPlaceholder', 'Write a reply…')} submitLabel={t('pullRequest.commentCard.reply', 'Reply')} submitting={replyBusy} + draftKey={actions.draftKey(comment.id)} onSubmit={submitReply} onCancel={() => setReplyOpen(false)} autoFocus diff --git a/mobile/src/components/pr-sidebar/PRCommentComposer.tsx b/mobile/src/components/pr-sidebar/PRCommentComposer.tsx index 152469d7281..18f3ac36c6a 100644 --- a/mobile/src/components/pr-sidebar/PRCommentComposer.tsx +++ b/mobile/src/components/pr-sidebar/PRCommentComposer.tsx @@ -7,6 +7,7 @@ import { useMobileTranslation } from '../../i18n/use-mobile-translation' import { isSubmittableCommentBody } from '../../source-control/review/pr-comment-actions' import { MessageBox } from '../composer-input/MessageBox' import { composerInputStyles } from '../composer-input/composer-input-styles' +import { useDurableDraftField } from '../../drafts/durable-draft-fields' type Props = { // Share plain-text composer across root comment, reply and edit. @@ -15,6 +16,8 @@ type Props = { submitting: boolean // Seeds the field for the edit case; the reply/add cases leave it empty. initialBody?: string + // Where a new comment or reply waits until it is posted, through closing and the app being killed. + draftKey?: string | null // Resolves to true on success; the composer clears + collapses (caller-driven via key remount or onSubmitted). onSubmit: (body: string) => Promise onCancel?: () => void @@ -27,6 +30,7 @@ export function PRCommentComposer({ submitLabel, submitting, initialBody, + draftKey = null, onSubmit, onCancel, autoFocus, @@ -34,7 +38,9 @@ export function PRCommentComposer({ }: Props) { useMobileColorScheme() const t = useMobileTranslation() - const [body, setBody] = useState(initialBody ?? '') + const [edited, setEdited] = useState(initialBody ?? '') + const [kept, setKept] = useDurableDraftField(draftKey, 'body') + const [body, setBody] = draftKey === null ? [edited, setEdited] : [kept, setKept] // Guard synchronously against double-submit before parent’s async submitting state rerenders. const inFlightRef = useRef(false) const canSubmit = isSubmittableCommentBody(body) && !submitting @@ -45,9 +51,11 @@ export function PRCommentComposer({ } inFlightRef.current = true try { - const ok = await onSubmit(body.trim()) + const posted = body.trim() + const ok = await onSubmit(posted) + // A reopened composer shares the draft; text typed there since stays. if (ok) { - setBody('') + setBody((current) => (current.trim() === posted ? '' : current)) } } finally { inFlightRef.current = false diff --git a/mobile/src/components/pr-sidebar/PRCommentsSection.tsx b/mobile/src/components/pr-sidebar/PRCommentsSection.tsx index dffdc4028d1..9c44d335d27 100644 --- a/mobile/src/components/pr-sidebar/PRCommentsSection.tsx +++ b/mobile/src/components/pr-sidebar/PRCommentsSection.tsx @@ -239,6 +239,7 @@ export function PRCommentsSection({ )} submitLabel={t('pullRequest.commentsSection.comment', 'Comment')} submitting={actions.isRootBusy} + draftKey={actions.draftKey(null)} onSubmit={actions.addRootComment} /> @@ -267,6 +268,7 @@ export function usePRCommentCardActions( isResolveBusy: actions.isResolveBusy, isEditBusy: actions.isEditBusy, isDeleteBusy: actions.isDeleteBusy, + draftKey: actions.draftKey, prRepo: prRepo ?? null } : undefined, diff --git a/mobile/src/components/pr-sidebar/PrCommentsScreen.tsx b/mobile/src/components/pr-sidebar/PrCommentsScreen.tsx index 7cec48386f9..329fbb36a17 100644 --- a/mobile/src/components/pr-sidebar/PrCommentsScreen.tsx +++ b/mobile/src/components/pr-sidebar/PrCommentsScreen.tsx @@ -91,6 +91,7 @@ export function PrCommentsScreen({ tab, onBack, bottomInset = 0, onOpenFile }: S isResolveBusy: commentActions.isResolveBusy, isEditBusy: commentActions.isEditBusy, isDeleteBusy: commentActions.isDeleteBusy, + draftKey: commentActions.draftKey, prRepo: data?.pr.prRepo ?? null } : undefined, @@ -195,6 +196,7 @@ export function PrCommentsScreen({ tab, onBack, bottomInset = 0, onOpenFile }: S })} submitLabel={t('pullRequest.commentsSection.comment', 'Comment')} submitting={commentActions.isRootBusy} + draftKey={commentActions.draftKey(null)} onSubmit={commentActions.addRootComment} inline /> diff --git a/mobile/src/components/pr-sidebar/PrCreateSheet.tsx b/mobile/src/components/pr-sidebar/PrCreateSheet.tsx index 8b2107d0ea3..8080e0faf27 100644 --- a/mobile/src/components/pr-sidebar/PrCreateSheet.tsx +++ b/mobile/src/components/pr-sidebar/PrCreateSheet.tsx @@ -1,11 +1,4 @@ -import { - useEffect, - useLayoutEffect, - useRef, - useState, - type Dispatch, - type SetStateAction -} from 'react' +import { useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { ActivityIndicator, Pressable, Text, View } from 'react-native' import { ChevronsUpDown, Sparkles } from 'lucide-react-native' import { colors, pressedStyle, spacing } from '../../theme/mobile-theme' @@ -30,8 +23,7 @@ import { import { cancelMobilePrFields, generateMobilePrFields, - loadMobilePrCreateDraft, - type MobilePrCreateDraft + loadMobilePrCreateDraft } from '../../source-control/mobile-pr-create-draft' import { getMobileCommitFailureStagedEntries, @@ -43,33 +35,52 @@ import type { MobilePrTab } from './use-mobile-pr-tab' import { prSheetStyles as sheetStyles } from './pr-tab-styles' import { PrBasePicker } from './PrBasePicker' import { FieldTextInput } from '../composer-input/FieldTextInput' +import { workspaceDraftRowKey } from '../../drafts/durable-draft-rows' +import { updateDurableDraftField, useDurableDraftField } from '../../drafts/durable-draft-fields' +import { useWorkspaceExecutionHost } from '../../navigation/workspace-execution-host' -type Fields = { title: string; body: string; base: string; draft: boolean } +// The provider rides along so a kept draft reopens without asking the host again. +type Fields = { title: string; body: string; base: string; draft: boolean; provider: string } -/** What Cancel leaves for the next open on the same workspace branch. */ -export type PrCreateKept = { key: string; draft: MobilePrCreateDraft; fields: Fields } +function keepCreateField(key: string, patch: Partial): void { + for (const [name, value] of Object.entries(patch)) { + updateDurableDraftField(key, name, value === true ? 'yes' : value === false ? '' : value) + } +} + +// The first edit keeps the whole form. The provider is never empty, so it is what marks a kept draft. +function keepCreateFields(key: string, fields: Fields | null): void { + keepCreateField(key, fields ?? { provider: '', title: '', body: '', base: '', draft: false }) +} + +function useKeptCreateFields(key: string | null): Fields | null { + const [provider] = useDurableDraftField(key, 'provider') + const [title] = useDurableDraftField(key, 'title') + const [body] = useDurableDraftField(key, 'body') + const [base] = useDurableDraftField(key, 'base') + const [draft] = useDurableDraftField(key, 'draft') + return useMemo( + () => (provider ? { title, body, base, draft: draft === 'yes', provider } : null), + [base, body, draft, provider, title] + ) +} // One Create flow for every entry point and all five providers: seed from commits, push if needed, land on the PR tab. -export function PrCreateSheet({ - tab, - kept, - onKeep -}: { - tab: MobilePrTab - kept: PrCreateKept | null - onKeep: Dispatch> -}) { +export function PrCreateSheet({ tab }: { tab: MobilePrTab }) { useMobileColorScheme() const t = useMobileTranslation() - const { client, worktreeId, gitBranch } = tab + const { client, hostId, worktreeId, gitBranch } = tab + const executionHost = useWorkspaceExecutionHost() const creating = tab.creatingPr const setCreating = tab.setCreatingPr - const keepKey = gitBranch ? `${worktreeId}\u0000${gitBranch}` : null - // Reopening on the same branch restores what was typed instead of re-seeding from commits. - const [restored] = useState(() => (kept && kept.key === keepKey ? kept : null)) - const [draft, setDraft] = useState(restored?.draft ?? null) - const [fields, setFields] = useState(restored?.fields ?? null) - const editedRef = useRef(restored !== null) + // What was typed waits, per workspace branch, through Cancel and the app being killed. + const keptKey = gitBranch + ? workspaceDraftRowKey('prCreate', { hostId, executionHost, worktreeId }, gitBranch) + : null + const kept = useKeptCreateFields(keptKey) + const [seeded, setSeeded] = useState(null) + const [seedError, setSeedError] = useState(null) + const fields = kept ?? seeded const [pickingBase, setPickingBase] = useState(false) const [generating, setGenerating] = useState(false) const [message, setMessage] = useState<{ text: string; error: boolean } | null>(null) @@ -89,11 +100,11 @@ export function PrCreateSheet({ useLayoutEffect(() => { commitsAheadRef.current = tab.commitsAhead }) - // Seed once per open: a reconnect or a status reload must not overwrite what the user typed. - const seededRef = useRef(restored !== null) + // Seed once per open, and not over a kept draft: a reconnect or a status reload must not overwrite what was typed. + const seedRef = useRef(kept !== null) useEffect(() => { - if (!client || !gitBranch || seededRef.current) { + if (!client || !gitBranch || seedRef.current) { return } let cancelled = false @@ -104,14 +115,19 @@ export function PrCreateSheet({ }) .then((loaded) => { if (!cancelled) { - seededRef.current = true - setDraft(loaded) - setFields({ title: loaded.title, body: loaded.body, base: loaded.base, draft: false }) + seedRef.current = true + setSeeded({ + title: loaded.title, + body: loaded.body, + base: loaded.base, + draft: false, + provider: loaded.prefill.provider + }) } }) .catch((err: unknown) => { if (!cancelled) { - setMessage({ text: err instanceof Error ? err.message : String(err), error: true }) + setSeedError(err instanceof Error ? err.message : String(err)) } }) return () => { @@ -119,13 +135,25 @@ export function PrCreateSheet({ } }, [client, gitBranch, worktreeId]) - const copy = hostedReviewCopy(draft?.prefill.provider) + const copy = hostedReviewCopy(fields?.provider) const title = t('pullRequest.createSheet.title', 'Create {{reviewLabel}}', { reviewLabel: copy.reviewLabel }) + // The first edit turns the seeded fields into a kept draft. Later ones write only what they change, so a + // late Write with AI answer can't undo a toggle made meanwhile. + const keptRef = useRef(kept) + useLayoutEffect(() => { + keptRef.current = kept + }) const update = (patch: Partial) => { - editedRef.current = true - setFields((current) => (current ? { ...current, ...patch } : current)) + if (!keptKey || !fields) { + return + } + if (keptRef.current) { + keepCreateField(keptKey, patch) + } else { + keepCreateFields(keptKey, { ...fields, ...patch }) + } } // Closing the sheet cancels a running generation, drops its late answer and keeps typed fields; a reconnect does none. @@ -134,9 +162,9 @@ export function PrCreateSheet({ useLayoutEffect(() => { generatingRef.current = generating }) - const closeTargetRef = useRef({ client, worktreeId, keepKey, draft, fields, onKeep }) + const closeTargetRef = useRef({ client, worktreeId }) useLayoutEffect(() => { - closeTargetRef.current = { client, worktreeId, keepKey, draft, fields, onKeep } + closeTargetRef.current = { client, worktreeId } }) useEffect( () => () => { @@ -145,25 +173,19 @@ export function PrCreateSheet({ if (generatingRef.current && target.client) { void cancelMobilePrFields(target.client, target.worktreeId).catch(() => {}) } - if (editedRef.current && target.keepKey && target.draft && target.fields) { - target.onKeep({ key: target.keepKey, draft: target.draft, fields: target.fields }) - } }, [] ) const writeWithAi = async () => { - if (!client || !fields || !draft || generating) { + if (!client || !fields || generating) { return } const generation = ++generationRef.current setGenerating(true) setMessage(null) try { - const result = await generateMobilePrFields(client, worktreeId, { - ...fields, - provider: draft.prefill.provider - }) + const result = await generateMobilePrFields(client, worktreeId, fields) if (generation !== generationRef.current) { return } @@ -190,7 +212,7 @@ export function PrCreateSheet({ return } createInFlightRef.current = true - const createdKey = keepKey + const createdKey = keptKey setCreating(true) setMessage(null) setCommitFailure(null) @@ -226,10 +248,12 @@ export function PrCreateSheet({ } triggerSuccess() setCreating(false) - // The PR exists now, so nothing carries over to the next Create on this branch, even text kept - // when a branch or client change closed this sheet mid-run. - editedRef.current = false - onKeep((current) => (current?.key === createdKey ? null : current)) + // The PR exists now, so nothing carries over to the next Create on this branch, even when a branch or + // client change closed this sheet mid-run. + setSeeded(fields) + if (createdKey) { + keepCreateFields(createdKey, null) + } const created = { url: outcome.url, warning: outcome.warning ?? null, @@ -281,8 +305,8 @@ export function PrCreateSheet({ 'Check out a branch before creating a pull request.' )} - ) : message ? ( - {message.text} + ) : seedError ? ( + {seedError} ) : ( )} diff --git a/mobile/src/components/pr-sidebar/PrThreadRow.tsx b/mobile/src/components/pr-sidebar/PrThreadRow.tsx index 2d9527ba450..be89feef5e9 100644 --- a/mobile/src/components/pr-sidebar/PrThreadRow.tsx +++ b/mobile/src/components/pr-sidebar/PrThreadRow.tsx @@ -44,6 +44,8 @@ export type PrThreadActions = { isResolveBusy: (threadId: string) => boolean isEditBusy: (commentId: number) => boolean isDeleteBusy: (commentId: number) => boolean + // Where a reply to a comment waits until it is posted. + draftKey: (replyTo: number) => string | null // Repo slug for the slug-addressed edit/delete RPCs; gates the affordances when absent. prRepo: PRCommentRepoSlug | null } diff --git a/mobile/src/components/pr-sidebar/PrThreadScreen.tsx b/mobile/src/components/pr-sidebar/PrThreadScreen.tsx index 3bef4466dd9..b1db7052d92 100644 --- a/mobile/src/components/pr-sidebar/PrThreadScreen.tsx +++ b/mobile/src/components/pr-sidebar/PrThreadScreen.tsx @@ -114,6 +114,7 @@ export function PrThreadScreen({ placeholder={t('pullRequest.screens.replyPlaceholder', 'Reply…')} submitLabel={t('pullRequest.commentCard.reply', 'Reply')} submitting={actions.isReplyBusy(root.id)} + draftKey={actions.draftKey(root.id)} onSubmit={(body) => actions.reply(root, body)} autoFocus={focusReply} inline diff --git a/mobile/src/components/pr-sidebar/pr-comment-composer-drafts.test.tsx b/mobile/src/components/pr-sidebar/pr-comment-composer-drafts.test.tsx new file mode 100644 index 00000000000..fa124007eb1 --- /dev/null +++ b/mobile/src/components/pr-sidebar/pr-comment-composer-drafts.test.tsx @@ -0,0 +1,99 @@ +import { createElement, type ReactNode } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +const hosts = vi.hoisted(() => { + const Box = (props: { children?: ReactNode }): ReactNode => props.children ?? null + Box.displayName = 'MessageBox' + return { Box } +}) +vi.mock('react-native', () => ({ Pressable: () => null })) +vi.mock('lucide-react-native', () => ({ X: () => null })) +vi.mock('../../theme/use-mobile-color-scheme', () => ({ useMobileColorScheme: () => 'light' })) +vi.mock('../composer-input/MessageBox', () => ({ MessageBox: hosts.Box })) +vi.mock('../composer-input/composer-input-styles', () => ({ composerInputStyles: {} })) + +import { publishPageDeviceStore } from '../../mobile-web-shell/bridge/page-device-store' +import { resetDurableDraftFieldsForTests } from '../../drafts/durable-draft-fields' +import { publishInMemoryDeviceStore } from '../../drafts/in-memory-device-store.test-support' +import { PRCommentComposer } from './PRCommentComposer' + +// The shell's draft table outlives the page; nothing else does. +const table = new Map() +let renderer: ReactTestRenderer | null = null +let onSubmit: (body: string) => Promise = async () => true + +const KEY = 'draft:prComment:wt-1:host-1::/acme/app#12#new' + +async function open(): Promise { + await act(async () => { + renderer = create( + createElement(PRCommentComposer, { + placeholder: 'Comment', + submitLabel: 'Comment', + submitting: false, + draftKey: KEY, + onSubmit: (body: string) => onSubmit(body) + }) + ) + await vi.advanceTimersByTimeAsync(0) + }) +} + +function box(): { value: string; onChangeText: (text: string) => void; onSend: () => void } { + const props = renderer?.root.findByType(hosts.Box).props + return { value: props?.value, onChangeText: props?.onChangeText, onSend: props?.onSend } +} + +async function type(text: string): Promise { + act(() => box().onChangeText(text)) + await act(async () => { + await vi.advanceTimersByTimeAsync(1000) + }) +} + +// A kill: the page and its memory go, the store stays. +function kill(): void { + act(() => renderer?.unmount()) + renderer = null + resetDurableDraftFieldsForTests() +} + +beforeEach(() => { + vi.useFakeTimers() + publishInMemoryDeviceStore(table) +}) + +afterEach(() => { + kill() + table.clear() + publishPageDeviceStore(null) + onSubmit = async () => true + vi.useRealTimers() +}) + +it('brings an unposted comment back after the app is killed', async () => { + await open() + await type('Can we keep the retry?') + kill() + + await open() + expect(box().value).toBe('Can we keep the retry?') +}) + +it('keeps text typed in a reopened composer when the earlier post lands', async () => { + let land: (ok: boolean) => void = () => {} + onSubmit = () => new Promise((resolve) => (land = resolve)) + await open() + await type('first thought') + act(() => box().onSend()) + act(() => renderer?.unmount()) + + await open() + await type('a second thought') + await act(async () => { + land(true) + await vi.advanceTimersByTimeAsync(1000) + }) + expect(box().value).toBe('a second thought') +}) diff --git a/mobile/src/components/pr-sidebar/pr-create-sheet-drafts.test.tsx b/mobile/src/components/pr-sidebar/pr-create-sheet-drafts.test.tsx new file mode 100644 index 00000000000..19c329130c7 --- /dev/null +++ b/mobile/src/components/pr-sidebar/pr-create-sheet-drafts.test.tsx @@ -0,0 +1,154 @@ +import { createElement, type ReactNode } from 'react' +import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +const hosts = vi.hoisted(() => { + const make = (name: string) => { + const Host = (props: { children?: ReactNode }): ReactNode => props.children ?? null + Host.displayName = name + return Host + } + return { + View: make('View'), + Text: make('Text'), + Pressable: make('Pressable'), + Field: make('FieldTextInput'), + Button: make('SheetPrimaryButton') + } +}) +vi.mock('react-native', () => ({ + View: hosts.View, + Text: hosts.Text, + Pressable: hosts.Pressable, + ActivityIndicator: hosts.View, + StyleSheet: { create: (styles: T) => styles, hairlineWidth: 1, absoluteFillObject: {} }, + Platform: { OS: 'ios', select: (options: Record) => options.ios } +})) +vi.mock('lucide-react-native', () => ({ ChevronsUpDown: hosts.View, Sparkles: hosts.View })) +vi.mock('../../theme/use-mobile-color-scheme', () => ({ useMobileColorScheme: () => 'light' })) +vi.mock('../nav-bar/SheetHeader', () => ({ SheetHeader: hosts.View })) +vi.mock('../SheetPrimaryButton', () => ({ SheetPrimaryButton: hosts.Button })) +vi.mock('../list/ListGroup', () => ({ ListGroup: hosts.View, ListRow: hosts.View })) +vi.mock('../MobileSwitch', () => ({ MobileSwitch: hosts.View })) +vi.mock('./PrBasePicker', () => ({ PrBasePicker: hosts.View })) +vi.mock('../composer-input/FieldTextInput', () => ({ FieldTextInput: hosts.Field })) +vi.mock('../../source-control/MobileCommitFailurePanel', () => ({ + MobileCommitFailurePanel: hosts.View +})) +vi.mock('../../source-control/use-mobile-commit-failure-recovery', () => ({ + useMobileCommitFailureRecovery: () => null +})) +const host = vi.hoisted(() => ({ + seed: vi.fn(), + create: vi.fn() +})) +vi.mock('../../source-control/mobile-pr-create-draft', () => ({ + loadMobilePrCreateDraft: host.seed, + generateMobilePrFields: vi.fn(), + cancelMobilePrFields: vi.fn(async () => {}) +})) +vi.mock('../../source-control/mobile-hosted-review-create-intent-runner', () => ({ + runMobileHostedReviewCreateIntent: host.create, + isMobileHostedReviewCommitFailure: () => false +})) + +import { publishPageDeviceStore } from '../../mobile-web-shell/bridge/page-device-store' +import { resetDurableDraftFieldsForTests } from '../../drafts/durable-draft-fields' +import { publishInMemoryDeviceStore } from '../../drafts/in-memory-device-store.test-support' +import { createFakeRpcClient } from '../../mobile-web-shell/bridge-host-test-fakes' +import { PrCreateSheet } from './PrCreateSheet' +import type { MobilePrTab } from './use-mobile-pr-tab' +import { mountFixture } from '../../test-support/rpc-recording/recorder-fixture-shape' + +// The shell's draft table outlives the page; nothing else does. +const table = new Map() +let renderer: ReactTestRenderer | null = null + +// The sheet reads only these fields of the PR tab. +const tab = mountFixture({ + client: createFakeRpcClient(), + hostId: 'host-1', + worktreeId: 'repo::/work/feature', + gitBranch: 'feature/login', + gitStatus: null, + commitsAhead: null, + connState: 'connected', + workspaceLabel: 'feature', + creatingPr: false, + setCreatingPr: () => {}, + closeSheet: () => {}, + closeSheetThen: () => {} +}) + +async function open(): Promise { + await act(async () => { + renderer = create(createElement(PrCreateSheet, { tab })) + await vi.advanceTimersByTimeAsync(0) + }) +} + +function titleField(): ReactTestInstance | undefined { + return renderer?.root.findAllByType(hosts.Field)[0] +} + +async function typeTitle(title: string): Promise { + act(() => titleField()?.props.onChangeText(title)) + await act(async () => { + await vi.advanceTimersByTimeAsync(1000) + }) +} + +// A kill: the page and its memory go, the store stays. +function kill(): void { + act(() => renderer?.unmount()) + renderer = null + resetDurableDraftFieldsForTests() +} + +beforeEach(() => { + vi.useFakeTimers() + host.seed.mockResolvedValue({ + prefill: { provider: 'github', base: 'main', title: '', body: '' }, + title: 'Seed from commits', + body: '', + base: 'main' + }) + publishInMemoryDeviceStore(table) +}) + +afterEach(() => { + kill() + table.clear() + publishPageDeviceStore(null) + host.seed.mockReset() + host.create.mockReset() + vi.useRealTimers() +}) + +it('brings typed fields back after the app is killed instead of the commit seed', async () => { + await open() + expect(titleField()?.props.value).toBe('Seed from commits') + await typeTitle('Fix the login race') + kill() + + await open() + expect(titleField()?.props.value).toBe('Fix the login race') +}) + +it('keeps nothing for the next Create once a Create lands after its sheet closed', async () => { + let land: (outcome: unknown) => void = () => {} + host.create.mockReturnValue(new Promise((resolve) => (land = resolve))) + await open() + await typeTitle('Fix the login race') + act(() => renderer?.root.findByType(hosts.Button).props.onPress()) + // A branch or client change closes the sheet while the Create is still running. + act(() => renderer?.unmount()) + await act(async () => { + land({ ok: true, url: 'https://example.test/pr/1', prefill: { provider: 'github' } }) + await vi.advanceTimersByTimeAsync(1000) + }) + kill() + + await open() + expect(titleField()?.props.value).toBe('Seed from commits') +}) diff --git a/mobile/src/drafts/document-edit-draft-rows.ts b/mobile/src/drafts/document-edit-draft-rows.ts index 81e3ea467a2..2dd2d91ed5e 100644 --- a/mobile/src/drafts/document-edit-draft-rows.ts +++ b/mobile/src/drafts/document-edit-draft-rows.ts @@ -1,11 +1,12 @@ import * as z from 'zod' import { durableDraftRow, - durableDraftRowKey, fitsDeviceStore, pageDraftRowBackend, resetDurableDraftRowsForTests, - type DurableDraftRow + workspaceDraftRowKey, + type DurableDraftRow, + type WorkspaceDraftScope } from './durable-draft-rows' /** Unsaved edits to a file or Markdown tab, with the hash of the host's version they were made against. */ @@ -23,19 +24,12 @@ type Slot = { } const slots = new Map() -/** The host and remote runtime a worktree lives on, so two hosts' same-named documents never share edits. */ -export type DocumentEditScope = { hostId: string; executionHost?: string; worktreeId: string } - -function scopedId(scope: DocumentEditScope, id: string): string { - return `${scope.hostId}:${scope.executionHost ?? ''}:${id}` +export function markdownTabDraftKey(scope: WorkspaceDraftScope, tabId: string): string { + return workspaceDraftRowKey('markdownTab', scope, tabId) } -export function markdownTabDraftKey(scope: DocumentEditScope, tabId: string): string { - return durableDraftRowKey('markdownTab', scope.worktreeId, scopedId(scope, tabId)) -} - -export function fileEditDraftKey(scope: DocumentEditScope, path: string): string { - return durableDraftRowKey('fileEdit', scope.worktreeId, scopedId(scope, path)) +export function fileEditDraftKey(scope: WorkspaceDraftScope, path: string): string { + return workspaceDraftRowKey('fileEdit', scope, path) } function slotFor(key: string): Slot { diff --git a/mobile/src/drafts/durable-draft-rows.ts b/mobile/src/drafts/durable-draft-rows.ts index 06be8f74839..0fed37a82ef 100644 --- a/mobile/src/drafts/durable-draft-rows.ts +++ b/mobile/src/drafts/durable-draft-rows.ts @@ -74,6 +74,11 @@ function fnv1a(text: string, seed: number): string { return hash.toString(16).padStart(8, '0') } +/** A 64-bit fingerprint of `text`, for keys and for telling whether text a draft was typed against changed. */ +export function draftTextHash(text: string): string { + return `${fnv1a(text, 0x811c9dc5)}${fnv1a(text, 0x01000193)}` +} + /** `draft:::`; the shell binds the host. */ export function durableDraftRowKey(namespace: string, worktreeId: string, id: string): string { const key = `draft:${namespace}:${worktreeId}:${id}` @@ -81,7 +86,22 @@ export function durableDraftRowKey(namespace: string, worktreeId: string, id: st return key } const rest = `${worktreeId}:${id}` - return `draft:${namespace}:#${fnv1a(rest, 0x811c9dc5)}${fnv1a(rest, 0x01000193)}` + return `draft:${namespace}:#${draftTextHash(rest)}` +} + +/** The host and remote runtime a worktree lives on, so two hosts' same-named worktrees never share a draft. */ +export type WorkspaceDraftScope = { hostId: string; executionHost?: string; worktreeId: string } + +export function workspaceDraftRowKey( + namespace: string, + scope: WorkspaceDraftScope, + id: string +): string { + return durableDraftRowKey( + namespace, + scope.worktreeId, + `${scope.hostId}:${scope.executionHost ?? ''}:${id}` + ) } export function deviceStoreDraftBackend( diff --git a/mobile/src/files/session-documents/MobileSessionFileReader.tsx b/mobile/src/files/session-documents/MobileSessionFileReader.tsx index 850c52c99a4..d87903a4fe1 100644 --- a/mobile/src/files/session-documents/MobileSessionFileReader.tsx +++ b/mobile/src/files/session-documents/MobileSessionFileReader.tsx @@ -35,6 +35,7 @@ import type { FileDocState, FileSyntaxState } from './session-document-state' import { DiffLineRow } from '../../source-control/review/MobileSessionDiffLineRow' import { sendableMobileDiffComments } from '../../source-control/review/mobile-diff-comments' import { MobileLoadErrorState, MobileStateView } from '../../components/MobileStateView' +import { useDurableDraftField } from '../../drafts/durable-draft-fields' type FileReaderProps = { doc: FileDocState | undefined @@ -44,6 +45,8 @@ type FileReaderProps = { documentId?: string language?: string diffCommentActions?: DiffCommentActions + // Where an unsaved note waits through leaving the file and the app being killed. + noteDraftKey?: string } export function FileReader(props: FileReaderProps) { @@ -58,7 +61,8 @@ function FileReaderDocument({ title, relativePath, language, - diffCommentActions + diffCommentActions, + noteDraftKey }: FileReaderProps) { useMobileColorScheme() const t = useMobileTranslation() @@ -69,7 +73,11 @@ function FileReaderDocument({ const [fileSyntax, setFileSyntax] = useState(null) const [diffSyntax, setDiffSyntax] = useState(null) const [activeCommentLine, setActiveCommentLine] = useState(null) - const [commentDraft, setCommentDraft] = useState('') + const [keptNote, setKeptNote] = useDurableDraftField(noteDraftKey ?? null, 'note') + const [localNote, setLocalNote] = useState('') + const [commentDraft, setCommentDraft] = noteDraftKey + ? [keptNote, setKeptNote] + : [localNote, setLocalNote] const plainDiffLines = useMemo( () => doc?.status === 'ready' && doc.kind === 'diff' @@ -105,10 +113,12 @@ function FileReaderDocument({ if (!diffCommentActions) { return } - void diffCommentActions.onAdd(relativePath, lineNumber, commentDraft).then((added) => { + const note = commentDraft + void diffCommentActions.onAdd(relativePath, lineNumber, note).then((added) => { if (added) { setActiveCommentLine(null) - setCommentDraft('') + // Only the saved text; a note typed since in a reopened reader stays. + setCommentDraft((current) => (current === note ? '' : current)) } }) }, diff --git a/mobile/src/session/MobileSessionActiveContent.tsx b/mobile/src/session/MobileSessionActiveContent.tsx index aee1d2a7bc2..06861569071 100644 --- a/mobile/src/session/MobileSessionActiveContent.tsx +++ b/mobile/src/session/MobileSessionActiveContent.tsx @@ -31,6 +31,7 @@ import { FileReader } from '../files/session-documents/MobileSessionFileReader' import { MobileSessionContentState } from './MobileSessionContentState' import { MarkdownReader } from '../files/session-documents/MobileSessionMarkdownReader' import { markdownTabDraftKey } from '../drafts/document-edit-draft-rows' +import { workspaceDraftRowKey } from '../drafts/durable-draft-rows' import { loadMarkdownPreviewImage } from '../files/mobile-markdown-image-source' export function MobileSessionActiveContent({ @@ -143,6 +144,9 @@ export function MobileSessionActiveContent({ controller ) const { presentedConnState, showOfflineState, forceReconnectHost } = controller + // Review notes are for working-tree diffs only. + const fileTabTakesNotes = + activeFileTab?.diffSource === 'staged' || activeFileTab?.diffSource === 'unstaged' const toast = toastMessage ? ( {toastMessage} @@ -195,8 +199,17 @@ export function MobileSessionActiveContent({ title={activeFileTab.title || t('session.activeContent.fileTitle', 'File')} relativePath={activeFileTab.relativePath} language={activeFileTab.language} + noteDraftKey={ + fileTabTakesNotes + ? workspaceDraftRowKey( + 'fileNote', + { hostId, executionHost, worktreeId }, + activeFileTab.relativePath + ) + : undefined + } diffCommentActions={ - activeFileTab.diffSource === 'staged' || activeFileTab.diffSource === 'unstaged' + fileTabTakesNotes ? { comments: diffComments, sendingCommentIds: sendingDiffCommentIds, diff --git a/mobile/src/source-control/review/use-mobile-diff-review-comment-actions.ts b/mobile/src/source-control/review/use-mobile-diff-review-comment-actions.ts index 8770531221a..a9d92e9dfd2 100644 --- a/mobile/src/source-control/review/use-mobile-diff-review-comment-actions.ts +++ b/mobile/src/source-control/review/use-mobile-diff-review-comment-actions.ts @@ -24,7 +24,7 @@ import { reviewDescriptorFromItem } from './mobile-diff-review-screen-model' import type { ReviewSheetIntents } from './mobile-diff-review-sheets' -import { useMobileDiffReviewNoteDraft } from './use-mobile-diff-review-note-draft' +import type { MobileDiffReviewNoteDraft } from './use-mobile-diff-review-note-draft' type CommentActionsInput = { client: RpcClient | null @@ -43,9 +43,12 @@ type CommentActionsInput = { navigationRef: { readonly current: number } /** GitHub's viewed mark for a PR file, or null when the file's viewed state lives on the phone. */ remoteViewed: (item: MobileDiffReviewQueueItem, viewed: boolean) => Promise | null - setComposerBody: Dispatch> + noteDraft: Pick< + MobileDiffReviewNoteDraft, + 'openNewNoteSheet' | 'openEditComposer' | 'clearSavedNote' + > setActionError: Dispatch> - sheets: Pick + sheets: Pick } export function useMobileDiffReviewCommentActions(input: CommentActionsInput) { @@ -64,11 +67,12 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) { selectIndex, navigationRef, remoteViewed, - setComposerBody, + noteDraft, setActionError, sheets } = input - const { openSheet, openSheetWhenIdle, closeSheet } = sheets + const { openSheetWhenIdle, closeSheet } = sheets + const { openNewNoteSheet, openEditComposer, clearSavedNote } = noteDraft const persistMetadata = useCallback( async ( @@ -147,12 +151,6 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) { [persistMetadata, screenState, setActionError, setScreenState, updateReadyState, worktreeId] ) - const { openNewNoteSheet, openEditComposer, clearSavedNote } = useMobileDiffReviewNoteDraft({ - composerBody, - setComposerBody, - openSheet - }) - const openComposer = useCallback( (lineNumber: number) => openNewNoteSheet({ kind: 'composer', composer: { mode: 'create', lineNumber } }), diff --git a/mobile/src/source-control/review/use-mobile-diff-review-controller.test.ts b/mobile/src/source-control/review/use-mobile-diff-review-controller.test.ts index 4b951d0b466..7bcd5c31744 100644 --- a/mobile/src/source-control/review/use-mobile-diff-review-controller.test.ts +++ b/mobile/src/source-control/review/use-mobile-diff-review-controller.test.ts @@ -6,6 +6,9 @@ import { createFakeRpcClient } from '../../mobile-web-shell/bridge-host-test-fak import type { ReviewScreenState } from './mobile-diff-review-screen-model' import { useMobileDiffReviewController } from './use-mobile-diff-review-controller' import { documentQueryClient } from '../../test-support/rpc-recording/hook-mount' +import { publishPageDeviceStore } from '../../mobile-web-shell/bridge/page-device-store' +import { resetDurableDraftFieldsForTests } from '../../drafts/durable-draft-fields' +import { publishInMemoryDeviceStore } from '../../drafts/in-memory-device-store.test-support' const loadSnapshot = vi.hoisted(() => vi.fn()) // Each build's notes-and-files read; status comes from the shared git.status query, held pending here. @@ -92,6 +95,8 @@ describe('useMobileDiffReviewController', () => { afterEach(() => { act(() => renderer?.unmount()) renderer = null + resetDurableDraftFieldsForTests() + publishPageDeviceStore(null) }) describe('Stage · next file', () => { @@ -204,5 +209,44 @@ describe('useMobileDiffReviewController', () => { act(() => controller?.openComposer(3)) expect(controller?.composerBody).toBe('') }) + + it('brings an unsaved new note back after the app is killed, not the text of an edit', async () => { + // The shell's draft table outlives the page; nothing else does. + const table = new Map() + publishInMemoryDeviceStore(table) + const openReview = async () => { + loadSnapshot.mockResolvedValueOnce(unstaged(['c.ts'])) + await act(async () => { + renderer = create(queries.wrap(createElement(StageProbe))) + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + } + await openReview() + act(() => controller?.openComposer(3)) + act(() => controller?.setComposerBody('rename this')) + act(() => controller?.closeComposer()) + act(() => + controller?.openEditComposer({ + id: 'saved', + worktreeId: 'wt-1', + filePath: 'c.ts', + lineNumber: 1, + side: 'modified', + body: 'older note', + createdAt: 1 + }) + ) + act(() => controller?.setComposerBody('older note, edited')) + // The draft row saves once typing settles. + await act(async () => { + await vi.waitFor(() => expect(table.size).toBe(1)) + }) + act(() => renderer?.unmount()) + resetDurableDraftFieldsForTests() + + await openReview() + act(() => controller?.openComposer(3)) + expect(controller?.composerBody).toBe('rename this') + }) }) }) diff --git a/mobile/src/source-control/review/use-mobile-diff-review-controller.ts b/mobile/src/source-control/review/use-mobile-diff-review-controller.ts index 6b7e9501774..8ee31adcf5d 100644 --- a/mobile/src/source-control/review/use-mobile-diff-review-controller.ts +++ b/mobile/src/source-control/review/use-mobile-diff-review-controller.ts @@ -27,6 +27,9 @@ import { useMobilePrSidebarController } from './use-mobile-pr-sidebar-controller import { useMobileDiffReviewGitHub } from './use-mobile-diff-review-github' import { mobileDiffReviewThreadsForItem } from './mobile-diff-review-github' import { mobileDiffReviewNotesForItem } from './mobile-diff-comments' +import { useMobileDiffReviewNoteDraft } from './use-mobile-diff-review-note-draft' +import { workspaceDraftRowKey } from '../../drafts/durable-draft-rows' +import { useWorkspaceExecutionHost } from '../../navigation/workspace-execution-host' const NO_COMMENTS: readonly DiffComment[] = [] @@ -74,7 +77,12 @@ export function useMobileDiffReviewController(input: ControllerInput) { const [sheets, dispatchSheets] = useReducer(reduceReviewSheets, NO_REVIEW_SHEETS) const sheetIntents = useMemo(() => reviewSheetIntents(dispatchSheets), []) const composer = reviewComposer(sheets) - const [composerBody, setComposerBody] = useState('') + const executionHost = useWorkspaceExecutionHost() + const noteDraft = useMobileDiffReviewNoteDraft({ + draftKey: workspaceDraftRowKey('reviewNote', { hostId, executionHost, worktreeId }, ''), + openSheet: sheetIntents.openSheet + }) + const { composerBody, setComposerBody } = noteDraft const [actionError, setActionError] = useState(null) const [busyAction, setBusyAction] = useState(null) const worktreeLabel = getWorktreeLabel(name, worktreeId) @@ -182,7 +190,7 @@ export function useMobileDiffReviewController(input: ControllerInput) { selectKey, moveSelection, remoteViewed: setRemoteViewed, - setComposerBody, + noteDraft, setActionError, setBusyAction, sheets: sheetIntents, diff --git a/mobile/src/source-control/review/use-mobile-diff-review-interactions.ts b/mobile/src/source-control/review/use-mobile-diff-review-interactions.ts index 6176651f97d..10bbfc7a0d6 100644 --- a/mobile/src/source-control/review/use-mobile-diff-review-interactions.ts +++ b/mobile/src/source-control/review/use-mobile-diff-review-interactions.ts @@ -6,6 +6,7 @@ import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue' import type { ComposerState, ReviewScreenState } from './mobile-diff-review-screen-model' import type { ReviewSheet, ReviewSheetIntents } from './mobile-diff-review-sheets' import type { MobileReviewTerminalTab } from './review-terminal-reply-schema' +import type { MobileDiffReviewNoteDraft } from './use-mobile-diff-review-note-draft' import { sourceFileDiffOpenRun } from '../mobile-source-file-open-operations' import { refusedRpcMessageOrFallback } from '../../transport/rpc/rpc-refusal-message' import { useMobileDiffReviewCommentActions } from './use-mobile-diff-review-comment-actions' @@ -33,7 +34,7 @@ type InteractionInput = { selectKey: (key: string) => void moveSelection: (direction: 'next' | 'previous') => void remoteViewed: (item: MobileDiffReviewQueueItem, viewed: boolean) => Promise | null - setComposerBody: Dispatch> + noteDraft: MobileDiffReviewNoteDraft setActionError: Dispatch> setBusyAction: Dispatch> sheets: ReviewSheetIntents @@ -61,7 +62,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) { selectKey, moveSelection, remoteViewed, - setComposerBody, + noteDraft, setActionError, setBusyAction, sheets, @@ -99,7 +100,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) { selectIndex, navigationRef, remoteViewed, - setComposerBody, + noteDraft, setActionError, sheets }) diff --git a/mobile/src/source-control/review/use-mobile-diff-review-note-draft.ts b/mobile/src/source-control/review/use-mobile-diff-review-note-draft.ts index 2a04dc98a06..cb50996f0ca 100644 --- a/mobile/src/source-control/review/use-mobile-diff-review-note-draft.ts +++ b/mobile/src/source-control/review/use-mobile-diff-review-note-draft.ts @@ -1,10 +1,11 @@ -import { useCallback, useRef, type Dispatch, type SetStateAction } from 'react' +import { useCallback, useState, type SetStateAction } from 'react' import type { DiffComment } from '../../../../src/shared/diff-comment-types' +import { useDurableDraftField } from '../../drafts/durable-draft-fields' import type { ReviewSheet, ReviewSheetIntents } from './mobile-diff-review-sheets' type NoteDraftInput = { - composerBody: string - setComposerBody: Dispatch> + // Where the new note waits through leaving the review and the app being killed. + draftKey: string openSheet: ReviewSheetIntents['openSheet'] } @@ -13,45 +14,46 @@ type NewNoteSheet = | { kind: 'composer'; composer: { mode: 'create'; lineNumber: number } } /** The review's unsaved new note: kept across Cancel, reopen and picking other lines, cleared only once saved. */ -export function useMobileDiffReviewNoteDraft({ - composerBody, - setComposerBody, - openSheet -}: NoteDraftInput) { - // Set while an edit borrows the text field; the new note comes back when a new-note sheet opens. - const setAsideNoteRef = useRef(null) +export function useMobileDiffReviewNoteDraft({ draftKey, openSheet }: NoteDraftInput) { + const [newNote, setNewNote] = useDurableDraftField(draftKey, 'note') + // An edit borrows the text field with its own text, until a new-note sheet opens; the new note is untouched + // underneath. Kept past the edit sheet closing, so its text stays while it slides away. + const [editBody, setEditBody] = useState(null) + const setEditText = useCallback( + (next: SetStateAction) => + setEditBody((current) => (typeof next === 'function' ? next(current ?? '') : next)), + [] + ) + const openNewNoteSheet = useCallback( (sheet: NewNoteSheet) => { - if (setAsideNoteRef.current !== null) { - setComposerBody(setAsideNoteRef.current) - setAsideNoteRef.current = null - } + setEditBody(null) openSheet(sheet) }, - [openSheet, setComposerBody] + [openSheet] ) // The user may have moved on while a note was saving; drop only the text that was saved. const clearSavedNote = useCallback( - (saved: string) => { - const isSaved = (text: string) => text.trim() === saved.trim() - if (setAsideNoteRef.current === null) { - setComposerBody((current) => (isSaved(current) ? '' : current)) - } else if (isSaved(setAsideNoteRef.current)) { - setAsideNoteRef.current = '' - } - }, - [setComposerBody] + (saved: string) => setNewNote((current) => (current.trim() === saved.trim() ? '' : current)), + [setNewNote] ) const openEditComposer = useCallback( (comment: DiffComment) => { - setAsideNoteRef.current ??= composerBody openSheet({ kind: 'composer', composer: { mode: 'edit', comment } }) - setComposerBody(comment.body) + setEditBody(comment.body) }, - [composerBody, openSheet, setComposerBody] + [openSheet] ) - return { openNewNoteSheet, openEditComposer, clearSavedNote } + return { + composerBody: editBody ?? newNote, + setComposerBody: editBody === null ? setNewNote : setEditText, + openNewNoteSheet, + openEditComposer, + clearSavedNote + } } + +export type MobileDiffReviewNoteDraft = ReturnType diff --git a/mobile/src/source-control/review/use-mobile-pr-comment-actions.ts b/mobile/src/source-control/review/use-mobile-pr-comment-actions.ts index 6df40877f65..d1f83cd1e96 100644 --- a/mobile/src/source-control/review/use-mobile-pr-comment-actions.ts +++ b/mobile/src/source-control/review/use-mobile-pr-comment-actions.ts @@ -15,6 +15,8 @@ import { } from './pr-comment-actions' import { prCommentTarget, prCommentWrite, type PrCommentWrite } from './pr-comment-writes' import type { PrQueryScope } from './mobile-pr-queries' +import { workspaceDraftRowKey } from '../../drafts/durable-draft-rows' +import { useWorkspaceExecutionHost } from '../../navigation/workspace-execution-host' export type PrCommentActionsInput = { hostId: string @@ -52,6 +54,22 @@ export function useMobilePrCommentActions(input: PrCommentActionsInput) { const { mutate, blockedOffline } = useHostMutation(prCommentWrite, { hostId, client }) const ready = client !== null && connState === 'connected' && !blockedOffline const target = useMemo(() => prCommentTarget(queries), [queries]) + const executionHost = useWorkspaceExecutionHost() + // A fork's #N and upstream #N keep separate drafts; null is a new root comment. + const draftKey = useCallback( + (replyTo: number | null): string | null => { + if (prNumber <= 0) { + return null + } + const repo = prRepo ? `${prRepo.host ?? ''}/${prRepo.owner}/${prRepo.repo}` : '' + return workspaceDraftRowKey( + 'prComment', + { hostId, executionHost, worktreeId }, + `${repo}#${prNumber}#${replyTo ?? 'new'}` + ) + }, + [executionHost, hostId, prNumber, prRepo, worktreeId] + ) const setBusy = useCallback((key: string, busy: boolean) => { setBusyKeys((prev) => { @@ -160,6 +178,7 @@ export function useMobilePrCommentActions(input: PrCommentActionsInput) { [busyKeys] ), isRootBusy: busyKeys.has(ROOT_KEY), + draftKey, reply, toggleResolve, addRootComment, diff --git a/mobile/src/source-control/use-mobile-source-control-commit-runners.ts b/mobile/src/source-control/use-mobile-source-control-commit-runners.ts index 5a9b6fbcc05..48bcfadf5bd 100644 --- a/mobile/src/source-control/use-mobile-source-control-commit-runners.ts +++ b/mobile/src/source-control/use-mobile-source-control-commit-runners.ts @@ -1,4 +1,4 @@ -import { useCallback, type MutableRefObject } from 'react' +import { useCallback, type Dispatch, type MutableRefObject, type SetStateAction } from 'react' import { translate } from '../i18n/mobile-i18n' import { triggerError, triggerSuccess } from '../platform/haptics' import type { LoadStatusOptions } from './mobile-source-control-screen-state' @@ -12,7 +12,8 @@ type SendGitRequest = (method: string, params?: Record) => P type RunGitWorkflow = ( actionId: string, runner: () => Promise, - options?: { clearCommitMessage?: boolean } + // The committed message; the field is cleared only while it still holds that text. + options?: { clearCommitMessage?: string } ) => Promise type Params = { @@ -28,7 +29,7 @@ type Params = { busyActionRef: MutableRefObject setBusyAction: (next: string | null) => void setActionError: (next: string | null) => void - setCommitMessage: (next: string) => void + setCommitMessage: Dispatch> recordCommitFailure: RecordMobileCommitFailure } @@ -80,7 +81,7 @@ export function useMobileSourceControlCommitRunners(params: Params) { throw err } }, - { clearCommitMessage: true } + { clearCommitMessage: message } ) }, [commitMessage, recordCommitFailure, runGitWorkflow, sendCommitRequest, stagedEntries]) @@ -101,11 +102,12 @@ export function useMobileSourceControlCommitRunners(params: Params) { try { await sendCommitRequest(message) didCommit = true + // The commit landed: a kill during the push must not bring its message back, and text typed since stays. + setCommitMessage((current) => (current.trim() === message ? '' : current)) await afterCommit() if (!mountedRef.current) { return false } - setCommitMessage('') triggerSuccess() await loadStatus({ force: true }) return true @@ -122,7 +124,6 @@ export function useMobileSourceControlCommitRunners(params: Params) { recordCommitFailure({ error: errorMessage, commitMessage: message, stagedEntries }) } if (didCommit) { - setCommitMessage('') await loadStatus({ clearActionErrorOnSuccess: false, force: true diff --git a/mobile/src/source-control/use-mobile-source-control-runners.ts b/mobile/src/source-control/use-mobile-source-control-runners.ts index 4fc63f59aba..956899af2a6 100644 --- a/mobile/src/source-control/use-mobile-source-control-runners.ts +++ b/mobile/src/source-control/use-mobile-source-control-runners.ts @@ -83,7 +83,7 @@ export function useMobileSourceControlRunners(params: Params) { async ( actionId: string, runner: () => Promise, - options?: { clearCommitMessage?: boolean; refreshOnError?: boolean } + options?: { clearCommitMessage?: string; refreshOnError?: boolean } ) => { if (busyActionRef.current) { return false @@ -94,12 +94,14 @@ export function useMobileSourceControlRunners(params: Params) { recordCommitFailure(null) try { await runner() + // The commit landed, so its message must not come back, even when the screen has left. + const committed = options?.clearCommitMessage + if (committed !== undefined) { + setCommitMessage((current) => (current.trim() === committed ? '' : current)) + } if (!mountedRef.current) { return false } - if (options?.clearCommitMessage) { - setCommitMessage('') - } triggerSuccess() await loadStatus({ force: true }) return true @@ -148,7 +150,7 @@ export function useMobileSourceControlRunners(params: Params) { ) const runGitSequence = useCallback( - async (actionId: string, steps: GitStep[], options?: { clearCommitMessage?: boolean }) => { + async (actionId: string, steps: GitStep[], options?: { clearCommitMessage?: string }) => { return await runGitWorkflow( actionId, async () => { diff --git a/mobile/src/source-control/use-mobile-source-control-state.ts b/mobile/src/source-control/use-mobile-source-control-state.ts index 168fa193856..7b1a96b3edd 100644 --- a/mobile/src/source-control/use-mobile-source-control-state.ts +++ b/mobile/src/source-control/use-mobile-source-control-state.ts @@ -4,6 +4,9 @@ import { useSafeAreaInsets } from 'react-native-safe-area-context' import { useForceReconnect } from '../transport/connection/host-client-hooks' import { useWorkspaceClient } from '../worktree/use-workspace-client' import { getWorktreeLabel } from '../worktree/worktree-label' +import { workspaceDraftRowKey } from '../drafts/durable-draft-rows' +import { useDurableDraftField } from '../drafts/durable-draft-fields' +import { useWorkspaceExecutionHost } from '../navigation/workspace-execution-host' import { useMobileGitRequests } from './use-mobile-git-requests' import { useMobileSourceControlLoaders } from './use-mobile-source-control-loaders' import { useMobileSourceControlOpeners } from './use-mobile-source-control-openers' @@ -48,7 +51,12 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara const { client, state: connState } = useWorkspaceClient(hostId) const forceReconnect = useForceReconnect() const [busyAction, setBusyAction] = useState(null) - const [commitMessage, setCommitMessage] = useState('') + const executionHost = useWorkspaceExecutionHost() + // The message waits through leaving the screen and the app being killed; a commit clears it. + const [commitMessage, setCommitMessage] = useDurableDraftField( + workspaceDraftRowKey('commit', { hostId, executionHost, worktreeId }, ''), + 'message' + ) const [generatingMessage, setGeneratingMessage] = useState(false) const [showBranchPicker, setShowBranchPicker] = useState(false) const [localBranches, setLocalBranches] = useState(null) diff --git a/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts b/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts index 60d9007938d..28944cd6c22 100644 --- a/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts +++ b/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts @@ -106,7 +106,13 @@ export function diffReviewActionMountAdapters( moveSelection: () => {}, // Viewed state stays on the phone here; GitHub-viewed files have their own recordings. remoteViewed: () => null, - setComposerBody: () => {}, + noteDraft: { + composerBody: '', + setComposerBody: () => {}, + openNewNoteSheet: () => {}, + openEditComposer: () => {}, + clearSavedNote: () => {} + }, setActionError: (update) => { actionError = typeof update === 'function' ? update(actionError) : update },