diff --git a/apps/mobile/src/features/review/ReviewCommentComposerSheet.tsx b/apps/mobile/src/features/review/ReviewCommentComposerSheet.tsx index 74ccc8cf0bcb..4bae4ef4b8ac 100644 --- a/apps/mobile/src/features/review/ReviewCommentComposerSheet.tsx +++ b/apps/mobile/src/features/review/ReviewCommentComposerSheet.tsx @@ -22,7 +22,7 @@ import { formatReviewCommentContext, getReviewUnifiedLineNumber, getSelectedReviewCommentLines, - useReviewCommentTarget, + getReviewCommentTarget, } from "./reviewCommentSelection"; import { useAppearanceCodeSurface } from "../settings/appearance/useAppearanceCodeSurface"; import { useAppearancePreferences } from "../settings/appearance/AppearancePreferencesProvider"; @@ -32,6 +32,9 @@ import { type ReviewHighlightedToken, } from "./shikiReviewHighlighter"; +import { useReviewCommentDismissal } from "./useReviewCommentDismissal"; +import { useReviewCommentSubmission } from "./useReviewCommentSubmission"; + const REVIEW_COMMENT_PREVIEW_MAX_LINES = 5; type ReviewCommentComposerSheetProps = StaticScreenProps<{ @@ -45,14 +48,18 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp const insets = useSafeAreaInsets(); const { width } = useWindowDimensions(); const { themeAppearance: selectedTheme } = useAppearancePreferences(); - const target = useReviewCommentTarget(); + // Freeze the target and destination together for this editor instance. + const [{ target, environmentId, threadId }] = useState(() => ({ + target: getReviewCommentTarget(), + ...props.route.params, + })); const { codeSurface } = useAppearanceCodeSurface(); - const { environmentId, threadId } = props.route.params; const [commentText, setCommentText] = useState(""); const [highlightedLinesById, setHighlightedLinesById] = useState< Record> >({}); const [attachments, setAttachments] = useState>([]); + const [pendingImages, setPendingImages] = useState(0); const [previewFile, setPreviewFile] = useState(null); const selectedLines = useMemo( @@ -78,11 +85,25 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp codeSurface.rowHeight, ); const previewViewportWidth = Math.max(width - 40, 280); - const dismissComposer = useCallback(() => { - clearReviewCommentTarget(); - navigation.goBack(); - }, [navigation]); + const { submitted, submit, accepted } = useReviewCommentSubmission(); + useReviewCommentDismissal({ + commentText, + attachmentCount: attachments.length, + pendingImages, + submitted, + accepted, + }); + useEffect( + () => () => { + // A newer selection belongs to another editor, even if this route is removed later. + if (getReviewCommentTarget() === target) clearReviewCommentTarget(); + }, + [target], + ); + const dismissComposer = useCallback(() => navigation.goBack(), [navigation]); const handleNativePaste = useNativePaste((uris) => { + if (submitted) return; + setPendingImages((count) => count + 1); void (async () => { try { const images = await convertPastedImagesToAttachments({ @@ -94,6 +115,8 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp } } catch (error) { console.error("[review comment] error converting pasted images", error); + } finally { + setPendingImages((count) => count - 1); } })(); }); @@ -127,12 +150,18 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp }, [selectedLines, selectedTheme, target]); async function handlePickImages(): Promise { - const result = await pickComposerImages({ existingCount: attachments.length }); - if (result.images.length > 0) { - setAttachments((current) => [...current, ...result.images]); - } - if (result.error) { - setPendingConnectionError(result.error); + if (submitted) return; + setPendingImages((count) => count + 1); + try { + const result = await pickComposerImages({ existingCount: attachments.length }); + if (result.images.length > 0) { + setAttachments((current) => [...current, ...result.images]); + } + if (result.error) { + setPendingConnectionError(result.error); + } + } finally { + setPendingImages((count) => count - 1); } } @@ -141,15 +170,20 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp return; } - appendReviewCommentToDraft({ - environmentId, - threadId, - text: formatReviewCommentContext(target, commentText), - attachments, - }); - setAttachments([]); - dismissComposer(); - }, [attachments, commentText, dismissComposer, environmentId, target, threadId]); + if (submitted || pendingImages > 0) return; + try { + submit(() => + appendReviewCommentToDraft({ + environmentId, + threadId, + text: formatReviewCommentContext(target, commentText), + attachments, + }), + ); + } catch { + setPendingConnectionError("Could not add the comment. Your input has been kept; try again."); + } + }, [attachments, commentText, environmentId, pendingImages, submit, submitted, target, threadId]); return ( @@ -256,6 +290,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp 0} icon="plus" onPress={() => void handlePickImages()} /> @@ -300,7 +336,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp icon="arrow.up" label="Comment" variant="primary" - disabled={!canSubmit} + disabled={!canSubmit || submitted || pendingImages > 0} onPress={handleSubmit} /> @@ -317,6 +353,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp > 0} icon="plus" onPress={() => void handlePickImages()} /> @@ -326,7 +363,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp icon="arrow.up" label="Comment" variant="primary" - disabled={!canSubmit} + disabled={!canSubmit || submitted || pendingImages > 0} onPress={handleSubmit} /> diff --git a/apps/mobile/src/features/review/useReviewCommentDismissal.test.ts b/apps/mobile/src/features/review/useReviewCommentDismissal.test.ts new file mode 100644 index 000000000000..d62e32bb8a92 --- /dev/null +++ b/apps/mobile/src/features/review/useReviewCommentDismissal.test.ts @@ -0,0 +1,117 @@ +import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import type { NavigationAction } from "@react-navigation/native"; +import type { AlertButton } from "react-native"; + +const harness = vi.hoisted(() => ({ + prevented: false, + onRemove: (_event: { data: { action: NavigationAction } }) => {}, + confirmation: { current: false }, + effects: [] as Array<() => unknown>, + buttons: [] as AlertButton[], + dispatch: vi.fn(), + goBack: vi.fn(), + alert: vi.fn(), +})); +vi.mock("react", () => ({ + useRef: () => harness.confirmation, + useEffect: (effect: () => unknown) => harness.effects.push(effect), +})); +vi.mock("@react-navigation/native", () => ({ + useNavigation: () => ({ dispatch: harness.dispatch, goBack: harness.goBack }), + usePreventRemove: (prevented: boolean, callback: typeof harness.onRemove) => { + harness.prevented = prevented; + harness.onRemove = callback; + }, +})); +vi.mock("react-native", () => ({ + Alert: { + alert: (...args: [string, string, AlertButton[]]) => { + harness.alert(...args); + harness.buttons = args[2]; + }, + }, +})); + +import { useReviewCommentDismissal } from "./useReviewCommentDismissal"; + +function render(overrides: Partial[0]> = {}) { + useReviewCommentDismissal({ + commentText: "", + attachmentCount: 0, + pendingImages: 0, + submitted: false, + accepted: { current: false }, + ...overrides, + }); +} +const action: NavigationAction = { type: "GO_BACK", source: "review-editor" }; +function remove() { + if (harness.prevented) harness.onRemove({ data: { action } }); +} +function choose(text: string) { + harness.buttons.find((button) => button.text === text)?.onPress?.(); +} + +beforeEach(() => { + vi.clearAllMocks(); + harness.confirmation.current = false; + harness.buttons = []; + harness.effects = []; +}); + +describe("review comment removal boundary", () => { + it("allows an empty editor to leave without confirmation", () => { + render(); + remove(); + expect(harness.prevented).toBe(false); + expect(harness.alert).not.toHaveBeenCalled(); + }); + it.each([{ commentText: "Unsent" }, { attachmentCount: 1 }])( + "protects invested input %j", + (input) => { + render(input); + remove(); + remove(); + expect(harness.prevented).toBe(true); + expect(harness.alert).toHaveBeenCalledTimes(1); + choose("Keep editing"); + expect(harness.dispatch).not.toHaveBeenCalled(); + remove(); + expect(harness.alert).toHaveBeenCalledTimes(2); + choose("Discard"); + expect(harness.dispatch).toHaveBeenCalledExactlyOnceWith(action); + }, + ); + it("keeps the sheet protected after a failed transfer", () => { + render({ commentText: "Unsent", attachmentCount: 1, submitted: false }); + remove(); + expect(harness.prevented).toBe(true); + expect(harness.alert).toHaveBeenCalledOnce(); + expect(harness.goBack).not.toHaveBeenCalled(); + }); + it("blocks removal while selected images are still being prepared", () => { + render({ pendingImages: 1 }); + remove(); + expect(harness.prevented).toBe(true); + expect(harness.alert).not.toHaveBeenCalled(); + expect(harness.dispatch).not.toHaveBeenCalled(); + }); + it("unlocks successful transfer without a discard prompt", () => { + render({ commentText: "Transferred", attachmentCount: 1, submitted: true }); + harness.effects.forEach((effect) => effect()); + expect(harness.goBack).toHaveBeenCalledOnce(); + remove(); + expect(harness.prevented).toBe(false); + expect(harness.alert).not.toHaveBeenCalled(); + }); +}); + +it("allows removal immediately after transfer, before the guard's render catches up", () => { + const accepted = { current: false }; + render({ commentText: "Transferred", accepted }); + expect(harness.prevented).toBe(true); + accepted.current = true; + remove(); + expect(harness.dispatch).toHaveBeenCalledExactlyOnceWith(action); + expect(harness.alert).not.toHaveBeenCalled(); +}); diff --git a/apps/mobile/src/features/review/useReviewCommentDismissal.ts b/apps/mobile/src/features/review/useReviewCommentDismissal.ts new file mode 100644 index 000000000000..887cac209e06 --- /dev/null +++ b/apps/mobile/src/features/review/useReviewCommentDismissal.ts @@ -0,0 +1,57 @@ +import { useNavigation, usePreventRemove } from "@react-navigation/native"; +import { useEffect, useRef, type RefObject } from "react"; +import { Alert } from "react-native"; + +export function useReviewCommentDismissal({ + commentText, + attachmentCount, + pendingImages, + submitted, + accepted, +}: { + readonly commentText: string; + readonly attachmentCount: number; + readonly pendingImages: number; + readonly submitted: boolean; + readonly accepted: RefObject; +}) { + const navigation = useNavigation(); + const confirmingDiscard = useRef(false); + usePreventRemove( + !submitted && (pendingImages > 0 || commentText.length > 0 || attachmentCount > 0), + ({ data }) => { + // The native removal guard can lag the successful synchronous transfer. + if (accepted.current) { + navigation.dispatch(data.action); + return; + } + if (pendingImages > 0 || confirmingDiscard.current) return; + confirmingDiscard.current = true; + Alert.alert( + "Discard comment?", + "Your comment and attachments have not been added to the draft.", + [ + { + text: "Keep editing", + style: "cancel", + onPress: () => { + confirmingDiscard.current = false; + }, + }, + { + text: "Discard", + style: "destructive", + onPress: () => { + confirmingDiscard.current = false; + navigation.dispatch(data.action); + }, + }, + ], + ); + }, + ); + useEffect(() => { + if (!submitted) return; + navigation.goBack(); + }, [navigation, submitted]); +} diff --git a/apps/mobile/src/features/review/useReviewCommentSubmission.test.ts b/apps/mobile/src/features/review/useReviewCommentSubmission.test.ts new file mode 100644 index 000000000000..5ff169fdcee1 --- /dev/null +++ b/apps/mobile/src/features/review/useReviewCommentSubmission.test.ts @@ -0,0 +1,44 @@ +import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +const state = vi.hoisted(() => ({ accepted: { current: false }, setSubmitted: vi.fn() })); +vi.mock("react", () => ({ + useRef: () => state.accepted, + useState: () => [false, state.setSubmitted], + useCallback: (callback: () => unknown) => callback, +})); +import { useReviewCommentSubmission } from "./useReviewCommentSubmission"; + +beforeEach(() => { + state.accepted.current = false; + vi.clearAllMocks(); +}); +describe("review comment submission", () => { + it("transfers exactly once when pressed twice before a rerender", () => { + const { submit } = useReviewCommentSubmission(); + const transfer = vi.fn(() => true); + submit(transfer); + submit(transfer); + expect(transfer).toHaveBeenCalledOnce(); + expect(state.setSubmitted).toHaveBeenCalledExactlyOnceWith(true); + }); + it("keeps failure editable and permits a successful retry", () => { + const { submit } = useReviewCommentSubmission(); + const transfer = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true); + submit(transfer); + expect(state.setSubmitted).not.toHaveBeenCalled(); + submit(transfer); + expect(transfer).toHaveBeenCalledTimes(2); + expect(state.setSubmitted).toHaveBeenCalledExactlyOnceWith(true); + }); + it("unlocks after a thrown transfer without marking the input submitted", () => { + const { submit } = useReviewCommentSubmission(); + expect(() => + submit(() => { + throw new Error("Transfer failed"); + }), + ).toThrow("Transfer failed"); + expect(state.setSubmitted).not.toHaveBeenCalled(); + submit(() => true); + expect(state.setSubmitted).toHaveBeenCalledExactlyOnceWith(true); + }); +}); diff --git a/apps/mobile/src/features/review/useReviewCommentSubmission.ts b/apps/mobile/src/features/review/useReviewCommentSubmission.ts new file mode 100644 index 000000000000..59ff3b4a3f95 --- /dev/null +++ b/apps/mobile/src/features/review/useReviewCommentSubmission.ts @@ -0,0 +1,19 @@ +import { useCallback, useRef, useState } from "react"; + +/** A synchronous transfer can be tapped twice before React commits the disabled button. */ +export function useReviewCommentSubmission() { + const accepted = useRef(false); + const [submitted, setSubmitted] = useState(false); + const submit = useCallback((transfer: () => boolean) => { + if (accepted.current) return; + accepted.current = true; + try { + if (transfer()) setSubmitted(true); + else accepted.current = false; + } catch (error) { + accepted.current = false; + throw error; + } + }, []); + return { submitted, submit, accepted }; +} diff --git a/apps/mobile/src/state/use-composer-drafts.test.ts b/apps/mobile/src/state/use-composer-drafts.test.ts index 6da72860009e..985488d1d43c 100644 --- a/apps/mobile/src/state/use-composer-drafts.test.ts +++ b/apps/mobile/src/state/use-composer-drafts.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it } from "@effect/vitest"; import { CommandId, + COMPOSER_CONTEXT_MAX_RECORDS, ComposerContextId, EnvironmentId, MessageId, @@ -10,6 +11,8 @@ import { } from "@t3tools/contracts"; import { onTestFinished, vi } from "vite-plus/test"; +vi.mock("../lib/uuid", () => ({ uuidv4: () => crypto.randomUUID() })); + const composerDraftFileMocks = vi.hoisted(() => { let document = JSON.stringify({ schemaVersion: 1, drafts: {} }); let readError: Error | null = null; @@ -185,6 +188,7 @@ import { retargetNewTaskDraft, setComposerDraftText, insertComposerDraftContext, + appendComposerDraftReviewComment, insertComposerDraftText, rememberComposerDraftSelection, setComposerDraftAttachmentUpload, @@ -238,6 +242,76 @@ function contextDraft(start: number, count: number): ComposerDraft { } describe("mobile composer drafts", () => { + const reviewText = + '\nKeep this comment\n```diff\n@@ -1,1 +1,1 @@\n line\n```\n'; + const reviewImage = { + type: "image" as const, + id: "review-image", + name: "review.png", + mimeType: "image/png" as const, + sizeBytes: 4, + dataUrl: "data:image/png;base64,YWJj", + previewUri: "data:image/png;base64,YWJj", + }; + + it("adds a review snapshot and image together to its scoped destination without replacing selected text", () => { + const key = "review-environment:review-thread"; + setComposerDraftText(key, "Existing draft"); + setComposerDraftText("other-environment:review-thread", "Other draft"); + rememberComposerDraftSelection(key, "Existing draft", { start: 0, end: 8 }); + expect(appendComposerDraftReviewComment(key, reviewText, [reviewImage])).toBe(true); + const draft = getComposerDraftSnapshot(key); + expect(draft.text).toMatch(/^Existing draft /); + expect(draft.attachments).toEqual([reviewImage]); + expect(draft.context?.records).toEqual( + expect.arrayContaining([ + expect.objectContaining({ kind: "review-comment", filePath: "src/a.ts" }), + expect.objectContaining({ kind: "image", attachmentId: "review-image" }), + ]), + ); + expect(getComposerDraftSnapshot("other-environment:review-thread").text).toBe("Other draft"); + }); + + it("leaves the destination and editor attachments intact when context capacity rejects a review", () => { + const key = "review-environment:full-context"; + const before = contextDraft(0, COMPOSER_CONTEXT_MAX_RECORDS); + appAtomRegistry.set(composerDraftsAtom, { [key]: before }); + expect(appendComposerDraftReviewComment(key, reviewText, [reviewImage])).toBe(false); + expect(getComposerDraftSnapshot(key)).toEqual(before); + expect(composerAttachmentCleanupMocks.remove).not.toHaveBeenCalled(); + }); + + it("does not insert the comment or release its files when the attachment limit rejects the transfer", () => { + const key = "review-environment:full-attachments"; + const before = { + text: "Existing draft", + attachments: Array.from({ length: 8 }, (_, index) => ({ + ...reviewImage, + id: `held-${index}`, + })), + }; + appAtomRegistry.set(composerDraftsAtom, { [key]: before }); + expect(appendComposerDraftReviewComment(key, reviewText, [reviewImage])).toBe(false); + expect(getComposerDraftSnapshot(key)).toEqual(before); + expect(composerAttachmentCleanupMocks.remove).not.toHaveBeenCalled(); + }); + + it("keeps a rejected review available for a successful retry", () => { + const key = "review-environment:retry"; + appAtomRegistry.set(composerDraftsAtom, { + [key]: contextDraft(0, COMPOSER_CONTEXT_MAX_RECORDS), + }); + expect(appendComposerDraftReviewComment(key, reviewText, [reviewImage])).toBe(false); + setComposerDraftText(key, "Room now"); + expect(appendComposerDraftReviewComment(key, reviewText, [reviewImage])).toBe(true); + expect(getComposerDraftSnapshot(key).attachments).toEqual([reviewImage]); + expect( + getComposerDraftSnapshot(key).context?.records.filter( + (record) => record.kind === "review-comment", + ), + ).toHaveLength(1); + }); + it.each([false, true])( "restores visible file chips from legacy drafts (archived: %s)", async (archived) => { diff --git a/apps/mobile/src/state/use-composer-drafts.ts b/apps/mobile/src/state/use-composer-drafts.ts index 734199dcef5e..92f7fbba3cf0 100644 --- a/apps/mobile/src/state/use-composer-drafts.ts +++ b/apps/mobile/src/state/use-composer-drafts.ts @@ -29,6 +29,9 @@ import { sanitizeComposerContextLabel, replaceComposerContextReferences, } from "@t3tools/shared/composerContextReferences"; +import { upgradeLegacyContextMessage } from "@t3tools/shared/composerContextLegacy"; +import { reidentifyComposerContext } from "../lib/composerContext"; +import { uuidv4 } from "../lib/uuid"; import { imageMimeType } from "@t3tools/shared/image"; import { videoMimeType } from "@t3tools/shared/video"; import { DraftComposerAttachmentSchema } from "../lib/composer-image-schema"; @@ -245,6 +248,38 @@ export function setComposerDraftContext( })); } +/** Transfers a review comment as one snapshot; rejected input remains owned by its editor. */ +export function appendComposerDraftReviewComment( + draftKey: string, + text: string, + attachments: ReadonlyArray = [], +): boolean { + const upgraded = upgradeLegacyContextMessage(text); + const content = reidentifyComposerContext(upgraded.text, upgraded.records, uuidv4); + const records = attachments.map((attachment) => attachmentContextRecord(attachment)); + let inserted = false; + updateComposerDrafts((current) => { + const draft = normalizeDraft(current[draftKey]); + if (draft.attachments.length + attachments.length > PROVIDER_SEND_TURN_MAX_ATTACHMENTS) { + return current; + } + // Append to the destination, without replacing its saved editor selection. + const next = draftWithInsertedContext( + draftKey, + { ...draft, attachments: [...draft.attachments, ...attachments] }, + { + text: [content.text, ...records.map(formatComposerContextReference)].join(" "), + context: { version: 1, records: [...content.context.records, ...records] }, + }, + { text: draft.text, start: draft.text.length, end: draft.text.length }, + ); + if (!next) return current; + inserted = true; + return { ...current, [draftKey]: next }; + }); + return inserted; +} + export function insertComposerDraftContext( draftKey: string, content: { diff --git a/apps/mobile/src/state/use-thread-composer-state.ts b/apps/mobile/src/state/use-thread-composer-state.ts index d21af7d79cc3..9676aa754911 100644 --- a/apps/mobile/src/state/use-thread-composer-state.ts +++ b/apps/mobile/src/state/use-thread-composer-state.ts @@ -24,9 +24,7 @@ import { type CodexFeedbackSubmission, } from "@t3tools/client-runtime/state/threads"; import { deriveActiveWorkStartedAt } from "@t3tools/shared/orchestrationTiming"; -import { upgradeLegacyContextMessage } from "@t3tools/shared/composerContextLegacy"; -import { composerContextSendBlockReason, reidentifyComposerContext } from "../lib/composerContext"; -import { uuidv4 } from "../lib/uuid"; +import { composerContextSendBlockReason } from "../lib/composerContext"; import { makeQueuedMessageMetadata } from "../lib/commandMetadata"; import { isModelSelectionUnavailable } from "../lib/modelOptions"; @@ -48,6 +46,7 @@ import { appAtomRegistry } from "../state/atom-registry"; import { pendingThreadCreationMessage } from "./pending-thread-creation"; import { appendComposerDraftAttachments, + appendComposerDraftReviewComment, captureComposerDraftInsertion, countComposerDraftAttachmentsAfterSelection, insertComposerDraftText, @@ -81,30 +80,19 @@ export function appendReviewCommentToDraft(input: { readonly threadId: ThreadId; readonly text: string; readonly attachments?: ReadonlyArray; -}): void { - const threadKey = scopedThreadKey(input.environmentId, input.threadId); - const upgraded = upgradeLegacyContextMessage(input.text); - if ( - !insertComposerDraftContext( - threadKey, - reidentifyComposerContext(upgraded.text, upgraded.records, uuidv4), - ) - ) { - Alert.alert("Too many context items", "Remove some context from the draft and try again."); - return; - } - if (input.attachments && input.attachments.length > 0) { - // Capped: a review comment is new content, not a send-failure restore, so - // it must not push the draft over the send limit. Overflow is released. - const rejectedCount = appendComposerDraftAttachments(threadKey, input.attachments, { - appendReference: true, - }); - if (rejectedCount > 0) { - setPendingConnectionError( - `${rejectedCount} comment attachment${rejectedCount === 1 ? " was" : "s were"} not added. Messages can contain at most ${PROVIDER_SEND_TURN_MAX_ATTACHMENTS} attachments.`, - ); - } +}): boolean { + const inserted = appendComposerDraftReviewComment( + scopedThreadKey(input.environmentId, input.threadId), + input.text, + input.attachments, + ); + if (!inserted) { + Alert.alert( + "Comment not added", + "Remove some attachments or context from the draft and try again.", + ); } + return inserted; } export function useThreadDraftForThread(input: { diff --git a/apps/web/src/components/diffs/AnnotatableCodeView.tsx b/apps/web/src/components/diffs/AnnotatableCodeView.tsx index 253a0f3d215e..8c38531342fc 100644 --- a/apps/web/src/components/diffs/AnnotatableCodeView.tsx +++ b/apps/web/src/components/diffs/AnnotatableCodeView.tsx @@ -18,6 +18,7 @@ import { type ReviewCommentContext, } from "~/reviewCommentContext"; +import { usePullRequestReviewStore } from "../pullRequest/pullRequestReviewStore"; import { nextFileCommentId } from "../files/fileCommentAnnotations"; import { DiffCommentAnnotation } from "./DiffCommentAnnotation"; import { StyledDiffCodeView, type StyledDiffCodeViewOptions } from "./StyledDiffCodeView"; @@ -125,11 +126,41 @@ export function AnnotatableCodeView({ id: string; range: SelectedLineRange; } | null>(null); - const [draft, setDraft] = useState<{ - fileKey: string; - annotation: DiffCommentLineAnnotation; - } | null>(null); - const [draftText, setDraftText] = useState(""); + const draftKey = JSON.stringify(["thread-diff", composerDraftTarget, sectionId]); + const retainedDraft = usePullRequestReviewStore((store) => store.inlineDrafts[draftKey]); + const setRetainedDraft = usePullRequestReviewStore((store) => store.setInlineDraft); + const draft = useMemo( + () => + retainedDraft + ? { + fileKey: retainedDraft.fileKey, + annotation: { + side: annotationSide(retainedDraft.range), + lineNumber: retainedDraft.range.end, + metadata: { + entries: [ + { + id: retainedDraft.comment.id, + kind: "draft" as const, + range: retainedDraft.range, + rangeLabel: retainedDraft.comment.rangeLabel, + text: "", + }, + ], + }, + }, + } + : null, + [retainedDraft?.fileKey, retainedDraft?.range, retainedDraft?.comment], + ); + const draftText = retainedDraft?.text ?? ""; + const setDraftText = useCallback( + (text: string) => { + const current = usePullRequestReviewStore.getState().inlineDrafts[draftKey]; + if (current) setRetainedDraft(draftKey, { ...current, text }); + }, + [draftKey, setRetainedDraft], + ); const filesByKey = useMemo(() => new Map(files.map((file) => [file.fileKey, file])), [files]); const items = useMemo[]>( @@ -179,13 +210,12 @@ export function AnnotatableCodeView({ (entryId: string) => { setSelectedLines(null); if (draft?.annotation.metadata.entries.some((entry) => entry.id === entryId)) { - setDraft(null); - setDraftText(""); + setRetainedDraft(draftKey, null); } else { removeReviewComment(composerDraftTarget, entryId); } }, - [composerDraftTarget, draft, removeReviewComment], + [composerDraftTarget, draft, draftKey, removeReviewComment, setRetainedDraft], ); const submitEntry = useCallback( @@ -193,23 +223,20 @@ export function AnnotatableCodeView({ const entry = draft?.annotation.metadata.entries.find( (candidate) => candidate.id === entryId, ); - const file = draft ? filesByKey.get(draft.fileKey) : undefined; - if (!entry || !file) return; - const comment = buildDiffReviewComment({ - id: entry.id, - sectionId, - sectionTitle, - filePath: file.filePath, - fileDiff: file.fileDiff, - range: entry.range, - text, - }); - if (comment) addReviewComment(composerDraftTarget, comment); + if (!entry) return; + if (!retainedDraft) return; + // Preserve the original selection even if the live diff changes while editing. + const comment = { ...retainedDraft.comment, text }; + addReviewComment(composerDraftTarget, comment); + const added = useComposerDraftStore + .getState() + .getComposerDraft(composerDraftTarget) + ?.reviewComments.some((current) => current.id === comment.id); + if (!added) return; setSelectedLines(null); - setDraft(null); - setDraftText(""); + setRetainedDraft(draftKey, null); }, - [addReviewComment, composerDraftTarget, draft, filesByKey, sectionId, sectionTitle], + [addReviewComment, composerDraftTarget, draft, draftKey, retainedDraft, setRetainedDraft], ); const beginComment = useCallback( @@ -230,73 +257,82 @@ export function AnnotatableCodeView({ text: "", }); if (!comment) return; - setDraftText(""); - setDraft({ - fileKey: item.id, - annotation: { - side: annotationSide(range), - lineNumber: range.end, - metadata: { - entries: [{ id, kind: "draft", range, rangeLabel: comment.rangeLabel, text: "" }], - }, - }, - }); + if (retainedDraft) return; + setRetainedDraft(draftKey, { fileKey: item.id, range, comment, text: "" }); }, - [filesByKey, sectionId, sectionTitle], + [draftKey, filesByKey, retainedDraft, sectionId, sectionTitle, setRetainedDraft], ); const hasOpenComment = draft !== null; return ( - - key={codeViewKey} - {...(viewerRef ? { viewerRef } : {})} - {...(className ? { className } : {})} - {...(unsafeCSSExtra ? { unsafeCSSExtra } : {})} - {...(renderHeaderMetadata - ? { - renderHeaderMetadata: (item: CodeViewItem) => - item.type === "diff" ? renderHeaderMetadata(item.fileDiff) : null, - } - : {})} - {...(renderCodeViewFooter ? { renderCodeViewFooter } : {})} - items={items} - selectedLines={selectedLines} - onSelectedLinesChange={setSelectedLines} - options={{ - ...options, - enableGutterUtility: !hasOpenComment, - enableLineSelection: !hasOpenComment, - onGutterUtilityClick: beginComment, - }} - renderHeaderFilenameSuffix={(item) => - item.type === "diff" ? renderHeaderFilenameSuffix(item.fileDiff) : null - } - renderHeaderPrefix={(item) => - item.type === "diff" - ? renderHeaderPrefix(item.fileDiff, item.id, item.collapsed === true) - : null - } - renderAnnotation={(annotation) => { - const hasDraft = annotation.metadata.entries.some((entry) => entry.kind === "draft"); - return ( -
- {annotation.metadata.entries.map((entry) => ( - removeEntry(entry.id)} - onComment={(text) => submitEntry(entry.id, text)} - onDelete={() => removeEntry(entry.id)} - /> - ))} -
- ); - }} - /> + <> + {retainedDraft && !filesByKey.has(retainedDraft.fileKey) ? ( +
+

+ Draft for {retainedDraft.comment.filePath}. The original selection is retained with your + comment. +

+ removeEntry(retainedDraft.comment.id)} + onComment={(text) => submitEntry(retainedDraft.comment.id, text)} + /> +
+ ) : null} + + key={codeViewKey} + {...(viewerRef ? { viewerRef } : {})} + {...(className ? { className } : {})} + {...(unsafeCSSExtra ? { unsafeCSSExtra } : {})} + {...(renderHeaderMetadata + ? { + renderHeaderMetadata: (item: CodeViewItem) => + item.type === "diff" ? renderHeaderMetadata(item.fileDiff) : null, + } + : {})} + {...(renderCodeViewFooter ? { renderCodeViewFooter } : {})} + items={items} + selectedLines={selectedLines} + onSelectedLinesChange={setSelectedLines} + options={{ + ...options, + enableGutterUtility: !hasOpenComment, + enableLineSelection: !hasOpenComment, + onGutterUtilityClick: beginComment, + }} + renderHeaderFilenameSuffix={(item) => + item.type === "diff" ? renderHeaderFilenameSuffix(item.fileDiff) : null + } + renderHeaderPrefix={(item) => + item.type === "diff" + ? renderHeaderPrefix(item.fileDiff, item.id, item.collapsed === true) + : null + } + renderAnnotation={(annotation) => { + const hasDraft = annotation.metadata.entries.some((entry) => entry.kind === "draft"); + return ( +
+ {annotation.metadata.entries.map((entry) => ( + removeEntry(entry.id)} + onComment={(text) => submitEntry(entry.id, text)} + onDelete={() => removeEntry(entry.id)} + /> + ))} +
+ ); + }} + /> + ); } diff --git a/apps/web/src/components/diffs/DiffCommentAnnotation.tsx b/apps/web/src/components/diffs/DiffCommentAnnotation.tsx index bce9f7e55f58..3d1bc80de904 100644 --- a/apps/web/src/components/diffs/DiffCommentAnnotation.tsx +++ b/apps/web/src/components/diffs/DiffCommentAnnotation.tsx @@ -1,6 +1,8 @@ import { MessageCircle, Trash2 } from "lucide-react"; import { useLayoutEffect, useRef, useState, type ReactNode } from "react"; +import { ensureLocalApi } from "~/localApi"; + import { Button } from "~/components/ui/button"; import { Textarea } from "~/components/ui/textarea"; @@ -24,6 +26,7 @@ interface DiffCommentAnnotationProps { placeholder?: string; submitLabel?: string; pending?: boolean; + submitDisabled?: boolean; secondaryAction?: DiffCommentSecondaryAction; focusOnMount?: boolean; } @@ -40,12 +43,30 @@ export function DiffCommentAnnotation({ placeholder = "Add a comment…", submitLabel = "Comment", pending = false, + submitDisabled = false, secondaryAction, focusOnMount = true, }: DiffCommentAnnotationProps) { const [localDraftText, setLocalDraftText] = useState(""); const displayedText = kind === "draft" && !onTextChange ? localDraftText : text; const trimmedText = displayedText.trim(); + const dismissing = useRef(false); + const cancel = async () => { + if (pending || dismissing.current) return; + dismissing.current = true; + try { + if ( + displayedText.length === 0 || + (await ensureLocalApi().dialogs.confirm( + "Discard this comment? Your text has not been added to the draft.", + { variant: "destructive" }, + )) + ) + onCancel(); + } finally { + dismissing.current = false; + } + }; const textareaRef = useRef(null); useLayoutEffect(() => { @@ -105,9 +126,10 @@ export function DiffCommentAnnotation({ onKeyDown={(event) => { if (event.key === "Escape") { event.preventDefault(); - onCancel(); + event.stopPropagation(); + void cancel(); } - if (isCommentSubmitShortcut(event, trimmedText, pending)) { + if (isCommentSubmitShortcut(event, trimmedText, pending || submitDisabled)) { event.preventDefault(); onComment(trimmedText); } @@ -119,7 +141,8 @@ export function DiffCommentAnnotation({ className="text-muted-foreground hover:text-foreground" variant="ghost" size="xs" - onClick={onCancel} + disabled={pending} + onClick={() => void cancel()} > Cancel @@ -127,14 +150,18 @@ export function DiffCommentAnnotation({ ) : null} - diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index b535c4f45e97..882054be872d 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -242,7 +242,6 @@ function PullRequestCodeTab({ id: string; range: SelectedLineRange; } | null>(null); - const [draft, setDraft] = useState(null); const [threadPending, setThreadPending] = useState(false); const [orphansOpen, setOrphansOpen] = useState(false); // Closed by default so the review form does not permanently eat vertical space below the @@ -263,10 +262,24 @@ function PullRequestCodeTab({ // One commit's own changes and the whole change are two different diffs, paged separately, so // everything below is keyed by both. const scopeKey = commit === null ? referenceKey : `${referenceKey}@${commit}`; - // The panel keeps this mounted across pull requests, so an open composer would otherwise - // survive the switch and attach its comment to whichever one is on screen when it is sent. + const lineDraftKey = JSON.stringify([environmentId, scopeKey]); + const draft = usePullRequestReviewStore((store) => store.lineDrafts[lineDraftKey] ?? null); + const setLineDraft = usePullRequestReviewStore((store) => store.setLineDraft); + const setDraft = useCallback( + (next: DraftAnchor | null) => { + setLineDraft(lineDraftKey, next === null ? null : { ...next, text: "" }); + }, + [lineDraftKey, setLineDraft], + ); + const setDraftText = useCallback( + (text: string) => { + const current = usePullRequestReviewStore.getState().lineDrafts[lineDraftKey]; + if (current) setLineDraft(lineDraftKey, { ...current, text }); + }, + [lineDraftKey, setLineDraft], + ); + // The editor is keyed separately; resetting the viewer must not discard its draft. useEffect(() => { - setDraft(null); setSelectedLines(null); setToggledFiles(new Set()); setFoldOverride(null); @@ -693,7 +706,7 @@ function PullRequestCodeTab({ const beginComment = useCallback( (range: SelectedLineRange | null, context: { item: CodeViewItem }) => { - if (!range || !canCommentOnLines) return; + if (!range || !canCommentOnLines || draft) return; const item = context.item; if (item.type !== "diff") return; const file = files.find((candidate) => buildFileDiffRenderKey(candidate) === item.id); @@ -712,7 +725,7 @@ function PullRequestCodeTab({ range, }); }, - [canCommentOnLines, files], + [canCommentOnLines, draft, files, setDraft], ); // Built here because the parsed diff only lives here, and built by the same function the @@ -733,11 +746,12 @@ function PullRequestCodeTab({ range: anchor.range, text, }); + if (comment === null) return; + onFinish(comment); setDraft(null); setSelectedLines(null); - if (comment !== null) onFinish(comment); }, - [detail.number, files], + [detail.number, files, setDraft], ); // The viewer's SlotPortals memoizes each visible file's header/annotation portal on these @@ -1002,8 +1016,10 @@ function PullRequestCodeTab({ buildFileDiffRenderKey(file) === draft.fileKey)} {...(onAddToAgentSelection ? { secondaryAction: { @@ -1020,6 +1036,7 @@ function PullRequestCodeTab({ setSelectedLines(null); }} onComment={(body) => { + if (!files.some((file) => buildFileDiffRenderKey(file) === draft.fileKey)) return; addComment(reviewKey, { id: nextPendingReviewCommentId(), path: draft.path, @@ -1038,13 +1055,31 @@ function PullRequestCodeTab({ addComment, draft, finishSelection, + files, onAddToAgentSelection, removeComment, renderThreadCard, reviewKey, + setDraft, + setDraftText, ], ); + const detachedDraft = + draft && !files.some((file) => buildFileDiffRenderKey(file) === draft.fileKey) ? ( +
+

+ Draft for {draft.path}. The original lines are not currently available; your text is + retained. +

+ {renderAnnotation({ + side: "additions", + lineNumber: 1, + metadata: { threads: [], pending: [], draft: true }, + })} +
+ ) : null; + /** * The review overlay belongs to the pull request, not to the patch: a change whose diff * cannot be structured — or read at all — is still one a reviewer can approve or reject, so @@ -1367,6 +1402,7 @@ function PullRequestCodeTab({ const withReviewBar = (body: ReactNode) => (
{toolbar} + {detachedDraft} {/* The overlay is anchored to this wrapper, not the scroller: absolute positioning inside an overflowing element tracks the content's bottom edge, which would carry the trigger away with the first scroll. */} @@ -1450,6 +1486,7 @@ function PullRequestCodeTab({ return (
{toolbar} + {detachedDraft} {/* Above the code, closed, and counted: these belong to the change rather than to any line of it, and in the stream they read as cards dropped into the patch. */} {orphanFiles.size > 0 ? ( diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.test.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.test.tsx new file mode 100644 index 000000000000..5ebc4d984714 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.test.tsx @@ -0,0 +1,84 @@ +import { act } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { EnvironmentId } from "@t3tools/contracts"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +vi.mock("../ui/button", () => ({ Button: "button" })); +vi.mock("../ui/textarea", () => ({ Textarea: "textarea" })); +vi.mock("../ui/toggle-group", () => ({ Toggle: "button", ToggleGroup: "div" })); +vi.mock("./PullRequestMarkdown", () => ({ PullRequestMarkdown: () => null })); +import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; +import { usePullRequestReviewStore } from "./pullRequestReviewStore"; + +let renderer: ReactTestRenderer | undefined; +const onCancel = vi.fn(); +const onSave = vi.fn(); +function editor(draftKey = "environment:review:comment-a", value = "Original") { + return ( + + ); +} +function edit(value: string) { + act(() => renderer!.root.findByType("textarea").props.onChange({ target: { value } })); +} +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.clearAllMocks(); + usePullRequestReviewStore.setState({ editorDrafts: {} }); +}); +afterEach(() => { + act(() => renderer?.unmount()); + renderer = undefined; + vi.unstubAllGlobals(); +}); + +describe("PR edit draft lifetime", () => { + it("retains edited text across Cancel and reopening the same target", () => { + act(() => { + renderer = create(editor()); + }); + edit("Invested input"); + act(() => + renderer!.root + .findAllByType("button") + .find((button) => button.children.includes("Cancel"))! + .props.onClick(), + ); + expect(onCancel).toHaveBeenCalledOnce(); + act(() => renderer!.unmount()); + act(() => { + renderer = create(editor()); + }); + expect(renderer!.root.findByType("textarea").props.value).toBe("Invested input"); + }); + + it("keeps each target's draft when a mounted editor changes subjects", () => { + act(() => { + renderer = create(editor()); + }); + edit("First target's edit"); + act(() => renderer!.update(editor("environment:review:comment-b", "Second target"))); + expect(renderer!.root.findByType("textarea").props.value).toBe("Second target"); + edit("Second target's edit"); + act(() => renderer!.update(editor())); + expect(renderer!.root.findByType("textarea").props.value).toBe("First target's edit"); + }); + + it("does not erase invested input when the remote body refreshes", () => { + act(() => { + renderer = create(editor()); + }); + edit("Local draft"); + act(() => renderer!.update(editor("environment:review:comment-a", "Remote update"))); + expect(renderer!.root.findByType("textarea").props.value).toBe("Local draft"); + }); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.tsx index a92418bbb862..e8df79eaa88e 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.tsx @@ -1,6 +1,8 @@ import { useState } from "react"; import type { EnvironmentId, ScopedThreadRef } from "@t3tools/contracts"; +import { usePullRequestReviewStore } from "./pullRequestReviewStore"; + import { cn } from "~/lib/utils"; import { Button } from "../ui/button"; @@ -9,15 +11,15 @@ import { Toggle, ToggleGroup } from "../ui/toggle-group"; import { PullRequestMarkdown } from "./PullRequestMarkdown"; /** - * The box a body is rewritten in — a description, or a remark already posted. It owns the draft - * and nothing else: the caller sends the request and says whether it is still in flight, so the - * same box serves every mutation without knowing which one it is. + * Edits a description or posted remark. Its scoped session draft survives panel dismissal; + * the caller clears the accepted snapshot only after the host confirms the save. * * Preview renders through the same component the saved body will be read through, which is the * only way to see what a host's markdown will actually become before it is sent. */ export function PullRequestMarkdownEditor({ value, + draftKey, cwd, environmentId, threadRef = null, @@ -30,6 +32,7 @@ export function PullRequestMarkdownEditor({ onCancel, }: { readonly value: string; + readonly draftKey: string; readonly cwd: string; readonly environmentId: EnvironmentId; /** Thread the editor sits beside, so links in its preview follow the link target setting. */ @@ -43,17 +46,10 @@ export function PullRequestMarkdownEditor({ readonly onSave: (next: string) => void; readonly onCancel: () => void; }) { - const [draft, setDraft] = useState(value); + const draft = usePullRequestReviewStore((store) => store.editorDrafts[draftKey] ?? value); + const setDraft = (text: string) => + usePullRequestReviewStore.getState().setEditorDraft(draftKey, text); const [preview, setPreview] = useState(false); - // The words this draft started from. React keeps a component instance wherever the same - // position and key come round again, so an editor opened on one remark can be handed another's - // words without being rebuilt — and saving would then write the first remark's text onto the - // second. Different words mean a different subject, and the draft starts again from them. - const [seed, setSeed] = useState(value); - if (seed !== value) { - setSeed(value); - setDraft(value); - } const empty = draft.trim().length === 0; const saveDisabled = saving || (empty && !allowEmpty); @@ -75,6 +71,7 @@ export function PullRequestMarkdownEditor({ } if (event.key !== "Escape" || saving) return; event.preventDefault(); + event.stopPropagation(); onCancel(); }} > diff --git a/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx b/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx index d49cb52cc3f2..00f3cf329e6b 100644 --- a/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx +++ b/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx @@ -33,7 +33,11 @@ import { PullRequestActorLabel } from "./pullRequestPresentation"; import { PullRequestMarkdown } from "./PullRequestMarkdown"; import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; import { PullRequestReactionBar } from "./PullRequestReactions"; -import type { PendingReviewComment } from "./pullRequestReviewStore"; +import { + reviewEditorKey, + usePullRequestReviewStore, + type PendingReviewComment, +} from "./pullRequestReviewStore"; const CARD_CLASS = "mx-3 my-2 rounded-xl border border-border/70 bg-background p-3 text-sm shadow-sm"; @@ -136,7 +140,10 @@ export function ReviewThreadCard({ // A resolved thread is finished work, so it opens collapsed and stays one line until asked for. const [expanded, setExpanded] = useState(!thread.isResolved); const [replying, setReplying] = useState(false); - const [reply, setReply] = useState(""); + const replyKey = reviewEditorKey(environmentId, reference, `reply:${thread.id}`); + const reply = usePullRequestReviewStore((store) => store.editorDrafts[replyKey] ?? ""); + const setReply = (text: string) => + usePullRequestReviewStore.getState().setEditorDraft(replyKey, text); const [editingId, setEditingId] = useState(null); const [savingEdit, setSavingEdit] = useState(false); const sendingRef = useRef(false); @@ -156,6 +163,9 @@ export function ReviewThreadCard({ const saved = await onEditComment(commentId, body); setSavingEdit(false); if (saved) { + usePullRequestReviewStore + .getState() + .clearEditorDraft(reviewEditorKey(environmentId, reference, `comment:${commentId}`), body); setLoadedPage((previous) => previous?.threadId === thread.id ? { @@ -186,7 +196,7 @@ export function ReviewThreadCard({ } : previous, ); - setReply(""); + usePullRequestReviewStore.getState().clearEditorDraft(replyKey, reply); setReplying(false); } } finally { @@ -280,6 +290,7 @@ export function ReviewThreadCard({ {editingId === comment.id ? ( { beforeEach(() => { - usePullRequestReviewStore.setState({ drafts: {}, summaries: {} }); + usePullRequestReviewStore.setState({ + drafts: {}, + summaries: {}, + editorDrafts: {}, + inlineDrafts: {}, + lineDrafts: {}, + }); + }); + + it("retains editor text across unmount and isolates environment, review and subject", () => { + const reference = { projectId: ProjectId.make("project"), repository: "owner/repo", number: 7 }; + const key = reviewEditorKey(EnvironmentId.make("one"), reference, "comment:42"); + usePullRequestReviewStore.getState().setEditorDraft(key, "Unsent edit"); + expect(usePullRequestReviewStore.getState().editorDrafts[key]).toBe("Unsent edit"); + expect( + usePullRequestReviewStore.getState().editorDrafts[ + reviewEditorKey(EnvironmentId.make("two"), reference, "comment:42") + ], + ).toBeUndefined(); + expect( + usePullRequestReviewStore.getState().editorDrafts[ + reviewEditorKey(EnvironmentId.make("one"), reference, "comment:43") + ], + ).toBeUndefined(); + expect( + usePullRequestReviewStore.getState().editorDrafts[ + reviewEditorKey(EnvironmentId.make("one"), { ...reference, number: 8 }, "comment:42") + ], + ).toBeUndefined(); + }); + + it("clears only a submitted editor snapshot and keeps a newer edit", () => { + const store = usePullRequestReviewStore.getState(); + store.setEditorDraft("reply", "Submitted"); + store.setEditorDraft("reply", "Newer input"); + store.clearEditorDraft("reply", "Submitted"); + expect(usePullRequestReviewStore.getState().editorDrafts.reply).toBe("Newer input"); + store.clearEditorDraft("reply", "Newer input"); + expect(usePullRequestReviewStore.getState().editorDrafts.reply).toBeUndefined(); + }); + + it("retains an inline PR draft's file and line with its text when another review is opened", () => { + const draft = { + fileKey: "file-a", + path: "src/a.ts", + oldPath: null, + position: { kind: "added" as const, newLine: 5 }, + range: { start: 5, end: 5, side: "additions" as const }, + text: "Keep this input", + }; + const store = usePullRequestReviewStore.getState(); + store.setLineDraft("environment-a/review-a", draft); + store.setLineDraft("environment-a/review-b", { + ...draft, + path: "src/b.ts", + text: "Other input", + }); + store.setLineDraft("environment-a/review-b", null); + expect(usePullRequestReviewStore.getState().lineDrafts["environment-a/review-a"]).toEqual( + draft, + ); + expect( + usePullRequestReviewStore.getState().lineDrafts["environment-a/review-b"], + ).toBeUndefined(); }); it("removes only the line comments included in a submitted snapshot", () => { diff --git a/apps/web/src/components/pullRequest/pullRequestReviewStore.ts b/apps/web/src/components/pullRequest/pullRequestReviewStore.ts index fba8d4c56e13..d2080219d394 100644 --- a/apps/web/src/components/pullRequest/pullRequestReviewStore.ts +++ b/apps/web/src/components/pullRequest/pullRequestReviewStore.ts @@ -6,8 +6,15 @@ * hosts that have no pending review of their own. That also means a draft lives only as long * as the tab does, which is why this is deliberately not persisted. */ -import type { PullRequestRef, PullRequestReviewCommentDraft } from "@t3tools/contracts"; +import type { + EnvironmentId, + PullRequestRef, + PullRequestReviewCommentDraft, + PullRequestReviewPosition, +} from "@t3tools/contracts"; import { create } from "zustand"; +import type { SelectedLineRange } from "@pierre/diffs"; +import type { ReviewCommentContext } from "~/reviewCommentContext"; export type PendingReviewComment = PullRequestReviewCommentDraft & { readonly id: string }; @@ -34,7 +41,38 @@ export function pullRequestReviewKey(reference: PullRequestRef): string { ]); } +export interface InlineReviewDraft { + readonly fileKey: string; + readonly range: SelectedLineRange; + readonly text: string; + readonly comment: ReviewCommentContext; +} + +export interface PullRequestLineDraft { + readonly fileKey: string; + readonly path: string; + readonly oldPath: string | null; + readonly position: PullRequestReviewPosition; + readonly range: SelectedLineRange; + readonly text: string; +} + +export function reviewEditorKey( + environmentId: EnvironmentId, + reference: PullRequestRef, + subject: string, +): string { + return JSON.stringify([environmentId, pullRequestReviewKey(reference), subject]); +} + interface PullRequestReviewStoreState { + readonly editorDrafts: Readonly>; + readonly inlineDrafts: Readonly>; + readonly lineDrafts: Readonly>; + readonly setEditorDraft: (key: string, text: string) => void; + readonly clearEditorDraft: (key: string, submittedText: string) => void; + readonly setInlineDraft: (key: string, draft: InlineReviewDraft | null) => void; + readonly setLineDraft: (key: string, draft: PullRequestLineDraft | null) => void; readonly drafts: Readonly>>; readonly summaries: Readonly>; readonly addComment: (key: string, comment: PendingReviewComment) => void; @@ -48,6 +86,27 @@ interface PullRequestReviewStoreState { const EMPTY: ReadonlyArray = []; export const usePullRequestReviewStore = create()((set) => ({ + editorDrafts: {}, + inlineDrafts: {}, + lineDrafts: {}, + setEditorDraft: (key, text) => + set((state) => ({ editorDrafts: { ...state.editorDrafts, [key]: text } })), + clearEditorDraft: (key, submittedText) => + set((state) => { + if (state.editorDrafts[key] !== submittedText) return state; + const { [key]: _removed, ...rest } = state.editorDrafts; + return { editorDrafts: rest }; + }), + setInlineDraft: (key, draft) => + set((state) => { + const { [key]: _removed, ...rest } = state.inlineDrafts; + return { inlineDrafts: draft === null ? rest : { ...rest, [key]: draft } }; + }), + setLineDraft: (key, draft) => + set((state) => { + const { [key]: _removed, ...rest } = state.lineDrafts; + return { lineDrafts: draft === null ? rest : { ...rest, [key]: draft } }; + }), drafts: {}, summaries: {}, addComment: (key, comment) =>