fix(mobile): stop clearing review and file notes on reopen (#38917)

* fix(mobile): stop clearing review and file notes on reopen

* test(mobile): type the edited note fixture
This commit is contained in:
Kelvin Amoaba
2026-11-06 16:44:29 +00:00
committed by GitHub
parent 612ec92a3a
commit 7151bb61e4
7 changed files with 128 additions and 34 deletions
@@ -79,7 +79,8 @@ export function MobileDiffReviewRangeSheet({
.commentOnGitHub({ item, startLine: range.startLine, line: range.line, body })
.then((posted) => {
if (posted.ok) {
// Only this sheet; one opened since keeps its own draft.
controller.clearSavedNote(body)
// Only this sheet; text typed since stays.
controller.closeSheet('range', range)
} else {
setPostError(posted.error)
@@ -141,10 +141,7 @@ export function MobileDiffReviewScreenView({ controller, hostConnection, onBack
threadsByLine={controller.threadsByLine}
threadActions={threadActions}
selectedRange={selectedRange}
onPickLines={(range) => {
controller.setComposerBody('')
controller.openSheet({ kind: 'range', ...range })
}}
onPickLines={(range) => controller.openNewNoteSheet({ kind: 'range', ...range })}
scrollToLine={controller.scrollToLine}
onScrolledToLine={controller.clearScrollToLine}
onEditNote={controller.openEditComposer}
+3 -11
View File
@@ -98,15 +98,8 @@ function FileReaderDocument({
return map
}, [diffCommentsForFile])
const startComment = useCallback((lineNumber: number) => {
setActiveCommentLine(lineNumber)
setCommentDraft('')
}, [])
const cancelComment = useCallback(() => {
setActiveCommentLine(null)
setCommentDraft('')
}, [])
// The draft follows the reader to another line and survives Cancel; only a saved note clears it.
const cancelComment = useCallback(() => setActiveCommentLine(null), [])
const submitComment = useCallback(
(lineNumber: number) => {
@@ -135,7 +128,7 @@ function FileReaderDocument({
activeCommentLine={activeCommentLine}
commentDraft={commentDraft}
commentsBusy={diffCommentActions?.busy === true}
onStartComment={startComment}
onStartComment={setActiveCommentLine}
onCancelComment={cancelComment}
onDraftChange={setCommentDraft}
onSubmitComment={submitComment}
@@ -152,7 +145,6 @@ function FileReaderDocument({
commentDraft,
diffCommentActions,
diffCommentsByLine,
startComment,
submitComment,
title
]
@@ -24,6 +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'
type CommentActionsInput = {
client: RpcClient | null
@@ -146,26 +147,19 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
[persistMetadata, screenState, setActionError, setScreenState, updateReadyState, worktreeId]
)
const { openNewNoteSheet, openEditComposer, clearSavedNote } = useMobileDiffReviewNoteDraft({
composerBody,
setComposerBody,
openSheet
})
const openComposer = useCallback(
(lineNumber: number) => {
openSheet({ kind: 'composer', composer: { mode: 'create', lineNumber } })
setComposerBody('')
},
[openSheet, setComposerBody]
(lineNumber: number) =>
openNewNoteSheet({ kind: 'composer', composer: { mode: 'create', lineNumber } }),
[openNewNoteSheet]
)
const openEditComposer = useCallback(
(comment: DiffComment) => {
openSheet({ kind: 'composer', composer: { mode: 'edit', comment } })
setComposerBody(comment.body)
},
[openSheet, setComposerBody]
)
const closeComposer = useCallback(() => {
closeSheet('composer')
setComposerBody('')
}, [closeSheet, setComposerBody])
const closeComposer = useCallback(() => closeSheet('composer'), [closeSheet])
const saveComposer = useCallback(async () => {
if (
@@ -200,7 +194,11 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
}
await saveCommentsAndReviewState(result.comments, screenState.reviewState)
closeComposer()
if (composer.mode === 'create') {
clearSavedNote(composerBody)
}
}, [
clearSavedNote,
closeComposer,
composer,
composerBody,
@@ -294,11 +292,13 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
)
return {
clearSavedNote,
closeComposer,
deleteComment,
markReviewed,
openComposer,
openEditComposer,
openNewNoteSheet,
saveCommentsAndReviewState,
saveComposer,
toggleItemViewed
@@ -222,6 +222,48 @@ describe('useMobileDiffReviewController', () => {
await stageCurrent(() => controller?.moveFile('previous'))
expect(controller?.currentItem?.filePath).toBe('b.ts')
})
it('keeps an unsaved note across reopen and edits, and clears it once saved', async () => {
loadSnapshot.mockResolvedValueOnce(unstaged(['c.ts']))
await act(async () => {
renderer = create(createElement(StageProbe))
await Promise.resolve()
})
act(() => controller?.openComposer(3))
act(() => controller?.setComposerBody('rename this'))
act(() => controller?.closeComposer())
act(() => controller?.openNewNoteSheet({ kind: 'range', startLine: 5, line: 7 }))
expect(controller?.composerBody).toBe('rename this')
act(() => controller?.closeSheet('range'))
act(() =>
controller?.openEditComposer({
id: 'saved',
worktreeId: 'wt-1',
filePath: 'c.ts',
lineNumber: 1,
side: 'modified',
body: 'older note',
createdAt: 1
})
)
expect(controller?.composerBody).toBe('older note')
act(() => controller?.closeComposer())
act(() => controller?.openComposer(3))
expect(controller?.composerBody).toBe('rename this')
let saving: Promise<void> | undefined
act(() => {
saving = controller?.saveComposer()
})
const save = client.requests.find((request) => request.method === 'worktree.set')
await act(async () => {
save?.resolve({ id: 'save', ok: true, result: {}, _meta: { runtimeId: 'r' } })
await saving
})
act(() => controller?.openComposer(3))
expect(controller?.composerBody).toBe('')
})
})
})
@@ -74,11 +74,13 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
const navigationRef = useRef(0)
const {
clearSavedNote,
closeComposer,
deleteComment,
markReviewed,
openComposer,
openEditComposer,
openNewNoteSheet,
saveCommentsAndReviewState,
saveComposer,
toggleItemViewed
@@ -139,6 +141,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
})
return {
clearSavedNote,
clearSentNotes,
closeComposer,
copyNotes,
@@ -155,6 +158,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
},
openComposer,
openEditComposer,
openNewNoteSheet,
openInSession: async () => {
// Only working-tree files have a desktop diff tab.
if (
@@ -193,7 +197,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
saveComposer,
sendPromptToTerminal,
loadTerminals,
// The sheet's own lines and the terminals it already listed; the draft clears when the next sheet opens.
// The sheet's own lines and the terminals it already listed; the draft clears once the note is saved.
sendRange: async (
sheet: Extract<ReviewSheet, { kind: 'range' }>,
terminals?: readonly MobileReviewTerminalTab[]
@@ -202,6 +206,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
if (!('comment' in saved)) {
return saved.error
}
clearSavedNote(saved.comment.body)
sheets.closeSheet('range', sheet)
await sendNoteNow(saved.comment, terminals)
return null
@@ -0,0 +1,57 @@
import { useCallback, useRef, type Dispatch, type SetStateAction } from 'react'
import type { DiffComment } from '../../../src/shared/diff-comment-types'
import type { ReviewSheet, ReviewSheetIntents } from './mobile-diff-review-sheets'
type NoteDraftInput = {
composerBody: string
setComposerBody: Dispatch<SetStateAction<string>>
openSheet: ReviewSheetIntents['openSheet']
}
type NewNoteSheet =
| Extract<ReviewSheet, { kind: 'range' }>
| { 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<string | null>(null)
const openNewNoteSheet = useCallback(
(sheet: NewNoteSheet) => {
if (setAsideNoteRef.current !== null) {
setComposerBody(setAsideNoteRef.current)
setAsideNoteRef.current = null
}
openSheet(sheet)
},
[openSheet, setComposerBody]
)
// 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]
)
const openEditComposer = useCallback(
(comment: DiffComment) => {
setAsideNoteRef.current ??= composerBody
openSheet({ kind: 'composer', composer: { mode: 'edit', comment } })
setComposerBody(comment.body)
},
[composerBody, openSheet, setComposerBody]
)
return { openNewNoteSheet, openEditComposer, clearSavedNote }
}