diff --git a/apps/swift-ios/Features/Review/FeatureDiffHydration.swift b/apps/swift-ios/Features/Review/FeatureDiffHydration.swift new file mode 100644 index 0000000000000..bf54571ee0e3d --- /dev/null +++ b/apps/swift-ios/Features/Review/FeatureDiffHydration.swift @@ -0,0 +1,37 @@ +/// Lines shown by a review diff while its full file contents load. A failed +/// load keeps the lines already on screen and reports the error for Retry; +/// a completion from a superseded attempt is ignored. +struct FeatureDiffHydration { + private(set) var lines: [FeatureDiffLine] + private(set) var isLoading = false + private(set) var errorMessage: String? + private var generation = FeatureAsyncGeneration() + + init(lines: [FeatureDiffLine]) { + self.lines = lines + } + + mutating func begin() -> UInt64 { + errorMessage = nil + isLoading = true + return generation.begin() + } + + /// `nil` means the server has no full contents, so the patch lines stay. + mutating func succeed(_ attempt: UInt64, lines loaded: [FeatureDiffLine]?) { + guard generation.accepts(attempt) else { return } + if let loaded { lines = loaded } + isLoading = false + } + + mutating func fail(_ attempt: UInt64, message: String) { + guard generation.accepts(attempt) else { return } + errorMessage = message + isLoading = false + } + + mutating func cancel(_ attempt: UInt64) { + guard generation.accepts(attempt) else { return } + isLoading = false + } +} diff --git a/apps/swift-ios/Features/Review/FeatureReviewView.swift b/apps/swift-ios/Features/Review/FeatureReviewView.swift index 0cf291b32555f..9aad5b2437012 100644 --- a/apps/swift-ios/Features/Review/FeatureReviewView.swift +++ b/apps/swift-ios/Features/Review/FeatureReviewView.swift @@ -12,6 +12,7 @@ public struct FeatureReviewView: View { @State private var review: FeatureReview? @State private var isLoading = true @State private var errorMessage: String? + @State private var loadGeneration = FeatureAsyncGeneration() public init( client: any FeatureClient, @@ -60,6 +61,14 @@ public struct FeatureReviewView: View { private func reviewList(_ review: FeatureReview) -> some View { List { + if let errorMessage { + Section { + FeatureRefreshFailureRow(message: errorMessage) { + Task { await load() } + } + } + } + Section { HStack { VStack(alignment: .leading, spacing: 3) { @@ -112,12 +121,20 @@ public struct FeatureReviewView: View { } private func load() async { + let generation = loadGeneration.begin() isLoading = true - defer { isLoading = false } + defer { + if loadGeneration.accepts(generation) { isLoading = false } + } do { - review = try await client.loadReview(threadID: threadID) + let loaded = try await client.loadReview(threadID: threadID) + guard loadGeneration.accepts(generation) else { return } + review = loaded errorMessage = nil + } catch is CancellationError { + return } catch { + guard loadGeneration.accepts(generation) else { return } errorMessage = error.localizedDescription } } @@ -204,8 +221,7 @@ private struct FeatureDiffView: View { let file: FeatureReviewFile let sendMessage: (FeatureMessageSubmission) async -> Bool - @State private var renderedLines: [FeatureDiffLine] - @State private var isHydrating = false + @State private var hydration: FeatureDiffHydration @State private var selectedLine: FeatureReviewLineSelection? @State private var isCommenting = false @State private var comment = "" @@ -223,15 +239,24 @@ private struct FeatureDiffView: View { self.threadID = threadID self.file = file self.sendMessage = sendMessage - _renderedLines = State(initialValue: file.lines) + _hydration = State(initialValue: FeatureDiffHydration(lines: file.lines)) } var body: some View { Group { - if renderedLines.isEmpty, isHydrating { + if hydration.lines.isEmpty, hydration.isLoading { ProgressView("Loading full diff…") .frame(maxWidth: .infinity, maxHeight: .infinity) - } else if renderedLines.isEmpty { + } else if let hydrationError = hydration.errorMessage, hydration.lines.isEmpty { + ContentUnavailableView { + Label("Couldn’t load this diff", systemImage: "exclamationmark.circle") + } description: { + Text(hydrationError) + } actions: { + Button("Try again") { Task { await hydrate() } } + .buttonStyle(.borderedProminent) + } + } else if hydration.lines.isEmpty { ContentUnavailableView( file.change == .binary ? "Binary file" : "Diff unavailable", systemImage: file.change == .binary ? "doc.richtext" : "doc.text.magnifyingglass", @@ -241,7 +266,7 @@ private struct FeatureDiffView: View { GeometryReader { proxy in ScrollView([.horizontal, .vertical]) { LazyVStack(alignment: .leading, spacing: 0) { - ForEach(renderedLines) { line in + ForEach(hydration.lines) { line in FeatureDiffLineRow( line: line, isSelected: selection(for: line) == selectedLine, @@ -278,6 +303,16 @@ private struct FeatureDiffView: View { commentComposer } } + .safeAreaInset(edge: .top, spacing: 0) { + if let hydrationError = hydration.errorMessage, !hydration.lines.isEmpty { + FeatureRefreshFailureRow(message: hydrationError) { + Task { await hydrate() } + } + .padding(.horizontal, 14) + .padding(.vertical, 10) + .background(T3Colors.surface) + } + } .task(id: file.id) { await hydrate() } } @@ -409,20 +444,23 @@ private struct FeatureDiffView: View { } private func hydrate() async { - isHydrating = true - defer { isHydrating = false } - guard let contents = try? await client.loadReviewFileContents( - threadID: threadID, - file: file - ) else { - return + let attempt = hydration.begin() + do { + let contents = try await client.loadReviewFileContents(threadID: threadID, file: file) + hydration.succeed( + attempt, + lines: contents.map { FeatureFullDiffHydrator.lines(for: file, contents: $0) } + ) + } catch is CancellationError { + hydration.cancel(attempt) + } catch { + hydration.fail(attempt, message: error.localizedDescription) } - renderedLines = FeatureFullDiffHydrator.lines(for: file, contents: contents) } private func sendComment() { guard !trimmedComment.isEmpty, !isSending else { return } - let record = reviewDraft.contextRecord(lines: renderedLines) + let record = reviewDraft.contextRecord(lines: hydration.lines) let prompt = reviewDraft.submissionText(contextRecord: record) isSending = true commentError = nil diff --git a/apps/swift-ios/Features/Shared/FeatureAsyncGeneration.swift b/apps/swift-ios/Features/Shared/FeatureAsyncGeneration.swift new file mode 100644 index 0000000000000..667e374f72585 --- /dev/null +++ b/apps/swift-ios/Features/Shared/FeatureAsyncGeneration.swift @@ -0,0 +1,12 @@ +struct FeatureAsyncGeneration { + private var value: UInt64 = 0 + + mutating func begin() -> UInt64 { + value &+= 1 + return value + } + + func accepts(_ generation: UInt64) -> Bool { + generation == value + } +} diff --git a/apps/swift-ios/Features/Shared/FeatureRefreshFailureRow.swift b/apps/swift-ios/Features/Shared/FeatureRefreshFailureRow.swift new file mode 100644 index 0000000000000..76bb7d0fe0f49 --- /dev/null +++ b/apps/swift-ios/Features/Shared/FeatureRefreshFailureRow.swift @@ -0,0 +1,20 @@ +import SwiftUI + +struct FeatureRefreshFailureRow: View { + let message: String + let retry: () -> Void + + var body: some View { + HStack(alignment: .top, spacing: 10) { + Image(systemName: "exclamationmark.circle") + .foregroundStyle(T3Colors.warning) + Text(message) + .font(T3Typography.supporting) + .foregroundStyle(T3Colors.textSecondary) + .frame(maxWidth: .infinity, alignment: .leading) + Button("Retry", action: retry) + .font(T3Typography.control.weight(.semibold)) + } + .accessibilityElement(children: .contain) + } +} diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureAsyncGenerationTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureAsyncGenerationTests.swift new file mode 100644 index 0000000000000..4e250eb1f61f8 --- /dev/null +++ b/apps/swift-ios/Tests/FeatureTests/FeatureAsyncGenerationTests.swift @@ -0,0 +1,15 @@ +import Testing +@testable import T3Code + +struct FeatureAsyncGenerationTests { + @Test + func asyncGenerationRejectsAnOlderCompletionAfterANewerOperationBegins() { + var generation = FeatureAsyncGeneration() + let staleLoad = generation.begin() + let mutation = generation.begin() + + #expect(!generation.accepts(staleLoad)) + #expect(generation.accepts(mutation)) + } + +} diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureDiffHydrationTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureDiffHydrationTests.swift new file mode 100644 index 0000000000000..032e81b3a9fc3 --- /dev/null +++ b/apps/swift-ios/Tests/FeatureTests/FeatureDiffHydrationTests.swift @@ -0,0 +1,75 @@ +import Testing +@testable import T3Code + +struct FeatureDiffHydrationTests { + private let patch = [ + FeatureDiffLine(id: "patch-1", kind: .deletion, oldLine: 2, text: "let color = blue"), + FeatureDiffLine(id: "patch-2", kind: .addition, newLine: 2, text: "let color = green"), + ] + private let full = [ + FeatureDiffLine(id: "full-1", kind: .context, oldLine: 1, newLine: 1, text: "import SwiftUI"), + FeatureDiffLine(id: "full-2", kind: .deletion, oldLine: 2, text: "let color = blue"), + FeatureDiffLine(id: "full-3", kind: .addition, newLine: 2, text: "let color = green"), + ] + + @Test + func failedLoadKeepsPatchLinesUntilRetrySucceeds() { + var hydration = FeatureDiffHydration(lines: patch) + + let failed = hydration.begin() + hydration.fail(failed, message: "The network connection was lost.") + + #expect(hydration.lines == patch) + #expect(hydration.errorMessage == "The network connection was lost.") + #expect(!hydration.isLoading) + + let retry = hydration.begin() + #expect(hydration.errorMessage == nil) + #expect(hydration.isLoading) + + hydration.succeed(retry, lines: full) + + #expect(hydration.lines == full) + #expect(hydration.errorMessage == nil) + #expect(!hydration.isLoading) + } + + @Test + func failedLoadWithoutPatchLinesReportsTheError() { + var hydration = FeatureDiffHydration(lines: []) + + let failed = hydration.begin() + hydration.fail(failed, message: "Request timed out.") + + #expect(hydration.lines.isEmpty) + #expect(hydration.errorMessage == "Request timed out.") + #expect(!hydration.isLoading) + } + + @Test + func supersededAttemptCannotOverwriteNewerResult() { + var hydration = FeatureDiffHydration(lines: patch) + + let stale = hydration.begin() + let current = hydration.begin() + hydration.succeed(current, lines: full) + hydration.fail(stale, message: "The request was cancelled.") + hydration.succeed(stale, lines: []) + + #expect(hydration.lines == full) + #expect(hydration.errorMessage == nil) + #expect(!hydration.isLoading) + } + + @Test + func missingFullContentsKeepPatchLinesWithoutError() { + var hydration = FeatureDiffHydration(lines: patch) + + let attempt = hydration.begin() + hydration.succeed(attempt, lines: nil) + + #expect(hydration.lines == patch) + #expect(hydration.errorMessage == nil) + #expect(!hydration.isLoading) + } +}