aboutsummaryrefslogtreecommitdiff
path: root/Hutch
diff options
context:
space:
mode:
authorChristian Cleberg <[email protected]>2026-07-15 22:02:55 -0500
committerChristian Cleberg <[email protected]>2026-07-15 22:02:55 -0500
commit2913d950a5c82d80226050e05bd64be7e30ff87d (patch)
treea5e178a4cb241c8ad362f864fd6ce082e0bbeefa /Hutch
parent575e62f6dab44b0c9836623fe8b7d17a219f8e0f (diff)
downloadhutch-2913d950a5c82d80226050e05bd64be7e30ff87d.tar.gz
hutch-2913d950a5c82d80226050e05bd64be7e30ff87d.tar.bz2
hutch-2913d950a5c82d80226050e05bd64be7e30ff87d.zip
fix: collapse patches to stop a recursive layout loop
Opening a patchset wedged the app. UICollectionView reported a row oscillating between 3674pt and 1647pt and trapped in a recursive layout loop, leaving the UI unresponsive. The detail view rendered every patch in the series expanded, so a List held one enormous self-sizing row per patch, each with a full diff. Self-sizing cells that large do not settle. Patches now start collapsed and expand on tap, so at most the ones a reviewer opens are measured. This is what ThreadDetailView already does — it collapses every message but the last, and renders the same diffs through the same DiffView without trouble. Reviewing a series one patch at a time is also closer to how the reading actually goes. The rendering of a block list is shared between the cover letter and patches rather than duplicated.
Diffstat (limited to 'Hutch')
-rw-r--r--Hutch/Views/Patchsets/PatchsetDetailView.swift104
1 files changed, 89 insertions, 15 deletions
diff --git a/Hutch/Views/Patchsets/PatchsetDetailView.swift b/Hutch/Views/Patchsets/PatchsetDetailView.swift
index 7bc5630..f904424 100644
--- a/Hutch/Views/Patchsets/PatchsetDetailView.swift
+++ b/Hutch/Views/Patchsets/PatchsetDetailView.swift
@@ -7,6 +7,7 @@ struct PatchsetDetailView: View {
@Environment(AppState.self) private var appState
@State private var viewModel: PatchsetDetailViewModel?
@State private var showStatusPicker = false
+ @State private var expandedPatchIDs: Set<Int> = []
var body: some View {
Group {
@@ -163,10 +164,31 @@ struct PatchsetDetailView: View {
}
}
+ /// Patches start collapsed.
+ ///
+ /// A diff is tall, and a series is many of them. Rendering every patch expanded
+ /// puts a dozen self-sizing diffs in one List, which drives UICollectionView
+ /// into a recursive layout loop and wedges the app. The inbox thread view
+ /// collapses all but the last message for the same reason.
@ViewBuilder
private func patchesSection(_ patchset: PatchsetDetail) -> some View {
- ForEach(patchset.patches) { patch in
- emailSection(patch, title: patch.seriesLabel.map { "Patch \($0)" } ?? "Patch")
+ Section("Patches") {
+ ForEach(patchset.patches) { patch in
+ PatchRow(
+ patch: patch,
+ isExpanded: expandedPatchIDs.contains(patch.id),
+ onToggle: {
+ withAnimation(.easeInOut(duration: 0.2)) {
+ if expandedPatchIDs.contains(patch.id) {
+ expandedPatchIDs.remove(patch.id)
+ } else {
+ expandedPatchIDs.insert(patch.id)
+ }
+ }
+ }
+ )
+ .themedRow()
+ }
}
}
@@ -178,19 +200,7 @@ struct PatchsetDetailView: View {
.font(.subheadline.weight(.semibold))
.textSelection(.enabled)
- ForEach(Array(email.contentBlocks.enumerated()), id: \.offset) { _, block in
- switch block {
- case .plainText(let text):
- Text(text)
- .font(.body)
- .textSelection(.enabled)
- .frame(maxWidth: .infinity, alignment: .leading)
- .fixedSize(horizontal: false, vertical: true)
- case .diff(let diff):
- DiffView(diff: diff)
- .textSelection(.enabled)
- }
- }
+ PatchsetContentBlocks(blocks: email.contentBlocks)
}
.padding(.vertical, 4)
.themedRow()
@@ -227,6 +237,70 @@ struct PatchsetDetailView: View {
}
}
+// MARK: - Patch Row
+
+private struct PatchRow: View {
+ let patch: PatchsetEmail
+ let isExpanded: Bool
+ let onToggle: () -> Void
+
+ var body: some View {
+ VStack(alignment: .leading, spacing: isExpanded ? 10 : 0) {
+ Button(action: onToggle) {
+ HStack(alignment: .top, spacing: 12) {
+ Image(systemName: isExpanded ? "chevron.down" : "chevron.right")
+ .font(.caption)
+ .foregroundStyle(.tertiary)
+ .padding(.top, 3)
+
+ VStack(alignment: .leading, spacing: 2) {
+ Text(patch.subject)
+ .font(.subheadline.weight(.medium))
+ .lineLimit(isExpanded ? nil : 2)
+ .multilineTextAlignment(.leading)
+ .frame(maxWidth: .infinity, alignment: .leading)
+
+ if let seriesLabel = patch.seriesLabel {
+ Text(seriesLabel)
+ .font(.caption)
+ .foregroundStyle(.secondary)
+ }
+ }
+ }
+ }
+ .buttonStyle(.plain)
+ .accessibilityHint(isExpanded ? "Collapses this patch" : "Expands this patch")
+
+ if isExpanded {
+ PatchsetContentBlocks(blocks: patch.contentBlocks)
+ }
+ }
+ .padding(.vertical, 4)
+ }
+}
+
+// MARK: - Content Blocks
+
+private struct PatchsetContentBlocks: View {
+ let blocks: [InboxMessageContentBlock]
+
+ var body: some View {
+ ForEach(Array(blocks.enumerated()), id: \.offset) { _, block in
+ switch block {
+ case .plainText(let text):
+ Text(text)
+ .font(.body)
+ .textSelection(.enabled)
+ .frame(maxWidth: .infinity, alignment: .leading)
+ .fixedSize(horizontal: false, vertical: true)
+ case .diff(let diff):
+ DiffView(diff: diff)
+ .textSelection(.enabled)
+ }
+ }
+ }
+}
+
// MARK: - Status Badge
struct PatchsetStatusBadge: View {